Skip to content

Commit 2127a09

Browse files
AlekSimpsontimtebeekknutwannheden
authored
Alek/remove to string calls from array instances (#126)
* new recipe that changes .equals() to .contentEquals() when comparing a string with a StringBuilder * polished somethings up and added license headers * Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java Co-authored-by: Tim te Beek <[email protected]> * Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek <[email protected]> * Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek <[email protected]> * Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek <[email protected]> * Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek <[email protected]> * Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek <[email protected]> * Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java Co-authored-by: Tim te Beek <[email protected]> * added missing import * updated license header year * added test to check that the recipe runs correctly on selects that are not just raw string variables but also things like method calls * added first test, need to test recipe * the recipe for the toString() replacement is mostly finished, except for two tests which are testing an edge cases related to String.format() * fixed edge case, all tests pass now * added preconidition check * added support for more methods that implicitly call toString() and also reworked recipe so that it uses cursor messaging * added RSPEC tags and updated import statements * The cursor message must be added before the `super` call * fixed cursor messaging, updated tests for more edge case methods, recipe also covers arrays concatenated with strings now * more changes from PR feedback * added support for static methods * Update src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java Co-authored-by: Knut Wannheden <[email protected]> * Update src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java --------- Co-authored-by: Tim te Beek <[email protected]> Co-authored-by: Knut Wannheden <[email protected]>
1 parent a6e524c commit 2127a09

File tree

3 files changed

+640
-1
lines changed

3 files changed

+640
-1
lines changed
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
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.staticanalysis;
17+
18+
import org.openrewrite.*;
19+
import org.openrewrite.java.JavaTemplate;
20+
import org.openrewrite.java.JavaVisitor;
21+
import org.openrewrite.java.MethodMatcher;
22+
import org.openrewrite.java.tree.*;
23+
24+
import java.util.Arrays;
25+
import java.util.Collections;
26+
import java.util.List;
27+
import java.util.Set;
28+
import java.util.stream.Collectors;
29+
30+
public class RemoveToStringCallsFromArrayInstances extends Recipe {
31+
private static final MethodMatcher VALUEOF_MATCHER = new MethodMatcher("java.lang.String valueOf(java.lang.Object)");
32+
private static final MethodMatcher OBJECTS_TOSTRING_MATCHER = new MethodMatcher("java.util.Objects toString(Object)");
33+
private static final MethodMatcher TOSTRING_MATCHER = new MethodMatcher("java.lang.Object toString()");
34+
35+
private static final List<String> PATTERNS = Arrays.asList(
36+
"java.io.PrintStream print*(Object)",
37+
"java.lang.String format*(..)",
38+
"java.lang.StringBuilder insert(int, Object)",
39+
"java.lang.StringBuilder append(Object)",
40+
"java.io.PrintStream format(String, Object[])",
41+
"java.io.PrintWriter print*(..)",
42+
"java.io.PrintWriter format(..)"
43+
);
44+
private static final List<MethodMatcher> METHOD_MATCHERS = PATTERNS.stream().map(MethodMatcher::new).collect(Collectors.toList());
45+
46+
@Override
47+
public Set<String> getTags() {
48+
return Collections.singleton("RSPEC-2116");
49+
}
50+
51+
@Override
52+
public String getDisplayName() {
53+
return "Remove `toString()` calls on arrays";
54+
}
55+
56+
@Override
57+
public String getDescription() {
58+
return "The result from `toString()` calls on arrays is largely useless. The output does not actually reflect" +
59+
" the contents of the array. `Arrays.toString(array)` give the contents of the array.";
60+
}
61+
62+
public TreeVisitor<?, ExecutionContext> getVisitor() {
63+
return new RemoveToStringFromArraysVisitor();
64+
}
65+
66+
private static class RemoveToStringFromArraysVisitor extends JavaVisitor<ExecutionContext> {
67+
@Override
68+
public J visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) {
69+
if (TOSTRING_MATCHER.matches(mi)) {
70+
Expression select = mi.getSelect();
71+
if (select == null) {
72+
return mi;
73+
}
74+
75+
return buildReplacement(select, mi);
76+
} else if (METHOD_MATCHERS.stream().anyMatch(matcher -> matcher.matches(mi))) {
77+
// deals with edge cases where .toString() is called implicitly
78+
List<Expression> arguments = mi.getArguments();
79+
for (Expression arg : arguments) {
80+
if (arg.getType() instanceof JavaType.Array) {
81+
getCursor().putMessage("METHOD_KEY", mi);
82+
break;
83+
}
84+
}
85+
}else if (OBJECTS_TOSTRING_MATCHER.matches(mi) || VALUEOF_MATCHER.matches(mi)) {
86+
// method is static
87+
Expression select = mi.getArguments().get(0);
88+
maybeRemoveImport("java.util.Objects");
89+
90+
return buildReplacement(select, mi);
91+
}
92+
93+
return super.visitMethodInvocation(mi, ctx);
94+
}
95+
96+
public J buildReplacement(Expression select, J.MethodInvocation mi) {
97+
if (!(select.getType() instanceof JavaType.Array)) {
98+
return mi;
99+
}
100+
101+
maybeAddImport("java.util.Arrays");
102+
return JavaTemplate.builder("Arrays.toString(#{anyArray(java.lang.Object)})")
103+
.imports("java.util.Arrays")
104+
.build()
105+
.apply(getCursor(), mi.getCoordinates().replace(), select);
106+
}
107+
108+
@Override
109+
public Expression visitExpression(Expression exp, ExecutionContext ctx) {
110+
Expression e = (Expression) super.visitExpression(exp, ctx);
111+
if (e instanceof TypedTree && e.getType() instanceof JavaType.Array) {
112+
Cursor c = getCursor().dropParentWhile(is -> is instanceof J.Parentheses || !(is instanceof Tree));
113+
if (c.getMessage("METHOD_KEY") != null || c.getMessage("BINARY_FOUND") != null) {
114+
maybeAddImport("java.util.Arrays");
115+
return JavaTemplate.builder("Arrays.toString(#{anyArray(java.lang.Object)})")
116+
.imports("java.util.Arrays")
117+
.build()
118+
.apply(getCursor(), e.getCoordinates().replace(), e);
119+
}
120+
}
121+
122+
return e;
123+
}
124+
125+
@Override
126+
public J.Binary visitBinary(J.Binary binary, ExecutionContext ctx) {
127+
Expression left = binary.getLeft();
128+
Expression right = binary.getRight();
129+
130+
if (binary.getOperator() == J.Binary.Type.Addition && (left.getType() instanceof JavaType.Array || right.getType() instanceof JavaType.Array)) {
131+
getCursor().putMessage("BINARY_FOUND", binary);
132+
}
133+
134+
return (J.Binary) super.visitBinary(binary, ctx);
135+
}
136+
}
137+
}

src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import static org.openrewrite.java.Assertions.java;
2525

2626
class EqualsToContentEqualsTest implements RewriteTest {
27-
2827
@Override
2928
public void defaults(RecipeSpec spec) {
3029
spec

0 commit comments

Comments
 (0)