Rubberduck: "Unreachable Case" inspection info needs to be reworded

Created on 29 Mar 2019  Â·  8Comments  Â·  Source: rubberduck-vba/Rubberduck

The current resource string simply says "Detects Case Clauses that will never execute", which is a fine description of what the inspection does, however InspectionInfo resource strings mean to be more than that. The description needs to be updated to something more substantative that justifies and explains the inspection result. The result string also needs tweaking.

InspectionResult string:

Unreachable: Never matches or is equivalent to a prior Case statement

Suggestion:

'Case' statement is heuristically unreachable

And then move the explanation to the InspectionInfo string, so instead of this:

Detects Case Clauses that will never execute

We would have something like this (suggestion):

A 'Case' condition either can never evaluate to True, or is equivalent to a prior 'Case' statement. This makes the block heuristically unreachable, i.e. "dead code". Consider removing, reordering, or modifying the block.


  • [ ] Neutral (en-US)
  • [ ] French
  • [ ] German
  • [ ] Czech
  • [ ] Spanish
difficulty-01-duckling enhancement feature-inspections resx up-for-grabs user-interface

Most helpful comment

There are a number of InspectionResult resource types for the UnreachableCaseInspection (UCI) that probably require modification if the 'unreachable' result-type does. Based on the above input, I've enumerated them below with some suggestions. I'm thinking it is probably better to hash out the wording here, rather than on the PR.

A couple comments:

  1. It seems to me that 'algorithmically' would be a more precise descriptor than 'heuristically'. A heuristic (full disclosure - I had to look up the definition) is considered a 'Rule of Thumb'. The flagged conditions in this inspection are definitely evaluated with more rigor than a 'Rule of Thumb'. That said, I would opt for leaving out either descriptor, since brevity matters and I would question whether either word provides useful clarification beyond 'unreachable'.
  2. The UCI detects a couple types of lurking Run-time errors as well. These blocks are definitely reachable - you just don't want to go there. Maybe these belong as a separate inspection...but at the moment they are here.

InspectionResults resources:
_UnreachableCaseInspection_Unreachable_
Existing: Unreachable: Never matches or is equivalent to a prior Case statement
New: 'Case' statement is unreachable

_UnreachableCaseInspection_CaseElse_
Existing: Unreachable Case Else: all matches exist within prior Case statement(s).
New: 'Case Else' statement is unreachable

_UnreachableCaseInspection_InherentlyUnreachable_
Existing: Unreachable: Case Statement contains invalid range clause(s).
New: 'Case' statement contains one or more malformed range clauses

_UnreachableCaseInspection_Overflow_
Existing: Unreachable: Case Statement will cause a Run-time error 6 (Overflow).
New: 'Case' statement will raise run-time error 6 (Overflow)

_UnreachableCaseInspection_TypeMismatch_
Existing: Unreachable: Case Statement will cause a Run-time error 13 (Mismatch).
New: 'Case' statement will raise run-time error 13 (type mismatch).

InspectionInfo resource:
_UnreachableCaseInspection_
Existing: Detects Case Clauses that will never execute.
New: A 'Case' condition either always evaluates to False, raises a run-time error, or the cumulative effect of prior 'Case' statements represents all possible values or a superset of this 'Case' statement's values. As a result, the 'Case' block will never execute and is "dead code", or the 'Case' statement is a run-time error waiting to happen. Consider removing, reordering, or modifying the 'Case' statement.

All 8 comments

I'd propose to adjust the InspectionInfo to be a bit clearer (and more correct) about the second part of the first sentence:

A 'Case' condition either can never evaluate to True or prior 'Case' conditions already cover it.

Going a bit off the literal wording you proposed

'Case'-Bedingung wird nie getroffen.


Eine 'Case'-Bedingung ist entweder unerfüllbar (niemals "True") oder von vorhergehenden 'Case'-Bedingungen vollständig abgedeckt. Dadurch wird der zugehörige Block nie ausgeführt, also "toter Code". Überlegen Sie den Block zu entfernen, umzuordnen oder anzupassen.

There are a number of InspectionResult resource types for the UnreachableCaseInspection (UCI) that probably require modification if the 'unreachable' result-type does. Based on the above input, I've enumerated them below with some suggestions. I'm thinking it is probably better to hash out the wording here, rather than on the PR.

A couple comments:

  1. It seems to me that 'algorithmically' would be a more precise descriptor than 'heuristically'. A heuristic (full disclosure - I had to look up the definition) is considered a 'Rule of Thumb'. The flagged conditions in this inspection are definitely evaluated with more rigor than a 'Rule of Thumb'. That said, I would opt for leaving out either descriptor, since brevity matters and I would question whether either word provides useful clarification beyond 'unreachable'.
  2. The UCI detects a couple types of lurking Run-time errors as well. These blocks are definitely reachable - you just don't want to go there. Maybe these belong as a separate inspection...but at the moment they are here.

InspectionResults resources:
_UnreachableCaseInspection_Unreachable_
Existing: Unreachable: Never matches or is equivalent to a prior Case statement
New: 'Case' statement is unreachable

_UnreachableCaseInspection_CaseElse_
Existing: Unreachable Case Else: all matches exist within prior Case statement(s).
New: 'Case Else' statement is unreachable

_UnreachableCaseInspection_InherentlyUnreachable_
Existing: Unreachable: Case Statement contains invalid range clause(s).
New: 'Case' statement contains one or more malformed range clauses

_UnreachableCaseInspection_Overflow_
Existing: Unreachable: Case Statement will cause a Run-time error 6 (Overflow).
New: 'Case' statement will raise run-time error 6 (Overflow)

_UnreachableCaseInspection_TypeMismatch_
Existing: Unreachable: Case Statement will cause a Run-time error 13 (Mismatch).
New: 'Case' statement will raise run-time error 13 (type mismatch).

InspectionInfo resource:
_UnreachableCaseInspection_
Existing: Detects Case Clauses that will never execute.
New: A 'Case' condition either always evaluates to False, raises a run-time error, or the cumulative effect of prior 'Case' statements represents all possible values or a superset of this 'Case' statement's values. As a result, the 'Case' block will never execute and is "dead code", or the 'Case' statement is a run-time error waiting to happen. Consider removing, reordering, or modifying the 'Case' statement.

Regarding this part:

New: 'Case' statement contains one or more malformed range clauses

What are examples of such "malformed" clauses that wouldn't be a compile-time errors?

VBA is happy to compile Case 2 To 10 as well as Case 10 To 2. Problem is, the latter will never execute.

I see. The thing is that I wouldn’t call it “malformed”... I would refer to it as logically impossible or self contradictory... the problem here isn’t the syntax or grammar (e.g. it is well formed) but rather the logic. The messsage should pount to that.

So “does not compute” statements implies?

How about a more direct message rather than general descriptors...

_UnreachableCaseInspection_InherentlyUnreachable_
Existing: Unreachable: Case Statement contains invalid range clause(s).
New: 'Case' statement Range Clauses must be expressed '[x] To [y]' where [x] is less than or equal to [y]'

Yes, I think that's most clear we can be. 👍

Was this page helpful?
0 / 5 - 0 ratings