Java Class move refactoring does not change references to the file

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:

new PsiElementResolveResult(psiFile);

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 and RenamePsiElementProcessorto 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. The MoveDirectoryWithClassesHelper.Default implementation handles half the refactoring because it does invoke the MoveFileHandler.forElement(psiFile).prepareMovedFile(psiFile, moveDestination, oldToNewElementsMapping);

However, it never calls the corresponding findUsages() nor retargetUsages()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.Defaultand in move()method call super.move()...then my own move handler’s retargetUsages(), did not need thefindUsages()call since I only need to adjust the file itself, the rest is already handled by reference search.

RenamePsiElementProcessorwas needed to handle PsiPackage and PsiDirectory 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 call getPostRenameCallback() for the first one:

  @Override
  public void performRefactoring(@NotNull UsageInfo[] usages) {
    List<Runnable> postRenameCallbacks = new ArrayList<>();

    final MultiMap<PsiElement, UsageInfo> classified = classifyUsages(myAllRenames.keySet(), usages);
    for (final PsiElement element : myAllRenames.keySet()) {
      if (!element.isValid()) {
        LOG.error(new PsiInvalidElementAccessException(element));
        continue;
      }
      String newName = myAllRenames.get(element);

      final RefactoringElementListener elementListener = getTransaction().getElementListener(element);
      
      /* ***************************************************************************************************/
      /* this will only invoke getPostRenameCallback() for the first rename processor for the element */
      final RenamePsiElementProcessor renamePsiElementProcessor = RenamePsiElementProcessor.forElement(element);
      /* ***************************************************************************************************/
      
      Runnable postRenameCallback = renamePsiElementProcessor.getPostRenameCallback(element, newName, elementListener);
      final Collection<UsageInfo> infos = classified.get(element);
      try {
        RenameUtil.doRename(element, newName, infos.toArray(UsageInfo.EMPTY_ARRAY), myProject, elementListener);
      }
      catch (final IncorrectOperationException e) {
        RenameUtil.showErrorMessage(e, element, myProject);
        return;
      }
      if (postRenameCallback != null) {
        postRenameCallbacks.add(postRenameCallback);
      }
    }

    // more code follows ....
    }

To get around that problem, I register my rename processor as order="first"and inside getPostRenameCallbackuse 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 abcis renamed to abc.xyzwhich is really a move of all files and subdirectories of abcto abc.xyzthen references to subdirectories of abcare not updated.

Requires implementation of MoveDirectoryWithClassesHelperwhich will add usages of such sub-directories in MoveDirectoryWithClassesHelper.findUsages(), then bind these references to moved directories in MoveDirectoryWithClassesHelper.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.