Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3f4e109
new recipe that changes .equals() to .contentEquals() when comparing …
AlekSimpson Jun 16, 2023
b4877a3
polished somethings up and added license headers
AlekSimpson Jun 16, 2023
ae41eb5
Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson Jun 16, 2023
90338a6
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson Jun 16, 2023
b17a46b
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson Jun 16, 2023
05753ca
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson Jun 16, 2023
74ed6f4
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson Jun 16, 2023
24c02a3
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson Jun 16, 2023
ac3ccee
Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson Jun 16, 2023
15089c1
added missing import
AlekSimpson Jun 16, 2023
86e3d79
updated license header year
AlekSimpson Jun 16, 2023
9423d0e
added test to check that the recipe runs correctly on selects that ar…
AlekSimpson Jun 17, 2023
a69675b
added first test, need to test recipe
AlekSimpson Jun 18, 2023
dbc8624
the recipe for the toString() replacement is mostly finished, except …
AlekSimpson Jun 19, 2023
83890a9
Merge branch 'main' into alek/RemoveToStringCallsFromArrayInstances
AlekSimpson Jun 20, 2023
a574b69
fixed edge case, all tests pass now
AlekSimpson Jun 20, 2023
3c4ed65
added preconidition check
AlekSimpson Jun 20, 2023
bd586b7
added support for more methods that implicitly call toString() and al…
AlekSimpson Jun 22, 2023
18e5c8c
added RSPEC tags and updated import statements
AlekSimpson Jun 22, 2023
40167dd
The cursor message must be added before the `super` call
knutwannheden Jun 23, 2023
5ca3b34
fixed cursor messaging, updated tests for more edge case methods, rec…
AlekSimpson Jun 23, 2023
ffadf48
more changes from PR feedback
AlekSimpson Jun 26, 2023
f488a15
added support for static methods
AlekSimpson Jun 26, 2023
522afbe
Update src/main/java/org/openrewrite/staticanalysis/RemoveToStringCal…
AlekSimpson Jun 27, 2023
81f193a
Update src/test/java/org/openrewrite/staticanalysis/RemoveToStringCal…
timtebeek Jun 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
* Copyright 2023 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.staticanalysis;

import org.openrewrite.*;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.tree.*;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class RemoveToStringCallsFromArrayInstances extends Recipe {
private static final MethodMatcher VALUEOF_MATCHER = new MethodMatcher("java.lang.String valueOf(java.lang.Object)");
private static final MethodMatcher OBJECTS_TOSTRING_MATCHER = new MethodMatcher("java.util.Objects toString(Object)");
private static final MethodMatcher TOSTRING_MATCHER = new MethodMatcher("java.lang.Object toString()");

private static final List<String> PATTERNS = Arrays.asList(
"java.io.PrintStream print*(Object)",
"java.lang.String format*(..)",
"java.lang.StringBuilder insert(int, Object)",
"java.lang.StringBuilder append(Object)",
"java.io.PrintStream format(String, Object[])",
"java.io.PrintWriter print*(..)",
"java.io.PrintWriter format(..)"
);
private static final List<MethodMatcher> METHOD_MATCHERS = PATTERNS.stream().map(MethodMatcher::new).collect(Collectors.toList());

@Override
public Set<String> getTags() {
return Collections.singleton("RSPEC-2116");
}

@Override
public String getDisplayName() {
return "Remove `toString()` calls on arrays";
}

@Override
public String getDescription() {
return "The result from `toString()` calls on arrays is largely useless. The output does not actually reflect" +
" the contents of the array. `Arrays.toString(array)` give the contents of the array.";
}

public TreeVisitor<?, ExecutionContext> getVisitor() {
return new RemoveToStringFromArraysVisitor();
}

private static class RemoveToStringFromArraysVisitor extends JavaVisitor<ExecutionContext> {
@Override
public J visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) {
if (TOSTRING_MATCHER.matches(mi)) {
Expression select = mi.getSelect();
if (select == null) {
return mi;
}

return buildReplacement(select, mi);
} else if (METHOD_MATCHERS.stream().anyMatch(matcher -> matcher.matches(mi))) {
// deals with edge cases where .toString() is called implicitly
List<Expression> arguments = mi.getArguments();
for (Expression arg : arguments) {
if (arg.getType() instanceof JavaType.Array) {
getCursor().putMessage("METHOD_KEY", mi);
break;
}
}
}else if (OBJECTS_TOSTRING_MATCHER.matches(mi) || VALUEOF_MATCHER.matches(mi)) {
// method is static
Expression select = mi.getArguments().get(0);
maybeRemoveImport("java.util.Objects");

return buildReplacement(select, mi);
}

return super.visitMethodInvocation(mi, ctx);
}

public J buildReplacement(Expression select, J.MethodInvocation mi) {
if (!(select.getType() instanceof JavaType.Array)) {
return mi;
}

maybeAddImport("java.util.Arrays");
return JavaTemplate.builder("Arrays.toString(#{anyArray(java.lang.Object)})")
.imports("java.util.Arrays")
.build()
.apply(getCursor(), mi.getCoordinates().replace(), select);
}

@Override
public Expression visitExpression(Expression exp, ExecutionContext ctx) {
Expression e = (Expression) super.visitExpression(exp, ctx);
if (e instanceof TypedTree && e.getType() instanceof JavaType.Array) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is more a question for Knut of Kun, but I had expected a call out to TypeUtils here rather than a direct use of instanceof. When would we use each?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could replace e.getType() instanceof JavaType.Array with TypeUtils.asArray(e.getType()) != null but I don't think it is really much better. IMHO the instanceof here is fine.

Cursor c = getCursor().dropParentWhile(is -> is instanceof J.Parentheses || !(is instanceof Tree));
if (c.getMessage("METHOD_KEY") != null || c.getMessage("BINARY_FOUND") != null) {
maybeAddImport("java.util.Arrays");
return JavaTemplate.builder("Arrays.toString(#{anyArray(java.lang.Object)})")
.imports("java.util.Arrays")
.build()
.apply(getCursor(), e.getCoordinates().replace(), e);
}
}

return e;
}

@Override
public J.Binary visitBinary(J.Binary binary, ExecutionContext ctx) {
Expression left = binary.getLeft();
Expression right = binary.getRight();

if (binary.getOperator() == J.Binary.Type.Addition && (left.getType() instanceof JavaType.Array || right.getType() instanceof JavaType.Array)) {
getCursor().putMessage("BINARY_FOUND", binary);
}

return (J.Binary) super.visitBinary(binary, ctx);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import static org.openrewrite.java.Assertions.java;

class EqualsToContentEqualsTest implements RewriteTest {

@Override
public void defaults(RecipeSpec spec) {
spec
Expand Down
Loading