Skip to content

Commit ac5b545

Browse files
timtebeektimtebeek
authored
Hoist matchers created per visit to static constants (#1225)
1 parent 8720326 commit ac5b545

15 files changed

Lines changed: 107 additions & 93 deletions

‎src/main/java/org/openrewrite/java/testing/arquillian/ReplaceArquillianInSequenceAnnotation.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@
2828
import java.util.Comparator;
2929

3030
public class ReplaceArquillianInSequenceAnnotation extends Recipe {
31+
private static final String IN_SEQUENCE = "org.jboss.arquillian.junit.InSequence";
32+
private static final String TEST_METHOD_ORDER = "org.junit.jupiter.api.TestMethodOrder";
33+
private static final String METHOD_ORDERER = "org.junit.jupiter.api.MethodOrderer";
34+
private static final AnnotationMatcher IN_SEQUENCE_MATCHER = new AnnotationMatcher("@" + IN_SEQUENCE);
35+
private static final AnnotationMatcher TEST_METHOD_ORDER_MATCHER = new AnnotationMatcher("@" + TEST_METHOD_ORDER);
3136

3237
@Getter
3338
final String displayName = "Arquillian JUnit 4 `@InSequence` to JUnit Jupiter `@Order`";
@@ -40,12 +45,6 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
4045
return Preconditions.check(
4146
new UsesType<>("org.jboss.arquillian.junit.InSequence", false),
4247
new JavaIsoVisitor<ExecutionContext>() {
43-
private final String IN_SEQUENCE = "org.jboss.arquillian.junit.InSequence";
44-
private final String TEST_METHOD_ORDER = "org.junit.jupiter.api.TestMethodOrder";
45-
private final String METHOD_ORDERER = "org.junit.jupiter.api.MethodOrderer";
46-
private final AnnotationMatcher IN_SEQUENCE_MATCHER = new AnnotationMatcher("@" + IN_SEQUENCE);
47-
private final AnnotationMatcher TEST_METHOD_ORDER_MATCHER = new AnnotationMatcher("@" + TEST_METHOD_ORDER);
48-
4948
@Override
5049
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) {
5150
doAfterVisit(new ChangeType(IN_SEQUENCE, "org.junit.jupiter.api.Order", true).getVisitor());

‎src/main/java/org/openrewrite/java/testing/cleanup/AssertionsArgumentOrder.java‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,15 @@
3838

3939
public class AssertionsArgumentOrder extends Recipe {
4040

41-
private static final MethodMatcher[] jupiterAssertionMatchers = new MethodMatcher[]{
41+
private static final MethodMatcher[] JUPITER_ASSERTION_MATCHERS = new MethodMatcher[]{
4242
new MethodMatcher("org.junit.jupiter.api.Assertions assertArrayEquals(..)"),
4343
new MethodMatcher("org.junit.jupiter.api.Assertions assertEquals(..)"),
4444
new MethodMatcher("org.junit.jupiter.api.Assertions assertNotEquals(..)"),
4545
new MethodMatcher("org.junit.jupiter.api.Assertions assertSame(..)"),
4646
new MethodMatcher("org.junit.jupiter.api.Assertions assertNotSame(..)")
4747
};
4848

49-
private static final MethodMatcher[] junitAssertMatchers = new MethodMatcher[]{
49+
private static final MethodMatcher[] JUNIT_ASSERT_MATCHERS = new MethodMatcher[]{
5050
new MethodMatcher("org.junit.Assert assertEquals(..)"),
5151
new MethodMatcher("org.junit.Assert assertEquals(..)"),
5252
new MethodMatcher("org.junit.Assert assertArrayEquals(..)"),
@@ -55,17 +55,17 @@ public class AssertionsArgumentOrder extends Recipe {
5555
new MethodMatcher("org.junit.Assert assert*Null(String, Object)")
5656
};
5757

58-
private static final MethodMatcher[] junitAssertWithMessageMatchers = new MethodMatcher[]{
58+
private static final MethodMatcher[] JUNIT_ASSERT_WITH_MESSAGE_MATCHERS = new MethodMatcher[]{
5959
new MethodMatcher("org.junit.Assert assertEquals(String, ..)"),
6060
new MethodMatcher("org.junit.Assert assertArrayEquals(String, ..)")
6161
};
62-
private static final MethodMatcher jupiterAssertIterableEqualsMatcher = new MethodMatcher("org.junit.jupiter.api.Assertions assertIterableEquals(..)");
62+
private static final MethodMatcher JUPITER_ASSERT_ITERABLE_EQUALS_MATCHER = new MethodMatcher("org.junit.jupiter.api.Assertions assertIterableEquals(..)");
6363

6464
// `assertNull("message", result())` should be `assertNull(result(), "message")`
65-
private static final MethodMatcher jupiterAssertNullMatcher = new MethodMatcher("org.junit.jupiter.api.Assertions assert*Null(Object, String)");
65+
private static final MethodMatcher JUPITER_ASSERT_NULL_MATCHER = new MethodMatcher("org.junit.jupiter.api.Assertions assert*Null(Object, String)");
6666

6767
// Moving a null literal into the expected position short circuits the equality check, rather than calling equals
68-
private static final MethodMatcher[] equalityMatchers = new MethodMatcher[]{
68+
private static final MethodMatcher[] EQUALITY_MATCHERS = new MethodMatcher[]{
6969
new MethodMatcher("org.junit.jupiter.api.Assertions assertEquals(..)"),
7070
new MethodMatcher("org.junit.jupiter.api.Assertions assertNotEquals(..)"),
7171
new MethodMatcher("org.junit.Assert assertEquals(..)"),
@@ -74,24 +74,31 @@ public class AssertionsArgumentOrder extends Recipe {
7474
new MethodMatcher("org.testng.Assert assertNotEquals(..)")
7575
};
7676

77-
private static final MethodMatcher[] testNgMatcher = new MethodMatcher[]{
77+
private static final MethodMatcher[] TESTNG_MATCHERS = new MethodMatcher[]{
7878
new MethodMatcher("org.testng.Assert assertSame(..)"),
7979
new MethodMatcher("org.testng.Assert assertNotSame(..)"),
8080
new MethodMatcher("org.testng.Assert assertEquals(..)"),
8181
new MethodMatcher("org.testng.Assert assertNotEquals(..)")
8282
};
8383

84-
private static final TreeVisitor<?, ExecutionContext> precondition;
84+
private static final MethodMatcher[] NEW_LIST_MATCHERS = new MethodMatcher[]{
85+
new MethodMatcher("java.util.List of(..)"),
86+
new MethodMatcher("java.util.Collections singleton(..)"),
87+
new MethodMatcher("java.util.Collections empty()"),
88+
new MethodMatcher("java.util.Arrays asList(..)"),
89+
};
90+
91+
private static final TreeVisitor<?, ExecutionContext> PRECONDITION;
8592

8693
static {
87-
List<MethodMatcher> matchers = new ArrayList<>(Arrays.asList(jupiterAssertionMatchers));
88-
matchers.addAll(Arrays.asList(junitAssertMatchers));
89-
matchers.addAll(Arrays.asList(junitAssertWithMessageMatchers));
90-
matchers.add(jupiterAssertIterableEqualsMatcher);
91-
matchers.add(jupiterAssertNullMatcher);
92-
matchers.addAll(Arrays.asList(testNgMatcher));
94+
List<MethodMatcher> matchers = new ArrayList<>(Arrays.asList(JUPITER_ASSERTION_MATCHERS));
95+
matchers.addAll(Arrays.asList(JUNIT_ASSERT_MATCHERS));
96+
matchers.addAll(Arrays.asList(JUNIT_ASSERT_WITH_MESSAGE_MATCHERS));
97+
matchers.add(JUPITER_ASSERT_ITERABLE_EQUALS_MATCHER);
98+
matchers.add(JUPITER_ASSERT_NULL_MATCHER);
99+
matchers.addAll(Arrays.asList(TESTNG_MATCHERS));
93100
//noinspection unchecked
94-
precondition = Preconditions.or(matchers.stream().map(UsesMethod::new).toArray(TreeVisitor[]::new));
101+
PRECONDITION = Preconditions.or(matchers.stream().map(UsesMethod::new).toArray(TreeVisitor[]::new));
95102
}
96103

97104
@Getter
@@ -105,17 +112,10 @@ public class AssertionsArgumentOrder extends Recipe {
105112

106113
@Override
107114
public TreeVisitor<?, ExecutionContext> getVisitor() {
108-
return Preconditions.check(precondition, new AssertionsArgumentOrderVisitor());
115+
return Preconditions.check(PRECONDITION, new AssertionsArgumentOrderVisitor());
109116
}
110117

111118
private static class AssertionsArgumentOrderVisitor extends JavaIsoVisitor<ExecutionContext> {
112-
private final MethodMatcher[] newListMatchers = new MethodMatcher[]{
113-
new MethodMatcher("java.util.List of(..)"),
114-
new MethodMatcher("java.util.Collections singleton(..)"),
115-
new MethodMatcher("java.util.Collections empty()"),
116-
new MethodMatcher("java.util.Arrays asList(..)"),
117-
};
118-
119119
@Override
120120
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
121121
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
@@ -154,7 +154,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
154154
}
155155

156156
private boolean isEqualityAssertion(J.MethodInvocation mi) {
157-
for (MethodMatcher equalityMatcher : equalityMatchers) {
157+
for (MethodMatcher equalityMatcher : EQUALITY_MATCHERS) {
158158
if (equalityMatcher.matches(mi)) {
159159
return true;
160160
}
@@ -171,7 +171,7 @@ private boolean isNullLiteral(Expression expression) {
171171
}
172172

173173
private boolean isCorrectOrder(Expression expected, Expression actual, J.MethodInvocation mi) {
174-
if (jupiterAssertNullMatcher.matches(mi)) {
174+
if (JUPITER_ASSERT_NULL_MATCHER.matches(mi)) {
175175
return isConstant(actual, mi) || !isConstant(expected, mi);
176176
}
177177
return isConstant(expected, mi) || !isConstant(actual, mi);
@@ -197,8 +197,8 @@ private boolean isConstant(Expression expression, J.MethodInvocation mi) {
197197
return var.hasFlags(Flag.Static, Flag.Final);
198198
}
199199

200-
if (jupiterAssertIterableEqualsMatcher.matches(mi)) {
201-
for (MethodMatcher iterableMatcher : newListMatchers) {
200+
if (JUPITER_ASSERT_ITERABLE_EQUALS_MATCHER.matches(mi)) {
201+
for (MethodMatcher iterableMatcher : NEW_LIST_MATCHERS) {
202202
if (iterableMatcher.matches(expression)) {
203203
return true;
204204
}
@@ -208,16 +208,16 @@ private boolean isConstant(Expression expression, J.MethodInvocation mi) {
208208
}
209209

210210
private boolean isJupiterAssertion(J.MethodInvocation mi) {
211-
for (MethodMatcher assertionMethodMatcher : jupiterAssertionMatchers) {
211+
for (MethodMatcher assertionMethodMatcher : JUPITER_ASSERTION_MATCHERS) {
212212
if (assertionMethodMatcher.matches(mi)) {
213213
return true;
214214
}
215215
}
216-
return jupiterAssertIterableEqualsMatcher.matches(mi) || jupiterAssertNullMatcher.matches(mi);
216+
return JUPITER_ASSERT_ITERABLE_EQUALS_MATCHER.matches(mi) || JUPITER_ASSERT_NULL_MATCHER.matches(mi);
217217
}
218218

219219
private boolean isTestNgAssertion(J.MethodInvocation mi) {
220-
for (MethodMatcher actExpMatcher : testNgMatcher) {
220+
for (MethodMatcher actExpMatcher : TESTNG_MATCHERS) {
221221
if (actExpMatcher.matches(mi)) {
222222
return true;
223223
}
@@ -226,7 +226,7 @@ private boolean isTestNgAssertion(J.MethodInvocation mi) {
226226
}
227227

228228
private boolean isJunitAssertion(J.MethodInvocation mi) {
229-
for (MethodMatcher assertionMethodMatcher : junitAssertMatchers) {
229+
for (MethodMatcher assertionMethodMatcher : JUNIT_ASSERT_MATCHERS) {
230230
if (assertionMethodMatcher.matches(mi)) {
231231
return true;
232232
}
@@ -235,7 +235,7 @@ private boolean isJunitAssertion(J.MethodInvocation mi) {
235235
}
236236

237237
private boolean isJunitAssertEqualsWithMessage(J.MethodInvocation mi) {
238-
for (MethodMatcher actExpMatcher : junitAssertWithMessageMatchers) {
238+
for (MethodMatcher actExpMatcher : JUNIT_ASSERT_WITH_MESSAGE_MATCHERS) {
239239
if (actExpMatcher.matches(mi)) {
240240
return true;
241241
}

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
@NoArgsConstructor
3636
@AllArgsConstructor
3737
public class HamcrestMatcherToAssertJ extends Recipe {
38+
private static final MethodMatcher ASSERT_THAT_MATCHER = new MethodMatcher("org.hamcrest.MatcherAssert assertThat(..)");
39+
private static final MethodMatcher SUB_MATCHER = new MethodMatcher("org.hamcrest.*Matchers *(org.hamcrest.Matcher)");
40+
private static final MethodMatcher CLOSE_TO_MATCHER = new MethodMatcher("org.hamcrest.Matchers closeTo(..)");
3841

3942
@Option(displayName = "Hamcrest matcher",
4043
description = "The Hamcrest `Matcher` to migrate to JUnit5.",
@@ -69,17 +72,15 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
6972
}
7073

7174
private class MigrateToAssertJVisitor extends JavaIsoVisitor<ExecutionContext> {
72-
private final MethodMatcher assertThatMatcher = new MethodMatcher("org.hamcrest.MatcherAssert assertThat(..)");
7375
private final MethodMatcher matchersMatcher = new MethodMatcher("org.hamcrest.*Matchers " + matcher + "(..)");
74-
private final MethodMatcher subMatcher = new MethodMatcher("org.hamcrest.*Matchers *(org.hamcrest.Matcher)");
7576

7677
// AssertJ assertions that don't take any arguments - matcher arguments should be ignored
7778
private final Set<String> noArgAssertions = new HashSet<>(Arrays.asList("isNotNull", "isNull"));
7879

7980
@Override
8081
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
8182
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
82-
if (assertThatMatcher.matches(mi)) {
83+
if (ASSERT_THAT_MATCHER.matches(mi)) {
8384
return replace(mi, ctx);
8485
}
8586
return mi;
@@ -90,7 +91,7 @@ private J.MethodInvocation replace(J.MethodInvocation mi, ExecutionContext ctx)
9091
Expression reasonArgument = mia.size() == 3 ? mia.get(0) : null;
9192
Expression actualArgument = mia.get(mia.size() - 2);
9293
Expression matcherArgument = mia.get(mia.size() - 1);
93-
if (!matchersMatcher.matches(matcherArgument) || subMatcher.matches(matcherArgument)) {
94+
if (!matchersMatcher.matches(matcherArgument) || SUB_MATCHER.matches(matcherArgument)) {
9495
return mi;
9596
}
9697
if (argumentType != null && !TypeUtils.isAssignableTo(argumentType, actualArgument.getType())) {
@@ -143,8 +144,6 @@ private J.MethodInvocation replace(J.MethodInvocation mi, ExecutionContext ctx)
143144
return template.apply(getCursor(), mi.getCoordinates().replace(), templateArguments.toArray());
144145
}
145146

146-
private final MethodMatcher CLOSE_TO_MATCHER = new MethodMatcher("org.hamcrest.Matchers closeTo(..)");
147-
148147
private Expression ensureMatchingNumericType(Expression toleranceExpr, @Nullable JavaType actualType) {
149148
JavaType toleranceType = toleranceExpr.getType();
150149
// Only cast if the actual value is a double/float and tolerance is an integer type

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@
3838
@NoArgsConstructor
3939
@AllArgsConstructor
4040
public class HamcrestNotMatcherToAssertJ extends Recipe {
41+
private static final MethodMatcher ASSERT_THAT_MATCHER = new MethodMatcher("org.hamcrest.MatcherAssert assertThat(..)");
42+
private static final MethodMatcher NOT_MATCHER = new MethodMatcher("org.hamcrest.*Matchers not(org.hamcrest.Matcher)");
43+
private static final MethodMatcher SUB_MATCHER = new MethodMatcher("org.hamcrest.*Matchers *(org.hamcrest.Matcher)");
4144

4245
@Option(displayName = "Hamcrest Matcher",
4346
description = "The Hamcrest `not(Matcher)` to migrate to JUnit5.",
@@ -65,10 +68,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
6568
}
6669

6770
private class MigrateToAssertJVisitor extends JavaIsoVisitor<ExecutionContext> {
68-
private final MethodMatcher ASSERT_THAT_MATCHER = new MethodMatcher("org.hamcrest.MatcherAssert assertThat(..)");
69-
private final MethodMatcher NOT_MATCHER = new MethodMatcher("org.hamcrest.*Matchers not(org.hamcrest.Matcher)");
70-
private final MethodMatcher MATCHERS_MATCHER = new MethodMatcher("org.hamcrest.*Matchers " + notMatcher + "(..)");
71-
private final MethodMatcher SUB_MATCHER = new MethodMatcher("org.hamcrest.*Matchers *(org.hamcrest.Matcher)");
71+
private final MethodMatcher matchersMatcher = new MethodMatcher("org.hamcrest.*Matchers " + notMatcher + "(..)");
7272

7373
@Override
7474
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
@@ -91,7 +91,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
9191

9292
private J.MethodInvocation handleTwoArgumentCase(J.MethodInvocation mi, Expression matcherArgument, ExecutionContext ctx) {
9393
Expression actualArgument = mi.getArguments().get(0);
94-
if (!MATCHERS_MATCHER.matches(matcherArgument) || SUB_MATCHER.matches(matcherArgument)) {
94+
if (!matchersMatcher.matches(matcherArgument) || SUB_MATCHER.matches(matcherArgument)) {
9595
return mi;
9696
}
9797
String actual = typeToIndicator(actualArgument.getType());
@@ -124,7 +124,7 @@ private J.MethodInvocation handleTwoArgumentCase(J.MethodInvocation mi, Expressi
124124
private J.MethodInvocation handleThreeArgumentCase(J.MethodInvocation mi, Expression matcherArgument, ExecutionContext ctx) {
125125
Expression reasonArgument = mi.getArguments().get(0);
126126
Expression actualArgument = mi.getArguments().get(1);
127-
if (!MATCHERS_MATCHER.matches(matcherArgument) || SUB_MATCHER.matches(matcherArgument)) {
127+
if (!matchersMatcher.matches(matcherArgument) || SUB_MATCHER.matches(matcherArgument)) {
128128
return mi;
129129
}
130130
String actual = typeToIndicator(actualArgument.getType());

‎src/main/java/org/openrewrite/java/testing/junit5/AddMissingNested.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@
2222
import org.openrewrite.Preconditions;
2323
import org.openrewrite.Recipe;
2424
import org.openrewrite.TreeVisitor;
25+
import org.openrewrite.java.AnnotationMatcher;
2526
import org.openrewrite.java.JavaIsoVisitor;
2627
import org.openrewrite.java.JavaParser;
2728
import org.openrewrite.java.JavaTemplate;
2829
import org.openrewrite.java.marker.JavaVersion;
29-
import org.openrewrite.java.search.FindAnnotations;
3030
import org.openrewrite.java.search.UsesType;
3131
import org.openrewrite.java.tree.Expression;
3232
import org.openrewrite.java.tree.J;
@@ -42,8 +42,10 @@
4242
import java.util.Comparator;
4343
import java.util.List;
4444
import java.util.Set;
45+
import java.util.concurrent.atomic.AtomicBoolean;
4546

4647
import static java.util.Collections.singleton;
48+
import static java.util.stream.Collectors.toList;
4749

4850
@Value
4951
@EqualsAndHashCode(callSuper = false)
@@ -55,6 +57,7 @@ public class AddMissingNested extends Recipe {
5557
"org.junit.jupiter.api.RepeatedTest",
5658
"org.junit.jupiter.params.ParameterizedTest",
5759
"org.junit.jupiter.api.TestFactory");
60+
private static final List<AnnotationMatcher> TEST_ANNOTATION_MATCHERS = TEST_ANNOTATIONS.stream().map(ann -> new AnnotationMatcher("@" + ann)).collect(toList());
5861

5962
@SuppressWarnings("unchecked")
6063
private static final TreeVisitor<?, ExecutionContext> PRECONDITION =
@@ -116,7 +119,15 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex
116119
}
117120

118121
private static boolean hasTestMethods(final J.ClassDeclaration cd) {
119-
return TEST_ANNOTATIONS.stream().anyMatch(ann -> !FindAnnotations.find(cd, "@" + ann).isEmpty());
122+
return new JavaIsoVisitor<AtomicBoolean>() {
123+
@Override
124+
public J.Annotation visitAnnotation(J.Annotation annotation, AtomicBoolean found) {
125+
if (TEST_ANNOTATION_MATCHERS.stream().anyMatch(matcher -> matcher.matches(annotation))) {
126+
found.set(true);
127+
}
128+
return super.visitAnnotation(annotation, found);
129+
}
130+
}.reduce(cd, new AtomicBoolean()).get();
120131
}
121132

122133
private boolean requiresManualMigration(J.ClassDeclaration cd) {

0 commit comments

Comments
 (0)