Skip to content

Commit

Permalink
Suggestions, introduce ErrorProneFork
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202
Stephan202 committed Sep 29, 2022
1 parent ec33d06 commit 1f4275a
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 61 deletions.
7 changes: 7 additions & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,10 @@
</module>
<module name="IllegalIdentifierName" />
<module name="IllegalImport">
<property name="illegalClasses" value="com.google.auto.common.MoreStreams">
<!-- Instead, please use Guava's
`java.util.stream.Collector`s. -->
</property>
<property name="illegalClasses" value="com.mongodb.lang.Nullable">
<!-- Instead, please use
`javax.annotation.Nullable`. -->
Expand Down Expand Up @@ -1262,6 +1266,9 @@
<artifactId>pitest-maven</artifactId>
<version>1.9.5</version>
<configuration>
<excludedClasses>
<excludedClass>*.AutoValue_*</excludedClass>
</excludedClasses>
<!-- Use multiple threads to speed things up. Extend
timeouts to prevent false positives as a result of
contention. -->
Expand Down
27 changes: 5 additions & 22 deletions ...c/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -127,25 +124,11 @@ private static <A extends Annotation> Optional<A> 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<Method> 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;
}
}
}
58 changes: 58 additions & 0 deletions refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/ErrorProneFork.java
View file
Original file line number Diff line number Diff line change
@@ -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 <a href="https://github.com/PicnicSupermarket/error-prone">Picnic's Error Prone fork</a>
*/
// 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<Method> 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 <a href="https://github.com/google/error-prone/pull/3301">google/error-prone#3301</a>
*/
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);
}
}
}
77 changes: 40 additions & 37 deletions refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -75,24 +74,6 @@ void identification() {

private static Stream<Arguments> reportedSeverityTestCases() {
/* { arguments, expectedSeverities } */

Stream<Arguments> 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")),
Expand Down Expand Up @@ -121,7 +102,45 @@ private static Stream<Arguments> 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());
}

/**
Expand Down Expand Up @@ -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"));
}
}
5 changes: 3 additions & 2 deletions ...upport/src/main/java/tech/picnic/errorprone/refaster/test/RefasterTemplateCollection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,11 @@ private static ImmutableRangeMap<Integer, String> indexTemplateMatches(
ImmutableRangeMap.Builder<Integer, String> templateMatches = ImmutableRangeMap.builder();

for (Description description : matches) {
String templateName = extractRefasterTemplateName(description);
Set<Replacement> replacements =
Iterables.getOnlyElement(description.fixes).getReplacements(endPositions);
for (Replacement replacement : replacements) {
templateMatches.put(replacement.range(), extractRefasterTemplateName(description));
templateMatches.put(replacement.range(), templateName);
}
}

Expand Down Expand Up @@ -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));
}

Expand Down

0 comments on commit 1f4275a

Please sign in to comment.