diff --git a/pom.xml b/pom.xml index 366430c5991..bc4628bfdbb 100644 --- a/pom.xml +++ b/pom.xml @@ -622,6 +622,10 @@ + + + @@ -1262,6 +1266,9 @@ pitest-maven 1.9.5 + + *.AutoValue_* + diff --git a/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java index 9a03e34436a..3ab7a7e17d4 100644 --- a/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java +++ b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java @@ -22,16 +22,13 @@ import com.sun.tools.javac.util.Context; import java.io.Serializable; import java.lang.annotation.Annotation; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; -import java.util.Arrays; import java.util.Iterator; import java.util.Optional; import java.util.function.Function; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; import tech.picnic.errorprone.refaster.annotation.Severity; +// XXX: Can we find a better name for this class? `CompositeAnnotatedCodeTransformer`, ...? // XXX: Test this class directly. (Right now it's only indirectly tested through `RefasterTest`.) @AutoValue abstract class AnnotatedCompositeCodeTransformer implements CodeTransformer, Serializable { @@ -127,25 +124,11 @@ private static Optional getAnnotationValue( } private static SeverityLevel overrideSeverity(SeverityLevel severity, Context context) { - ErrorProneOptions errorProneOptions = context.get(ErrorProneOptions.class); - SeverityLevel minSeverity = allSuggestionsAsWarnings(errorProneOptions) ? WARNING : SUGGESTION; - SeverityLevel maxSeverity = errorProneOptions.isDropErrorsToWarnings() ? WARNING : ERROR; + ErrorProneOptions options = context.get(ErrorProneOptions.class); + SeverityLevel minSeverity = + ErrorProneFork.isSuggestionsAsWarningsEnabled(options) ? WARNING : SUGGESTION; + SeverityLevel maxSeverity = options.isDropErrorsToWarnings() ? WARNING : ERROR; return Comparators.max(Comparators.min(severity, minSeverity), maxSeverity); } - - private static boolean allSuggestionsAsWarnings(ErrorProneOptions errorProneOptions) { - try { - Optional isSuggestionsAsWarningsMethod = - Arrays.stream(errorProneOptions.getClass().getDeclaredMethods()) - .filter(m -> Modifier.isPublic(m.getModifiers())) - .filter(m -> m.getName().equals("isSuggestionsAsWarnings")) - .findFirst(); - return isSuggestionsAsWarningsMethod.isPresent() - && Boolean.TRUE.equals( - isSuggestionsAsWarningsMethod.orElseThrow().invoke(errorProneOptions)); - } catch (IllegalAccessException | InvocationTargetException e) { - return false; - } - } } diff --git a/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/ErrorProneFork.java b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/ErrorProneFork.java new file mode 100644 index 00000000000..3b8711fc5a8 --- /dev/null +++ b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/ErrorProneFork.java @@ -0,0 +1,58 @@ +package tech.picnic.errorprone.refaster.plugin; + +import com.google.errorprone.ErrorProneOptions; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.Optional; + +/** + * Utility class that enables the runtime to determine whether Picnic's fork of Error Prone is on + * the classpath. + * + * @see Picnic's Error Prone fork + */ +// XXX: Add tests for this class. We can at least test `#isErrorProneForkAvailable()` by having +// Maven inject a corresponding system property. +public final class ErrorProneFork { + private static final Optional ERROR_PRONE_OPTIONS_IS_SUGGESTIONS_AS_WARNINGS_METHOD = + Arrays.stream(ErrorProneOptions.class.getDeclaredMethods()) + .filter(m -> Modifier.isPublic(m.getModifiers())) + .filter(m -> "isSuggestionsAsWarnings".equals(m.getName())) + .findFirst(); + + private ErrorProneFork() {} + + /** + * Tells whether Picnic's fork of Error Prone is available. + * + * @return {@code true} iff classpath introspection indicates the presence of Error Prone + * modifications that are assumed to be present only in Picnic's fork. + */ + public static boolean isErrorProneForkAvailable() { + return ERROR_PRONE_OPTIONS_IS_SUGGESTIONS_AS_WARNINGS_METHOD.isPresent(); + } + + /** + * Tells whether the custom {@code -XepAllSuggestionsAsWarnings} flag is set. + * + * @param options The currently active Error Prone options. + * @return {@code true} iff {@link #isErrorProneForkAvailable() the Error Prone fork is available} + * and the aforementioned flag is configured. + * @see google/error-prone#3301 + */ + public static boolean isSuggestionsAsWarningsEnabled(ErrorProneOptions options) { + return ERROR_PRONE_OPTIONS_IS_SUGGESTIONS_AS_WARNINGS_METHOD + .filter(m -> Boolean.TRUE.equals(invoke(m, options))) + .isPresent(); + } + + private static Object invoke(Method method, Object obj, Object... args) { + try { + return method.invoke(obj, args); + } catch (IllegalAccessException | InvocationTargetException e) { + throw new IllegalStateException(String.format("Failed to invoke method '%s'", method), e); + } + } +} diff --git a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java index 4c99a90032f..26d221f3cfb 100644 --- a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java +++ b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java @@ -11,14 +11,13 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; -import java.lang.reflect.Modifier; -import java.util.Arrays; import java.util.regex.Pattern; import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import tech.picnic.errorprone.refaster.plugin.ErrorProneFork; final class RefasterTest { private final CompilationTestHelper compilationHelper = @@ -75,24 +74,6 @@ void identification() { private static Stream reportedSeverityTestCases() { /* { arguments, expectedSeverities } */ - - Stream forkTestCases = - isBuiltWithErrorProneFork() - ? Stream.of( - arguments( - ImmutableList.of("-Xep:Refaster:OFF", "-XepAllSuggestionsAsWarnings"), - ImmutableList.of()), - arguments( - ImmutableList.of("-Xep:Refaster:DEFAULT", "-XepAllSuggestionsAsWarnings"), - ImmutableList.of("warning", "warning", "error", "warning")), - arguments( - ImmutableList.of("-Xep:Refaster:WARN", "-XepAllSuggestionsAsWarnings"), - ImmutableList.of("warning", "warning", "warning", "warning")), - arguments( - ImmutableList.of("-Xep:Refaster:ERROR", "-XepAllSuggestionsAsWarnings"), - ImmutableList.of("error", "error", "error", "error"))) - : Stream.empty(); - return Stream.concat( Stream.of( arguments(ImmutableList.of(), ImmutableList.of("Note", "warning", "error", "Note")), @@ -121,7 +102,45 @@ private static Stream reportedSeverityTestCases() { arguments( ImmutableList.of("-Xep:Refaster:ERROR", "-XepAllErrorsAsWarnings"), ImmutableList.of("warning", "warning", "warning", "warning"))), - forkTestCases); + ErrorProneFork.isErrorProneForkAvailable() + ? Stream.of( + arguments( + ImmutableList.of("-Xep:Refaster:OFF", "-XepAllSuggestionsAsWarnings"), + ImmutableList.of()), + arguments( + ImmutableList.of("-Xep:Refaster:DEFAULT", "-XepAllSuggestionsAsWarnings"), + ImmutableList.of("warning", "warning", "error", "warning")), + arguments( + ImmutableList.of("-Xep:Refaster:WARN", "-XepAllSuggestionsAsWarnings"), + ImmutableList.of("warning", "warning", "warning", "warning")), + arguments( + ImmutableList.of("-Xep:Refaster:ERROR", "-XepAllSuggestionsAsWarnings"), + ImmutableList.of("error", "error", "error", "error")), + arguments( + ImmutableList.of( + "-Xep:Refaster:OFF", + "-XepAllErrorsAsWarnings", + "-XepAllSuggestionsAsWarnings"), + ImmutableList.of()), + arguments( + ImmutableList.of( + "-Xep:Refaster:DEFAULT", + "-XepAllErrorsAsWarnings", + "-XepAllSuggestionsAsWarnings"), + ImmutableList.of("warning", "warning", "warning", "warning")), + arguments( + ImmutableList.of( + "-Xep:Refaster:WARN", + "-XepAllErrorsAsWarnings", + "-XepAllSuggestionsAsWarnings"), + ImmutableList.of("warning", "warning", "warning", "warning")), + arguments( + ImmutableList.of( + "-Xep:Refaster:ERROR", + "-XepAllErrorsAsWarnings", + "-XepAllSuggestionsAsWarnings"), + ImmutableList.of("warning", "warning", "warning", "warning"))) + : Stream.empty()); } /** @@ -224,20 +243,4 @@ void restrictedReplacement() { "}") .doTest(TestMode.TEXT_MATCH); } - - private static boolean isBuiltWithErrorProneFork() { - Class clazz; - try { - clazz = - Class.forName( - "com.google.errorprone.ErrorProneOptions", - /* initialize= */ false, - Thread.currentThread().getContextClassLoader()); - } catch (ClassNotFoundException e) { - throw new IllegalStateException("Can't load `ErrorProneOptions` class", e); - } - return Arrays.stream(clazz.getDeclaredMethods()) - .filter(m -> Modifier.isPublic(m.getModifiers())) - .anyMatch(m -> m.getName().equals("isSuggestionsAsWarnings")); - } } diff --git a/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterTemplateCollection.java b/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterTemplateCollection.java index 811a6318834..7cf467c647b 100644 --- a/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterTemplateCollection.java +++ b/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterTemplateCollection.java @@ -179,10 +179,11 @@ private static ImmutableRangeMap indexTemplateMatches( ImmutableRangeMap.Builder templateMatches = ImmutableRangeMap.builder(); for (Description description : matches) { + String templateName = extractRefasterTemplateName(description); Set replacements = Iterables.getOnlyElement(description.fixes).getReplacements(endPositions); for (Replacement replacement : replacements) { - templateMatches.put(replacement.range(), extractRefasterTemplateName(description)); + templateMatches.put(replacement.range(), templateName); } } @@ -232,7 +233,7 @@ private void reportViolations( private static String extractRefasterTemplateName(Description description) { String message = description.getRawMessage(); int index = message.indexOf(':'); - checkState(index >= 0, "String '%s' does not contain character ':'", message); + checkState(index >= 0, "Failed to extract Refaster template name from string '%s'", message); return getSubstringAfterFinalDelimiter('.', message.substring(0, index)); }