Skip to content

Commit 8a037e8

Browse files
velotimtebeek
velo
and
timtebeek
authored
Chained AssertJ assertions should be simplified to the corresponding dedicated assertion (#556)
* Chained AssertJ assertions should be simplified to the corresponding dedicated assertion * Adopt Traits.methodAccess and match overrides * Apply formatter to SonarDedicatedAssertions * Fix imports * Group IsEqualTo* Logic into reusable ShortenChainedAssertJAssertion * Apply formatter * Rename to have differences stand out more * Clear out empty lines * Cover one additional case * Rename class to reflect tested recipe * Fix expectations around Path * Enable one more case after updating expected outcome * Update documented options for clarity * Move test to illustrate reliance on recipe order * Remove unnecessary classpath entry --------- Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent 37e9667 commit 8a037e8

12 files changed

+749
-655
lines changed

‎src/main/java/org/openrewrite/java/testing/assertj/IsEqualToBoolean.java‎

Lines changed: 0 additions & 76 deletions
This file was deleted.

‎src/main/java/org/openrewrite/java/testing/assertj/IsEqualToEmptyString.java‎

Lines changed: 0 additions & 68 deletions
This file was deleted.

‎src/main/java/org/openrewrite/java/testing/assertj/SimplifyAssertJAssertion.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
* <p>
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.java.testing.assertj;
17+
18+
import lombok.AllArgsConstructor;
19+
import lombok.NoArgsConstructor;
20+
import org.openrewrite.ExecutionContext;
21+
import org.openrewrite.Option;
22+
import org.openrewrite.Recipe;
23+
import org.openrewrite.TreeVisitor;
24+
import org.openrewrite.internal.lang.Nullable;
25+
import org.openrewrite.java.JavaIsoVisitor;
26+
import org.openrewrite.java.JavaParser;
27+
import org.openrewrite.java.JavaTemplate;
28+
import org.openrewrite.java.MethodMatcher;
29+
import org.openrewrite.java.tree.J;
30+
import org.openrewrite.java.tree.TypeUtils;
31+
32+
@AllArgsConstructor
33+
@NoArgsConstructor
34+
public class SimplifyAssertJAssertion extends Recipe {
35+
36+
@Option(displayName = "AssertJ assertion",
37+
description = "The assertion method that should be replaced.",
38+
example = "hasSize",
39+
required = false)
40+
@Nullable
41+
String assertToReplace;
42+
43+
@Option(displayName = "Assertion argument literal",
44+
description = "The literal argument passed into the assertion to replace; use \"null\" for `null`.",
45+
example = "0")
46+
String literalArgument;
47+
48+
@Option(displayName = "Dedicated assertion",
49+
description = "The zero argument assertion to adopt instead.",
50+
example = "isEmpty")
51+
String dedicatedAssertion;
52+
53+
@Option(displayName = "Required type",
54+
description = "The type of the actual assertion argument.",
55+
example = "java.lang.String")
56+
String requiredType;
57+
58+
@Override
59+
public String getDisplayName() {
60+
return "Simplify AssertJ assertions with literal arguments";
61+
}
62+
63+
@Override
64+
public String getDescription() {
65+
return "Simplify AssertJ assertions by replacing them with more expressiove dedicated assertions.";
66+
}
67+
68+
@Override
69+
public TreeVisitor<?, ExecutionContext> getVisitor() {
70+
return new ShorthenChainedAssertJAssertionsVisitor();
71+
}
72+
73+
private class ShorthenChainedAssertJAssertionsVisitor extends JavaIsoVisitor<ExecutionContext> {
74+
private final MethodMatcher ASSERT_THAT_MATCHER = new MethodMatcher("org.assertj.core.api.Assertions assertThat(..)");
75+
private final MethodMatcher ASSERT_TO_REPLACE = new MethodMatcher("org.assertj.core.api.* " + assertToReplace + "(..)");
76+
77+
@Override
78+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) {
79+
J.MethodInvocation mi = super.visitMethodInvocation(methodInvocation, ctx);
80+
81+
// Match the end of the chain first, then the select to avoid matching the wrong method chain
82+
if (!ASSERT_TO_REPLACE.matches(mi) || !ASSERT_THAT_MATCHER.matches(mi.getSelect())) {
83+
return mi;
84+
}
85+
86+
// Compare argument with passed in literal
87+
if (!(mi.getArguments().get(0) instanceof J.Literal) ||
88+
!literalArgument.equals(((J.Literal) mi.getArguments().get(0)).getValueSource())) { // Implies "null" is `null`
89+
return mi;
90+
}
91+
92+
// Check argument type of assertThat
93+
if (!TypeUtils.isAssignableTo(requiredType, ((J.MethodInvocation) mi.getSelect()).getArguments().get(0).getType())) {
94+
return mi;
95+
}
96+
97+
// Assume zero argument replacement method
98+
return JavaTemplate.builder(dedicatedAssertion + "()")
99+
.contextSensitive()
100+
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24"))
101+
.build()
102+
.apply(getCursor(), mi.getCoordinates().replaceMethod());
103+
}
104+
}
105+
}

