Skip to content

Commit 6ea4619

Browse files
knutwannhedenknutwannheden
authored
Fix bugs in HamcrestMatcherToAssertJ and HamcrestIsMatcherToAssertJ (#863)
* `HamcrestMatcherToAssertJ`: `assertThat(<double>, closeTo(<int>))` An assertion `assertThat(<double>, closeTo(<int>))` needs to be treated separately, as AssertJ requires a `within(<double>)` overload to be called. Fixes: #861 * `HamcrestIsMatcherToAssertJ`: Only transform Hamcrest `assertThat()` There was a bug in `HamcrestIsMatcherToAssertJ`: It always registered an after visitor to transform all `is()` assertions, even though some may require `containsExactly()` and others `isEqualTo()`. Fixes: #861
1 parent bd7ec84 commit 6ea4619

File tree

4 files changed

+164
-16
lines changed

4 files changed

+164
-16
lines changed

‎src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestIsMatcherToAssertJ.java‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import org.openrewrite.Recipe;
2121
import org.openrewrite.TreeVisitor;
2222
import org.openrewrite.java.JavaIsoVisitor;
23+
import org.openrewrite.java.JavaParser;
24+
import org.openrewrite.java.JavaTemplate;
2325
import org.openrewrite.java.MethodMatcher;
2426
import org.openrewrite.java.search.UsesMethod;
2527
import org.openrewrite.java.tree.Expression;
@@ -39,22 +41,57 @@ public String getDescription() {
3941
return "Migrate Hamcrest `is(Object)` to AssertJ `Assertions.assertThat(..)`.";
4042
}
4143

42-
static final MethodMatcher IS_OBJECT_MATCHER = new MethodMatcher("org.hamcrest.*Matchers is(..)");
44+
private static final MethodMatcher ASSERT_THAT_MATCHER = new MethodMatcher("org.hamcrest.MatcherAssert assertThat(..)");
45+
private static final MethodMatcher IS_MATCHER = new MethodMatcher("org.hamcrest.*Matchers is(..)");
46+
private static final MethodMatcher IS_WITH_NESTED_MATCHER = new MethodMatcher("org.hamcrest.*Matchers is(org.hamcrest.Matcher)");
4347

4448
@Override
4549
public TreeVisitor<?, ExecutionContext> getVisitor() {
46-
return Preconditions.check(new UsesMethod<>(IS_OBJECT_MATCHER), new JavaIsoVisitor<ExecutionContext>() {
50+
return Preconditions.check(new UsesMethod<>(IS_MATCHER), new JavaIsoVisitor<ExecutionContext>() {
4751
@Override
48-
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) {
52+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
53+
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
54+
if (!ASSERT_THAT_MATCHER.matches(mi)) {
55+
return mi;
56+
}
4957

50-
// Switch between one or the other depending on whether actual argument is an array or not
51-
List<Expression> arguments = methodInvocation.getArguments();
52-
String replacement = 2 <= arguments.size() &&
53-
TypeUtils.asArray(arguments.get(arguments.size() - 2).getType()) != null ?
54-
"containsExactly" : "isEqualTo";
55-
doAfterVisit(new HamcrestMatcherToAssertJ("is", replacement, null).getVisitor());
58+
List<Expression> args = mi.getArguments();
59+
Expression reasonArgument = args.size() == 3 ? args.get(0) : null;
60+
Expression actualArgument = args.get(args.size() - 2);
61+
Expression matcherArgument = args.get(args.size() - 1);
5662

57-
return super.visitMethodInvocation(methodInvocation, ctx);
63+
// Only handle is() matcher, but not is(Matcher) which nests another matcher
64+
if (!IS_MATCHER.matches(matcherArgument) || IS_WITH_NESTED_MATCHER.matches(matcherArgument)) {
65+
return mi;
66+
}
67+
68+
// Choose assertion based on whether actual argument is an array
69+
boolean isArray = TypeUtils.asArray(actualArgument.getType()) != null;
70+
71+
J.MethodInvocation isInvocation = (J.MethodInvocation) matcherArgument;
72+
Expression expectedArgument = isInvocation.getArguments().get(0);
73+
74+
maybeRemoveImport("org.hamcrest.Matchers.is");
75+
maybeRemoveImport("org.hamcrest.CoreMatchers.is");
76+
maybeRemoveImport("org.hamcrest.MatcherAssert");
77+
maybeRemoveImport("org.hamcrest.MatcherAssert.assertThat");
78+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
79+
80+
String reason = reasonArgument != null ? ".as(#{any(String)})" : "";
81+
// Using `#{any(Object)}` and `#{any(Object[])}` here as the actual types could be from the source path
82+
String template = isArray ?
83+
"assertThat(#{any(Object[])})" + reason + ".containsExactly(#{any(Object[])})" :
84+
"assertThat(#{any(Object)})" + reason + ".isEqualTo(#{any(Object)})";
85+
86+
Object[] templateArgs = reasonArgument != null ?
87+
new Object[]{actualArgument, reasonArgument, expectedArgument} :
88+
new Object[]{actualArgument, expectedArgument};
89+
90+
return JavaTemplate.builder(template)
91+
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3"))
92+
.staticImports("org.assertj.core.api.Assertions.assertThat")
93+
.build()
94+
.apply(getCursor(), mi.getCoordinates().replace(), templateArgs);
5895
}
5996
});
6097
}

‎src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToAssertJ.java‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@
2424
import org.openrewrite.java.JavaTemplate;
2525
import org.openrewrite.java.MethodMatcher;
2626
import org.openrewrite.java.search.UsesMethod;
27-
import org.openrewrite.java.tree.Expression;
28-
import org.openrewrite.java.tree.J;
29-
import org.openrewrite.java.tree.JavaType;
30-
import org.openrewrite.java.tree.TypeUtils;
27+
import org.openrewrite.java.tree.*;
28+
import org.openrewrite.marker.Markers;
3129

3230
import java.util.ArrayList;
3331
import java.util.List;
@@ -124,16 +122,46 @@ actual, assertion, getArgumentsTemplate(matcherArgumentMethod)))
124122
if (reasonArgument != null) {
125123
templateArguments.add(reasonArgument);
126124
}
127-
for (Expression originalArgument : matcherArgumentMethod.getArguments()) {
125+
boolean isCloseTo = CLOSE_TO_MATCHER.matches(matcherArgumentMethod);
126+
List<Expression> matcherArgs = matcherArgumentMethod.getArguments();
127+
for (int i = 0; i < matcherArgs.size(); i++) {
128+
Expression originalArgument = matcherArgs.get(i);
128129
if (!(originalArgument instanceof J.Empty)) {
129-
templateArguments.add(originalArgument);
130+
// For closeTo, the tolerance (second argument) type must match the actual value type for within()
131+
if (isCloseTo && i == 1) {
132+
templateArguments.add(ensureMatchingNumericType(originalArgument, actualArgument.getType()));
133+
} else {
134+
templateArguments.add(originalArgument);
135+
}
130136
}
131137
}
132138
return template.apply(getCursor(), mi.getCoordinates().replace(), templateArguments.toArray());
133139
}
134140

