Javafx: FXML inspection that validates event handler methods' signature. Code cleanup, move reused methods to utility class. Tests updated. (IDEA-151694)

2026-09-27 10:03:11 +07:00 · 2016-03-23 13:51:13 +03:00
parent 84ef850cfe
commit 4112f22f44
13 changed files with 162 additions and 118 deletions
@@ -48,6 +48,10 @@ public class JavaFxEventHandlerInspectionTest extends AbstractJavaFXTestCase {
    doHighlightingTest();
  }

+  public void testHighlightSuper() throws Exception {
+    doHighlightingTest();
+  }
+
  public void testHighlightWildcard() throws Exception {
    doHighlightingTest();
  }
@@ -121,7 +121,7 @@ public class JavaFxPsiUtil {
        }
      }
    }
-    return psiClass;
+    return null;
  }

  public static void insertImportWhenNeeded(XmlFile xmlFile,
@@ -825,22 +825,8 @@ public class JavaFxPsiUtil {
           InheritanceUtil.isInheritor(fieldType, JavaFxCommonNames.JAVAFX_COLLECTIONS_OBSERVABLE_MAP);
  }

-  public static boolean isNotFullyResolvedGeneric(@NotNull PsiClassType classType) {
-    final PsiClassType.ClassResolveResult resolveResult = classType.resolveGenerics();
-    final PsiClass psiClass = resolveResult.getElement();
-    if (psiClass == null || psiClass instanceof PsiTypeParameter) return true;
-    final PsiSubstitutor substitutor = resolveResult.getSubstitutor();
-    for (PsiTypeParameter parameter : PsiUtil.typeParametersIterable(psiClass)) {
-      final PsiType substitute = substitutor.substitute(parameter);
-      if (substitute == null || substitute instanceof PsiClassType && isNotFullyResolvedGeneric((PsiClassType)substitute)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
  @Nullable
-  public static PsiSubstitutor getTagClassSubstitutor(@NotNull XmlAttribute xmlAttribute, @NotNull PsiClass controllerClass) {
+  private static PsiSubstitutor getTagClassSubstitutor(@NotNull XmlAttribute xmlAttribute, @NotNull PsiClass controllerClass) {
    final XmlTag xmlTag = xmlAttribute.getParent();
    final PsiClass tagClass = getTagClass(xmlTag);
    if (tagClass != null) {
@@ -860,7 +846,26 @@ public class JavaFxPsiUtil {
  }

  @Nullable
-  public static PsiType getEventHandlerPropertyType(@NotNull PsiClass tagClass, @NotNull String eventName) {
+  public static PsiClassType getDeclaredEventType(@NotNull XmlAttribute xmlAttribute) {
+    final PsiClass tagClass = getTagClass(xmlAttribute.getParent());
+    if (tagClass != null) {
+      final PsiType eventHandlerPropertyType = getEventHandlerPropertyType(tagClass, xmlAttribute.getName());
+      if (eventHandlerPropertyType != null) {
+        final PsiClass controllerClass = getControllerClass(xmlAttribute.getContainingFile());
+        if (controllerClass != null) {
+          final PsiSubstitutor tagClassSubstitutor = getTagClassSubstitutor(xmlAttribute, controllerClass);
+
+          final PsiType handlerType = tagClassSubstitutor != null ?
+                                      tagClassSubstitutor.substitute(eventHandlerPropertyType) : eventHandlerPropertyType;
+          return substituteEventType(handlerType, xmlAttribute.getProject());
+        }
+      }
+    }
+    return null;
+  }
+
+  @Nullable
+  private static PsiType getEventHandlerPropertyType(@NotNull PsiClass tagClass, @NotNull String eventName) {
    final PsiMethod[] handlerSetterCandidates = tagClass.findMethodsByName(PropertyUtil.suggestSetterName(eventName), true);
    for (PsiMethod handlerSetter : handlerSetterCandidates) {
      if (!handlerSetter.hasModifierProperty(PsiModifier.STATIC) &&
@@ -880,15 +885,33 @@ public class JavaFxPsiUtil {
  }

  @Nullable
-  public static PsiType substituteEventType(@NotNull PsiClassType eventHandlerClass, @NotNull Project project) {
+  private static PsiClassType substituteEventType(@Nullable PsiType eventHandlerType, @NotNull Project project) {
+    if (!(eventHandlerType instanceof PsiClassType)) return null;
+    final PsiClassType.ClassResolveResult resolveResult = ((PsiClassType)eventHandlerType).resolveGenerics();
+    final PsiClass eventHandlerClass = resolveResult.getElement();
+    if (eventHandlerClass == null) return null;
+    final PsiSubstitutor eventHandlerClassSubstitutor = resolveResult.getSubstitutor();
+
    final PsiClass eventHandlerInterface =
      JavaPsiFacade.getInstance(project).findClass(JavaFxCommonNames.JAVAFX_EVENT_EVENT_HANDLER, GlobalSearchScope.allScope(project));
    if (eventHandlerInterface == null) return null;
+    if (!InheritanceUtil.isInheritorOrSelf(eventHandlerClass, eventHandlerInterface, true)) return null;
    final PsiTypeParameter[] typeParameters = eventHandlerInterface.getTypeParameters();
    if (typeParameters.length != 1) return null;
-    final PsiTypeParameter typeParameter = typeParameters[0];
-    final PsiSubstitutor substitutor = TypeConversionUtil.getSuperClassSubstitutor(eventHandlerInterface, eventHandlerClass);
-    return substitutor.substitute(typeParameter);
+    final PsiTypeParameter eventTypeParameter = typeParameters[0];
+    final PsiSubstitutor substitutor =
+      TypeConversionUtil.getSuperClassSubstitutor(eventHandlerInterface, eventHandlerClass, eventHandlerClassSubstitutor);
+    final PsiType eventType = substitutor.substitute(eventTypeParameter);
+    if (eventType instanceof PsiClassType) {
+      return (PsiClassType)eventType;
+    }
+    if (eventType instanceof PsiWildcardType) {
+      final PsiType boundType = ((PsiWildcardType)eventType).getBound();
+      if (boundType instanceof PsiClassType) {
+        return (PsiClassType)boundType;
+      }
+    }
+    return null;
  }

  private static class JavaFxControllerCachedValueProvider implements CachedValueProvider<PsiClass> {
@@ -1,17 +1,23 @@
 package org.jetbrains.plugins.javaFX.fxml.codeInsight.inspections;

-import com.intellij.codeInsight.intention.IntentionAction;
-import com.intellij.codeInsight.intention.QuickFixFactory;
-import com.intellij.codeInspection.IntentionWrapper;
+import com.intellij.codeInsight.FileModificationService;
+import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil;
 import com.intellij.codeInspection.LocalQuickFix;
+import com.intellij.codeInspection.LocalQuickFixOnPsiElement;
 import com.intellij.codeInspection.ProblemsHolder;
 import com.intellij.codeInspection.XmlSuppressableInspectionTool;
 import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
+import com.intellij.openapi.application.ApplicationManager;
+import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
 import com.intellij.psi.util.InheritanceUtil;
 import com.intellij.psi.xml.XmlAttribute;
 import com.intellij.psi.xml.XmlAttributeValue;
 import com.intellij.psi.xml.XmlFile;
+import com.intellij.refactoring.changeSignature.ChangeSignatureProcessor;
+import com.intellij.refactoring.changeSignature.JavaChangeSignatureDialog;
+import com.intellij.refactoring.changeSignature.ParameterInfoImpl;
+import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.javaFX.fxml.JavaFxCommonNames;
@@ -19,13 +25,11 @@ import org.jetbrains.plugins.javaFX.fxml.JavaFxFileTypeFactory;
 import org.jetbrains.plugins.javaFX.fxml.JavaFxPsiUtil;

 import javax.swing.*;
-import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;

-import static org.jetbrains.plugins.javaFX.fxml.JavaFxPsiUtil.getTagClass;
-
 /**
 * @author Pavel.Dolgov
 */
@@ -73,8 +77,7 @@ public class JavaFxEventHandlerInspection extends XmlSuppressableInspectionTool
            .ifPresent(ignored -> holder.registerProblem(xmlAttributeValue, "Return type of event handler should be void"));
        }

-        final PsiSubstitutor tagClassSubstitutor = JavaFxPsiUtil.getTagClassSubstitutor(attribute, controllerClass);
-        final PsiClassType declaredType = getDeclaredEventType(attribute, tagClassSubstitutor);
+        final PsiClassType declaredType = JavaFxPsiUtil.getDeclaredEventType(attribute);
        if (declaredType == null) return;

        for (PsiMethod method : eventHandlerMethods) {
@@ -83,7 +86,7 @@ public class JavaFxEventHandlerInspection extends XmlSuppressableInspectionTool
            final PsiType actualType = parameters[0].getType();
            if (actualType instanceof PsiClassType) {
              if (!actualType.isAssignableFrom(declaredType)) {
-                final LocalQuickFix quickFix = createChangeArgumentTypeQuickFix(controllerClass, declaredType, method, tagClassSubstitutor);
+                final LocalQuickFix quickFix = new ChangeParameterTypeQuickFix(method, declaredType);

                final PsiClassType actualRawType = ((PsiClassType)actualType).rawType();
                final PsiClassType expectedRawType = declaredType.rawType();
@@ -107,46 +110,6 @@ public class JavaFxEventHandlerInspection extends XmlSuppressableInspectionTool
    };
  }

-  @NotNull
-  private static LocalQuickFix createChangeArgumentTypeQuickFix(@NotNull PsiClass controllerClass,
-                                                                @NotNull PsiClassType declaredType,
-                                                                @NotNull PsiMethod method,
-                                                                @Nullable PsiSubstitutor tagClassSubstitutor) {
-    final PsiElementFactory psiElementFactory = JavaPsiFacade.getElementFactory(controllerClass.getProject());
-    final PsiExpression expression = psiElementFactory.createExpressionFromText("new " + declaredType.getCanonicalText() + "(){}", method);
-    final IntentionAction intentionAction = QuickFixFactory.getInstance()
-      .createChangeMethodSignatureFromUsageFix(method, new PsiExpression[]{expression},
-                                               tagClassSubstitutor != null ? tagClassSubstitutor : PsiSubstitutor.EMPTY,
-                                               controllerClass, false, 2);
-    return new IntentionWrapper(intentionAction, method.getContainingFile());
-  }
-
-  @Nullable
-  private static PsiClassType getDeclaredEventType(@NotNull XmlAttribute attribute, @Nullable PsiSubstitutor tagClassSubstitutor) {
-    final PsiClass tagClass = getTagClass(attribute.getParent());
-    if (tagClass == null) return null;
-
-    final PsiType eventHandlerPropertyType = JavaFxPsiUtil.getEventHandlerPropertyType(tagClass, attribute.getName());
-    if (eventHandlerPropertyType == null) return null;
-
-    final PsiType handlerType =
-      tagClassSubstitutor != null ? tagClassSubstitutor.substitute(eventHandlerPropertyType) : eventHandlerPropertyType;
-    if (handlerType instanceof PsiClassType) {
-      final PsiType eventType = JavaFxPsiUtil.substituteEventType((PsiClassType)handlerType, tagClass.getProject());
-      if (eventType instanceof PsiClassType) {
-        return (PsiClassType)eventType;
-      }
-      if (eventType instanceof PsiWildcardType) {
-        PsiWildcardType wildcardType = (PsiWildcardType)eventType;
-        if (wildcardType.isSuper()) {
-          final PsiType bound = wildcardType.getBound();
-          if (bound instanceof PsiClassType) return (PsiClassType)bound;
-        }
-      }
-    }
-    return null;
-  }
-
  private static boolean hasEventArgument(@NotNull PsiMethod method) {
    final PsiParameter[] parameters = method.getParameterList().getParameters();
    return parameters.length == 0 ||
@@ -158,4 +121,68 @@ public class JavaFxEventHandlerInspection extends XmlSuppressableInspectionTool
  public JComponent createOptionsPanel() {
    return new SingleCheckboxOptionsPanel("Detect event handler method having non-void return type", this, "myDetectNonVoidReturnType");
  }
+
+  private static class ChangeParameterTypeQuickFix extends LocalQuickFixOnPsiElement {
+    final PsiType mySuggestedParameterType;
+    final String myText;
+
+    public ChangeParameterTypeQuickFix(@NotNull PsiMethod method,
+                                       @NotNull PsiType suggestedParameterType) {
+      super(method);
+      mySuggestedParameterType = suggestedParameterType;
+      myText = "Change parameter type of '" + JavaHighlightUtil.formatMethod(method) +
+               "' to " + mySuggestedParameterType.getPresentableText();
+    }
+
+    @Override
+    public boolean startInWriteAction() {
+      return false;
+    }
+
+    @NotNull
+    @Override
+    public String getText() {
+      return myText;
+    }
+
+    @Nls
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Change parameter type of event handler method";
+    }
+
+    @Override
+    public boolean isAvailable(@NotNull Project project,
+                               @NotNull PsiFile file,
+                               @NotNull PsiElement startElement,
+                               @NotNull PsiElement endElement) {
+      return startElement instanceof PsiMethod &&
+             mySuggestedParameterType.isValid();
+    }
+
+    @Override
+    public void invoke(@NotNull Project project, @NotNull PsiFile file, @NotNull PsiElement startElement, @NotNull PsiElement endElement) {
+      if (!FileModificationService.getInstance().prepareFileForWrite(file)) return;
+      if (!(startElement instanceof PsiMethod)) return;
+      final PsiMethod method = (PsiMethod)startElement;
+      final PsiParameter[] parameters = method.getParameterList().getParameters();
+      final String parameterName = parameters.length >= 1 ? parameters[0].getName() : "e";
+
+      final ParameterInfoImpl parameterInfo = new ParameterInfoImpl(0, parameterName, mySuggestedParameterType);
+      if (ApplicationManager.getApplication().isUnitTestMode()) {
+        final ChangeSignatureProcessor processor =
+          new ChangeSignatureProcessor(project, method, false, null, method.getName(), method.getReturnType(),
+                                       new ParameterInfoImpl[]{parameterInfo});
+        processor.run();
+      }
+      else {
+        final List<ParameterInfoImpl> parameterInfos = Collections.singletonList(parameterInfo);
+        final JavaChangeSignatureDialog dialog =
+          JavaChangeSignatureDialog.createAndPreselectNew(project, method, parameterInfos, false, null);
+        dialog.setParameterInfos(parameterInfos);
+        dialog.show();
+      }
+    }
+  }
 }
@@ -37,13 +37,11 @@ import java.util.List;
 * Date: 1/16/13
 */
 public class JavaFxEventHandlerReference extends PsiReferenceBase<XmlAttributeValue> {
-  private final PsiClass myCurrentTagClass;
  private final PsiMethod myEventHandler;
  private final PsiClass myController;

-  public JavaFxEventHandlerReference(XmlAttributeValue element, PsiClass currentTagClass, final PsiMethod method, PsiClass controller) {
+  public JavaFxEventHandlerReference(XmlAttributeValue element, final PsiMethod method, PsiClass controller) {
    super(element);
-    myCurrentTagClass = currentTagClass;
    myEventHandler = method;
    myController = controller;
  }
@@ -104,27 +102,10 @@ public class JavaFxEventHandlerReference extends PsiReferenceBase<XmlAttributeVa
      final XmlAttributeValue element = ref.getElement();
      String canonicalText = JavaFxCommonNames.JAVAFX_EVENT;
      final PsiElement parent = element.getParent();
-      if (parent instanceof XmlAttribute && ref.myCurrentTagClass != null) {
-        final XmlAttribute xmlAttribute = (XmlAttribute)parent;
-        final PsiType eventHandlerPropertyType = JavaFxPsiUtil.getEventHandlerPropertyType(ref.myCurrentTagClass, xmlAttribute.getName());
-        if (eventHandlerPropertyType != null) {
-          final PsiSubstitutor tagClassSubstitutor = JavaFxPsiUtil.getTagClassSubstitutor(xmlAttribute, ref.myController);
-          final PsiType handlerType =
-            tagClassSubstitutor != null ? tagClassSubstitutor.substitute(eventHandlerPropertyType) : eventHandlerPropertyType;
-          if (handlerType instanceof PsiClassType) {
-            PsiType eventType = JavaFxPsiUtil.substituteEventType((PsiClassType)handlerType, element.getProject());
-            if (eventType instanceof PsiWildcardType && ((PsiWildcardType)eventType).isSuper()) {
-              eventType = ((PsiWildcardType)eventType).getBound();
-            }
-            if (eventType != null) {
-              if (eventType instanceof PsiClassType && JavaFxPsiUtil.isNotFullyResolvedGeneric((PsiClassType)eventType)) {
-                canonicalText = ((PsiClassType)eventType).rawType().getCanonicalText();
-              }
-              else {
-                canonicalText = eventType.getCanonicalText();
-              }
-            }
-          }
+      if (parent instanceof XmlAttribute) {
+        final PsiClassType eventType = JavaFxPsiUtil.getDeclaredEventType((XmlAttribute)parent);
+        if (eventType != null) {
+          canonicalText = eventType.getCanonicalText();
        }
      }
      return "public void " + element.getValue().substring(1) + "(" + canonicalText + " e)";
@@ -50,17 +50,12 @@ class JavaFxEventHandlerReferenceProvider extends JavaFxControllerBasedReference
    final XmlAttribute attribute = (XmlAttribute)xmlAttributeValue.getContext();
    if (attribute == null) return PsiReference.EMPTY_ARRAY;
    if (!JavaFxPsiUtil.checkIfAttributeHandler(attribute)) return PsiReference.EMPTY_ARRAY;
-    final XmlElementDescriptor descriptor = attribute.getParent().getDescriptor();
-    if (descriptor == null) return PsiReference.EMPTY_ARRAY;
-    final PsiElement declaration = descriptor.getDeclaration();
-    if (!(declaration instanceof PsiClass)) return PsiReference.EMPTY_ARRAY;
-    final PsiClass currentTagClass = ((PsiClass)declaration);
    final String eventHandlerName = attValueString.substring(1);
    final PsiMethod[] methods = controllerClass.findMethodsByName(eventHandlerName, true);

    final PsiReference[] references = Arrays.stream(methods)
      .filter(JavaFxEventHandlerReference::isHandlerMethod)
-      .map(handlerMethod -> new JavaFxEventHandlerReference(xmlAttributeValue, currentTagClass, handlerMethod, controllerClass))
+      .map(handlerMethod -> new JavaFxEventHandlerReference(xmlAttributeValue, handlerMethod, controllerClass))
      .toArray(PsiReference.ARRAY_FACTORY::create);

    if (references.length == 1) {
@@ -76,6 +71,6 @@ class JavaFxEventHandlerReferenceProvider extends JavaFxControllerBasedReference
        return PsiReference.EMPTY_ARRAY;
      }
    }
-    return new PsiReference[]{new JavaFxEventHandlerReference(xmlAttributeValue, currentTagClass, null, controllerClass)};
+    return new PsiReference[]{new JavaFxEventHandlerReference(xmlAttributeValue, null, controllerClass)};
  }
 }
@@ -1,12 +1,13 @@
 import javafx.fxml.FXML;
 import javafx.scene.control.*;
+import javafx.scene.input.*;

 public class HighlightRaw {
    @FXML TableView positive;
    @FXML TableView negative;

    public void sortPositive(SortEvent e) {}
-    public void scrollPositive(ScrollToEvent e) {}
+    public void scrollPositive(ScrollToEvent<TableColumn> e) {}

    public void sortNegative(ScrollToEvent e) {}
    public void scrollNegative(ScrollToEvent<TableView> e) {}
@@ -0,0 +1,12 @@
+import javafx.event.Event;
+import javafx.fxml.FXML;
+import javafx.scene.control.*;
+import javafx.scene.input.*;
+
+public class HighlightSuper {
+    @FXML private void onSameArg(MouseEvent e) {}
+    @FXML private void onSuperArg(Event e) {}
+    @FXML private void onNoArg() {}
+    @FXML private void onNotSuper(MouseDragEvent e) {}
+    @FXML private void onNotRelated(SortEvent e) {}
+}
@@ -1,13 +1,8 @@
 import javafx.event.Event;
 import javafx.fxml.FXML;
-import javafx.scene.control.SortEvent;
-import javafx.scene.input.MouseDragEvent;
-import javafx.scene.input.MouseEvent;
+import javafx.scene.control.*;

 public class HighlightWildcard {
-    @FXML private void onSameArg(MouseEvent e) {}
-    @FXML private void onSuperArg(Event e) {}
-    @FXML private void onNoArg() {}
-    @FXML private void onNotSuper(MouseDragEvent e) {}
-    @FXML private void onNotRelated(SortEvent e) {}
+    @FXML private void scrollPositive(ScrollToEvent<? extends Number> e) {}
+    @FXML private void scrollNegative(ScrollToEvent<? super Number> e) {}
 }
@@ -6,7 +6,7 @@ import javafx.util.Pair;
 public class QuickfixHalfRaw {
  @FXML TableView<Pair> table;

-    public void onSort(SortEvent sortEvent) {
+    public void onSort(SortEvent<TableView<Pair>> tableViewSortEvent) {

    }
 }
@@ -5,7 +5,7 @@ import javafx.scene.control.TableView;
 public class QuickfixRaw {
  @FXML TableView table;

-    public void onSort(SortEvent sortEvent) {
+    public void onSort(SortEvent<TableView> tableViewSortEvent) {

    }
 }
@@ -0,0 +1,9 @@
+<?import javafx.scene.control.*?>
+<?import javafx.scene.layout.HBox?>
+<HBox xmlns:fx="http://javafx.com/fxml" fx:controller="HighlightSuper">
+    <Label text=" SameArg " onMouseClicked="#onSameArg"/>
+    <Label text=" SuperArg " onMouseClicked="#onSuperArg"/>
+    <Label text=" NoArg " onMouseClicked="#onNoArg"/>
+    <Label text=" NotSuper " onMouseClicked=<warning descr="Incompatible event handler argument: javafx.scene.input.MouseDragEvent is not assignable from javafx.scene.input.MouseEvent">"#onNotSuper"</warning>/>
+    <Label text=" NotRelated " onMouseClicked=<warning descr="Incompatible event handler argument: javafx.scene.control.SortEvent is not assignable from javafx.scene.input.MouseEvent">"#onNotRelated"</warning>/>
+</HBox>
@@ -1,9 +1,6 @@
 <?import javafx.scene.control.*?>
 <?import javafx.scene.layout.HBox?>
 <HBox xmlns:fx="http://javafx.com/fxml" fx:controller="HighlightWildcard">
-    <Label text=" SameArg " onMouseClicked="#onSameArg"/>
-    <Label text=" SuperArg " onMouseClicked="#onSuperArg"/>
-    <Label text=" NoArg " onMouseClicked="#onNoArg"/>
-    <Label text=" NotSuper " onMouseClicked=<warning descr="Incompatible event handler argument: javafx.scene.input.MouseDragEvent is not assignable from javafx.scene.input.MouseEvent">"#onNotSuper"</warning>/>
-    <Label text=" NotRelated " onMouseClicked=<warning descr="Incompatible event handler argument: javafx.scene.control.SortEvent is not assignable from javafx.scene.input.MouseEvent">"#onNotRelated"</warning>/>
+    <TableView onScrollTo="#scrollPositive"/>
+    <TableView onScrollTo=<warning descr="Incompatible generic parameter of event handler argument: javafx.scene.control.ScrollToEvent<? super java.lang.Number> is not assignable from javafx.scene.control.ScrollToEvent<java.lang.Integer>">"#scrollNegative"</warning>/>
 </HBox>
@@ -8,7 +8,7 @@ public class CreateControllerMethodHalfRaw {
    @FXML
    TableView<Map> table;

-    public void onSort(SortEvent sortEvent) {
+    public void onSort(SortEvent<TableView<Map>> tableViewSortEvent) {

    }
 }