From 259435bbc8e209c102d01eb0c61688b73e298fb0 Mon Sep 17 00:00:00 2001
From: Pavel Dolgov <pavel.dolgov@jetbrains.com>
Date: Thu, 12 May 2016 19:37:40 +0300
Subject: [PATCH] Javafx: Highlighting for private event handler methods marked
 with @FXML (IDEA-151633)

---
 .../javaFX/fxml/JavaFXHighlightingTest.java   | 27 ++++++
 .../JavaFxImplicitUsageProvider.java          | 93 +++++++++++++++----
 .../javaFX/fxml/refs/JavaFxScopeEnlarger.java |  2 +-
 .../indexing/JavaFxControllerClassIndex.java  | 10 +-
 .../javaFX/indexing/JavaFxIdsIndex.java       |  4 +-
 .../EventHandlerInSuperclass.java             |  9 ++
 .../highlighting/FxIdInSuperclass.java        |  9 ++
 .../highlighting/PrivateEventHandler.java     |  9 ++
 .../highlighting/privateEventHandler.fxml     |  6 ++
 9 files changed, 147 insertions(+), 22 deletions(-)
 create mode 100644 plugins/javaFX/testData/highlighting/EventHandlerInSuperclass.java
 create mode 100644 plugins/javaFX/testData/highlighting/FxIdInSuperclass.java
 create mode 100644 plugins/javaFX/testData/highlighting/PrivateEventHandler.java
 create mode 100644 plugins/javaFX/testData/highlighting/privateEventHandler.fxml

diff --git a/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXHighlightingTest.java b/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXHighlightingTest.java
index d419f44313e1..af1d00f56f6a 100644
--- a/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXHighlightingTest.java
+++ b/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXHighlightingTest.java
@@ -444,6 +444,33 @@ public class JavaFXHighlightingTest extends AbstractJavaFXTestCase {
     doTest();
   }
 
+  public void testPrivateEventHandler() throws Exception {
+    myFixture.configureByFiles(getTestName(true) + ".fxml", getTestName(false) + ".java");
+    myFixture.testHighlighting(true, true, true, getTestName(false) + ".java");
+  }
+
+  public void testFxIdInSuperclass() throws Exception {
+    doTestControllerSuperclass();
+  }
+
+  public void testEventHandlerInSuperclass() throws Exception {
+    doTestControllerSuperclass();
+  }
+
+  private void doTestControllerSuperclass() {
+    final String superclass = getTestName(false);
+    myFixture.copyFileToProject(superclass + ".java");
+    myFixture.addClass("public class SubclassingController extends " + superclass + " {}");
+    myFixture.addFileToProject("sample.fxml", "<?import javafx.scene.layout.VBox?>\n" +
+                                              "<?import javafx.scene.control.Button?>\n" +
+                                              "<VBox xmlns:fx=\"http://javafx.com/fxml/1\" xmlns=\"http://javafx.com/javafx/8\"\n" +
+                                              "      fx:controller=\"SubclassingController\">\n" +
+                                              "    <Button fx:id=\"inheritedButton\" onAction=\"#onAction\"/>\n" +
+                                              "</VBox>");
+
+    myFixture.testHighlighting(true, true, true, superclass + ".java");
+  }
+
   private void doTest() throws Exception {
     myFixture.testHighlighting(false, false, false, getTestName(true) + ".fxml");
   }
diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxImplicitUsageProvider.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxImplicitUsageProvider.java
index 655f4890452c..19462e48a04c 100644
--- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxImplicitUsageProvider.java
+++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxImplicitUsageProvider.java
@@ -15,29 +15,56 @@
  */
 package org.jetbrains.plugins.javaFX.fxml.codeInsight.inspections;
 
+import com.intellij.codeInsight.AnnotationUtil;
 import com.intellij.codeInsight.daemon.ImplicitUsageProvider;
 import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.Ref;
 import com.intellij.openapi.vfs.VirtualFile;
