-
Notifications
You must be signed in to change notification settings - Fork 93
Alek/remove to string calls from array instances #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 15 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 b4877a3
polished somethings up and added license headers
AlekSimpson ae41eb5
Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson 90338a6
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson b17a46b
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson 05753ca
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson 74ed6f4
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson 24c02a3
Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson ac3ccee
Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEq…
AlekSimpson 15089c1
added missing import
AlekSimpson 86e3d79
updated license header year
AlekSimpson 9423d0e
added test to check that the recipe runs correctly on selects that ar…
AlekSimpson a69675b
added first test, need to test recipe
AlekSimpson dbc8624
the recipe for the toString() replacement is mostly finished, except …
AlekSimpson 83890a9
Merge branch 'main' into alek/RemoveToStringCallsFromArrayInstances
AlekSimpson a574b69
fixed edge case, all tests pass now
AlekSimpson 3c4ed65
added preconidition check
AlekSimpson bd586b7
added support for more methods that implicitly call toString() and al…
AlekSimpson 18e5c8c
added RSPEC tags and updated import statements
AlekSimpson 40167dd
The cursor message must be added before the `super` call
knutwannheden 5ca3b34
fixed cursor messaging, updated tests for more edge case methods, rec…
AlekSimpson ffadf48
more changes from PR feedback
AlekSimpson f488a15
added support for static methods
AlekSimpson 522afbe
Update src/main/java/org/openrewrite/staticanalysis/RemoveToStringCal…
AlekSimpson 81f193a
Update src/test/java/org/openrewrite/staticanalysis/RemoveToStringCal…
timtebeek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
87 changes: 87 additions & 0 deletions
87
src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| package org.openrewrite.staticanalysis; | ||
|
|
||
| import org.openrewrite.ExecutionContext; | ||
| import org.openrewrite.Preconditions; | ||
| import org.openrewrite.Recipe; | ||
| import org.openrewrite.TreeVisitor; | ||
| import org.openrewrite.java.JavaIsoVisitor; | ||
| import org.openrewrite.java.JavaTemplate; | ||
| import org.openrewrite.java.MethodMatcher; | ||
| import org.openrewrite.java.search.UsesType; | ||
| import org.openrewrite.java.tree.Expression; | ||
| import org.openrewrite.java.tree.J; | ||
| import org.openrewrite.java.tree.JavaType; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class RemoveToStringCallsFromArrayInstances extends Recipe { | ||
| private static final MethodMatcher TO_STRING_MATCHER = new MethodMatcher("java.lang.Object toString(..)"); | ||
AlekSimpson marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| private static final MethodMatcher PRINTLN_MATCHER = new MethodMatcher("java.io.PrintStream println(..)"); | ||
AlekSimpson marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| private static final MethodMatcher STR_FORMAT_MATCHER = new MethodMatcher("java.lang.String format(..)"); | ||
knutwannheden marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| @Override | ||
| public String getDisplayName() { | ||
| return "hashCode and toString should not be called on array instances"; | ||
AlekSimpson marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| @Override | ||
| public String getDescription() { | ||
| return "hashCode and toString should not be called on array instances."; | ||
| } | ||
|
|
||
| public TreeVisitor<?, ExecutionContext> getVisitor() { | ||
| //return Preconditions.check(new UsesType<>("java.lang.Object[]", false), new RemoveToStringFromArraysVisitor()); | ||
| return new RemoveToStringFromArraysVisitor(); | ||
| } | ||
|
|
||
| private static class RemoveToStringFromArraysVisitor extends JavaIsoVisitor<ExecutionContext> { | ||
knutwannheden marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| @Override | ||
| public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) { | ||
| J.MethodInvocation m = super.visitMethodInvocation(mi, ctx); | ||
|
|
||
| if (TO_STRING_MATCHER.matches(m)) { | ||
| String builder_string = "Arrays.toString(#{anyArray(java.lang.String)})"; | ||
| Expression select = m.getSelect(); | ||
| assert select != null; | ||
|
|
||
| return buildReplacement(builder_string, m, select); | ||
|
|
||
| }else if (PRINTLN_MATCHER.matches(m)) { | ||
| Expression select = m.getArguments().get(0); | ||
| String builder_string = "System.out.println(Arrays.toString(#{anyArray(java.lang.String)}))"; | ||
|
|
||
| return buildReplacement(builder_string, m, select); | ||
AlekSimpson marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| }else if (STR_FORMAT_MATCHER.matches(m)) { | ||
| List<Expression> arguments = m.getArguments(); | ||
|
|
||
| for (Expression arg : arguments) { | ||
| if (arg.getType() instanceof JavaType.Array) { | ||
| arg = JavaTemplate.builder("Arrays.toString(#{anyArray(java.lang.String)})") | ||
| .imports("java.util.Arrays") | ||
| .build() | ||
| .apply(getCursor(), arg.getCoordinates().replace(), arg); | ||
| } | ||
| } | ||
| maybeAddImport("java.util.Arrays"); | ||
|
|
||
| return m.withArguments(arguments); | ||
| } | ||
|
|
||
| return m; | ||
| } | ||
|
|
||
| public J.MethodInvocation buildReplacement(String builder_string, J.MethodInvocation m, Expression select) { | ||
| if (!(select.getType() instanceof JavaType.Array)) { | ||
| return m; | ||
| } | ||
|
|
||
| J.MethodInvocation retVal = JavaTemplate.builder(builder_string) | ||
| .imports("java.util.Arrays") | ||
| .build() | ||
| .apply(getCursor(), m.getCoordinates().replace(), select); | ||
| maybeAddImport("java.util.Arrays"); | ||
| return retVal; | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
219 changes: 219 additions & 0 deletions
219
src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| package org.openrewrite.staticanalysis; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.openrewrite.DocumentExample; | ||
| import org.openrewrite.Issue; | ||
| import org.openrewrite.java.JavaParser; | ||
| import org.openrewrite.test.RecipeSpec; | ||
| import org.openrewrite.test.RewriteTest; | ||
|
|
||
| import static org.openrewrite.java.Assertions.java; | ||
|
|
||
| @SuppressWarnings({"ImplicitArrayToString", "UnnecessaryLocalVariable", "RedundantStringFormatCall", "MalformedFormatString", "PrimitiveArrayArgumentToVarargsMethod"}) | ||
| public class RemoveToStringCallsFromArrayInstancesTest implements RewriteTest { | ||
| @Override | ||
| public void defaults(RecipeSpec spec) { | ||
| spec | ||
| .parser(JavaParser.fromJavaVersion()) | ||
AlekSimpson marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| .recipe(new RemoveToStringCallsFromArrayInstances()); | ||
| } | ||
|
|
||
| @Test | ||
| @DocumentExample | ||
| @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/44") | ||
| public void fixNonCompliantToString() { | ||
| //language=java | ||
| rewriteRun( | ||
| java( | ||
| """ | ||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| String argStr = args.toString(); | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import java.util.Arrays; | ||
|
|
||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| String argStr = Arrays.toString(args); | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void doesNotRunOnNonArrayInstances() { | ||
| //language=java | ||
| rewriteRun( | ||
| java( | ||
| """ | ||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| int number = 5; | ||
| System.out.println(number.toString()); | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void runsOnNonStringArrays() { | ||
| //language=java | ||
| rewriteRun( | ||
| java( | ||
| """ | ||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| String arrStr = getNumArr().toString(); | ||
| } | ||
|
|
||
| public int[] getNumArr() { | ||
| int[] nums = {1, 2, 3, 4}; | ||
| return nums; | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import java.util.Arrays; | ||
|
|
||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| String arrStr = Arrays.toString(getNumArr()); | ||
| } | ||
|
|
||
| public int[] getNumArr() { | ||
| int[] nums = {1, 2, 3, 4}; | ||
| return nums; | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void selectIsAMethod() { | ||
| //language=java | ||
| rewriteRun( | ||
| java( | ||
| """ | ||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| String arrStr = getArr().toString(); | ||
| } | ||
|
|
||
| public String[] getArr() { | ||
| String[] arr = {"test", "array"}; | ||
| return arr; | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import java.util.Arrays; | ||
|
|
||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| String arrStr = Arrays.toString(getArr()); | ||
| } | ||
|
|
||
| public String[] getArr() { | ||
| String[] arr = {"test", "array"}; | ||
| return arr; | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void printlnEdgeCase() { | ||
| //language=java | ||
| rewriteRun( | ||
| java( | ||
| """ | ||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| int[] s = new int[]{1,2,3}; | ||
| System.out.println(s); | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import java.util.Arrays; | ||
|
|
||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| int[] s = new int[]{1,2,3}; | ||
| System.out.println(Arrays.toString(s)); | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void stringFormatEdgeCase() { | ||
| //language=java | ||
| rewriteRun( | ||
| java( | ||
| """ | ||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| int[] s = new int[]{1, 2, 3}; | ||
| System.out.println(String.format("s=%s", s)); | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import java.util.Arrays; | ||
|
|
||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| int[] s = new int[]{1, 2, 3}; | ||
| System.out.println(String.format("s=%s", Arrays.toString(s))); | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void stringFormatMultipleArraysPassedIn() { | ||
| //language=java | ||
| rewriteRun( | ||
| java( | ||
| """ | ||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| int[] s1 = new int[]{1, 2, 3}; | ||
| int[] s2 = new int[]{4, 5, 6}; | ||
|
|
||
| System.out.println(String.format("s1=%s, s2=%s", s1, s2)); | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import java.util.Arrays; | ||
|
|
||
| class SomeClass { | ||
| public static void main(String[] args) { | ||
| int[] s1 = new int[]{1, 2, 3}; | ||
| int[] s2 = new int[]{4, 5, 6}; | ||
|
|
||
| System.out.println(String.format("s1=%s, s2=%s", Arrays.toString(s1), Arrays.toString(s2))); | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.