Renaming enum member to conflicting class name doesn't produce warning

[Energeer]

Rubberduck version information
Version 2.5.9.6321
OS: Microsoft Windows NT 10.0.19044.0, x64
Host Product: Microsoft Office x64
Host Version: 16.0.18324.20194
Host Executable: EXCEL.EXE

Description
Renaming an Enum member to the same name as a class breaks code. Rubberduck does not appear to warn about the conflict.

This probably falls somewhere between bug and feature request, but since I think notification of naming conflicts is a feature and doesn't appear in this case, I have labelled as a bug.

To Reproduce
Steps to reproduce the behavior:

1. Create the classes and module below.
2. Use Rubberduck to rename Rectangular to Rectangle in IShape.
3. Run PrintShapeEnumAndName() Run-time error '424': Object required

IShape.cls

Public Enum Shape
    Rectangular
End Enum

Rectangle.cls

Public Property Get Name() as String
    Name = "Rectangle"
End Property

Module1.bas

Public Sub PrintShapeEnumAndName()
    Debug.Print Rectangular

    With New Rectangle
        Debug.Print .Name
    End With
End Sub

Expected behavior
A warning that there is a naming conflict and code may break.

Additional context
This might seem like a small issue, and it’s on the user to name things properly to avoid conflicts. But in a big project with a lot of renaming, if you mess up, there doesn’t seem to be an easy way to fix it.

[retailcoder]

Wow, everything around this was very thoroughly tested, ...nice find, that's the first rename bug found in years!

A bug with the collision validation actually - resolution seems correct to me; the enum member is picked up as such, but the rename box does not warn about it...

...as it does when you rename, say, a blatantly conflicting one:

Meanwhile it qualifies the enum member with the containing module without validating that it can legally do that (or assumes the enum is defined in a standard module), which breaks the code regardless of the naming collision:

Instead Rubberduck should qualify the enum member with its parent, so in this case ShapeType.Rectangle; it's safe to assume that the parent enum is public, or at least accessible from wherever a member is referenced.

---

On a tangentially related note, the Shadowed declaration inspection should probably have a word about this too. It ships disabled by default because it's an expensive one, but in a small test project it doesn't hurt to enable it:

...but while it (correctly) identifies shadowed Excel.IShape and Excel.Rectangle declarations, not a word about the Rectangle enum member shadowing the class name:

This is not unexpected, because the enum member and the class name can never be used in the same syntactical context, so while the names are confusing to a human reader, to VBA (and Rubberduck) the identical names are unambiguous - which is also why the rename refactoring isn't issuing a warning either.

---

I note that I did not locally reproduce the exact same problem: whatever I do, Debug.Print Rectangular always becomes Debug.Print IShape.Rectangle - which is still a bug because the qualifier is wrong, but renaming Rectangular to Rectangle is not illegal VBA code in this context:

Adding a @PredeclaredId to Rectangle, and making the Name property its @DefaultMember, now we're confusing VBA... but still not Rubberduck:

That's the only way I could alter the semantics of the code by renaming the enum member - but then again the correct semantics are restored by fixing the qualifier to be the parent enum type name rather than the containing module name.

So, I'm kind of not quite reproducing the issue here, because the enum member is always qualified with my local debug build, and that bad qualifier appears to be absent from your bug report.

Still, the refactor/rename collision validation should probably warn about this (edge) case.

[Energeer]

Thanks for the detailed response and investigation, I may need to re-read it a few times to fully digest it all...

Meanwhile it qualifies the enum member with the containing module without validating that it can legally do that (or assumes the enum is defined in a standard module), which breaks the code regardless of the naming collision:

Yes, that's exactly it. It likely has nothing to do with the Rectangle class itself, but rather the IShape qualifier. Apologies for not mentioning that earlier, I don't think I fully understood the issue when creating the example.

I've been following the convention of placing Enums in interface classes ever since I read this:

I could describe this type as follows (the enums don’t belong to the interface, they’re just public types that were convenient to define there)

(https://rubberduckvba.blog/2016/07/05/oop-vba-pt-2-factories-and-cheap-hotels/)

This is not unexpected, because the enum member and the class name can never be used in the same syntactical context, so while the names are confusing to a human reader, to VBA (and Rubberduck) the identical names are unambiguous - which is also why the rename refactoring isn't issuing a warning either.

What about the following code, or am I misunderstanding the point made?

Public Sub DoSomething()
    Debug.Print (TypeOf Rectangle Is Rectangle)
End Sub

[retailcoder]

With a @PredeclaredId on the Rectangle class, (TypeOf Rectangle Is Rectangle) is a legal Boolean expression where the first Rectangle refers to the default instance, and the second Rectangle refers to the class type.

Without the predeclared ID, the first Rectangle resolves to the enum member, which VBA considers a type mismatch compile-time error. Because it's a compile-time error and the basic premise is that Rubberduck is looking at compilable code, there's no inspection to warn about it (the 3.x parser would feel differently about this, but in 2.x we're generally not issuing compiler errors).

The mis-qualifying of the renamed enum member should be a relatively easy fix; I don't think the collision validation needs to change, because if the enum member gets qualified with the correct parent declaration identifier (i.e. the name of the parent enum type), then the semantics of the code are preserved, the code remains compilable, and there is no collision introduced... so the prompt/warning would be confusing, I think.

[retailcoder]

Aight, got a fix for this one.