Use Guice to instantiate BugCheckers, rather than our own hand-rolled…

… DI.

PiperOrigin-RevId: 500834527

check_api/pom.xml

@@ -136,6 +136,12 @@
       <version>1.2</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <!-- Apache 2.0 -->
+      <groupId>com.google.inject</groupId>
+      <artifactId>guice</artifactId>
+      <version>${guice.version}</version>
+    </dependency>
   </dependencies>
 
   <build>

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 | com.google.inject.ConfigurationException 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

core/pom.xml

@@ -179,7 +179,7 @@
       <!-- Apache 2.0 -->
       <groupId>com.google.inject</groupId>
       <artifactId>guice</artifactId>
-      <version>5.1.0</version>
+      <version>${guice.version}</version>
       <scope>test</scope>
     </dependency>
     <dependency>

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);
   }

pom.xml

@@ -46,6 +46,7 @@
     <protobuf.version>3.19.2</protobuf.version>
     <grpc.version>1.43.2</grpc.version>
     <jspecify.version>0.2.0</jspecify.version>
+    <guice.version>5.1.0</guice.version>
   </properties>
 
   <organization>

test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java

@@ -406,109 +406,6 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
     }
   }
 
-  @BugPattern(
-      summary = "A checker that Error Prone can't instantiate because its constructor is private.",
-      severity = ERROR)
-  public static class PrivateConstructorChecker extends BugChecker
-      implements CompilationUnitTreeMatcher {
-    private PrivateConstructorChecker() {}
-
-    @Override
-    public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
-      return NO_MATCH;
-    }
-  }
-
-  @Test
-  public void cannotInstantiateCheckerWithPrivateConstructor() {
-    AssertionError expected =
-        assertThrows(
-            AssertionError.class,
-            () ->
-                CompilationTestHelper.newInstance(PrivateConstructorChecker.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 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 {}