Java Class move refactoring does not change references to the file
已完成
In Markdown Navigator links are created to files and find usages generates references to files.
If a single Java file is moved then no file references to the Java file will be updated causing broken links.
However, if the Java Class file is moved as part of several files then file references are updated.
This seems inconsistent and probably is an omission since a Java Class move involves a file move and should update references to the file, not just the class.
Is there something I am missing or should I open a YouTrack issue?
请先登录再写评论。
The behavior is expected. You could provide an additional <referencesSearch> to find corresponding references in .MD files in this case.
Could you please point me to the place where those references are created + post a minimum snippet reproducing this? Thanks.
Yann, the code is part of Markdown Navigator link reference resolve. If the link resolves to a PsiFile then the following is used to create ResolveResult instance as part of the multiResolve return array:
If the file and the class have different names, so that file and class show as separate elements in the project tree, then find usages for the file will show links referencing the file. If the file and class have the same name then find usages does not find the links which refer to the file since only class references are searched.
The same for rename refactoring. If the file is renamed then links are updated, if the class is renamed (which causes a file rename) then links are not updated.
So doing the class rename as 2 steps: first rename file to the new name then rename the class to the same name updates links.
The behaviour appears to show that if the rename is through the class then the implied file rename does not handle updates to the file references.
As for the file moves, if only Java class files are selected then file drag/drop move presents the move class dialog
and links referring to these files are not updated. If a non-java file is included in the move, then the file move dialog is presented
And then the links are updated to the new location. In both cases the package inside the java files is updated to the new location. This leads me to believe that the java class move does not handle the implied file move refactoring.
I can try and create a dummy plugin which uses file references but it needs minimal support code to make the plugin workable otherwise there is no references to update. Something like a custom plugin which takes text lines with path relative to project and generates references to files for these. When bindToElement() is invoked on the reference it will update the line to the file's path relative to project directory..
Yann, to make sure I understand correctly what you mean by find corresponding references via <referencesSearch>.
I do provide <referencesSearch> but at the moment references to PsiFiles are provided. What you are saying is that Java Classes also have to be included so that references to Java Classes are returned by the file references in the links when the class is contained by that file.
I can do this although it seems like a kludge since the links are references not to Java classes but to the file that contains them. Also a java class file can contain more than one class and the contained class name does not have to match the file name. During a file move all the contained classes would result in additional re-bind to the same file and this extra logic also has to be addressed by the plugin.
I would also need to validate and handle the class/file duality for other IDEs like PyCharm, RubyMine, WebStorm, PhpStorm, CLion, etc. Since these also may have a similar issue.
Is there a logical reason why file references are not handled by Java class refactoring other than this use case was not an issue when references were seen as references to Java classes only?
It seems logical to address file rename/move refactoring when doing a class rename/move since the file refactoring is implied.
A bit of hacking had to be added for both the
MoveDirectoryWithClassesHelper
andRenamePsiElementProcessor
to get all Java class/package rename/move refactoring working.MoveDirectoryWithClassesHelper.java
was needed so that when package is moved the plugin gets a chance to adjust links of all down-path files. TheMoveDirectoryWithClassesHelper.Default
implementation handles half the refactoring because it does invoke theMoveFileHandler.forElement(psiFile).prepareMovedFile(psiFile, moveDestination, oldToNewElementsMapping);
However, it never calls the corresponding
findUsages()
norretargetUsages()
for the move file handler, in my case this is needed to complete the file move refactoring of markdown files.I got around this by extending from
MoveDirectoryWithClassesHelper.Default
and inmove()
method callsuper.move()...
then my own move handler’sretargetUsages()
, did not need thefindUsages()
call since I only need to adjust the file itself, the rest is already handled by reference search.RenamePsiElementProcessor
was needed to handlePsiPackage
andPsiDirectory
renames because links to all down-path files/directories have to be refactored for the path change.Got it working but with a bit of hacking. I need to collect references before rename is performed. Easy enough to do it in the
RenamePsiElementProcessor.getPostRenameCallback()
which is called before the rename and callback invoked after rename.However,
RenameProcessor
will invoke methods of all matching rename processors for the element but will only callgetPostRenameCallback()
for the first one:To get around that problem, I register my rename processor as
order="first"
and insidegetPostRenameCallback
use the EP to get the next extension’s post rename callback which I invoke when my post rename call back is invoked.This is the “a bit of hacking” that I referred to because if another plugin needs to be first in the rename processor list because it needs
getPostRenameCallback()
invoked then which ever is loaded first.The rename refactoring should really allow all rename processors to contribute to the post rename callback call. Most will return null but not having this handled in a generic fashion is confusing and un-intuitive. It also necessitates brittle implementation which can create a coupling between unrelated plugins.
Unfortunately, the prediction that order="first" was bound to cause an issue has come true. RubyMine RubyRenamePsiDirectoryProcessor also sets order first so Markdown Navigator directory rename handling does not work under RubyMine.
I will either have to live with this or figure out another way of "hacking" a solution.
Got around all limitations by re-implementing using MarkdownRefactoringListenerProvider instead of multiple handlers.
The only complication was handling duplicated listeners for package rename refactoring. I did not investigate it but ignored the bindToElement() if the element is not valid (because it was already bound to a new element by first listener callback).
Now works with Android Studio and Ruby Mine with a single code base.
Thanks for sharing your findings here, indeed RefactoringListenerProvider seems to be the best approach to cover all cases at once.
For posterity:
Found a use case which fails. If a package
abc
is renamed toabc.xyz
which is really a move of all files and subdirectories ofabc
toabc.xyz
then references to subdirectories ofabc
are not updated.Requires implementation of
MoveDirectoryWithClassesHelper
which will add usages of such sub-directories inMoveDirectoryWithClassesHelper.findUsages()
, then bind these references to moved directories inMoveDirectoryWithClassesHelper.postProcessUsages()
The implementation is simple and is based on
MoveDirectoryWithClassesHelper.Default
, tweaked for specifics of adding subdirectory references of moved directories which are not indirectoriesToMove
.Java-Class-Move-Does-Not-Use-File-References.md
Another discovery that others may find useful.
It seems:
prepareMovedFile
. I was so convinced that a move file handler gets a chance to prepare for the move that I missed it in the name for the method.In the trace I could see that my refactoring listener was calling the file handler’s
prepareMovedFile
before the file was moved. Then the IDE would call the handler for the same file after the move. Too late, all the page relative links in the file are potentially broken so theoldToNewMap
used by the IDE was mostly empty.Since my refactoring listener calls the move handler before the file is moved, I cache the entries generated for the file in file’s userData on first call, then return it to the subsequent callers. The same will happen if the order of calls are reversed. First call computes, the rest get pre-computed values until
retargetUsages
for the file is called.Now file move refactoring works like a charm and I got rid of all code that was there to mask the real issue.