Inspection for literal boolean conditions

[ThunderFrame]

We've all done it... You're debugging and you want to skip or enter an If block.... Only you forget to reinstate the condition, so next time the code runs in PROD, the nukes either fail to launch, or they're all launched at once....

Rubberduck should have an inspection for code like:

If True Then
'...

If False Then
'...

If Not False Then
'...

If Not True Then
`...

If LaunchNukes Or True Then
`...

Select Case True
    Case False
        'Unreachable Code
    Case True
        'Will always run
    Case Else
        'Unreachable Code
End Select

Quickfixes might need to be aware of comments that have preserved the "real" condition:

If True Then 'LaunchNukes` Then
'...

If Not True Then 'LaunchNukes Then
'...

[ThunderFrame]

Might also be worth checking any literal that will be cast to Boolean literal:

    If "TRUE" Then
      'This runs
      Beep
    End If

    If "FALSE" Then
      'Unreachable
      Beep
    End If

    If "1E23" Then
      'This runs
      Beep
    End If

    If 10 And 11 Then
      'This runs
      Beep
    End If

    If 42 Then
      'This runs
      Beep
    End If

    If -42 Then
      'This runs
      Beep
    End If

    If 0 Then
      'This is unreachable code
      Beep
    End If

[retailcoder]

The good news is that there's already a ParseTreeListener walking the parse trees and picking up empty string literals - here it's simply a matter of looking at the node's Parent and verifying if it's an If or a Case statement. The listener is issuing the inspection results directly though (the inspection itself merely fetches the results from the parser state) - we're looking at something with similar mechanics here.

The problem might be with performance: literals are [literally] everywhere, and walking the parse trees is expensive. OTOH we won't know how bad it is until we try it.

---

Quickfixes might need to be aware of comments that have preserved the "real" condition

Comments are never parsed as code, Rubberduck has no way of knowing that the commented-out condition is actually dead code. I think it's best to treat comments as what they are: not code.

Besides, code-in-comments should be punishable by ... by Rubberduck forcing your solution onto source control and automatically removing them all. Would be nice to be able to tell code from gibberish in comments though.

[ThunderFrame]

We need to be careful, as Select Case True is a valid usage, and sometimes the only way of expressing a Select Case statement that meets the needs of the conditions - particularly where the conditions relate to different variables. For example:

Sub foo()
  Dim ignoreResults
  ignoreResults = True

  Dim rng As Range
  Set rng = Range("A1:C5")

  'Execute the first Case that evaluates to true
  Select Case True
    Case ignoreResults
      Beep
    Case Not rng Is Nothing
      Beep
    Case Else
      Beep
  End Select
End Sub

[retailcoder]

I wouldn't trip a warning on Select Case True - on Case {bool-literal} is another story though. Right?

[ThunderFrame]

Exactly

[retailcoder]

So, we have an inspection that says:

Expression is always [true|false].

What's the quick-fix? Do we suggest implementing the condition as a precompiler directive?

[ThunderFrame]

At the very least I'd implement a constant and give the user the option of its name and scope.