diff --git a/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXDefaultTagInspectionTest.java b/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXDefaultTagInspectionTest.java index 62f9c3f2190b..28d7fa26ce56 100644 --- a/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXDefaultTagInspectionTest.java +++ b/plugins/javaFX/javaFX-CE/testSrc/org/jetbrains/plugins/javaFX/fxml/JavaFXDefaultTagInspectionTest.java @@ -39,10 +39,27 @@ public class JavaFXDefaultTagInspectionTest extends AbstractJavaFXQuickFixTest { doLaunchQuickfixTest("children"); } + public void testFxCollectionsHighlighting() throws Exception { + doHighlightingTest(); + } + + public void testEmptyListHighlighting() throws Exception { + doHighlightingTest(); + } + + public void testEmptyCollapsedListHighlighting() throws Exception { + doHighlightingTest(); + } + public void testStylesheets() throws Exception { checkQuickFixNotAvailable("stylesheets"); } + private void doHighlightingTest() throws Exception { + myFixture.configureByFiles(getTestName(true) + ".fxml"); + myFixture.checkHighlighting(); + } + @Override protected String getHint(String tagName) { return "Unwrap '" + tagName + "'"; 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 3441c200191b..b72970c4e850 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/JavaFxPsiUtil.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/JavaFxPsiUtil.java @@ -978,7 +978,7 @@ public class JavaFxPsiUtil { InheritanceUtil.isInheritor(fieldType, JavaFxCommonNames.JAVAFX_COLLECTIONS_OBSERVABLE_MAP); } - private static boolean isObservableCollection(@Nullable PsiClass psiClass) { + public static boolean isObservableCollection(@Nullable PsiClass psiClass) { return psiClass != null && (InheritanceUtil.isInheritor(psiClass, JavaFxCommonNames.JAVAFX_COLLECTIONS_OBSERVABLE_LIST) || InheritanceUtil.isInheritor(psiClass, JavaFxCommonNames.JAVAFX_COLLECTIONS_OBSERVABLE_SET) || @@ -1077,7 +1077,7 @@ public class JavaFxPsiUtil { } @Nullable - public static PsiClass getFactoryProducedClass(@Nullable PsiClass psiClass, @Nullable String factoryMethodName) { + private static PsiClass getFactoryProducedClass(@Nullable PsiClass psiClass, @Nullable String factoryMethodName) { if (psiClass == null || factoryMethodName == null) return null; final PsiMethod[] methods = psiClass.findMethodsByName(factoryMethodName, true); for (PsiMethod method : methods) { @@ -1112,6 +1112,33 @@ public class JavaFxPsiUtil { return propertyName != null ? propertyName : memberName; } + @NotNull + public static PsiClass getTagClassReplacement(@NotNull XmlTag xmlTag) { + return getTagClassReplacement(xmlTag, getTagClass(xmlTag)).getFirst(); + } + + @NotNull + public static Pair getTagClassReplacement(@NotNull XmlTag xmlTag, @Nullable PsiClass tagClass) { + if (tagClass != null) { + final XmlAttribute constAttr = xmlTag.getAttribute(FxmlConstants.FX_CONSTANT); + if (constAttr != null) { + final PsiField constField = tagClass.findFieldByName(constAttr.getValue(), true); + if (constField != null) { + final PsiType constType = constField.getType(); + return Pair.create(PsiUtil.resolveClassInClassTypeOnly( + constType instanceof PsiPrimitiveType ? ((PsiPrimitiveType)constType).getBoxedType(xmlTag) : constType), true); + } + } + else { + final XmlAttribute factoryAttr = xmlTag.getAttribute(FxmlConstants.FX_FACTORY); + if (factoryAttr != null) { + return Pair.create(getFactoryProducedClass(tagClass, factoryAttr.getValue()), true); + } + } + } + return Pair.create(tagClass, false); + } + private static class JavaFxControllerCachedValueProvider implements CachedValueProvider { private final Project myProject; private final PsiFile myContainingFile; diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxDefaultTagInspection.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxDefaultTagInspection.java index 5c52cb633ca0..a74362ac99d2 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxDefaultTagInspection.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/JavaFxDefaultTagInspection.java @@ -15,11 +15,16 @@ */ package org.jetbrains.plugins.javaFX.fxml.codeInsight.inspections; -import com.intellij.codeInspection.*; +import com.intellij.codeInspection.LocalInspectionToolSession; +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.codeInspection.XmlSuppressableInspectionTool; import com.intellij.openapi.util.Comparing; +import com.intellij.openapi.util.TextRange; import com.intellij.psi.*; +import com.intellij.psi.util.PsiUtil; import com.intellij.psi.xml.XmlTag; import com.intellij.xml.XmlElementDescriptor; +import com.intellij.xml.util.XmlTagUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.plugins.javaFX.fxml.JavaFxPsiUtil; import org.jetbrains.plugins.javaFX.fxml.descriptors.JavaFxPropertyTagDescriptor; @@ -39,19 +44,36 @@ public class JavaFxDefaultTagInspection extends XmlSuppressableInspectionTool{ super.visitXmlTag(tag); final XmlElementDescriptor descriptor = tag.getDescriptor(); if (descriptor instanceof JavaFxPropertyTagDescriptor) { - final XmlTag parentTag = tag.getParentTag(); - if (parentTag != null) { - final String propertyName = JavaFxPsiUtil.getDefaultPropertyName(JavaFxPsiUtil.getTagClass(parentTag)); + final PsiClass parentTagClass = JavaFxPsiUtil.getTagClass(tag.getParentTag()); + if (parentTagClass != null) { + final String propertyName = JavaFxPsiUtil.getDefaultPropertyName(parentTagClass); final String tagName = tag.getName(); - if (Comparing.strEqual(tagName, propertyName)) { - holder.registerProblem(tag.getFirstChild(), - "Default property tag could be removed", - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - new UnwrapTagFix(tagName)); + if (Comparing.strEqual(tagName, propertyName) && !isCollectionAssignment(parentTagClass, propertyName, tag)) { + final TextRange startTagRange = XmlTagUtil.getStartTagRange(tag); + final TextRange rangeInElement = startTagRange != null ? startTagRange.shiftRight(-tag.getTextOffset()) : null; + holder.registerProblem(tag, rangeInElement, "Default property tag could be removed", new UnwrapTagFix(tagName)); } } } } }; } + + private static boolean isCollectionAssignment(@NotNull PsiClass parentTagClass, @NotNull String propertyName, @NotNull XmlTag tag) { + final XmlTag[] subTags = tag.getSubTags(); + if (subTags.length != 0) { + final PsiClass contentClass = JavaFxPsiUtil.getTagClassReplacement(subTags[subTags.length - 1]); + if (JavaFxPsiUtil.isObservableCollection(contentClass)) { + final PsiMember property = JavaFxPsiUtil.collectWritableProperties(parentTagClass).get(propertyName); + if (property != null) { + final PsiType propertyType = JavaFxPsiUtil.getWritablePropertyType(parentTagClass, property); + final PsiClass propertyClass = PsiUtil.resolveClassInClassTypeOnly(propertyType); + if (JavaFxPsiUtil.isObservableCollection(propertyClass)) { + return true; + } + } + } + } + return false; + } } diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/UnwrapTagFix.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/UnwrapTagFix.java index 46ca065d3d38..e22f6f11e1a6 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/UnwrapTagFix.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/inspections/UnwrapTagFix.java @@ -49,7 +49,7 @@ public class UnwrapTagFix implements LocalQuickFix { @NotNull @Override public String getFamilyName() { - return getName(); + return "Unwrap tag"; } @Override @@ -57,18 +57,19 @@ public class UnwrapTagFix implements LocalQuickFix { final PsiElement element = descriptor.getPsiElement(); if (element != null) { final PsiFile containingFile = element.getContainingFile(); - LOG.assertTrue(containingFile != null && JavaFxFileTypeFactory.isFxml(containingFile), containingFile == null ? "no containing file found" : "containing file: " + containingFile.getName()); - final XmlTag xmlTag = PsiTreeUtil.getParentOfType(element, XmlTag.class); + LOG.assertTrue(containingFile != null && JavaFxFileTypeFactory.isFxml(containingFile), + containingFile == null ? "no containing file found" : "containing file: " + containingFile.getName()); + final XmlTag xmlTag = PsiTreeUtil.getParentOfType(element, XmlTag.class, false); if (xmlTag != null) { final XmlTag parentTag = xmlTag.getParentTag(); final PsiElement[] children = PsiTreeUtil.getChildrenOfType(xmlTag, XmlTagChild.class); - if (children != null) { - if (!FileModificationService.getInstance().preparePsiElementsForWrite(element)) return; - if (children.length > 0) { - parentTag.addRange(children[0], children[children.length - 1]); - } - xmlTag.delete(); - CodeStyleManager.getInstance(project).reformat(parentTag); + if (!FileModificationService.getInstance().preparePsiElementsForWrite(element)) return; + if (children != null && children.length > 0 && parentTag != null) { + parentTag.addRange(children[0], children[children.length - 1]); + } + xmlTag.delete(); + if (parentTag != null) { + CodeStyleManager.getInstance(project).reformat(parentTag, true); } } } diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/descriptors/JavaFxClassTagDescriptorBase.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/descriptors/JavaFxClassTagDescriptorBase.java index 8a3909922f3f..a87589d2765c 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/descriptors/JavaFxClassTagDescriptorBase.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/descriptors/JavaFxClassTagDescriptorBase.java @@ -4,6 +4,7 @@ import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.daemon.Validator; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; +import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; @@ -311,24 +312,11 @@ public abstract class JavaFxClassTagDescriptorBase implements XmlElementDescript host.addMessage(attribute.getNameElement(), "fx:controller can only be applied to root element", ValidationHost.ErrorType.ERROR); //todo add delete/move to upper tag fix } } - PsiClass aClass = getPsiClass(); - final XmlAttribute constAttr = context.getAttribute(FxmlConstants.FX_CONSTANT); - final XmlAttribute factoryAttr = context.getAttribute(FxmlConstants.FX_FACTORY); - if (constAttr != null && aClass != null) { - final PsiField constField = aClass.findFieldByName(constAttr.getValue(), true); - if (constField != null) { - final PsiType constType = constField.getType(); - aClass = PsiUtil.resolveClassInClassTypeOnly( - constType instanceof PsiPrimitiveType ? ((PsiPrimitiveType)constType).getBoxedType(context) : constType); - } - } else { - if (factoryAttr != null) { - aClass = JavaFxPsiUtil.getFactoryProducedClass(aClass, factoryAttr.getValue()); - } - } + final Pair replacement = JavaFxPsiUtil.getTagClassReplacement(context, getPsiClass()); + final PsiClass aClass = replacement.getFirst(); JavaFxPsiUtil.isClassAcceptable(parentTag, aClass, (errorMessage, errorType) -> host.addMessage(context.getNavigationElement(), errorMessage, errorType)); - boolean needInstantiate = constAttr == null && factoryAttr == null; + boolean needInstantiate = !replacement.getSecond(); if (needInstantiate && aClass != null && aClass.isValid()) { JavaFxPsiUtil.isAbleToInstantiate(aClass, errorMessage -> host.addMessage(context, errorMessage, ValidationHost.ErrorType.ERROR)); diff --git a/plugins/javaFX/testData/inspections/defaultTag/emptyCollapsedListHighlighting.fxml b/plugins/javaFX/testData/inspections/defaultTag/emptyCollapsedListHighlighting.fxml new file mode 100644 index 000000000000..10057dd937fb --- /dev/null +++ b/plugins/javaFX/testData/inspections/defaultTag/emptyCollapsedListHighlighting.fxml @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file diff --git a/plugins/javaFX/testData/inspections/defaultTag/emptyListHighlighting.fxml b/plugins/javaFX/testData/inspections/defaultTag/emptyListHighlighting.fxml new file mode 100644 index 000000000000..258c4383f977 --- /dev/null +++ b/plugins/javaFX/testData/inspections/defaultTag/emptyListHighlighting.fxml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/plugins/javaFX/testData/inspections/defaultTag/fxCollectionsHighlighting.fxml b/plugins/javaFX/testData/inspections/defaultTag/fxCollectionsHighlighting.fxml new file mode 100644 index 000000000000..0aaa964196a0 --- /dev/null +++ b/plugins/javaFX/testData/inspections/defaultTag/fxCollectionsHighlighting.fxml @@ -0,0 +1,17 @@ + + + + + +
+ + + + + + + + + +
+
\ No newline at end of file