-import com.intellij.psi.PsiClass;
-import com.intellij.psi.PsiElement;
-import com.intellij.psi.PsiField;
+import com.intellij.psi.*;
+import com.intellij.psi.search.GlobalSearchScope;
+import com.intellij.psi.search.PsiSearchHelper;
+import com.intellij.psi.search.searches.ReferencesSearch;
+import com.intellij.psi.util.InheritanceUtil;
+import com.intellij.util.Query;
+import gnu.trove.THashSet;
+import org.jetbrains.plugins.javaFX.fxml.JavaFxCommonNames;
+import org.jetbrains.plugins.javaFX.fxml.refs.JavaFxScopeEnlarger;
 import org.jetbrains.plugins.javaFX.indexing.JavaFxControllerClassIndex;
 import org.jetbrains.plugins.javaFX.indexing.JavaFxIdsIndex;
 
-import java.util.Collection;
 import java.util.List;
+import java.util.Set;
 
 /**
  * User: anna
  * Date: 3/22/13
  */
 public class JavaFxImplicitUsageProvider implements ImplicitUsageProvider {
-  
+
   @Override
   public boolean isImplicitUsage(PsiElement element) {
+    if (element instanceof PsiMethod) {
+      return isImplicitMethodUsage((PsiMethod)element);
+    }
     return isImplicitWrite(element);
   }
 
+  public boolean isImplicitMethodUsage(PsiMethod method) {
+    if (!isImplicitFxmlAccess(method)) return false;
+    final Project project = method.getProject();
+    final GlobalSearchScope projectScope = GlobalSearchScope.projectScope(project);
+    final GlobalSearchScope fxmlScope = new JavaFxScopeEnlarger.GlobalFxmlSearchScope(projectScope);
+    final PsiSearchHelper searchHelper = PsiSearchHelper.SERVICE.getInstance(project);
+    final PsiSearchHelper.SearchCostResult searchCost = RefResolveService.getInstance(project).isUpToDate()
+                                                        ? PsiSearchHelper.SearchCostResult.FEW_OCCURRENCES
+                                                        : searchHelper.isCheapEnoughToSearch(method.getName(), fxmlScope, null, null);
+    if (searchCost == PsiSearchHelper.SearchCostResult.FEW_OCCURRENCES) {
+      final Query<PsiReference> query = ReferencesSearch.search(method, fxmlScope);
+      return query.findFirst() != null;
+    }
+    return false;
+  }
+
   @Override
   public boolean isImplicitRead(PsiElement element) {
     return false;
@@ -46,22 +73,54 @@ public class JavaFxImplicitUsageProvider implements ImplicitUsageProvider {
   @Override
   public boolean isImplicitWrite(PsiElement element) {
     if (element instanceof PsiField) {
-      final String name = ((PsiField)element).getName();
-      final PsiClass containingClass = ((PsiField)element).getContainingClass();
-      if (containingClass != null) {
-        final String qualifiedName = containingClass.getQualifiedName();
-        if (qualifiedName != null) {
-          final Project project = element.getProject();
-          final List<VirtualFile> fxmls = JavaFxControllerClassIndex.findFxmlsWithController(project, qualifiedName);
-          if (!fxmls.isEmpty()) {
-            final Collection<String> filePaths = JavaFxIdsIndex.getFilePaths(project, name);
-            for (VirtualFile fxml : fxmls) {
-              if (filePaths.contains(fxml.getPath())) return true;
+      final PsiField field = (PsiField)element;
+      if (!isImplicitFxmlAccess(field)) return false;
+      final String fieldName = field.getName();
+      if (fieldName == null) return false;
+
+      final PsiClass containingClass = field.getContainingClass();
+      if (containingClass == null) return false;
+      final String qualifiedName = containingClass.getQualifiedName();
+      if (qualifiedName == null) return false;
+      final Project project = element.getProject();
+      final Set<VirtualFile> visitedFxmls = new THashSet<>();
+      // is the field declared in a controller class?
+      final List<VirtualFile> fxmls = JavaFxControllerClassIndex.findFxmlsWithController(project, qualifiedName);
+      if (!fxmls.isEmpty()) {
+        final Set<String> fxIdFilePaths = JavaFxIdsIndex.getFilePaths(project, fieldName);
+        for (VirtualFile fxml : fxmls) {
+          visitedFxmls.add(fxml);
+          if (fxIdFilePaths.contains(fxml.getPath())) return true;
+        }
+      }
+      // is the field declared in a superclass of a controller class?
+      final Set<String> fxIdFilePaths = JavaFxIdsIndex.getFilePaths(project, fieldName);
+      if (!fxIdFilePaths.isEmpty()) {
+        final Ref<Boolean> refFound = new Ref<>(false);
+        final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project);
+        final GlobalSearchScope resolveScope = containingClass.getResolveScope();
+        JavaFxControllerClassIndex.processControllerClassNames(project, resolveScope, className -> {
+          final List<VirtualFile> fxmlCandidates = JavaFxControllerClassIndex.findFxmlsWithController(project, className, resolveScope);
+          for (VirtualFile fxml : fxmlCandidates) {
+            if (!visitedFxmls.add(fxml)) continue;
+            if (fxIdFilePaths.contains(fxml.getPath())) {
+              final PsiClass aClass = psiFacade.findClass(className, resolveScope);
+              if (InheritanceUtil.isInheritorOrSelf(aClass, containingClass, true)) {
+                refFound.set(true);
+                return false;
+              }
             }
           }
-        }
+          return true;
+        });
+        return refFound.get();
       }
     }
     return false;
   }
+
+  private static boolean isImplicitFxmlAccess(PsiModifierListOwner member) {
+    return !member.hasModifierProperty(PsiModifier.PUBLIC) &&
+           AnnotationUtil.isAnnotated(member, JavaFxCommonNames.JAVAFX_FXML_ANNOTATION, false);
+  }
 }
diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxScopeEnlarger.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxScopeEnlarger.java
index 06e58a145ea0..8fdc9fec9c66 100644
--- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxScopeEnlarger.java
+++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxScopeEnlarger.java
@@ -100,7 +100,7 @@ public class JavaFxScopeEnlarger extends UseScopeEnlarger {
            AnnotationUtil.isAnnotated(method, JavaFxCommonNames.JAVAFX_FXML_ANNOTATION, false);
   }
 
-  static class GlobalFxmlSearchScope extends DelegatingGlobalSearchScope {
+  public static class GlobalFxmlSearchScope extends DelegatingGlobalSearchScope {
     public GlobalFxmlSearchScope(GlobalSearchScope baseScope) {
       super(baseScope);
     }
diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxControllerClassIndex.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxControllerClassIndex.java
index 20dd73f69dcc..92109f2c4776 100644
--- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxControllerClassIndex.java
+++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxControllerClassIndex.java
@@ -165,10 +165,16 @@ public class JavaFxControllerClassIndex extends ScalarIndexExtension<String> {
   }
 
   public static List<VirtualFile> findFxmlsWithController(final Project project, @NotNull String className) {
-    return findFxmlWithController(project, className, Functions.id(), ProjectScope.getAllScope(project));
+    return findFxmlsWithController(project, className, ProjectScope.getAllScope(project));
   }
 
-  public static <T> List<T> findFxmlWithController(final Project project,
+  public static List<VirtualFile> findFxmlsWithController(final Project project,
+                                                          @NotNull String className,
+                                                          @NotNull GlobalSearchScope scope) {
+    return findFxmlWithController(project, className, Functions.id(), scope);
+  }
+
+  private static <T> List<T> findFxmlWithController(final Project project,
                                                      @NotNull final String className,
                                                      final Function<VirtualFile, T> f,
                                                      final GlobalSearchScope scope) {
diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxIdsIndex.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxIdsIndex.java
index 1de41873363d..2994766b4e03 100644
--- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxIdsIndex.java
+++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/indexing/JavaFxIdsIndex.java
@@ -96,8 +96,8 @@ public class JavaFxIdsIndex extends FileBasedIndexExtension<String, Set<String>>
   }
 
   @NotNull
-  public static Collection<String> getFilePaths(Project project, String id) {
+  public static Set<String> getFilePaths(Project project, String id) {
     final List<Set<String>> values = FileBasedIndex.getInstance().getValues(KEY, id, GlobalSearchScope.projectScope(project));
-    return (Collection<String>)(values.isEmpty() ? Collections.emptySet() : values.get(0));
+    return values.isEmpty() ? Collections.emptySet() : values.get(0);
   }
 }
\ No newline at end of file
diff --git a/plugins/javaFX/testData/highlighting/EventHandlerInSuperclass.java b/plugins/javaFX/testData/highlighting/EventHandlerInSuperclass.java
new file mode 100644
index 000000000000..a55704af8e8b
--- /dev/null
+++ b/plugins/javaFX/testData/highlighting/EventHandlerInSuperclass.java
@@ -0,0 +1,9 @@
+import javafx.event.ActionEvent;
+import javafx.fxml.FXML;
+
+public class EventHandlerInSuperclass {
+    @FXML
+    private void onAction(ActionEvent e) { System.out.println(e); }
+    @FXML
+    private void <warning descr="Private method 'onUnused(javafx.event.ActionEvent)' is never used">onUnused</warning>(ActionEvent e) { System.out.println(e); }
+}
\ No newline at end of file
diff --git a/plugins/javaFX/testData/highlighting/FxIdInSuperclass.java b/plugins/javaFX/testData/highlighting/FxIdInSuperclass.java
new file mode 100644
index 000000000000..308f019dba04
--- /dev/null
+++ b/plugins/javaFX/testData/highlighting/FxIdInSuperclass.java
@@ -0,0 +1,9 @@
+import javafx.fxml.FXML;
+import javafx.scene.control.Button;
+
+public class FxIdInSuperclass {
+    @FXML
+    private Button <warning descr="Private field 'inheritedButton' is assigned but never accessed">inheritedButton</warning>;
+    @FXML
+    private Button <warning descr="Private field 'unusedButton' is never used">unusedButton</warning>;
+}
\ No newline at end of file
diff --git a/plugins/javaFX/testData/highlighting/PrivateEventHandler.java b/plugins/javaFX/testData/highlighting/PrivateEventHandler.java
new file mode 100644
index 000000000000..0962188d0306
--- /dev/null
+++ b/plugins/javaFX/testData/highlighting/PrivateEventHandler.java
@@ -0,0 +1,9 @@
+import javafx.event.ActionEvent;
+import javafx.fxml.FXML;
+
+public class PrivateEventHandler {
+  @FXML
+  private void onAction(ActionEvent e) { System.out.println(e); }
+  @FXML
+  private void <warning descr="Private method 'onUnused(javafx.event.ActionEvent)' is never used">onUnused</warning>(ActionEvent e) { System.out.println(e); }
+}
\ No newline at end of file
diff --git a/plugins/javaFX/testData/highlighting/privateEventHandler.fxml b/plugins/javaFX/testData/highlighting/privateEventHandler.fxml
new file mode 100644
index 000000000000..ac5b295a75ed
--- /dev/null
+++ b/plugins/javaFX/testData/highlighting/privateEventHandler.fxml
@@ -0,0 +1,6 @@
+<?import javafx.scene.layout.VBox?>
+<?import javafx.scene.control.Button?>
+<VBox xmlns:fx="http://javafx.com/fxml/1" xmlns="http://javafx.com/javafx/8"
+      fx:controller="PrivateEventHandler">
+  <Button onAction="#onAction"/>
+</VBox>
\ No newline at end of file