rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.91k stars 299 forks source link

Inspection for "implicitly declared local resizable array by the redim statement" #1580

Open ghost opened 8 years ago

ghost commented 8 years ago

Example by @comintern:

Option Explicit

Private Sub Test()
    ReDim foo(10)
    Debug.Print TypeName(foo)
End Sub

Here's the relevant VBA spec: 5.4.3.3 ReDim Statement:

If the name has no matches, then the <redim-statement> is instead interpreted as a <local-variable-declaration> with a <variable-declaration-list> declaring a resizable array with the specified name and the following rules do not apply.

The Option Explicit directive should actually make this invalid but it still works. foo is treated as a simple name expression for lookup, and the simple name expression rules state that if the variable declaration mode is "explicit" and there's no match then the expression is invalid. I guess this is an exception to the rule?

@retailcoder suggested we make an inspection to catch these cases which might require the binder/resolver to flag the created variable somehow so as to be identifiable later on in the inspections.

retailcoder commented 8 years ago

It's currently implementable without changing anything. The ParserRuleContext for the declaration of foo would point to an Identifier context; we can inspect the variable declarations and look at their Parent (or the Parent's Parent) and if that's a ReDimStmtContext then we know to fire up an inspection result.

Or wait a minute, does the DeclarationsListener even ever pick up ReDim statements as declarations?

...this may end up being trickier than it looks.

ghost commented 8 years ago

I don't think any declarations are created yet. Only the resolver knows when to create one. I'd suggest we don't rely on the context and its parent chain too much since that's quite fragile. We could just add a flag to the variable declaration or perhaps even add a new declaration type "ReDimVariable" or something?

retailcoder commented 8 years ago

lol that's getting very ugly, very fast!

Whatever it takes... I'm not too crazy about a new DeclarationType for that though... it's technically just a Variable, only the way it's declared changes. And you're right, working with the ParserRuleContext directly pretty much casts the grammar in stone, which we'd like to avoid.