Javafx: Don't report default tag problem when the default tag is a collection and its contents is also a collection. Move a reused method to utility class. Tests added. (IDEA-153662)

This commit is contained in:
Pavel Dolgov
2016-05-18 16:23:02 +03:00
parent 788df85d0c
commit 85c75a9f79
8 changed files with 124 additions and 37 deletions
@@ -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 + "'";
@@ -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<PsiClass, Boolean> 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<PsiClass> {
private final Project myProject;
private final PsiFile myContainingFile;
@@ -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;
}
}
@@ -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);
}
}
}
@@ -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<PsiClass, Boolean> 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));
@@ -0,0 +1,7 @@
<?import javafx.scene.layout.VBox?>
<?import javafx.scene.control.ListView?>
<VBox xmlns:fx="http://javafx.com/fxml">
<ListView fx:id="list">
<warning descr="Default property tag could be removed"><items/></warning>
</ListView>
</VBox>
@@ -0,0 +1,8 @@
<?import javafx.scene.layout.VBox?>
<?import javafx.scene.control.ListView?>
<VBox xmlns:fx="http://javafx.com/fxml">
<ListView fx:id="list">
<warning descr="Default property tag could be removed"><items></warning>
</items>
</ListView>
</VBox>
@@ -0,0 +1,17 @@
<?import javafx.scene.layout.BorderPane?>
<?import javafx.scene.control.ListView?>
<?import javafx.collections.FXCollections?>
<?import java.lang.String?>
<BorderPane xmlns:fx="http://javafx.com/fxml">
<center>
<ListView fx:id="list">
<items>
<FXCollections fx:factory="observableArrayList">
<String fx:value="line1"/>
<String fx:value="line2"/>
<String fx:value="line3"/>
</FXCollections>
</items>
</ListView>
</center>
</BorderPane>