Automatic code cleanup.

PiperOrigin-RevId: 496613902
google · Jan 9, 2023 · e902cb9 · e902cb9
1 parent bb9ede9
commit e902cb9
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 71 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplierImpl.java b/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplierImpl.java
@@ -27,6 +27,9 @@
 import com.google.errorprone.BugPattern.SeverityLevel;
 import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.ProvisionException;
 import java.io.Serializable;
 import java.lang.reflect.Constructor;
 import java.util.Arrays;
@@ -42,6 +45,8 @@ class ScannerSupplierImpl extends ScannerSupplier implements Serializable {
   private final ImmutableMap<String, SeverityLevel> severities;
   private final ImmutableSet<String> disabled;
   private final ErrorProneFlags flags;
+  // Lazily initialized to make serialization easy.
+  private transient Injector injector;
 
   ScannerSupplierImpl(
       ImmutableBiMap<String, BugCheckerInfo> checks,
@@ -61,6 +66,20 @@ class ScannerSupplierImpl extends ScannerSupplier implements Serializable {
   }
 
   private BugChecker instantiateChecker(BugCheckerInfo checker) {
+    if (injector == null) {
+      injector =
+          Guice.createInjector(binder -> binder.bind(ErrorProneFlags.class).toInstance(flags));
+    }
+    try {
+      return injector.getInstance(checker.checkerClass());
+    } catch (ProvisionException e) {
+      // Fall back to the old path for external checks.
+      // TODO(b/263227221): Consider stripping this internally after careful testing.
+      return instantiateCheckerOldPath(checker);
+    }
+  }
+
+  private BugChecker instantiateCheckerOldPath(BugCheckerInfo checker) {
     // Invoke BugChecker(ErrorProneFlags) constructor, if it exists.
     @SuppressWarnings("unchecked")
     /* getConstructors() actually returns Constructor<BugChecker>[], though the return type is

diff --git a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java
@@ -34,6 +34,7 @@
 import com.google.errorprone.BugCheckerInfo;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.ErrorProneJavaCompilerTest;
 import com.google.errorprone.ErrorProneJavaCompilerTest.UnsuppressibleArrayEquals;
 import com.google.errorprone.ErrorProneOptions;
@@ -625,6 +626,21 @@ final MapSubject flagsMap() {
     }
   }
 
+  /** A check missing `@Inject`. */
+  @SuppressWarnings("InjectOnBugCheckers") // intentional for testing
+  @BugPattern(summary = "", severity = ERROR)
+  public static class MissingInject extends BugChecker {
+    public MissingInject(ErrorProneFlags flags) {}
+  }
+
+  @Test
+  public void missingInject_stillProvisioned() {
+    ScannerSupplier ss1 = ScannerSupplier.fromBugCheckerClasses(MissingInject.class);
+
+    // We're only testing that this doesn't fail.
+    var unused = ss1.get();
+  }
+
   private static ScannerSupplierSubject assertScanner(ScannerSupplier scannerSupplier) {
     return assertAbout(ScannerSupplierSubject::new).that(scannerSupplier);
   }

diff --git a/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java b/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java
@@ -438,77 +438,6 @@ public void cannotInstantiateCheckerWithPrivateConstructor() {
         .contains("Are both the class and the zero-arg constructor public?");
   }
 
-  @BugPattern(
-      summary =
-          "A checker that Error Prone can't instantiate because it is private and has a default"
-              + " constructor.",
-      severity = ERROR)
-  private static class PrivateChecker extends BugChecker implements CompilationUnitTreeMatcher {
-
-    // By JLS 8.8.9, if there are no constructor declarations, then a default constructor with the
-    // same access modifier as the class is implicitly declared.  So here there is a default
-    // constructor with private access.
-
-    @Override
-    public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
-      return NO_MATCH;
-    }
-  }
-
-  @Test
-  public void cannotInstantiatePrivateChecker() {
-    AssertionError expected =
-        assertThrows(
-            AssertionError.class,
-            () ->
-                CompilationTestHelper.newInstance(PrivateChecker.class, getClass())
-                    .addSourceLines(
-                        "test/Test.java",
-                        "package test;",
-                        "// BUG: Diagnostic contains:",
-                        "public class Test {}")
-                    .doTest());
-    assertThat(expected).hasMessageThat().contains("Could not instantiate BugChecker");
-    assertThat(expected)
-        .hasMessageThat()
-        .contains("Are both the class and the zero-arg constructor public?");
-  }
-
-  @BugPattern(
-      summary =
-          "A checker that Error Prone can't instantiate because the class is private even though"
-              + " the constructor is public.",
-      severity = ERROR)
-  private static class PrivateCheckerWithPublicConstructor extends BugChecker
-      implements CompilationUnitTreeMatcher {
-    public PrivateCheckerWithPublicConstructor() {}
-
-    @Override
-    public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
-      return NO_MATCH;
-    }
-  }
-
-  @Test
-  public void cannotInstantiatePrivateCheckerWithPublicConstructor() {
-    AssertionError expected =
-        assertThrows(
-            AssertionError.class,
-            () ->
-                CompilationTestHelper.newInstance(
-                        PrivateCheckerWithPublicConstructor.class, getClass())
-                    .addSourceLines(
-                        "test/Test.java",
-                        "package test;",
-                        "// BUG: Diagnostic contains:",
-                        "public class Test {}")
-                    .doTest());
-    assertThat(expected).hasMessageThat().contains("Could not instantiate BugChecker");
-    assertThat(expected)
-        .hasMessageThat()
-        .contains("Are both the class and the zero-arg constructor public?");
-  }
-
   /** Test classes used for withClassPath tests */
   public static class WithClassPath extends WithClassPathSuper {}