UnnecessarilyFullyQualified: handle java.lang and inconsistent impo…

…rt usages
PicnicSupermarket · Oct 12, 2021 · b368b93 · b368b93
1 parent 971baea
commit b368b93
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 14 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualified.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualified.java
@@ -46,10 +46,8 @@
 import com.sun.tools.javac.code.Symbol.TypeSymbol;
 import com.sun.tools.javac.util.Position;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.lang.model.element.Name;
 
@@ -74,7 +72,6 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
       return NO_MATCH;
     }
     Table<Name, TypeSymbol, List<TreePath>> table = HashBasedTable.create();
-    Set<Name> identifiersSeen = new HashSet<>();
     new SuppressibleTreePathScanner<Void, Void>() {
       @Override
       public Void visitImport(ImportTree importTree, Void unused) {
@@ -89,12 +86,6 @@ public Void visitMemberSelect(MemberSelectTree memberSelectTree, Void unused) {
         return super.visitMemberSelect(memberSelectTree, null);
       }
 
-      @Override
-      public Void visitIdentifier(IdentifierTree identifierTree, Void unused) {
-        identifiersSeen.add(identifierTree.getName());
-        return null;
-      }
-
       private boolean shouldIgnore() {
         // Don't report duplicate hits if we're not at the tail of a series of member selects on
         // classes.
@@ -161,20 +152,25 @@ public Void visitIdentifier(IdentifierTree identifierTree, Void aVoid) {
       if (Ascii.isLowerCase(name.charAt(0))) {
         continue;
       }
-      if (identifiersSeen.contains(name)) {
-        continue;
-      }
       String nameString = name.toString();
       if (EXEMPTED_NAMES.contains(nameString)) {
         continue;
       }
+      TypeSymbol type = getOnlyElement(types.keySet());
       List<TreePath> pathsToFix = getOnlyElement(types.values());
       if (pathsToFix.stream()
-          .anyMatch(path -> findIdent(nameString, state.withPath(path), VAL_TYP) != null)) {
+          .anyMatch(
+              path -> {
+                Symbol ident = findIdent(nameString, state.withPath(path), VAL_TYP);
+                return ident != null && !ident.equals(type);
+              })) {
         continue;
       }
       SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
-      fixBuilder.addImport(getOnlyElement(types.keySet()).getQualifiedName().toString());
+      String newImport = type.getQualifiedName().toString();
+      if (!newImport.startsWith("java.lang.")) {
+        fixBuilder.addImport(newImport);
+      }
       for (TreePath path : pathsToFix) {
         fixBuilder.replace(path.getLeaf(), nameString);
       }

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualifiedTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualifiedTest.java
@@ -27,6 +27,30 @@ public final class UnnecessarilyFullyQualifiedTest {
   private final BugCheckerRefactoringTestHelper helper =
       BugCheckerRefactoringTestHelper.newInstance(UnnecessarilyFullyQualified.class, getClass());
 
+  @Test
+  public void javaLang() {
+    helper
+        .addInputLines(
+            "Test.java",
+            "public class Test {",
+            "  @java.lang.Deprecated",
+            "  public java.lang.String foo(java.lang.String s) {",
+            "    java.lang.Integer i = 1;",
+            "    return s;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "public class Test {",
+            "  @Deprecated",
+            "  public String foo(String s) {",
+            "    Integer i = 1;",
+            "    return s;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void singleUse() {
     helper
@@ -81,6 +105,28 @@ public void refersToMultipleTypes() {
         .doTest();
   }
 
+  @Test
+  public void inconsistentImportUsage() {
+    helper
+        .addInputLines(
+            "Test.java",
+            "import java.util.List;",
+            "public class Test {",
+            "  public java.util.List foo(List list) {",
+            "    return list;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import java.util.List;",
+            "public class Test {",
+            "  public List foo(List list) {",
+            "    return list;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void clashesWithTypeInSuperType() {
     helper