API Change Request: PsiNavigationUtil to use element's Navigatable.navigate() before default navigation to containing file.

Relates to: NavigationGutterIconRenderer goes to file/offset of element instead of invoking element’s navigate() but a simpler solution.

Currently, PsiNavigationUtil navigates to an element based on its containingFile’s virtual file. This means that if an element implements Navigatable with its own alternative navigation, for line markers for example, the element’s navigate() method will only be called if it returns null for its containing file.

public class PsiNavigateUtil {
  public static void navigate(@Nullable final PsiElement psiElement) {
    navigate(psiElement, true);
  }

  public static void navigate(@Nullable final PsiElement psiElement, boolean requestFocus) {
    if (psiElement != null && psiElement.isValid()) {
      final PsiElement navigationElement = psiElement.getNavigationElement();
      
      if (navigationElement instanceof Navigatable) {
        navigatable = (Navigatable) navigationElement;
      } else {
        final int offset = navigationElement instanceof PsiFile ? -1 : navigationElement.getTextOffset();

        VirtualFile virtualFile = PsiUtilCore.getVirtualFile(psiElement);
        Navigatable navigatable;
        if (virtualFile != null && virtualFile.isValid()) {
          navigatable = PsiNavigationSupport.getInstance().createNavigatable(navigationElement.getProject(), virtualFile, offset);
        } else {
          return;
        }
      }

      navigatable.navigate(requestFocus);
    }
  }
}

Why is the element not tested to see if it implements Navigatable or if it does at least test if it returns true for canNavigateToSource() before using default navigation.

public class PsiNavigateUtil {
  public static void navigate(@Nullable final PsiElement psiElement) {
    navigate(psiElement, true);
  }

  public static void navigate(@Nullable final PsiElement psiElement, boolean requestFocus) {
    if (psiElement != null && psiElement.isValid()) {
      final PsiElement navigationElement = psiElement.getNavigationElement();
      if (navigationElement instanceof Navigatable && !((Navigatable)navigationElement).canNavigateToSource()) {
        navigatable = (Navigatable)navigationElement;
      } else {
        final int offset = navigationElement instanceof PsiFile ? -1 : navigationElement.getTextOffset();
        
        VirtualFile virtualFile = PsiUtilCore.getVirtualFile(psiElement);
        Navigatable navigatable;
        if (virtualFile != null && virtualFile.isValid()) {
          navigatable = PsiNavigationSupport.getInstance().createNavigatable(navigationElement.getProject(), virtualFile, offset);
        }
        else {
          return;
        }
      }
      navigatable.navigate(requestFocus);
    }
  }
}

This would allow custom navigation without resorting to hacks of returning null for getContainingFile() to prevent default from being used when the element cannot navigate to source.

Please sign in to leave a comment.