diff --git a/check_api/pom.xml b/check_api/pom.xml index 74acc5059c7..e9ba7b7e15e 100644 --- a/check_api/pom.xml +++ b/check_api/pom.xml @@ -136,6 +136,12 @@ 1.2 test + + + com.google.inject + guice + ${guice.version} + 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 index b94c6835877..276f478a9fc 100644 --- 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 severities; private final ImmutableSet disabled; private final ErrorProneFlags flags; + // Lazily initialized to make serialization easy. + private transient Injector injector; ScannerSupplierImpl( ImmutableBiMap 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[], though the return type is diff --git a/core/pom.xml b/core/pom.xml index 7803d9fcee5..c7676b6a53f 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -179,7 +179,7 @@ com.google.inject guice - 5.1.0 + ${guice.version} test diff --git a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java index 1c5efa6d03a..bcad4438de3 100644 --- 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/pom.xml b/pom.xml index 515f8b7edb6..ea1267b0b1d 100644 --- a/pom.xml +++ b/pom.xml @@ -46,6 +46,7 @@ 3.19.2 1.43.2 0.2.0 + 5.1.0 diff --git a/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java b/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java index 0fede355ec4..e21dd5aab8a 100644 --- a/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java +++ b/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 {}