Complete fewer symbols in BoxedPrimitiveConstructor

Fixes #441 MOE_MIGRATED_REVID=128764851
google · Jul 29, 2016 · ddaea8d · ddaea8d
1 parent 1885c84
commit ddaea8d
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 11 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/BoxedPrimitiveConstructor.java b/core/src/main/java/com/google/errorprone/bugpatterns/BoxedPrimitiveConstructor.java
@@ -17,8 +17,10 @@
 package com.google.errorprone.bugpatterns;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.matchers.Matchers.toType;
 import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+import static com.google.errorprone.util.ASTHelpers.getType;
 
 import com.google.common.primitives.Longs;
 import com.google.errorprone.BugPattern;
@@ -36,6 +38,8 @@
 import com.sun.source.tree.LiteralTree;
 import com.sun.source.tree.NewClassTree;
 import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symtab;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
 import com.sun.tools.javac.tree.JCTree;
@@ -64,21 +68,29 @@ public class BoxedPrimitiveConstructor extends BugChecker implements NewClassTre
 
   @Override
   public Description matchNewClass(NewClassTree tree, VisitorState state) {
-    Type type = ASTHelpers.getType(tree);
-    if (type == null) {
-      return Description.NO_MATCH;
+    Symbol sym = ASTHelpers.getSymbol(tree.getIdentifier());
+    if (sym == null) {
+      return NO_MATCH;
     }
     Types types = state.getTypes();
-    type = types.unboxedTypeOrType(type);
-    if (!type.isPrimitive()) {
-      // TODO(cushon): consider handling String also
-      return Description.NO_MATCH;
+    Symtab symtab = state.getSymtab();
+    // TODO(cushon): consider handling String also
+    if (sym.equals(types.boxedClass(symtab.byteType))
+        || sym.equals(types.boxedClass(symtab.charType))
+        || sym.equals(types.boxedClass(symtab.shortType))
+        || sym.equals(types.boxedClass(symtab.intType))
+        || sym.equals(types.boxedClass(symtab.longType))
+        || sym.equals(types.boxedClass(symtab.doubleType))
+        || sym.equals(types.boxedClass(symtab.floatType))
+        || sym.equals(types.boxedClass(symtab.booleanType))) {
+      return describeMatch(tree, buildFix(tree, state));
     }
-    return describeMatch(tree, buildFix(tree, state, type));
+    return NO_MATCH;
   }
 
-  private Fix buildFix(NewClassTree tree, VisitorState state, Type type) {
+  private Fix buildFix(NewClassTree tree, VisitorState state) {
     boolean autoboxFix = shouldAutoboxFix(state);
+    Type type = state.getTypes().unboxedTypeOrType(getType(tree));
     if (state.getTypes().isSameType(type, state.getSymtab().booleanType)) {
       Object value = literalValue(tree.getArguments().iterator().next());
       if (value instanceof Boolean) {
@@ -89,10 +101,10 @@ private Fix buildFix(NewClassTree tree, VisitorState state, Type type) {
       }
     }
     JCTree.JCExpression arg = (JCTree.JCExpression) getOnlyElement(tree.getArguments());
-    if (autoboxFix && ASTHelpers.getType(arg).isPrimitive()) {
+    if (autoboxFix && getType(arg).isPrimitive()) {
       return SuggestedFix.builder()
           .replace(((JCTree) tree).getStartPosition(), arg.getStartPosition(), "")
-          .replace(state.getEndPosition(arg), state.getEndPosition((JCTree) tree), "")
+          .replace(state.getEndPosition(arg), state.getEndPosition(tree), "")
           .build();
     }
     JCTree parent = (JCTree) state.getPath().getParentPath().getParentPath().getLeaf();

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/BoxedPrimitiveConstructorTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/BoxedPrimitiveConstructorTest.java
@@ -16,9 +16,19 @@
 
 package com.google.errorprone.bugpatterns;
 
+import com.google.common.io.ByteStreams;
 import com.google.errorprone.CompilationTestHelper;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Arrays;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
@@ -224,4 +234,40 @@ public void longHashCode() {
             "}")
         .doTest();
   }
+
+  @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
+
+  public static class Super {}
+
+  public static class Inner extends Super {}
+
+  // TODO(b/30478325): create a better way to write this style of test
+  static void addClassToJar(JarOutputStream jos, Class<?> clazz) throws IOException {
+    String entryPath = clazz.getName().replace('.', '/') + ".class";
+    try (InputStream is = clazz.getClassLoader().getResourceAsStream(entryPath)) {
+      jos.putNextEntry(new JarEntry(entryPath));
+      ByteStreams.copy(is, jos);
+    }
+  }
+
+  @Test
+  public void incompleteClasspath() throws Exception {
+    File libJar = tempFolder.newFile("lib.jar");
+    try (FileOutputStream fis = new FileOutputStream(libJar);
+        JarOutputStream jos = new JarOutputStream(fis)) {
+      addClassToJar(jos, BoxedPrimitiveConstructorTest.class);
+      addClassToJar(jos, Inner.class);
+    }
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import " + Inner.class.getCanonicalName() + ";",
+            "class Test {",
+            "  void m() {",
+            "    new Inner();",
+            "  }",
+            "}")
+        .setArgs(Arrays.asList("-cp", libJar.toString()))
+        .doTest();
+  }
 }