135141
private final MethodMatcher CLOSE_TO_MATCHER = new MethodMatcher("org.hamcrest.Matchers closeTo(..)");
136142

143+
private Expression ensureMatchingNumericType(Expression toleranceExpr, JavaType actualType) {
144+
JavaType toleranceType = toleranceExpr.getType();
145+
// Only cast if the actual value is a double/float and tolerance is an integer type
146+
if ((actualType == JavaType.Primitive.Double || TypeUtils.isOfClassType(actualType, "java.lang.Double")) &&
147+
(toleranceType == JavaType.Primitive.Int || toleranceType == JavaType.Primitive.Long)) {
148+
// Wrap with (double) cast to ensure correct within() overload is called
149+
return new J.TypeCast(
150+
Tree.randomId(),
151+
toleranceExpr.getPrefix(),
152+
Markers.EMPTY,
153+
new J.ControlParentheses<>(
154+
Tree.randomId(),
155+
Space.EMPTY,
156+
Markers.EMPTY,
157+
JRightPadded.build(new J.Primitive(Tree.randomId(), Space.EMPTY, Markers.EMPTY, JavaType.Primitive.Double))
158+
),
159+
toleranceExpr.withPrefix(Space.SINGLE_SPACE)
160+
);
161+
}
162+
return toleranceExpr;
163+
}
164+
137165
private String getArgumentsTemplate(J.MethodInvocation matcherArgument) {
138166
List<Expression> methodArguments = matcherArgument.getArguments();
139167
if (CLOSE_TO_MATCHER.matches(matcherArgument)) {

‎src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestIsMatcherToAssertJTest.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.junit.jupiter.api.Test;
1919
import org.openrewrite.DocumentExample;
2020
import org.openrewrite.InMemoryExecutionContext;
21+
import org.openrewrite.Issue;
2122
import org.openrewrite.java.JavaParser;
2223
import org.openrewrite.test.RecipeSpec;
2324
import org.openrewrite.test.RewriteTest;
@@ -210,4 +211,48 @@ void testEquals() {
210211
)
211212
);
212213
}
214+
215+
@Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/861")
216+
@Test
217+
void isMatcherWithOtherMethodCallWithArrayArg() {
218+
rewriteRun(
219+
//language=java
220+
java(
221+
"""
222+
import org.junit.jupiter.api.Test;
223+
import java.util.Arrays;
224+
import static org.hamcrest.MatcherAssert.assertThat;
225+
import static org.hamcrest.Matchers.is;
226+
227+
class ATest {
228+
@Test
229+
void testEquals() {
230+
String[] arr = new String[]{"a"};
231+
String[] copy = Arrays.copyOf(arr, 5);
232+
String str1 = "Hello";
233+
String str2 = "Hello";
234+
assertThat(str1, is(str2));
235+
}
236+
}
237+
""",
238+
"""
239+
import org.junit.jupiter.api.Test;
240+
import java.util.Arrays;
241+
242+
import static org.assertj.core.api.Assertions.assertThat;
243+
244+
class ATest {
245+
@Test
246+
void testEquals() {
247+
String[] arr = new String[]{"a"};
248+
String[] copy = Arrays.copyOf(arr, 5);
249+
String str1 = "Hello";
250+
String str2 = "Hello";
251+
assertThat(str1).isEqualTo(str2);
252+
}
253+
}
254+
"""
255+
)
256+
);
257+
}
213258
}

