From e902cb9d04456d6011c939acf92db4e89b9ed5a5 Mon Sep 17 00:00:00 2001 From: ghm Date: Tue, 20 Dec 2022 03:08:53 -0800 Subject: [PATCH] Automatic code cleanup. PiperOrigin-RevId: 496613902 --- .../scanner/ScannerSupplierImpl.java | 19 +++++ .../scanner/ScannerSupplierTest.java | 16 +++++ .../errorprone/CompilationTestHelperTest.java | 71 ------------------- 3 files changed, 35 insertions(+), 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 index b94c68358776..d510254f2660 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 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/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java index 1c5efa6d03ab..bcad4438de3a 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/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java b/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java index 0fede355ec4c..59b6961851de 100644 --- 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 {}