diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrder.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrder.java new file mode 100644 index 0000000000..46575300b3 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrder.java @@ -0,0 +1,139 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.errorprone.BugPattern.LinkType.CUSTOM; +import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; +import static com.google.errorprone.BugPattern.StandardTags.STYLE; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.hasAnnotation; +import static java.util.Comparator.comparing; +import static java.util.stream.Collectors.toCollection; +import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; + +import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Streams; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreeScanner; +import com.sun.tools.javac.code.Symbol; +import java.util.Comparator; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import javax.lang.model.element.Name; +import org.jspecify.annotations.Nullable; +import tech.picnic.errorprone.bugpatterns.util.SourceCode; + +/** + * A {@link BugChecker} that flags Refaster methods with a non-canonical parameter order. + * + *

To a first approximation, parameters should be ordered by their first usage in an + * {@code @AfterTemplate} method. Ties are broken by preferring the order dictated by methods with a + * larger number of parameters. + */ +// XXX: This check can introduce suggestions that are incompatible with Error Prone's +// `InconsistentOverloads` check. Review whether/how to improve this. +@AutoService(BugChecker.class) +@BugPattern( + summary = "Refaster template parameters should be listed in a canonical order", + link = BUG_PATTERNS_BASE_URL + "RefasterMethodParameterOrder", + linkType = CUSTOM, + severity = SUGGESTION, + tags = STYLE) +public final class RefasterMethodParameterOrder extends BugChecker implements ClassTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Matcher BEFORE_TEMPLATE_METHOD = hasAnnotation(BeforeTemplate.class); + private static final Matcher BEFORE_OR_AFTER_TEMPLATE_METHOD = + anyOf(BEFORE_TEMPLATE_METHOD, hasAnnotation(AfterTemplate.class)); + + /** Instantiates a new {@link RefasterMethodParameterOrder} instance. */ + public RefasterMethodParameterOrder() {} + + @Override + public Description matchClass(ClassTree tree, VisitorState state) { + ImmutableList methods = getMethodsByPriority(tree, state); + if (methods.isEmpty()) { + return Description.NO_MATCH; + } + + Comparator canonicalOrder = determineCanonicalParameterOrder(methods); + + return methods.stream() + .flatMap(m -> tryReorderParameters(m, canonicalOrder, state)) + .reduce(SuggestedFix.Builder::merge) + .map(SuggestedFix.Builder::build) + .map(fix -> describeMatch(tree, fix)) + .orElse(Description.NO_MATCH); + } + + private static ImmutableList getMethodsByPriority( + ClassTree tree, VisitorState state) { + return tree.getMembers().stream() + .filter(m -> BEFORE_OR_AFTER_TEMPLATE_METHOD.matches(m, state)) + .map(MethodTree.class::cast) + .sorted( + comparing((MethodTree m) -> BEFORE_TEMPLATE_METHOD.matches(m, state)) + .thenComparingInt(m -> -m.getParameters().size())) + .collect(toImmutableList()); + } + + private static Comparator determineCanonicalParameterOrder( + ImmutableList methods) { + Set canonicalOrder = new LinkedHashSet<>(); + methods.forEach(m -> processParameters(m, canonicalOrder)); + + ImmutableList reversedCanonicalOrder = ImmutableList.copyOf(canonicalOrder).reverse(); + return comparing( + VariableTree::getName, + Comparator.comparingInt(reversedCanonicalOrder::indexOf) + .reversed() + .thenComparing(Name::toString)); + } + + private static void processParameters(MethodTree method, Set orderedParams) { + Set toBeOrdered = + method.getParameters().stream() + .map(ASTHelpers::getSymbol) + .collect(toCollection(HashSet::new)); + + new TreeScanner<@Nullable Void, @Nullable Void>() { + @Override + public @Nullable Void visitIdentifier(IdentifierTree node, @Nullable Void unused) { + if (toBeOrdered.remove(ASTHelpers.getSymbol(node))) { + orderedParams.add(node.getName()); + } + return super.visitIdentifier(node, null); + } + }.scan(method, null); + } + + private static Stream tryReorderParameters( + MethodTree method, Comparator canonicalOrder, VisitorState state) { + List originalOrder = method.getParameters(); + ImmutableList orderedParams = + ImmutableList.sortedCopyOf(canonicalOrder, originalOrder); + + return originalOrder.equals(orderedParams) + ? Stream.empty() + : Streams.zip( + originalOrder.stream(), + orderedParams.stream().map(p -> SourceCode.treeToString(p, state)), + SuggestedFix.builder()::replace); + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java index 84bac00845..c1b73ecf01 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java @@ -530,13 +530,13 @@ MapAssert after(Map map, K key, V value) { static final class AssertThatStreamContainsAnyElementsOf { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsAnyElementsOf(iterable); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsAnyElementsOf(iterable); } @@ -551,13 +551,13 @@ ListAssert after(Stream stream, Iterable iterable) { static final class AssertThatStreamContainsAnyOf { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).containsAnyOf(array); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).containsAnyOf(array); } @@ -573,14 +573,14 @@ static final class AssertThatStreamContainsAnyOfVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).containsAnyOf(Refaster.asVarargs(elements)); } @BeforeTemplate @SuppressWarnings("AssertThatStreamContainsAnyOf" /* Varargs converted to array. */) ListAssert before2( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).containsAnyOf(Refaster.asVarargs(elements)); } @@ -596,13 +596,13 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamContainsAll { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsAll(iterable); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsAll(iterable); } @@ -617,13 +617,13 @@ ListAssert after(Stream stream, Iterable iterable) { static final class AssertThatStreamContains { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).contains(array); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).contains(array); } @@ -639,14 +639,14 @@ static final class AssertThatStreamContainsVarArgs @BeforeTemplate @SuppressWarnings("AssertThatStreamContains" /* Varargs converted to array. */) IterableAssert before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).contains(Refaster.asVarargs(elements)); } @BeforeTemplate @SuppressWarnings("AssertThatStreamContains" /* Varargs converted to array. */) ListAssert before2( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).contains(Refaster.asVarargs(elements)); } @@ -661,7 +661,7 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamContainsExactlyElementsOf { @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsExactlyElementsOf(iterable); } @@ -676,7 +676,7 @@ ListAssert after(Stream stream, Iterable iterable) { static final class AssertThatStreamContainsExactly { @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).containsExactly(array); } @@ -692,7 +692,7 @@ static final class AssertThatStreamContainsExactlyVarargs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).containsExactly(Refaster.asVarargs(elements)); } @@ -708,13 +708,13 @@ static final class AssertThatStreamContainsExactlyInAnyOrderElementsOf< S, T extends S, U extends T> { @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsExactlyInAnyOrderElementsOf(iterable); } @BeforeTemplate AbstractCollectionAssert before2( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsExactlyInAnyOrderElementsOf(iterable); } @@ -729,13 +729,13 @@ ListAssert after(Stream stream, Iterable iterable) { static final class AssertThatStreamContainsExactlyInAnyOrder { @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).containsExactlyInAnyOrder(array); } @BeforeTemplate AbstractCollectionAssert before2( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).containsExactlyInAnyOrder(array); } @@ -751,7 +751,7 @@ static final class AssertThatStreamContainsExactlyInAnyOrderVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)) .containsExactlyInAnyOrder(Refaster.asVarargs(elements)); } @@ -759,7 +759,7 @@ ListAssert before( @BeforeTemplate @SuppressWarnings("AssertThatStreamContainsExactlyInAnyOrder" /* Varargs converted to array. */) AbstractCollectionAssert before2( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)) .containsExactlyInAnyOrder(Refaster.asVarargs(elements)); } @@ -776,13 +776,13 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamContainsSequence { @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsSequence(iterable); } @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, U[] iterable) { + Stream stream, U[] iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsSequence(iterable); } @@ -798,7 +798,7 @@ static final class AssertThatStreamContainsSequenceVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).containsSequence(Refaster.asVarargs(elements)); } @@ -814,13 +814,13 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamContainsSubsequence { @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsSubsequence(iterable); } @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, U[] iterable) { + Stream stream, U[] iterable, Collector> collector) { return assertThat(stream.collect(collector)).containsSubsequence(iterable); } @@ -836,7 +836,7 @@ static final class AssertThatStreamContainsSubsequenceVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)) .containsSubsequence(Refaster.asVarargs(elements)); } @@ -853,13 +853,13 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamDoesNotContainAnyElementsOf { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContainAnyElementsOf(iterable); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContainAnyElementsOf(iterable); } @@ -874,13 +874,13 @@ ListAssert after(Stream stream, Iterable iterable) { static final class AssertThatStreamDoesNotContain { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContain(array); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContain(array); } @@ -896,14 +896,14 @@ static final class AssertThatStreamDoesNotContainVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContain(Refaster.asVarargs(elements)); } @BeforeTemplate @SuppressWarnings("AssertThatStreamDoesNotContain" /* Varargs converted to array. */) ListAssert before2( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContain(Refaster.asVarargs(elements)); } @@ -918,13 +918,13 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamDoesNotContainSequence { @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContainSequence(iterable); } @BeforeTemplate ListAssert before( - Stream stream, Collector> collector, U[] iterable) { + Stream stream, U[] iterable, Collector> collector) { return assertThat(stream.collect(collector)).doesNotContainSequence(iterable); } @@ -940,7 +940,7 @@ static final class AssertThatStreamDoesNotContainSequenceVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)) .doesNotContainSequence(Refaster.asVarargs(elements)); } @@ -957,13 +957,13 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamHasSameElementsAs { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).hasSameElementsAs(iterable); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).hasSameElementsAs(iterable); } @@ -978,13 +978,13 @@ ListAssert after(Stream stream, Iterable iterable) { static final class AssertThatStreamContainsOnly { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).containsOnly(array); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, U[] array) { + Stream stream, U[] array, Collector> collector) { return assertThat(stream.collect(collector)).containsOnly(array); } @@ -1000,14 +1000,14 @@ static final class AssertThatStreamContainsOnlyVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).containsOnly(Refaster.asVarargs(elements)); } @BeforeTemplate @SuppressWarnings("AssertThatStreamContainsOnly" /* Varargs converted to array. */) ListAssert before2( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).containsOnly(Refaster.asVarargs(elements)); } @@ -1022,25 +1022,25 @@ ListAssert after(Stream stream, @Repeated U elements) { static final class AssertThatStreamIsSubsetOf { @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).isSubsetOf(iterable); } @BeforeTemplate IterableAssert before( - Stream stream, Collector> collector, U[] iterable) { + Stream stream, U[] iterable, Collector> collector) { return assertThat(stream.collect(collector)).isSubsetOf(iterable); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, Iterable iterable) { + Stream stream, Iterable iterable, Collector> collector) { return assertThat(stream.collect(collector)).isSubsetOf(iterable); } @BeforeTemplate ListAssert before2( - Stream stream, Collector> collector, U[] iterable) { + Stream stream, U[] iterable, Collector> collector) { return assertThat(stream.collect(collector)).isSubsetOf(iterable); } @@ -1056,14 +1056,14 @@ static final class AssertThatStreamIsSubsetOfVarArgs before( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).isSubsetOf(Refaster.asVarargs(elements)); } @BeforeTemplate @SuppressWarnings("AssertThatStreamIsSubsetOf" /* Varargs converted to array. */) ListAssert before2( - Stream stream, Collector> collector, @Repeated U elements) { + Stream stream, @Repeated U elements, Collector> collector) { return assertThat(stream.collect(collector)).isSubsetOf(Refaster.asVarargs(elements)); } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java index 74bd22df35..0a653c520a 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java @@ -455,14 +455,14 @@ static final class AssertThatThrownByIOExceptionHasMessageNotContaining { static final class AssertThatThrownBy { @BeforeTemplate AbstractObjectAssert before( - Class exceptionType, ThrowingCallable throwingCallable) { + ThrowingCallable throwingCallable, Class exceptionType) { return assertThatExceptionOfType(exceptionType).isThrownBy(throwingCallable); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) AbstractObjectAssert after( - Class exceptionType, ThrowingCallable throwingCallable) { + ThrowingCallable throwingCallable, Class exceptionType) { return assertThatThrownBy(throwingCallable).isInstanceOf(exceptionType); } } @@ -471,8 +471,8 @@ static final class AssertThatThrownByHasMessage { @BeforeTemplate @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */) AbstractObjectAssert before( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatExceptionOfType(exceptionType) .isThrownBy(throwingCallable) @@ -482,8 +482,8 @@ static final class AssertThatThrownByHasMessage { @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) AbstractObjectAssert after( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatThrownBy(throwingCallable).isInstanceOf(exceptionType).hasMessage(message); } @@ -493,8 +493,8 @@ static final class AssertThatThrownByHasMessageParameters { @BeforeTemplate @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */) AbstractObjectAssert before( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message, @Repeated Object parameters) { return assertThatExceptionOfType(exceptionType) @@ -505,8 +505,8 @@ static final class AssertThatThrownByHasMessageParameters { @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) AbstractObjectAssert after( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message, @Repeated Object parameters) { return assertThatThrownBy(throwingCallable) @@ -519,8 +519,8 @@ static final class AssertThatThrownByHasMessageStartingWith { @BeforeTemplate @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */) AbstractObjectAssert before( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatExceptionOfType(exceptionType) .isThrownBy(throwingCallable) @@ -530,8 +530,8 @@ static final class AssertThatThrownByHasMessageStartingWith { @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) AbstractObjectAssert after( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatThrownBy(throwingCallable) .isInstanceOf(exceptionType) @@ -543,8 +543,8 @@ static final class AssertThatThrownByHasMessageContaining { @BeforeTemplate @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */) AbstractObjectAssert before( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatExceptionOfType(exceptionType) .isThrownBy(throwingCallable) @@ -554,8 +554,8 @@ static final class AssertThatThrownByHasMessageContaining { @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) AbstractObjectAssert after( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatThrownBy(throwingCallable) .isInstanceOf(exceptionType) @@ -567,8 +567,8 @@ static final class AssertThatThrownByHasMessageNotContaining { @BeforeTemplate @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */) AbstractObjectAssert before( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatExceptionOfType(exceptionType) .isThrownBy(throwingCallable) @@ -578,8 +578,8 @@ static final class AssertThatThrownByHasMessageNotContaining { @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) AbstractObjectAssert after( - Class exceptionType, ThrowingCallable throwingCallable, + Class exceptionType, String message) { return assertThatThrownBy(throwingCallable) .isInstanceOf(exceptionType) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java index 119f1634f3..e47918d4bc 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java @@ -157,23 +157,13 @@ Predicate after() { } /** Avoid contrived ways of handling {@code null} values during equality testing. */ - static final class EqualsLhsNullable { + static final class Equals { @BeforeTemplate boolean before(T value1, S value2) { - return Optional.ofNullable(value1).equals(Optional.of(value2)); - } - - @AfterTemplate - boolean after(T value1, S value2) { - return value2.equals(value1); - } - } - - /** Avoid contrived ways of handling {@code null} values during equality testing. */ - static final class EqualsRhsNullable { - @BeforeTemplate - boolean before(T value1, S value2) { - return Optional.of(value1).equals(Optional.ofNullable(value2)); + return Refaster.anyOf( + Optional.of(value1).equals(Optional.of(value2)), + Optional.of(value1).equals(Optional.ofNullable(value2)), + Optional.ofNullable(value2).equals(Optional.of(value1))); } @AfterTemplate @@ -183,7 +173,7 @@ boolean after(T value1, S value2) { } /** Avoid contrived ways of handling {@code null} values during equality testing. */ - static final class EqualsLhsAndRhsNullable { + static final class ObjectsEquals { @BeforeTemplate boolean before(T value1, S value2) { return Optional.ofNullable(value1).equals(Optional.ofNullable(value2)); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java index 1848c268b9..aca741d389 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java @@ -118,17 +118,17 @@ ImmutableList after(Collection iterable) { */ static final class ImmutableListSortedCopyOfWithCustomComparator { @BeforeTemplate - ImmutableList before(Iterable iterable, Comparator cmp) { + ImmutableList before(Comparator cmp, Iterable iterable) { return Streams.stream(iterable).sorted(cmp).collect(toImmutableList()); } @BeforeTemplate - ImmutableList before(Collection iterable, Comparator cmp) { + ImmutableList before(Comparator cmp, Collection iterable) { return iterable.stream().sorted(cmp).collect(toImmutableList()); } @AfterTemplate - ImmutableList after(Collection iterable, Comparator cmp) { + ImmutableList after(Comparator cmp, Collection iterable) { return ImmutableList.sortedCopyOf(cmp, iterable); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java index 2f919e7327..7eeda27c3c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java @@ -276,26 +276,26 @@ void after(Object actual, Object expected) { static final class AssertThatWithFailMessageStringIsSameAs { @BeforeTemplate - void before(Object actual, Object expected, String message) { + void before(Object actual, String message, Object expected) { assertSame(expected, actual, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, String message) { + void after(Object actual, String message, Object expected) { assertThat(actual).withFailMessage(message).isSameAs(expected); } } static final class AssertThatWithFailMessageSupplierIsSameAs { @BeforeTemplate - void before(Object actual, Object expected, Supplier supplier) { + void before(Object actual, Supplier supplier, Object expected) { assertSame(expected, actual, supplier); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, Supplier supplier) { + void after(Object actual, Supplier supplier, Object expected) { assertThat(actual).withFailMessage(supplier).isSameAs(expected); } } @@ -315,26 +315,26 @@ void after(Object actual, Object expected) { static final class AssertThatWithFailMessageStringIsNotSameAs { @BeforeTemplate - void before(Object actual, Object expected, String message) { + void before(Object actual, String message, Object expected) { assertNotSame(expected, actual, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, String message) { + void after(Object actual, String message, Object expected) { assertThat(actual).withFailMessage(message).isNotSameAs(expected); } } static final class AssertThatWithFailMessageSupplierIsNotSameAs { @BeforeTemplate - void before(Object actual, Object expected, Supplier supplier) { + void before(Object actual, Supplier supplier, Object expected) { assertNotSame(expected, actual, supplier); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, Supplier supplier) { + void after(Object actual, Supplier supplier, Object expected) { assertThat(actual).withFailMessage(supplier).isNotSameAs(expected); } } @@ -355,13 +355,13 @@ void after(ThrowingCallable throwingCallable, Class clazz) { static final class AssertThatThrownByWithFailMessageStringIsExactlyInstanceOf< T extends Throwable> { @BeforeTemplate - void before(Executable throwingCallable, Class clazz, String message) { + void before(Executable throwingCallable, String message, Class clazz) { assertThrowsExactly(clazz, throwingCallable, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(ThrowingCallable throwingCallable, Class clazz, String message) { + void after(ThrowingCallable throwingCallable, String message, Class clazz) { assertThatThrownBy(throwingCallable).withFailMessage(message).isExactlyInstanceOf(clazz); } } @@ -369,13 +369,13 @@ void after(ThrowingCallable throwingCallable, Class clazz, String message) { static final class AssertThatThrownByWithFailMessageSupplierIsExactlyInstanceOf< T extends Throwable> { @BeforeTemplate - void before(Executable throwingCallable, Class clazz, Supplier supplier) { + void before(Executable throwingCallable, Supplier supplier, Class clazz) { assertThrowsExactly(clazz, throwingCallable, supplier); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(ThrowingCallable throwingCallable, Class clazz, Supplier supplier) { + void after(ThrowingCallable throwingCallable, Supplier supplier, Class clazz) { assertThatThrownBy(throwingCallable).withFailMessage(supplier).isExactlyInstanceOf(clazz); } } @@ -395,26 +395,26 @@ void after(ThrowingCallable throwingCallable, Class clazz) { static final class AssertThatThrownByWithFailMessageStringIsInstanceOf { @BeforeTemplate - void before(Executable throwingCallable, Class clazz, String message) { + void before(Executable throwingCallable, String message, Class clazz) { assertThrows(clazz, throwingCallable, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(ThrowingCallable throwingCallable, Class clazz, String message) { + void after(ThrowingCallable throwingCallable, String message, Class clazz) { assertThatThrownBy(throwingCallable).withFailMessage(message).isInstanceOf(clazz); } } static final class AssertThatThrownByWithFailMessageSupplierIsInstanceOf { @BeforeTemplate - void before(Executable throwingCallable, Class clazz, Supplier supplier) { + void before(Executable throwingCallable, Supplier supplier, Class clazz) { assertThrows(clazz, throwingCallable, supplier); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(ThrowingCallable throwingCallable, Class clazz, Supplier supplier) { + void after(ThrowingCallable throwingCallable, Supplier supplier, Class clazz) { assertThatThrownBy(throwingCallable).withFailMessage(supplier).isInstanceOf(clazz); } } @@ -488,26 +488,26 @@ void after(Object actual, Class clazz) { static final class AssertThatWithFailMessageStringIsInstanceOf { @BeforeTemplate - void before(Object actual, Class clazz, String message) { + void before(Object actual, String message, Class clazz) { assertInstanceOf(clazz, actual, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Class clazz, String message) { + void after(Object actual, String message, Class clazz) { assertThat(actual).withFailMessage(message).isInstanceOf(clazz); } } static final class AssertThatWithFailMessageSupplierIsInstanceOf { @BeforeTemplate - void before(Object actual, Class clazz, Supplier supplier) { + void before(Object actual, Supplier supplier, Class clazz) { assertInstanceOf(clazz, actual, supplier); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Class clazz, Supplier supplier) { + void after(Object actual, Supplier supplier, Class clazz) { assertThat(actual).withFailMessage(supplier).isInstanceOf(clazz); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java index 977248a2d8..927adb4217 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java @@ -437,10 +437,12 @@ Flux after(Flux flux, T object) { static final class FluxEmpty> { @BeforeTemplate Flux before( - int prefetch, Function combinator, + int prefetch, Comparator comparator) { return Refaster.anyOf( + Flux.zip(combinator), + Flux.zip(combinator, prefetch), Flux.concat(), Flux.concatDelayError(), Flux.firstWithSignal(), @@ -456,13 +458,11 @@ Flux before( Flux.mergePriorityDelayError(prefetch, comparator), Flux.mergeSequential(), Flux.mergeSequential(prefetch), - Flux.mergeSequentialDelayError(prefetch), - Flux.zip(combinator), - Flux.zip(combinator, prefetch)); + Flux.mergeSequentialDelayError(prefetch)); } @BeforeTemplate - Flux before(int prefetch, Function combinator) { + Flux before(Function combinator, int prefetch) { return Refaster.anyOf( Flux.combineLatest(combinator), Flux.combineLatest(combinator, prefetch)); } @@ -727,7 +727,7 @@ abstract static class FluxMap { abstract S transformation(@MayOptionallyUse T value); @BeforeTemplate - Flux before(Flux flux, boolean delayUntilEnd, int maxConcurrency, int prefetch) { + Flux before(Flux flux, int prefetch, boolean delayUntilEnd, int maxConcurrency) { return Refaster.anyOf( flux.concatMap(x -> Mono.just(transformation(x))), flux.concatMap(x -> Flux.just(transformation(x))), @@ -795,7 +795,7 @@ abstract static class FluxMapNotNull { @BeforeTemplate @SuppressWarnings("java:S138" /* Method is long, but not complex. */) - Publisher before(Flux flux, boolean delayUntilEnd, int maxConcurrency, int prefetch) { + Publisher before(Flux flux, int prefetch, boolean delayUntilEnd, int maxConcurrency) { return Refaster.anyOf( flux.concatMap( x -> @@ -1125,9 +1125,9 @@ Flux before( Function function, @Matches(IsIdentityOperation.class) Function> identityOperation, + int prefetch, boolean delayUntilEnd, - int maxConcurrency, - int prefetch) { + int maxConcurrency) { return Refaster.anyOf( mono.map(function).flatMapMany(identityOperation), mono.flux().concatMap(function), @@ -1669,7 +1669,7 @@ StepVerifier.Step after(StepVerifier.Step step, T object) { // a `@Matches(DoesNotDropElements.class)` or `@NotMatches(MayDropElements.class)` guard. static final class FluxAsStepVerifierExpectNext> { @BeforeTemplate - StepVerifier.Step before(Flux flux, Collector listCollector, T object) { + StepVerifier.Step before(Flux flux, T object, Collector listCollector) { return flux.collect(listCollector) .as(StepVerifier::create) .assertNext(list -> assertThat(list).containsExactly(object)); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TestNGToAssertJRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TestNGToAssertJRules.java index 8c9521ca3a..89b0973563 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TestNGToAssertJRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TestNGToAssertJRules.java @@ -237,13 +237,13 @@ void after(Object actual, Object expected) { static final class AssertSameWithMessage { @BeforeTemplate - void before(Object actual, Object expected, String message) { + void before(Object actual, String message, Object expected) { assertSame(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, String message) { + void after(Object actual, String message, Object expected) { assertThat(actual).withFailMessage(message).isSameAs(expected); } } @@ -263,13 +263,13 @@ void after(Object actual, Object expected) { static final class AssertNotSameWithMessage { @BeforeTemplate - void before(Object actual, Object expected, String message) { + void before(Object actual, String message, Object expected) { assertNotSame(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, String message) { + void after(Object actual, String message, Object expected) { assertThat(actual).withFailMessage(message).isNotSameAs(expected); } } @@ -339,63 +339,63 @@ void after(Object actual, Object expected) { static final class AssertEqualWithMessage { @BeforeTemplate - void before(boolean actual, boolean expected, String message) { + void before(boolean actual, String message, boolean expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(byte actual, byte expected, String message) { + void before(byte actual, String message, byte expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(char actual, char expected, String message) { + void before(char actual, String message, char expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(short actual, short expected, String message) { + void before(short actual, String message, short expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(int actual, int expected, String message) { + void before(int actual, String message, int expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(long actual, long expected, String message) { + void before(long actual, String message, long expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(float actual, float expected, String message) { + void before(float actual, String message, float expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(double actual, double expected, String message) { + void before(double actual, String message, double expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(Object actual, Object expected, String message) { + void before(Object actual, String message, Object expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(String actual, String expected, String message) { + void before(String actual, String message, String expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(Map actual, Map expected, String message) { + void before(Map actual, String message, Map expected) { assertEquals(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, String message) { + void after(Object actual, String message, Object expected) { assertThat(actual).withFailMessage(message).isEqualTo(expected); } } @@ -415,13 +415,13 @@ void after(float actual, float expected, float delta) { static final class AssertEqualFloatsWithDeltaWithMessage { @BeforeTemplate - void before(float actual, float expected, float delta, String message) { + void before(float actual, String message, float expected, float delta) { assertEquals(actual, expected, delta, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(float actual, float expected, float delta, String message) { + void after(float actual, String message, float expected, float delta) { assertThat(actual).withFailMessage(message).isCloseTo(expected, offset(delta)); } } @@ -441,13 +441,13 @@ void after(double actual, double expected, double delta) { static final class AssertEqualDoublesWithDeltaWithMessage { @BeforeTemplate - void before(double actual, double expected, double delta, String message) { + void before(double actual, String message, double expected, double delta) { assertEquals(actual, expected, delta, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(double actual, double expected, double delta, String message) { + void after(double actual, String message, double expected, double delta) { assertThat(actual).withFailMessage(message).isCloseTo(expected, offset(delta)); } } @@ -507,53 +507,53 @@ void after(Object[] actual, Object[] expected) { static final class AssertEqualArrayIterationOrderWithMessage { @BeforeTemplate - void before(boolean[] actual, boolean[] expected, String message) { + void before(boolean[] actual, String message, boolean[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(byte[] actual, byte[] expected, String message) { + void before(byte[] actual, String message, byte[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(char[] actual, char[] expected, String message) { + void before(char[] actual, String message, char[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(short[] actual, short[] expected, String message) { + void before(short[] actual, String message, short[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(int[] actual, int[] expected, String message) { + void before(int[] actual, String message, int[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(long[] actual, long[] expected, String message) { + void before(long[] actual, String message, long[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(float[] actual, float[] expected, String message) { + void before(float[] actual, String message, float[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(double[] actual, double[] expected, String message) { + void before(double[] actual, String message, double[] expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(Object[] actual, Object[] expected, String message) { + void before(Object[] actual, String message, Object[] expected) { assertEquals(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object[] actual, Object[] expected, String message) { + void after(Object[] actual, String message, Object[] expected) { assertThat(actual).withFailMessage(message).containsExactly(expected); } } @@ -573,13 +573,13 @@ void after(Object[] actual, Object[] expected) { static final class AssertEqualArraysIrrespectiveOfOrderWithMessage { @BeforeTemplate - void before(Object[] actual, Object[] expected, String message) { + void before(Object[] actual, String message, Object[] expected) { assertEqualsNoOrder(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object[] actual, Object[] expected, String message) { + void after(Object[] actual, String message, Object[] expected) { assertThat(actual).withFailMessage(message).containsExactlyInAnyOrder(expected); } } @@ -601,13 +601,13 @@ void after(Iterator actual, Iterator expected) { static final class AssertEqualIteratorIterationOrderWithMessage { @BeforeTemplate - void before(Iterator actual, Iterator expected, String message) { + void before(Iterator actual, String message, Iterator expected) { assertEquals(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Iterator actual, Iterator expected, String message) { + void after(Iterator actual, String message, Iterator expected) { // XXX: This is not `null`-safe. // XXX: The `ImmutableList.copyOf` should actually *not* be imported statically. assertThat(actual) @@ -639,18 +639,18 @@ void after(Iterable actual, Iterable expected) { static final class AssertEqualIterableIterationOrderWithMessage { @BeforeTemplate - void before(Iterable actual, Iterable expected, String message) { + void before(Iterable actual, String message, Iterable expected) { assertEquals(actual, expected, message); } @BeforeTemplate - void before(Collection actual, Collection expected, String message) { + void before(Collection actual, String message, Collection expected) { assertEquals(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Iterable actual, Iterable expected, String message) { + void after(Iterable actual, String message, Iterable expected) { assertThat(actual).withFailMessage(message).containsExactlyElementsOf(expected); } } @@ -670,13 +670,13 @@ void after(Set actual, Set expected) { static final class AssertEqualSetsWithMessage { @BeforeTemplate - void before(Set actual, Set expected, String message) { + void before(Set actual, String message, Set expected) { assertEquals(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Set actual, Set expected, String message) { + void after(Set actual, String message, Set expected) { assertThat(actual).withFailMessage(message).hasSameElementsAs(expected); } } @@ -751,68 +751,68 @@ void after(Object actual, Object expected) { static final class AssertUnequalWithMessage { @BeforeTemplate - void before(boolean actual, boolean expected, String message) { + void before(boolean actual, String message, boolean expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(byte actual, byte expected, String message) { + void before(byte actual, String message, byte expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(char actual, char expected, String message) { + void before(char actual, String message, char expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(short actual, short expected, String message) { + void before(short actual, String message, short expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(int actual, int expected, String message) { + void before(int actual, String message, int expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(long actual, long expected, String message) { + void before(long actual, String message, long expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(float actual, float expected, String message) { + void before(float actual, String message, float expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(double actual, double expected, String message) { + void before(double actual, String message, double expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(Object actual, Object expected, String message) { + void before(Object actual, String message, Object expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(String actual, String expected, String message) { + void before(String actual, String message, String expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(Set actual, Set expected, String message) { + void before(Set actual, String message, Set expected) { assertNotEquals(actual, expected, message); } @BeforeTemplate - void before(Map actual, Map expected, String message) { + void before(Map actual, String message, Map expected) { assertNotEquals(actual, expected, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Object actual, Object expected, String message) { + void after(Object actual, String message, Object expected) { assertThat(actual).withFailMessage(message).isNotEqualTo(expected); } } @@ -832,13 +832,13 @@ void after(float actual, float expected, float delta) { static final class AssertUnequalFloatsWithDeltaWithMessage { @BeforeTemplate - void before(float actual, float expected, float delta, String message) { + void before(float actual, String message, float expected, float delta) { assertNotEquals(actual, expected, delta, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(float actual, float expected, float delta, String message) { + void after(float actual, String message, float expected, float delta) { assertThat(actual).withFailMessage(message).isNotCloseTo(expected, offset(delta)); } } @@ -858,13 +858,13 @@ void after(double actual, double expected, double delta) { static final class AssertUnequalDoublesWithDeltaWithMessage { @BeforeTemplate - void before(double actual, double expected, double delta, String message) { + void before(double actual, String message, double expected, double delta) { assertNotEquals(actual, expected, delta, message); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(double actual, double expected, double delta, String message) { + void after(double actual, String message, double expected, double delta) { assertThat(actual).withFailMessage(message).isNotCloseTo(expected, offset(delta)); } } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrderTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrderTest.java new file mode 100644 index 0000000000..6ac21f65ab --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrderTest.java @@ -0,0 +1,130 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class RefasterMethodParameterOrderTest { + @Test + void identification() { + CompilationTestHelper.newInstance(RefasterMethodParameterOrder.class, getClass()) + .addSourceLines( + "A.java", + "import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;", + "import static org.assertj.core.api.Assertions.assertThat;", + "", + "import com.google.errorprone.refaster.annotation.AfterTemplate;", + "import com.google.errorprone.refaster.annotation.BeforeTemplate;", + "import com.google.errorprone.refaster.annotation.Placeholder;", + "import com.google.errorprone.refaster.annotation.UseImportPolicy;", + "import java.util.Map;", + "", + "class A {", + " class UnusedLexicographicallyOrderedParameters {", + " @BeforeTemplate", + " void singleParam(int a) {}", + "", + " @BeforeTemplate", + " void twoParams(int a, int b) {}", + "", + " @Placeholder", + " void notATemplateMethod(int b, int a) {}", + " }", + "", + " class NonParameterValueIdentifierIsIgnored {", + " @AfterTemplate", + " @UseImportPolicy(value = STATIC_IMPORT_ALWAYS)", + " void after(Map map, V value) {", + " assertThat(map).containsValue(value);", + " }", + " }", + "", + " // BUG: Diagnostic contains:", + " class UnusedLexicographicallyUnorderedParameters {", + " @BeforeTemplate", + " void foo(int a, int b) {}", + "", + " @BeforeTemplate", + " void bar(int b, int a) {}", + " }", + "}") + .doTest(); + } + + @Test + void replacement() { + BugCheckerRefactoringTestHelper.newInstance(RefasterMethodParameterOrder.class, getClass()) + .addInputLines( + "A.java", + "import com.google.errorprone.refaster.annotation.AfterTemplate;", + "import com.google.errorprone.refaster.annotation.BeforeTemplate;", + "", + "class A {", + " class UnusedUnsortedParameters {", + " @BeforeTemplate", + " void before(int b, int a) {}", + " }", + "", + " class UnsortedParametersWithoutAfterTemplate {", + " @BeforeTemplate", + " int before(int a, int b, int c, int d) {", + " return b + a + d + b + c;", + " }", + " }", + "", + " class UnsortedParametersWithMultipleMethodsAndParameterCounts {", + " @BeforeTemplate", + " int before(int b, int a, int g, int f, int d) {", + " return f + a + g + b + d;", + " }", + "", + " @AfterTemplate", + " int after(int a, int b) {", + " return b + a;", + " }", + "", + " @AfterTemplate", + " int after2(int a, int d, int f) {", + " return d + a + f;", + " }", + " }", + "}") + .addOutputLines( + "A.java", + "import com.google.errorprone.refaster.annotation.AfterTemplate;", + "import com.google.errorprone.refaster.annotation.BeforeTemplate;", + "", + "class A {", + " class UnusedUnsortedParameters {", + " @BeforeTemplate", + " void before(int a, int b) {}", + " }", + "", + " class UnsortedParametersWithoutAfterTemplate {", + " @BeforeTemplate", + " int before(int b, int a, int d, int c) {", + " return b + a + d + b + c;", + " }", + " }", + "", + " class UnsortedParametersWithMultipleMethodsAndParameterCounts {", + " @BeforeTemplate", + " int before(int d, int a, int f, int b, int g) {", + " return f + a + g + b + d;", + " }", + "", + " @AfterTemplate", + " int after(int a, int b) {", + " return b + a;", + " }", + "", + " @AfterTemplate", + " int after2(int d, int a, int f) {", + " return d + a + f;", + " }", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestInput.java index 93b9764079..016580a647 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestInput.java @@ -69,15 +69,14 @@ Predicate testPredicateLambda() { return not(v -> v.isEmpty()); } - boolean testEqualsLhsNullable() { - return Optional.ofNullable("foo").equals(Optional.of("bar")); - } - - boolean testEqualsRhsNullable() { - return Optional.of("foo").equals(Optional.ofNullable("bar")); + ImmutableSet testEquals() { + return ImmutableSet.of( + Optional.of("foo").equals(Optional.of("bar")), + Optional.of("baz").equals(Optional.ofNullable("qux")), + Optional.ofNullable("quux").equals(Optional.of("quuz"))); } - boolean testEqualsLhsAndRhsNullable() { + boolean testObjectsEquals() { return Optional.ofNullable("foo").equals(Optional.ofNullable("bar")); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestOutput.java index c61eb76748..39bab876fa 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/EqualityRulesTestOutput.java @@ -69,15 +69,11 @@ Predicate testPredicateLambda() { return v -> !v.isEmpty(); } - boolean testEqualsLhsNullable() { - return "bar".equals("foo"); + ImmutableSet testEquals() { + return ImmutableSet.of("foo".equals("bar"), "baz".equals("qux"), "quuz".equals("quux")); } - boolean testEqualsRhsNullable() { - return "foo".equals("bar"); - } - - boolean testEqualsLhsAndRhsNullable() { + boolean testObjectsEquals() { return Objects.equals("foo", "bar"); } }