Avoid flagging variables and same file classes

PicnicSupermarket · Jan 6, 2023 · 679f597 · 679f597
1 parent 56a6741
commit 679f597
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 6 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/BadStaticImport.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/BadStaticImport.java
@@ -3,6 +3,7 @@
 import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
 import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
 import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
+import static com.sun.tools.javac.code.Kinds.Kind.TYP;
 import static tech.picnic.errorprone.bugpatterns.StaticImport.STATIC_IMPORT_CANDIDATE_MEMBERS;
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
 
@@ -18,8 +19,11 @@
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.ImportTree;
 import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
 import com.sun.tools.javac.code.Symbol;
+import tech.picnic.errorprone.bugpatterns.util.SourceCode;
 
 /** A {@link BugChecker} that flags methods and constants that should not be statically imported. */
 // XXX: Also introduce checks that disallows the following candidates:
@@ -119,7 +123,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
 
   @Override
   public Description matchIdentifier(IdentifierTree tree, VisitorState state) {
-    if (isMatch(tree)) {
+    if (isMatch(tree, state)) {
       return getDescription(tree, state);
     }
 
@@ -142,19 +146,23 @@ private static String getImportToRemove(Symbol symbol) {
         ".", symbol.getEnclosingElement().getQualifiedName(), symbol.getSimpleName());
   }
 
-  private static boolean isMatch(IdentifierTree tree) {
+  private static boolean isMatch(IdentifierTree tree, VisitorState state) {
     Symbol symbol = ASTHelpers.getSymbol(tree);
     if (symbol == null) {
       return false;
     }
 
     Symbol enclosingSymbol = symbol.getEnclosingElement();
-    if (enclosingSymbol == null) {
+    if (enclosingSymbol == null || enclosingSymbol.kind != TYP) {
       return false;
     }
 
-    String qualifiedTypeName = enclosingSymbol.getQualifiedName().toString();
     String identifierName = symbol.getSimpleName().toString();
+    if (!isIdentifierStaticallyImported(identifierName, state)) {
+      return false;
+    }
+
+    String qualifiedTypeName = enclosingSymbol.getQualifiedName().toString();
     return !isExempted(qualifiedTypeName, identifierName)
         && isCandidate(qualifiedTypeName, identifierName);
   }
@@ -168,4 +176,17 @@ private static boolean isCandidate(String qualifiedTypeName, String identifierNa
   private static boolean isExempted(String qualifiedTypeName, String identifierName) {
     return STATIC_IMPORT_CANDIDATE_MEMBERS.containsEntry(qualifiedTypeName, identifierName);
   }
+
+  private static boolean isIdentifierStaticallyImported(String identifierName, VisitorState state) {
+    return state.getPath().getCompilationUnit().getImports().stream()
+        .filter(ImportTree::isStatic)
+        .map(ImportTree::getQualifiedIdentifier)
+        .map(tree -> getStaticImportIdentifier(tree, state))
+        .anyMatch(identifierName::contentEquals);
+  }
+
+  private static CharSequence getStaticImportIdentifier(Tree tree, VisitorState state) {
+    String source = SourceCode.treeToString(tree, state);
+    return source.subSequence(source.lastIndexOf('.') + 1, source.length());
+  }
 }
diff --git a/...r-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/BadStaticImportTest.java b/...r-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/BadStaticImportTest.java
@@ -40,7 +40,7 @@ void badIdentifiersDontClashWithStaticImportCandidates() {
   }
 
   @Test
-  void identifySimpleMethodInvocation() {
+  void identification() {
     compilationTestHelper
         .addSourceLines(
             "A.java",
@@ -81,13 +81,22 @@ void identifySimpleMethodInvocation() {
             "",
             "    // BUG: Diagnostic contains:",
             "    ImmutableSet.of(min(ImmutableSet.of()));",
+            "",
+            "    Object builder = null;",
+            "    // Not flagged because identifier is variable name",
+            "    Object lBuilder = ImmutableList.of(builder);",
+            "",
+            "    // Not flagged because method is not statically imported",
+            "    create();",
             "  }",
+            "",
+            "  void create() {}",
             "}")
         .doTest();
   }
 
   @Test
-  void replaceSimpleMethodInvocation() {
+  void replacement() {
     refactoringTestHelper
         .addInputLines(
             "A.java",