Inspection: Static code analysis for Err.Raise

[bclothier]

Given code like this:

Select Case Foo
  Case 1
    DoThis
  Case 2
    DoThat
  Case 3
    DoNothing
  Case Else
    Err.Raise 5, "Fail", "Foo can be only 1 , 2 or 3, moron!"
End Select

It would be useful to have an inspection where Foo is assigned a literal value or similar case that would case it to reach the Err.Raise statement and thus alert the user.

This can be useful for maintaining / updating enumerations that must be complete. Imagine adding a new flag to the enum but not remembering to update the Select Case that should process all possible values of the enumeration. There might be also other cases where the use of Err.Raise warrant additional analysis as the code get updated.

[retailcoder]

I think this has more to do with Select Case statements involving an enum variable than it has to do with Err.Raise. Flagging Select Case statements that involve an enum variable but don't have a Case block for every possible member is something that can easily be done right now, without any code path analysis required. Wouldn't that be even more effective?

[daFreeMan]

Additionally, not everyone is going to Err.Raise on the Case Else - it may be a Logger.Log or something, so basing the inspection on the existence of Err in the Case Else is a bit narrow.

I do, however approve of the error message! 😀

[bclothier]

Good point. However, I know in my case not all Select Case + enum variable require all to be enumerated. This is more common in the initialization branch of the project, as a way to validate the inputs. But once validated, there might be more paths where we only need to do some extra actions for certain enumerations but not other enumerations, so in that situation, we'd have another Select Case without implementing all the enumeration members. But in that case, this Select Case would typically have no Case Else block + Err.Raise. I don't think I want an inspection for those Select Cases, or I'd end up adding ignore to way too many or even disabling the inspection altogether.

Narrowing the inspection to only where it's made explicit that the Select Case is intended to be complete enumeration by use of Err.Raise, or a comment 'Do Nothing or even MsgBox "Error!" would be more helpful, I think. daFreeMan is right about not everyone doing an Err.Raise for this type of Select Case scenario. My original thought was to help ensure we don't have any literals that would send us down the path where we will always encounter Err.Raise which was why I focused on that, using Select Case as an example.

[bclothier]

Just another example of why analyzing for Err.Raise might be helpful. Suppose we have a class with this stub:

  Err.Raise 5, "This class", "Not implemented"
End Property```

Then given the calling code:

```Dim thisThing As ISomeInterface

Set thisThing = New ThisClass 'Implements ISomeinterface

thisThing.SomeProperty = "abc"

RD should be able to warn about that case since this implementation doesn't implement this member. In real life, the code might be a bit more opaque so it'd be easy to miss that a variable is going to end up bumping against Err.Raise.