‎src/main/java/org/openrewrite/java/testing/assertj/SimplifyChainedAssertJAssertion.java‎

Lines changed: 31 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -38,29 +38,29 @@
3838
@AllArgsConstructor
3939
@NoArgsConstructor
4040
public class SimplifyChainedAssertJAssertion extends Recipe {
41-
@Option(displayName = "AssertJ Assertion",
41+
@Option(displayName = "AssertJ chained assertion",
4242
description = "The chained AssertJ assertion to move to dedicated assertion.",
4343
example = "equals",
4444
required = false)
4545
@Nullable
4646
String chainedAssertion;
4747

48-
@Option(displayName = "AssertJ Assertion",
48+
@Option(displayName = "AssertJ replaced assertion",
4949
description = "The AssertJ assert that should be replaced.",
5050
example = "isTrue",
5151
required = false)
5252
@Nullable
5353
String assertToReplace;
5454

55-
@Option(displayName = "AssertJ Assertion",
55+
@Option(displayName = "AssertJ replacement assertion",
5656
description = "The AssertJ method to migrate to.",
5757
example = "isEqualTo",
5858
required = false)
5959
@Nullable
6060
String dedicatedAssertion;
6161

62-
@Option(displayName = "Required Type",
63-
description = "Specifies the type the recipe should run on.",
62+
@Option(displayName = "Required type",
63+
description = "The type of the actual assertion argument.",
6464
example = "java.lang.String",
6565
required = false)
6666
@Nullable
@@ -120,11 +120,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat
120120
List<Expression> arguments = new ArrayList<>();
121121
arguments.add(actual);
122122

123-
// Special case for more expressive assertions: assertThat(x.size()).isEqualTo(0) -> isEmpty()
124-
if ("size".equals(chainedAssertion) && "isEqualTo".equals(assertToReplace) && hasZeroArgument(mi)) {
125-
return applyTemplate("assertThat(#{any()}).isEmpty()", arguments, mi, ctx);
126-
}
127-
128123
String template = getStringTemplateAndAppendArguments(assertThatArg, mi, arguments);
129124
return applyTemplate(String.format(template, dedicatedAssertion), arguments, mi, ctx);
130125
}
@@ -136,38 +131,38 @@ private J.MethodInvocation applyTemplate(String formattedTemplate, List<Expressi
136131
.build()
137132
.apply(getCursor(), mi.getCoordinates().replace(), arguments.toArray());
138133
}
139-
}
140134

141-
private String getStringTemplateAndAppendArguments(J.MethodInvocation assertThatArg, J.MethodInvocation methodToReplace, List<Expression> arguments) {
142-
Expression assertThatArgument = assertThatArg.getArguments().get(0);
143-
Expression methodToReplaceArgument = methodToReplace.getArguments().get(0);
144-
boolean assertThatArgumentIsEmpty = assertThatArgument instanceof J.Empty;
145-
boolean methodToReplaceArgumentIsEmpty = methodToReplaceArgument instanceof J.Empty;
135+
private String getStringTemplateAndAppendArguments(J.MethodInvocation assertThatArg, J.MethodInvocation methodToReplace, List<Expression> arguments) {
136+
Expression assertThatArgument = assertThatArg.getArguments().get(0);
137+
Expression methodToReplaceArgument = methodToReplace.getArguments().get(0);
138+
boolean assertThatArgumentIsEmpty = assertThatArgument instanceof J.Empty;
139+
boolean methodToReplaceArgumentIsEmpty = methodToReplaceArgument instanceof J.Empty;
146140

147-
// If both arguments are empty, then the select is already added to the arguments list, and we use a minimal template
148-
if (assertThatArgumentIsEmpty && methodToReplaceArgumentIsEmpty) {
149-
return "assertThat(#{any()}).%s()";
150-
}
151-
152-
// If both arguments are not empty, then we add both to the arguments to the arguments list, and return a template with two arguments
153-
if (!assertThatArgumentIsEmpty && !methodToReplaceArgumentIsEmpty) {
154-
// This should only happen for map assertions using a key and value
155-
arguments.add(assertThatArgument);
156-
arguments.add(methodToReplaceArgument);
157-
return "assertThat(#{any()}).%s(#{any()}, #{any()})";
158-
}
141+
// If both arguments are empty, then the select is already added to the arguments list, and we use a minimal template
142+
if (assertThatArgumentIsEmpty && methodToReplaceArgumentIsEmpty) {
143+
return "assertThat(#{any()}).%s()";
144+
}
159145

160-
// If either argument is empty, we choose which one to add to the arguments list, and optionally extract the select
161-
arguments.add(extractEitherArgument(assertThatArgumentIsEmpty, assertThatArgument, methodToReplaceArgument));
146+
// If both arguments are not empty, then we add both to the arguments to the arguments list, and return a template with two arguments
147+
if (!assertThatArgumentIsEmpty && !methodToReplaceArgumentIsEmpty) {
148+
// This should only happen for map assertions using a key and value
149+
arguments.add(assertThatArgument);
150+
arguments.add(methodToReplaceArgument);
151+
return "assertThat(#{any()}).%s(#{any()}, #{any()})";
152+
}
162153

163-
// Special case for Path.of() assertions
164-
if ("java.nio.file.Path".equals(requiredType) && dedicatedAssertion.contains("Raw")
165-
&& TypeUtils.isAssignableTo("java.lang.String", assertThatArgument.getType())) {
166-
return "assertThat(#{any()}).%s(Path.of(#{any()}))";
167-
}
154+
// If either argument is empty, we choose which one to add to the arguments list, and optionally extract the select
155+
arguments.add(extractEitherArgument(assertThatArgumentIsEmpty, assertThatArgument, methodToReplaceArgument));
168156

169-
return "assertThat(#{any()}).%s(#{any()})";
157+
// Special case for Path.of() assertions
158+
if ("java.nio.file.Path".equals(requiredType) && dedicatedAssertion.contains("Raw")
159+
&& TypeUtils.isAssignableTo("java.lang.String", assertThatArgument.getType())) {
160+
maybeAddImport("java.nio.file.Path");
161+
return "assertThat(#{any()}).%s(Path.of(#{any()}))";
162+
}
170163

164+
return "assertThat(#{any()}).%s(#{any()})";
165+
}
171166
}
172167

173168
private static Expression extractEitherArgument(boolean assertThatArgumentIsEmpty, Expression assertThatArgument, Expression methodToReplaceArgument) {
@@ -183,13 +178,4 @@ private static Expression extractEitherArgument(boolean assertThatArgumentIsEmpt
183178
}
184179
return assertThatArgument;
185180
}
186-
187-
private boolean hasZeroArgument(J.MethodInvocation method) {
188-
List<Expression> arguments = method.getArguments();
189-
if (arguments.size() == 1 && arguments.get(0) instanceof J.Literal) {
190-
J.Literal literalArg = (J.Literal) arguments.get(0);
191-
return literalArg.getValue() != null && literalArg.getValue().equals(0);
192-
}
193-
return false;
194-
}
195181
}

0 commit comments

Comments
 (0)