Cross Module renaming

[vidit-od]

Fixes #4816

This PR aims at implementing Cross Module Renaming. Recently Cross Module Renaming was made True by default in HLS, but current implementation shows Renaming of an exported name is unsupported and Renaming of an imported name is unsupported when attempting to do so.

The main reason for this was the failWhenImportOrExport function blocking rename whenever any imported or exported variables are renamed.

The current implementation carefully checks and distinguish bw local , exported, imported variables. If cross Module is disabled. Blocks renaming imported and exported variables renaming. The older constrain of having an Explicit export list is still maintained.

The below comment ( link ) also talks about a scenario where relying completely on HieDb can lead to incorrect results when local references and imported/ exported references have same name. Hence Ast is also referred for renaming of local variables.

[vidit-od]

This PR aims at implementing cross module renaming.

It is ready for review and most of the functionality has been check by me manually. Raising this PR to see how the tests perform on various environments.

Will add testcases for this Feature once we can verify old ones work properly.

[fendor]

Could you briefly explain the strategy of the solution? I was under the impression, that the rename feature could entirely rely on hiedb to find references to a Name and replacing them, but it seems like you are doing quite a lot more with the GHC AST directly?

[vidit-od]

Reason for using AST

Implementation with HieDb :

Here changing the local variable leads to changing of all other variables of same names (Local and Global). The reason for this is HieDB uses OccName for finding references. AST presents absolute truth and should be used to rely on for preventing inconsistencies.

This similar break can be observed in any and all examples where a local variable has same name as imported variable. Although this is a very specific example of : Importing a variable -> having a same name variable as a local variable -> user triggers rename on that variable.

current implementation enables :

IMP : HieDB does cover all of the simpler cases. considering that AST traversal takes comparatively longer time we have to choose b/w faster but little inconsistent implementation vs slower but always correct one ; both version of codes are available with me :)

Hopefully this makes everything clear ! @fendor

[wz1000]

Can you offer a overview of what the bug was, and your approach towards fixing it please? It would be good to add this to the commit message as well.

[wz1000]

It would also be good to add the example from #4816 as a regression test.

[vidit-od]

• i have updated the PR description with an overview explanation,
• also new regression tests including the ones listed in original issue as well as a few more cases have been added.
• The one review regarding usage of Modsummary has been reverted because of the reasons listed in above conversation.

On a side note: the tests for rename plugin gave me a very hard time. Tho , I dont know if we can do anything about it or not :)

requesting a re-review @wz1000 @fendor

[vidit-od]

changed this Regression test.
we now trigger rename only when cursor name is properly on identifier.