Remove unused imports in TreePruner

to avoid reporting types as used in .jdeps that were only needed to process otherwise unused imports. PiperOrigin-RevId: 260525951
bazelbuild · Jul 29, 2019 · 019586a · 019586a
1 parent 9a5883f
commit 019586a
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 12 deletions.
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/TreePruner.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/TreePruner.java
@@ -19,6 +19,7 @@
 import com.sun.source.tree.BinaryTree;
 import com.sun.source.tree.ConditionalExpressionTree;
 import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.ImportTree;
 import com.sun.source.tree.LambdaExpressionTree.BodyKind;
 import com.sun.source.tree.LiteralTree;
 import com.sun.source.tree.MemberSelectTree;
@@ -27,15 +28,18 @@
 import com.sun.source.tree.TypeCastTree;
 import com.sun.source.tree.UnaryTree;
 import com.sun.source.util.SimpleTreeVisitor;
+import com.sun.source.util.TreePathScanner;
 import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symtab;
 import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.JCTree.JCBlock;
 import com.sun.tools.javac.tree.JCTree.JCClassDecl;
+import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
 import com.sun.tools.javac.tree.JCTree.JCExpression;
 import com.sun.tools.javac.tree.JCTree.JCExpressionStatement;
 import com.sun.tools.javac.tree.JCTree.JCFieldAccess;
 import com.sun.tools.javac.tree.JCTree.JCIdent;
+import com.sun.tools.javac.tree.JCTree.JCImport;
 import com.sun.tools.javac.tree.JCTree.JCLambda;
 import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
 import com.sun.tools.javac.tree.JCTree.JCMethodInvocation;
@@ -47,6 +51,8 @@
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.List;
 import com.sun.tools.javac.util.Name;
+import java.util.HashSet;
+import java.util.Set;
 
 /**
  * Prunes AST nodes that are not required for header compilation.
@@ -67,8 +73,9 @@ public class TreePruner {
    *   <li>initializers of definitely non-constant fields
    * </ul>
    */
-  static void prune(Context context, JCTree tree) {
-    tree.accept(new PruningVisitor(context));
+  static void prune(Context context, JCCompilationUnit unit) {
+    unit.accept(new PruningVisitor(context));
+    removeUnusedImports(unit);
   }
 
   /** A {@link TreeScanner} that deletes method bodies and blocks from the AST. */
@@ -317,4 +324,58 @@ public boolean reduce(Boolean... bx) {
           return r;
         }
       };
+
+  private static void removeUnusedImports(JCCompilationUnit unit) {
+    Set<String> usedNames = new HashSet<>();
+    // TODO(cushon): consider folding this into PruningVisitor to avoid a second pass
+    new TreePathScanner<Void, Void>() {
+      @Override
+      public Void visitImport(ImportTree importTree, Void usedSymbols) {
+        return null;
+      }
+
+      @Override
+      public Void visitIdentifier(IdentifierTree tree, Void unused) {
+        if (tree == null) {
+          return null;
+        }
+        usedNames.add(tree.getName().toString());
+        return null;
+      }
+    }.scan(unit, null);
+    com.sun.tools.javac.util.List<JCTree> replacements = com.sun.tools.javac.util.List.nil();
+    for (JCTree def : unit.defs) {
+      if (!def.hasTag(JCTree.Tag.IMPORT) || !isUnused(unit, usedNames, (JCImport) def)) {
+        replacements = replacements.append(def);
+      }
+    }
+    unit.defs = replacements;
+  }
+
+  private static boolean isUnused(
+      JCCompilationUnit unit, Set<String> usedNames, JCImport importTree) {
+    String simpleName =
+        importTree.getQualifiedIdentifier() instanceof JCIdent
+            ? ((JCIdent) importTree.getQualifiedIdentifier()).getName().toString()
+            : ((JCFieldAccess) importTree.getQualifiedIdentifier()).getIdentifier().toString();
+    String qualifier =
+        ((JCFieldAccess) importTree.getQualifiedIdentifier()).getExpression().toString();
+    if (qualifier.equals("java.lang")) {
+      return true;
+    }
+    if (unit.getPackageName() != null && unit.getPackageName().toString().equals(qualifier)) {
+      // remove imports of classes from the current package
+      return true;
+    }
+    if (importTree.getQualifiedIdentifier() instanceof JCFieldAccess
+        && ((JCFieldAccess) importTree.getQualifiedIdentifier())
+            .getIdentifier()
+            .contentEquals("*")) {
+      return false;
+    }
+    if (usedNames.contains(simpleName)) {
+      return false;
+    }
+    return true;
+  }
 }
diff --git a/...ols/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java b/...ols/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java
@@ -1224,8 +1224,8 @@ public void ignoreStrictDepsErrorsForFailingCompilations() throws Exception {
     addSourceLines(
         "Hello.java",
         "import " + Lib.class.getCanonicalName() + ";",
-        "import no.such.Class;",
         "class Hello extends Lib {",
+        "  no.such.Class f;",
         "}");
 
     optionsBuilder.addClassPathEntries(ImmutableList.of(lib.toString()));

diff --git a/...tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/TreePrunerTest.java b/...tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/TreePrunerTest.java
@@ -24,7 +24,6 @@
 import com.sun.tools.javac.parser.ParserFactory;
 import com.sun.tools.javac.tree.JCTree.JCClassDecl;
 import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
-import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
 import com.sun.tools.javac.util.Context;
 import java.io.IOError;
 import java.io.IOException;
@@ -60,16 +59,11 @@ JCCompilationUnit parseLines(String... lines) {
     }
   }
 
-  JCVariableDecl parseField(String line) {
+  String printPruned(String line) {
     JCCompilationUnit unit = parseLines("class T {", line, "}");
+    TreePruner.prune(context, unit);
     JCClassDecl classDecl = (JCClassDecl) getOnlyElement(unit.defs);
-    return (JCVariableDecl) getOnlyElement(classDecl.defs);
-  }
-
-  String printPruned(String line) {
-    JCVariableDecl tree = parseField(line);
-    TreePruner.prune(context, tree);
-    return tree.toString();
+    return getOnlyElement(classDecl.defs).toString();
   }
 
   @Test
@@ -333,4 +327,29 @@ public void qualifiedSuperConstructorChaining() {
     };
     assertThat(prettyPrint(tree)).isEqualTo(Joiner.on('\n').join(expected));
   }
+
+  @Test
+  public void unusedImport() {
+    String[] lines = {
+      "package p;",
+      "import com.google.common.collect.ImmutableMap;",
+      "public class Test {",
+      "  public Test() {",
+      "    ImmutableMap<String, String> map = ImmutableMap.of();",
+      "  }",
+      "}",
+    };
+    JCCompilationUnit tree = parseLines(lines);
+    TreePruner.prune(context, tree);
+    String[] expected = {
+      "package p;", //
+      "",
+      "public class Test {",
+      "    ",
+      "    public Test() {",
+      "    }",
+      "}",
+    };
+    assertThat(prettyPrint(tree)).isEqualTo(Joiner.on('\n').join(expected));
+  }
 }