‎src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToAssertJTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.junit.jupiter.api.Test;
2020
import org.openrewrite.DocumentExample;
2121
import org.openrewrite.InMemoryExecutionContext;
22+
import org.openrewrite.Issue;
2223
import org.openrewrite.java.JavaParser;
2324
import org.openrewrite.test.RecipeSpec;
2425
import org.openrewrite.test.RewriteTest;
@@ -464,6 +465,43 @@ void replaceCloseTo() {
464465
);
465466
}
466467

468+
@Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/861")
469+
@Test
470+
void closeToWithIntTolerance() {
471+
rewriteRun(
472+
spec -> spec.recipe(new HamcrestMatcherToAssertJ("closeTo", "isCloseTo", null)),
473+
//language=java
474+
java(
475+
"""
476+
import org.junit.jupiter.api.Test;
477+
478+
import static org.hamcrest.MatcherAssert.assertThat;
479+
import static org.hamcrest.Matchers.closeTo;
480+
481+
class ATest {
482+
@Test
483+
void replaceCloseTo() {
484+
assertThat(123.123, closeTo(123.123, 1));
485+
}
486+
}
487+
""",
488+
"""
489+
import org.junit.jupiter.api.Test;
490+
491+
import static org.assertj.core.api.Assertions.assertThat;
492+
import static org.assertj.core.api.Assertions.within;
493+
494+
class ATest {
495+
@Test
496+
void replaceCloseTo() {
497+
assertThat(123.123).isCloseTo(123.123, within((double) 1));
498+
}
499+
}
500+
"""
501+
)
502+
);
503+
}
504+
467505
@Test
468506
void closeToWorksWithBigDecimal() {
469507
rewriteRun(

0 commit comments

Comments
 (0)