diff --git a/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFxEventHandlerInspectionTest.java b/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFxEventHandlerInspectionTest.java index 80d507164041..7a39d85a2283 100644 --- a/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFxEventHandlerInspectionTest.java +++ b/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFxEventHandlerInspectionTest.java @@ -48,6 +48,10 @@ public class JavaFxEventHandlerInspectionTest extends AbstractJavaFXTestCase { doHighlightingTest(); } + public void testHighlightSuper() throws Exception { + doHighlightingTest(); + } + public void testHighlightWildcard() throws Exception { doHighlightingTest(); } diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/JavaFxPsiUtil.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/JavaFxPsiUtil.java index 59516d09cc67..e05619e28208 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/JavaFxPsiUtil.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/JavaFxPsiUtil.java @@ -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 { diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxEventHandlerInspection.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxEventHandlerInspection.java index 962abbd1eb7d..8fa72e20d433 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxEventHandlerInspection.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxEventHandlerInspection.java @@ -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 parameterInfos = Collections.singletonList(parameterInfo); + final JavaChangeSignatureDialog dialog = + JavaChangeSignatureDialog.createAndPreselectNew(project, method, parameterInfos, false, null); + dialog.setParameterInfos(parameterInfos); + dialog.show(); + } + } + } } diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxEventHandlerReference.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxEventHandlerReference.java index b23d38455079..2bbad76d3b8f 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxEventHandlerReference.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/refs/JavaFxEventHandlerReference.java @@ -37,13 +37,11 @@ import java.util.List; * Date: 1/16/13 */ public class JavaFxEventHandlerReference extends PsiReferenceBase { - 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 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)}; } } diff --git a/plugins/javaFX/testData/inspections/eventHandler/HighlightRaw.java b/plugins/javaFX/testData/inspections/eventHandler/HighlightRaw.java index fee21b0a7793..6b999fb91b6b 100644 --- a/plugins/javaFX/testData/inspections/eventHandler/HighlightRaw.java +++ b/plugins/javaFX/testData/inspections/eventHandler/HighlightRaw.java @@ -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 e) {} public void sortNegative(ScrollToEvent e) {} public void scrollNegative(ScrollToEvent e) {} diff --git a/plugins/javaFX/testData/inspections/eventHandler/HighlightSuper.java b/plugins/javaFX/testData/inspections/eventHandler/HighlightSuper.java new file mode 100644 index 000000000000..fbe229947dec --- /dev/null +++ b/plugins/javaFX/testData/inspections/eventHandler/HighlightSuper.java @@ -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) {} +} diff --git a/plugins/javaFX/testData/inspections/eventHandler/HighlightWildcard.java b/plugins/javaFX/testData/inspections/eventHandler/HighlightWildcard.java index 40c969b14e88..c9c236c3a27e 100644 --- a/plugins/javaFX/testData/inspections/eventHandler/HighlightWildcard.java +++ b/plugins/javaFX/testData/inspections/eventHandler/HighlightWildcard.java @@ -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 e) {} + @FXML private void scrollNegative(ScrollToEvent e) {} } diff --git a/plugins/javaFX/testData/inspections/eventHandler/QuickfixHalfRaw_after.java b/plugins/javaFX/testData/inspections/eventHandler/QuickfixHalfRaw_after.java index c6dd5f73ae91..95e7e418114f 100644 --- a/plugins/javaFX/testData/inspections/eventHandler/QuickfixHalfRaw_after.java +++ b/plugins/javaFX/testData/inspections/eventHandler/QuickfixHalfRaw_after.java @@ -6,7 +6,7 @@ import javafx.util.Pair; public class QuickfixHalfRaw { @FXML TableView table; - public void onSort(SortEvent sortEvent) { + public void onSort(SortEvent> tableViewSortEvent) { } } diff --git a/plugins/javaFX/testData/inspections/eventHandler/QuickfixRaw_after.java b/plugins/javaFX/testData/inspections/eventHandler/QuickfixRaw_after.java index 5a0886c9dfca..73bd806e4d9c 100644 --- a/plugins/javaFX/testData/inspections/eventHandler/QuickfixRaw_after.java +++ b/plugins/javaFX/testData/inspections/eventHandler/QuickfixRaw_after.java @@ -5,7 +5,7 @@ import javafx.scene.control.TableView; public class QuickfixRaw { @FXML TableView table; - public void onSort(SortEvent sortEvent) { + public void onSort(SortEvent tableViewSortEvent) { } } diff --git a/plugins/javaFX/testData/inspections/eventHandler/highlightSuper.fxml b/plugins/javaFX/testData/inspections/eventHandler/highlightSuper.fxml new file mode 100644 index 000000000000..3534349be426 --- /dev/null +++ b/plugins/javaFX/testData/inspections/eventHandler/highlightSuper.fxml @@ -0,0 +1,9 @@ + + + + diff --git a/plugins/javaFX/testData/inspections/eventHandler/highlightWildcard.fxml b/plugins/javaFX/testData/inspections/eventHandler/highlightWildcard.fxml index 017f0e8641b8..ed6b598e2c27 100644 --- a/plugins/javaFX/testData/inspections/eventHandler/highlightWildcard.fxml +++ b/plugins/javaFX/testData/inspections/eventHandler/highlightWildcard.fxml @@ -1,9 +1,6 @@ - diff --git a/plugins/javaFX/testData/quickfix/CreateControllerMethodHalfRaw_after.java b/plugins/javaFX/testData/quickfix/CreateControllerMethodHalfRaw_after.java index 2c68db990f3d..d064fe1f4bb2 100644 --- a/plugins/javaFX/testData/quickfix/CreateControllerMethodHalfRaw_after.java +++ b/plugins/javaFX/testData/quickfix/CreateControllerMethodHalfRaw_after.java @@ -8,7 +8,7 @@ public class CreateControllerMethodHalfRaw { @FXML TableView table; - public void onSort(SortEvent sortEvent) { + public void onSort(SortEvent> tableViewSortEvent) { } }