Add new refactoring "Inject Module Qualifier"...

[jim-oflaherty-jr-qalocate-com]

Justification As a codebase grows, the probability of a naming conflict arising among public subs and functions as external modules and/or new modules are added increases quickly. There is a dynamic tension between using simple straightforward names for subs and functions within a module and those names having already been chosen in another code-base's modules which now must be merged and integrated. Manually adding the "Module Prefix" with a dot to clarify the definition context becomes mandatory in resolving the naming conflicts in at least one of the two sides of the conflict.

Description When activating the refactoring, the context of the sub or function is used to determine its definition module's name (including the one within which the refactoring is occurring). And then the module's name is inserted prior to the sub or function name followed by a dot. This disambiguates the sub or function ownership.

For example...

'Code inside module "mFoo"
Dim headerNames() As String
Dim headerNamesSize As Long

headerNamesSize = size(headerNames) 'Original unrefactored code - "size" is a function in module "mArrays"
headerNamesSize = mArrays.size(headerNames) 'Following the first refactoring pass, the "size" function has the module's name, "mArrays" within which it is defined, prepended
headerNamesSize = Geocoder.mArrays.size(headerNames) 'Following the second refactoring pass, the "size" function has the project's name, "Geocoder" within which the "mArrays" module is defined, prepended

Additional context Assuming the context of a successfully compiled state, the existing parsed state already holds EXACTLY these relationships. In fact, that is precisely the information being displayed to the right of the "Ready" button.

I don't have enough experience refactoring Excel yet to know if also providing the project name prefix with a dot in front of the module would be as useful. If so, then perhaps this refactoring could prepend a module name to a sub or function. And then if there is already a module name, prepend the project name to the already existing module name.

I also sense there is a possibility of adding some sort of inspection which identifies calls to simpler named subs and functions be prepended with its defining module to proactively avoid the naming conflicts and refactoring efforts later.

[MDoerner]

I like this suggestion. I would simply call it Qualify. It would also apply to properties and fields on modules. We could also qualify with the project, if the reference points to a referenced library.

This should not be hard to implement since we store the necessary information on the references anyway. The biggest problem is probably figuring out in an efficient way when the refactoring should be available.

We could actually wrap the refactoring in an inspection quickfix pair. That would allow to find all unqualified calls and qualify them. I think we should not flag calls to methods/fields in the same module/class, though.

[jim-oflaherty-jr-qalocate-com]

We could actually wrap the refactoring in an inspection quickfix pair. That would allow to find all unqualified calls and qualify them.

I really like this. It completely captures both sides of the software engineering conflict.

I think we should not flag calls to methods/fields in the same module/class, though.

Ah. Okay. As long as the compiler rule is that the local module scope is ALWAYS where the resolution occurs first, then this probably mostly works.

The only concern I would have is during a Rename refactoring where one module is renaming to an identifier that is the same as an already existing identifier in another module that is unqualified locally. Wouldn't that give rise to the current refactoring error message of same name confusion? If so, then to aid in my own future refactorings, I would personally be manually engaging the Qualify refactoring even for my module local context, at least for the more simple names like size, to entirely avoid the warning refactoring message.

Does that make sense? Or am I missing something here?

[BZngr]

I also would agree that this is a good candidate for an Inspection/QuickFix combination. Further, I believe much of the needed code already exists.

There is a ReplaceReferencesRefactoringAction with a ModuleQualifyExternalReferences flag (defaulted to true) on its ReplaceReferencesModel. This refactoring action is used only by the EncapsulateFieldRefactoring ATM.

With some (hopefully) minor changes I would think that both a new QuickFix and RenameRefactoring could incorporate the ReplaceReferencesRefactoringAction. Perhaps module qualification should be the standard behavior for both RenameRefactoring and EncapsulateFieldRefactoring with no option to enable/disable.(?)

Also, my suspicion is that RenameRefactoring's conflict detection is currently preventing a rename where renaming external references would create a problem. So, there would be some work there as well.