QuickFixGetFamilyNameViolationInspection updated to provide less false-positives and be more green

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<String> 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<PsiJavaCodeReferenceElement> 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;
+  }
 }

plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByExternalParameter.java

@@ -8,7 +8,7 @@ class A {
         return "some name";
       };
 
-      public String <warning descr="QuickFix's getFamilyName() implementation must not depend on a specific context">getFamilyName</warning>() {
+      public String getFamilyName() {
         return someParameter + "123";
       };
     };

plugins/devkit/testData/inspections/getFamilyNameViolation/NotViolatedByField.java

@@ -8,7 +8,7 @@ class MyQuickFix implements QuickFix {
     return "some name";
   };
 
-  public String <warning descr="QuickFix's getFamilyName() implementation must not depend on a specific context">getFamilyName</warning>() {
+  public String getFamilyName() {
     return someField + getName() + "123";
   };
 

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";
+  };
+
+
+}

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 <warning descr="QuickFix's getFamilyName() implementation must not depend on a specific context">getFamilyName</warning>() {
-    return getName() + "123";
+    return "error is here: " + String.valueOf(myElement);
   };
 
-
 }

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");
+  }
 }