diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java index 70fb2a6f9..a93d1c40d 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java @@ -18,9 +18,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import com.google.auto.service.AutoService; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; @@ -39,6 +41,7 @@ import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.Tree; import com.sun.source.util.SimpleTreeVisitor; +import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import java.util.List; import java.util.Optional; @@ -548,7 +551,15 @@ public boolean apply(Type type, VisitorState state) { // Note: cannot match array/vararg arguments private static final Matcher HAS_ITEMS = MethodMatchers.staticMethod() .onClass(MATCHERS) - .namedAnyOf("hasItems", "arrayContainingInAnyOrder"); + .named("hasItems"); + + private static final Matcher CONTAINS_EXACTLY_ANY_ORDER = MethodMatchers.staticMethod() + .onClass(MATCHERS) + .namedAnyOf("arrayContainingInAnyOrder", "containsInAnyOrder"); + + private static final Matcher CONTAINS_EXACTLY_ORDERED = MethodMatchers.staticMethod() + .onClass(MATCHERS) + .named("contains"); private static final Matcher IS_EMPTY = Matchers.anyOf( MethodMatchers.staticMethod() @@ -634,8 +645,7 @@ public Optional visitMethodInvocation(MethodInvocationTree node, Visitor return Optional.of(".hasSize(" + argSource(node, state, 0) + ')'); } - if (HAS_ITEMS.matches(node, state) - && checkNotNull(ASTHelpers.getSymbol(node), "symbol").isVarArgs()) { + if (HAS_ITEMS.matches(node, state) && isObjectVarArgs(node, state)) { if (negated) { // this negates to 'doesNotContainAll' which doesn't exist. AssertJ doesNotContain // evaluates as 'does not contain any'. @@ -645,7 +655,39 @@ && checkNotNull(ASTHelpers.getSymbol(node), "symbol").isVarArgs()) { .map(state::getSourceForNode) .collect(Collectors.joining(", ")) + ')'); } + if (CONTAINS_EXACTLY_ANY_ORDER.matches(node, state) && isObjectVarArgs(node, state)) { + if (negated) { + // this negates to 'doesNotContainExactlyInAnyOrder' which doesn't exist. + return Optional.empty(); + } + return Optional.of(".containsExactlyInAnyOrder(" + node.getArguments().stream() + .map(state::getSourceForNode) + .collect(Collectors.joining(", ")) + ')'); + } + if (CONTAINS_EXACTLY_ORDERED.matches(node, state) && isObjectVarArgs(node, state)) { + if (negated) { + // this negates to 'doesNotContainExactly' which doesn't exist. + return Optional.empty(); + } + return Optional.of(".containsExactly(" + node.getArguments().stream() + .map(state::getSourceForNode) + .collect(Collectors.joining(", ")) + ')'); + } return Optional.empty(); } } + + private static boolean isObjectVarArgs(MethodInvocationTree tree, VisitorState state) { + Symbol.MethodSymbol methodSymbol = checkNotNull(ASTHelpers.getSymbol(tree), "symbol"); + if (!methodSymbol.isVarArgs()) { + return false; + } + Symbol.VarSymbol varSymbol = Iterables.getLast(methodSymbol.getParameters()); + checkState(varSymbol.type instanceof Type.ArrayType, "Expected an array as last argument of a vararg method"); + Type.ArrayType arrayType = (Type.ArrayType) varSymbol.type; + return ASTHelpers.isSameType( + arrayType.getComponentType(), + state.getTypeFromString(Object.class.getName()), + state); + } } diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferAssertjTests.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferAssertjTests.java index 58d448133..5e87060ab 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferAssertjTests.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferAssertjTests.java @@ -1040,6 +1040,8 @@ public void fix_assertThat_contains() { " assertThat(arrayValue, hasItemInArray(\"str\"));", " assertThat(value, hasItems(\"one\", \"two\"));", " assertThat(arrayValue, arrayContainingInAnyOrder(\"one\", \"two\"));", + " assertThat(value, containsInAnyOrder(\"one\", \"two\"));", + " assertThat(value, contains(\"one\", \"two\"));", " assertThat(value, not(hasItem(\"str\")));", " assertThat(arrayValue, not(hasItemInArray(\"str\")));", " assertThat(arrayValue[0], containsString(\"str\"));", @@ -1054,7 +1056,9 @@ public void fix_assertThat_contains() { " assertThat(value).contains(\"str\");", " assertThat(arrayValue).contains(\"str\");", " assertThat(value).contains(\"one\", \"two\");", - " assertThat(arrayValue).contains(\"one\", \"two\");", + " assertThat(arrayValue).containsExactlyInAnyOrder(\"one\", \"two\");", + " assertThat(value).containsExactlyInAnyOrder(\"one\", \"two\");", + " assertThat(value).containsExactly(\"one\", \"two\");", " assertThat(value).doesNotContain(\"str\");", " assertThat(arrayValue).doesNotContain(\"str\");", " assertThat(arrayValue[0]).contains(\"str\");", diff --git a/changelog/@unreleased/pr-859.v2.yml b/changelog/@unreleased/pr-859.v2.yml new file mode 100644 index 000000000..f37c3289f --- /dev/null +++ b/changelog/@unreleased/pr-859.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Fix hamcrest arrayContainingInAnyOrder conversion + links: + - https://github.com/palantir/gradle-baseline/pull/859