Parser does not find hidden EnumerationMember references

[BZngr]

When the declaration of EnumerationMember [_Last] is selected (e.g., to rename), the usage/reference in MsgBox CStr(FruitType.[_Last]) is not discovered/listed as a reference.

Option Explicit

Public Enum FruitType
    [_First] = 1
    Apple = 1
    Orange = 2
    Plum = 3
    [_Last] = 3
End Enum

Sub DoSomething()
    MsgBox CStr(FruitType.[_Last])
End Sub

[ThunderFrame]

Indeed, using an enum member in an expression, results in the member being identified as a runtime expression, or unrecognized. There's also a resolver bug with naming conflicts, and it's impossible to rename an enum member and retain the leading underscore (I'll raise separate issues):

Public Enum FoodType
  Apple = 4
  Orange = 5
  Plum = 3
End Enum

Public Enum FruitType
  [_First] = 1
  Apple = 1
  Orange = 2
  Plum = [FoodType].[Plum] 'These are identified as Enum and Enum member
  Applorange = Apple Or Orange 'VBA's resolved value is 3, but in RD, Apple is recognized as FoodType.Apple, and Orange as FoodType.Orange
  [_Last] = 3
  [_Count1] = [_Last] - [_First] + 1 '[_Last] and [_First] expression variables are identified as "runtime expression"
  [_Count2] = [FruitType].[_Last] - FruitType.[_First] + 1 '[FruitType] and FruitType are correctly identified, [_Last] and [_First] are unrecognized tokens
End Enum

[retailcoder]

@ThunderFrame unrecognized tokens, as in unresolved?

[ThunderFrame]

yep - as in RD toolbar doesn't know what to make of them.

[MDoerner]

This is a problem with with how MatchName in DeclarationFinder works. Before searching for the name, it normalizes it, i.e. it removes enclosing brackets and converts to lower case. However, we only convert to lower case when building the backing dictionary. Consequently, the declarations whose identifier name contains enclosing square brackets cannot be found by name.

We should be consistent between the backing dictionary and the function. I would propose to remove the brackets as well when building the dictionary since enclosing a reference in brackets is a legal way to reference the same declaration.

As an additional consideration, do we want to include the brackets in the identifier in the first place? Technically, they are not part of the name and only present to make an otherwise illegal name legal. Moreover, having them in there will probably cause wrong references in examples as the one below:

Private Enum FruitTypes
    Apple
    [[Apple]]
    [Orange]
    [_Strawberry]
    [[_Strawberry]]
End Enum

Private Sub Test()
    Debug.Print FruitTypes.Apple            'Prints 0
    Debug.Print FruitTypes.[Apple]          'Prints 0
    Debug.Print FruitTypes.[[Apple]]        'Prints 1
    Debug.Print FruitTypes.Orange           'Prints 2
    Debug.Print FruitTypes.[Orange]         'Prints 2
    Debug.Print FruitTypes.[_Strawberry]    'Prints 3
    Debug.Print FruitTypes.[[_Strawberry]]  'Prints 4
End Sub

Note that IntelliSense and auto-completing are confused here as well. [_Strawberry] does not appear in the IntelliSense dropdown and selecting [Apple] will insert [Apple] instead of the required [[Apple]].

So, considering this, I would suggest to leave the mismatch in removing the brackets between the backing dictionary and MatchName as-is and remove the first set of enclosing brackets from the identifier name of declarations instead.

[BZngr]

FWIW, I did some experiments based on the DeclarationFinder code comments. Hopefully, some of this information might be new/useful to you.

Based on your above comments, I would expect the following assumptions and logic to be true:

1. [Apple] is a valid variable VBA Identifier without the first layer of surrounding brackets
   Normalized within DeclarationFinder.MatchName as apple
   As stored in DeclarationFinder._declarationsByName: [apple]
DeclarationFinder.MatchName needs to look for: apple AND [apple]

2. [_Apple] is an invalid variable VBA Identifier without the first layer of surrounding brackets
   Normalized within DeclarationFinder.MatchName as _apple
   As stored in DeclarationFinder._declarationsByName: [_apple]
DeclarationFinder.MatchName needs to look for: [_apple]

3. [[Apple]] is an invalid variable VBA Identifier without the first layer of surrounding brackets
   Normalized within DeclarationFinder.MatchName as [apple]
   As stored in DeclarationFinder._declarationsByName: [[apple]]
DeclarationFinder.MatchName needs to look for: [[apple]]

4. [[_Apple]] is an invalid variable VBA Identifier without the first layer of surrounding brackets
   Normalized within DeclarationFinder.MatchName as [_apple]
   As stored in DeclarationFinder._declarationsByName: [[_apple]]
DeclarationFinder.MatchName needs to look for: [[_apple]]

I did some experiments using a modified version of the VBAIdentifierValidator that allowed bracketed enum expressions during RenameRefactoring. And, I hacked a version of the DeclarationFinder.MatchName to attempt to find matches as described above. Renaming single-bracketed declarations (examples 1 & 2 above) started working (a nice surprise). Declarations (and their references) for identifiers like [[Apple]] are not handled properly using the above assumptions and logic. So, I'm thinking my understanding of how declarations identifiers like [[Apple]] are stored in the _declarationsByName dictionary is not correct.

Also, the 'success' of applying the above logic to single bracketed expressions (which required some use of identifier validation code) made me wonder if there was enough 'mutual dependency' to justify making name validation part of the DeclarationFinder exposed as public methods or as an interface. This makes name validation code as accessible to other objects as the DeclarationFinder itself (for refactorings, both are almost always needed). And, if it is 'owned' by the DeclarationFinder, then name validation can easily incorporate name-conflict detection as part of the identifier validation.

[MDoerner]

First, I think changing the declaration finder is the wrong approach here. The problem is that we do not follow the VBA logic of ignoring the outermost enclosing brackets at the declaration. If you look at what IntelliSense shows, you will only see the versions with one less pair of brackets.

Regarding putting name validation on the DeclarationFinder. Please don't. This class does not need more responsibilities, but less. It should be split into a caching part and other parts instead. If you want a general name validation logic provider, set up an interface for it and inject that into whatever requires name validation.