diff --git a/plugins/devkit/src/inspections/QuickFixGetFamilyNameViolationInspection.java b/plugins/devkit/src/inspections/QuickFixGetFamilyNameViolationInspection.java index 8446d03a51c3..5d8498a7ea22 100644 --- a/plugins/devkit/src/inspections/QuickFixGetFamilyNameViolationInspection.java +++ b/plugins/devkit/src/inspections/QuickFixGetFamilyNameViolationInspection.java @@ -17,9 +17,13 @@ package org.jetbrains.idea.devkit.inspections; import com.intellij.codeInspection.*; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.extensions.AreaInstance; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.pom.Navigatable; import com.intellij.psi.*; import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.containers.ContainerUtil; import gnu.trove.THashSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -33,6 +37,11 @@ import java.util.Set; public class QuickFixGetFamilyNameViolationInspection extends DevKitInspectionBase { private final static Logger LOG = Logger.getInstance(QuickFixGetFamilyNameViolationInspection.class); + private final static Set BASE_CONTEXT_AWARE_CLASSES = ContainerUtil.newHashSet(PsiElement.class.getName(), + Navigatable.class.getName(), + AreaInstance.class.getName(), + VirtualFile.class.getName()); + @Nullable @Override public ProblemDescriptor[] checkMethod(@NotNull PsiMethod method, @NotNull InspectionManager manager, boolean isOnTheFly) { @@ -55,28 +64,35 @@ public class QuickFixGetFamilyNameViolationInspection extends DevKitInspectionBa if (!processed.add(method) || method.hasModifierProperty(PsiModifier.STATIC)) return false; final PsiCodeBlock body = method.getBody(); if (body == null) return false; + if (isContextDependentType(method.getReturnType())) { + return true; + } final Collection referenceIterator = PsiTreeUtil.findChildrenOfType(body, PsiJavaCodeReferenceElement.class); for (PsiJavaCodeReferenceElement reference : referenceIterator) { final PsiElement resolved = reference.resolve(); if (resolved instanceof PsiVariable) { - if ((resolved instanceof PsiLocalVariable || resolved instanceof PsiParameter) && !PsiTreeUtil.isAncestor(body, resolved, false)) { - return true; - } - if (resolved instanceof PsiField && !((PsiField)resolved).hasModifierProperty(PsiModifier.STATIC)) { + if (!(resolved instanceof PsiField && ((PsiField)resolved).hasModifierProperty(PsiModifier.STATIC)) && isContextDependentType(((PsiVariable)resolved).getType())) { return true; } } - if (resolved instanceof PsiMethod && !((PsiMethod)resolved).hasModifierProperty(PsiModifier.STATIC)) { - final PsiClass resolvedContainingClass = ((PsiMethod)resolved).getContainingClass(); + if (resolved instanceof PsiMethod) { + final PsiMethod resolvedMethod = (PsiMethod)resolved; + final PsiClass resolvedContainingClass = resolvedMethod.getContainingClass(); + //if (resolvedMethod.getName().equals("getName") && + // resolvedMethod.getParameterList().getParametersCount() == 0 && + // !resolvedMethod.hasModifierProperty(PsiModifier.STATIC) && + // InheritanceUtil.isInheritor(resolvedContainingClass, QuickFix.class.getName())) { + // return true; + //} final PsiClass methodContainingClass = method.getContainingClass(); if (resolvedContainingClass != null && methodContainingClass != null && (methodContainingClass == resolvedContainingClass || methodContainingClass.isInheritor(resolvedContainingClass, true))) { - if (doesMethodViolate((PsiMethod)resolved, processed)) { + if (doesMethodViolate(resolvedMethod, processed)) { return true; } } @@ -84,4 +100,16 @@ public class QuickFixGetFamilyNameViolationInspection extends DevKitInspectionBa } return false; } + + private static boolean isContextDependentType(@Nullable PsiType type) { + if (type == null) return false; + + for (String aClass : BASE_CONTEXT_AWARE_CLASSES) { + if (InheritanceUtil.isInheritor(type, aClass)) { + return true; + } + } + + return false; + } } diff --git a/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByExternalParameter.java b/plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByExternalParameter.java similarity index 64% rename from plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByExternalParameter.java rename to plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByExternalParameter.java index dc4014552931..af497ebf5053 100644 --- a/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByExternalParameter.java +++ b/plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByExternalParameter.java @@ -8,7 +8,7 @@ class A { return "some name"; }; - public String getFamilyName() { + public String getFamilyName() { return someParameter + "123"; }; }; diff --git a/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByField.java b/plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByField.java similarity index 60% rename from plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByField.java rename to plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByField.java index c948ae538462..44ff062f6740 100644 --- a/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByField.java +++ b/plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByField.java @@ -8,7 +8,7 @@ class MyQuickFix implements QuickFix { return "some name"; }; - public String getFamilyName() { + public String getFamilyName() { return someField + getName() + "123"; }; diff --git a/plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByGetName.java b/plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByGetName.java new file mode 100644 index 000000000000..4f465db74d12 --- /dev/null +++ b/plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByGetName.java @@ -0,0 +1,16 @@ +import com.intellij.codeInspection.QuickFix; + +class MyQuickFix implements QuickFix { + + String someField; + + public String getName() { + return someField; + }; + + public String getFamilyName() { + return getName() + "123"; + }; + + +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByGetName.java b/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByPsiElementFieldUsage.java similarity index 72% rename from plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByGetName.java rename to plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByPsiElementFieldUsage.java index 35a461b33a20..abb5a55ca0a8 100644 --- a/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByGetName.java +++ b/plugins/devkit/testData/inspections/getFamilyNameViolation/ViolationByPsiElementFieldUsage.java @@ -1,16 +1,17 @@ import com.intellij.codeInspection.QuickFix; +import com.intellij.psi.PsiElement; class MyQuickFix implements QuickFix { String someField; + PsiElement myElement; public String getName() { return someField; }; public String getFamilyName() { - return getName() + "123"; + return "error is here: " + String.valueOf(myElement); }; - } \ No newline at end of file diff --git a/plugins/devkit/testSources/inspections/QuickFixGetFamilyNameViolationInspectionTest.java b/plugins/devkit/testSources/inspections/QuickFixGetFamilyNameViolationInspectionTest.java index 10d4d984aa20..e090ff198a7a 100644 --- a/plugins/devkit/testSources/inspections/QuickFixGetFamilyNameViolationInspectionTest.java +++ b/plugins/devkit/testSources/inspections/QuickFixGetFamilyNameViolationInspectionTest.java @@ -39,17 +39,19 @@ public class QuickFixGetFamilyNameViolationInspectionTest extends JavaCodeInsigh " String getName();" + " String getFamilyName();" + "}"); + myFixture.addClass("package com.intellij.psi;" + + "public interface PsiElement {}"); } - public void testViolationByField() { + public void testNotViolatedByField() { myFixture.testHighlighting(getTestName(false) + ".java"); } - public void testViolationByGetName() { + public void testNotViolatedByGetName() { myFixture.testHighlighting(getTestName(false) + ".java"); } - public void testViolationByExternalParameter() { + public void testNotViolatedByExternalParameter() { myFixture.testHighlighting(getTestName(false) + ".java"); } @@ -64,4 +66,8 @@ public class QuickFixGetFamilyNameViolationInspectionTest extends JavaCodeInsigh public void testNotViolatedGetNameMethod() { myFixture.testHighlighting(getTestName(false) + ".java"); } + + public void testViolationByPsiElementFieldUsage() { + myFixture.testHighlighting(getTestName(false) + ".java"); + } }