From 3f4e109e54fea654132457d8303fa8232a8625ce Mon Sep 17 00:00:00 2001 From: AlekSimpson Date: Fri, 16 Jun 2023 13:58:23 -0700 Subject: [PATCH 01/14] new recipe that changes .equals() to .contentEquals() when comparing a string with a StringBuilder --- .../staticanalysis/EqualsToContentEquals.java | 80 +++++++++++++ .../EqualsToContentEqualsTest.java | 105 ++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java new file mode 100644 index 0000000000..c285c84625 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -0,0 +1,80 @@ +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.MethodMatcher; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.*; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +public class EqualsToContentEquals extends Recipe { + private static final MethodMatcher equals_matcher = new MethodMatcher("java.lang.String equals(..)"); + private static final List TYPE_NAMES = Arrays.asList( + "java.lang.StringBuffer", + "java.lang.StringBuilder", + "java.lang.CharSequence" + ); + @SuppressWarnings("unchecked") + private static final TreeVisitor PRECONDITION = + Preconditions.or(TYPE_NAMES.stream().map(s -> new UsesType<>(s, false)).toArray(UsesType[]::new)); + private static final List toString_matchers = TYPE_NAMES.stream() + .map(obj -> new MethodMatcher(obj + " toString()")).collect(Collectors.toList()); + + @Override + public String getDisplayName() { + return "Use contentEquals to compare StringBuilder to a String"; + } + @Override + public String getDescription() { + return "Use contentEquals to compare StringBuilder to a String."; + } + + public TreeVisitor getVisitor() { + return Preconditions.check(PRECONDITION, new EqualsToContentEqualsVisitor()); + } + + private static class EqualsToContentEqualsVisitor extends JavaIsoVisitor { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(mi, ctx); + J.Identifier methodName = m.getName(); + // create method matcher on equals(String) + if (equals_matcher.matches(m)) { + Expression argument = m.getArguments().get(0); + + // checks whether the argument is a toString() method call on a StringBuffer or CharSequence + if (toString_matchers.stream().anyMatch(matcher -> matcher.matches(argument))) { + J.MethodInvocation inv = (J.MethodInvocation) argument; + Expression newArg = inv.getSelect(); + if (inv.getSelect() == null) { return m; } + + JavaType sb_type = JavaType.buildType("java.lang.StringBuilder"); + JavaType string_buffer_type = JavaType.buildType("java.lang.StringBuffer"); + JavaType char_sq_type = JavaType.buildType("java.lang.CharSequence"); + + if ( + TypeUtils.isOfType(newArg.getType(), sb_type) || + TypeUtils.isOfType(newArg.getType(), string_buffer_type) || + TypeUtils.isOfType(newArg.getType(), char_sq_type) + ) { + // strip out the toString() on the argument + List args = new ArrayList<>(1); + args.add(newArg); + m = m.withArguments(args); + // rename the method to contentEquals + methodName = m.getName().withSimpleName("contentEquals"); + } + } + } + + return m.withName(methodName); + } + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java new file mode 100644 index 0000000000..e1378be95a --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java @@ -0,0 +1,105 @@ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +public class EqualsToContentEqualsTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec + .parser(JavaParser.fromJavaVersion()) + .recipe(new EqualsToContentEquals()); + } + + @Test + public void replaceStringBuilder() { + //language=java + rewriteRun( + java( + """ + class SomeClass { + boolean foo(StringBuilder sb) { + String str = "example string"; + return str.equals(sb.toString()); + } + } + """, + """ + class SomeClass { + boolean foo(StringBuilder sb) { + String str = "example string"; + return str.contentEquals(sb); + } + } + """ + ) + ); + } + + @Test + public void onlyRunsOnCorrectInvocations() { + //language=java + rewriteRun( + java( + """ + class SomeClass { + boolean foo(int number, String str) { + return str.equals(number.toString()); + } + } + """ + ) + ); + } + + @Test + void runsOnStringBuffer() { + //language=java + rewriteRun( + java( + """ + class SomeClass { + boolean foo(StringBuffer sb, String str) { + return str.equals(sb.toString()); + } + } + """, + """ + class SomeClass { + boolean foo(StringBuffer sb, String str) { + return str.contentEquals(sb); + } + } + """ + ) + ); + } + + @Test + void runsOnCharSequence() { + //language=java + rewriteRun( + java( + """ + class SomeClass { + boolean foo(CharSequence cs, String str) { + return str.equals(cs.toString()); + } + } + """, + """ + class SomeClass { + boolean foo(CharSequence cs, String str) { + return str.contentEquals(cs); + } + } + """ + ) + ); + } +} From b4877a34f9b97fbee40a5216b1e7d9c0e74488f7 Mon Sep 17 00:00:00 2001 From: AlekSimpson Date: Fri, 16 Jun 2023 14:08:24 -0700 Subject: [PATCH 02/14] polished somethings up and added license headers --- .../staticanalysis/EqualsToContentEquals.java | 30 ++++++++++++++----- .../EqualsToContentEqualsTest.java | 15 ++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index c285c84625..b31f3e6884 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -1,3 +1,18 @@ +/* + * Copyright 2021 the original author or authors. + *

+ * 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 + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.ExecutionContext; @@ -13,6 +28,7 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; +import java.util.stream.Stream; public class EqualsToContentEquals extends Recipe { private static final MethodMatcher equals_matcher = new MethodMatcher("java.lang.String equals(..)"); @@ -55,15 +71,13 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, Execution Expression newArg = inv.getSelect(); if (inv.getSelect() == null) { return m; } - JavaType sb_type = JavaType.buildType("java.lang.StringBuilder"); - JavaType string_buffer_type = JavaType.buildType("java.lang.StringBuffer"); - JavaType char_sq_type = JavaType.buildType("java.lang.CharSequence"); + Stream TYPES = Stream.of( + JavaType.buildType("java.lang.StringBuilder"), + JavaType.buildType("java.lang.StringBuffer"), + JavaType.buildType("java.lang.CharSequence") + ); - if ( - TypeUtils.isOfType(newArg.getType(), sb_type) || - TypeUtils.isOfType(newArg.getType(), string_buffer_type) || - TypeUtils.isOfType(newArg.getType(), char_sq_type) - ) { + if (TYPES.anyMatch(type -> TypeUtils.isOfType(newArg.getType(), type))) { // strip out the toString() on the argument List args = new ArrayList<>(1); args.add(newArg); diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java index e1378be95a..a31ba124ae 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2021 the original author or authors. + *

+ * 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 + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.junit.jupiter.api.Test; From ae41eb5c13660591d1e184f7ebf317ad80a999dc Mon Sep 17 00:00:00 2001 From: Aleksandar A Simpson Date: Fri, 16 Jun 2023 15:36:16 -0700 Subject: [PATCH 03/14] Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java Co-authored-by: Tim te Beek --- .../openrewrite/staticanalysis/EqualsToContentEqualsTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java index a31ba124ae..0abf7db61c 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java @@ -32,6 +32,7 @@ public void defaults(RecipeSpec spec) { } @Test + @DocumentExample public void replaceStringBuilder() { //language=java rewriteRun( From 90338a69d7e8c57a24ee4f7b40c2b976522bfd1e Mon Sep 17 00:00:00 2001 From: Aleksandar A Simpson Date: Fri, 16 Jun 2023 15:36:47 -0700 Subject: [PATCH 04/14] Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek --- .../staticanalysis/EqualsToContentEquals.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index b31f3e6884..dbbbece362 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -32,16 +32,14 @@ public class EqualsToContentEquals extends Recipe { private static final MethodMatcher equals_matcher = new MethodMatcher("java.lang.String equals(..)"); - private static final List TYPE_NAMES = Arrays.asList( - "java.lang.StringBuffer", - "java.lang.StringBuilder", - "java.lang.CharSequence" - ); - @SuppressWarnings("unchecked") - private static final TreeVisitor PRECONDITION = - Preconditions.or(TYPE_NAMES.stream().map(s -> new UsesType<>(s, false)).toArray(UsesType[]::new)); - private static final List toString_matchers = TYPE_NAMES.stream() - .map(obj -> new MethodMatcher(obj + " toString()")).collect(Collectors.toList()); + private static final TreeVisitor PRECONDITION = Preconditions.or( + new UsesType<>("java.lang.StringBuffer", false), + new UsesType<>("java.lang.StringBuilder", false), + new UsesType<>("java.lang.CharSequence", false)); + private static final List TOSTRING_MATCHERS = Arrays.asList( + new MethodMatcher("java.lang.String toString()"), + new MethodMatcher("java.lang.StringBuffer toString()"), + new MethodMatcher("java.lang.StringBuilder toString()")); @Override public String getDisplayName() { From b17a46b87cc7105abe5547c65a3985fa5b6f8014 Mon Sep 17 00:00:00 2001 From: Aleksandar A Simpson Date: Fri, 16 Jun 2023 15:36:56 -0700 Subject: [PATCH 05/14] Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek --- .../org/openrewrite/staticanalysis/EqualsToContentEquals.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index dbbbece362..856801f89a 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -86,7 +86,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, Execution } } - return m.withName(methodName); + return m; } } } From 05753cadc9f4d5d49a198d06bf0e3b5c22d6ec37 Mon Sep 17 00:00:00 2001 From: Aleksandar A Simpson Date: Fri, 16 Jun 2023 15:37:04 -0700 Subject: [PATCH 06/14] Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek --- .../org/openrewrite/staticanalysis/EqualsToContentEquals.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index 856801f89a..2b6b5eb5ec 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -58,7 +58,6 @@ private static class EqualsToContentEqualsVisitor extends JavaIsoVisitor Date: Fri, 16 Jun 2023 15:37:14 -0700 Subject: [PATCH 07/14] Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek --- .../org/openrewrite/staticanalysis/EqualsToContentEquals.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index 2b6b5eb5ec..8b2b9d47f2 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -63,7 +63,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, Execution Expression argument = m.getArguments().get(0); // checks whether the argument is a toString() method call on a StringBuffer or CharSequence - if (toString_matchers.stream().anyMatch(matcher -> matcher.matches(argument))) { + if (TOSTRING_MATCHERS.stream().anyMatch(matcher -> matcher.matches(argument))) { J.MethodInvocation inv = (J.MethodInvocation) argument; Expression newArg = inv.getSelect(); if (inv.getSelect() == null) { return m; } From 24c02a335853f3ea6d8b69931eddba1e70ca9ee0 Mon Sep 17 00:00:00 2001 From: Aleksandar A Simpson Date: Fri, 16 Jun 2023 15:37:24 -0700 Subject: [PATCH 08/14] Update src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java Co-authored-by: Tim te Beek --- .../openrewrite/staticanalysis/EqualsToContentEquals.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index 8b2b9d47f2..a937c3d782 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -76,11 +76,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, Execution if (TYPES.anyMatch(type -> TypeUtils.isOfType(newArg.getType(), type))) { // strip out the toString() on the argument - List args = new ArrayList<>(1); - args.add(newArg); - m = m.withArguments(args); - // rename the method to contentEquals - methodName = m.getName().withSimpleName("contentEquals"); + return m.withArguments(Collections.singletonList(newArg)) + .withName(m.getName().withSimpleName("contentEquals")); } } } From ac3ccee536c91c803cb76b51f4e633a2a8e2541f Mon Sep 17 00:00:00 2001 From: Aleksandar A Simpson Date: Fri, 16 Jun 2023 15:37:36 -0700 Subject: [PATCH 09/14] Update src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java Co-authored-by: Tim te Beek --- .../openrewrite/staticanalysis/EqualsToContentEqualsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java index 0abf7db61c..805aee1d67 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java @@ -64,7 +64,7 @@ public void onlyRunsOnCorrectInvocations() { java( """ class SomeClass { - boolean foo(int number, String str) { + boolean foo(Integer number, String str) { return str.equals(number.toString()); } } From 15089c18d9dfabe3bc2aa7ed289b22e043f75a08 Mon Sep 17 00:00:00 2001 From: AlekSimpson Date: Fri, 16 Jun 2023 15:41:17 -0700 Subject: [PATCH 10/14] added missing import --- .../org/openrewrite/staticanalysis/EqualsToContentEquals.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index a937c3d782..0961c52db1 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -24,10 +24,9 @@ import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.*; -import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; import java.util.stream.Stream; public class EqualsToContentEquals extends Recipe { From 86e3d7926deb8277b1efb5b89dba3e8e103b4e88 Mon Sep 17 00:00:00 2001 From: AlekSimpson Date: Fri, 16 Jun 2023 15:44:42 -0700 Subject: [PATCH 11/14] updated license header year --- gradle/licenseHeader.txt | 2 +- .../openrewrite/staticanalysis/EqualsToContentEqualsTest.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/gradle/licenseHeader.txt b/gradle/licenseHeader.txt index bcb1afc63e..701ddf254d 100644 --- a/gradle/licenseHeader.txt +++ b/gradle/licenseHeader.txt @@ -1,4 +1,4 @@ -Copyright 2021 the original author or authors. +Copyright 2023 the original author or authors.

Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java index 805aee1d67..883c16860b 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java @@ -16,6 +16,7 @@ package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; From 9423d0e20210770f4c8bfaa860cda98138000ab8 Mon Sep 17 00:00:00 2001 From: AlekSimpson Date: Sat, 17 Jun 2023 10:40:26 -0700 Subject: [PATCH 12/14] added test to check that the recipe runs correctly on selects that are not just raw string variables but also things like method calls --- .../staticanalysis/EqualsToContentEquals.java | 5 +-- .../EqualsToContentEqualsTest.java | 33 ++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index 0961c52db1..844614296b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 the original author or authors. + * Copyright 2023 the original author or authors. *

* Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -38,7 +38,8 @@ public class EqualsToContentEquals extends Recipe { private static final List TOSTRING_MATCHERS = Arrays.asList( new MethodMatcher("java.lang.String toString()"), new MethodMatcher("java.lang.StringBuffer toString()"), - new MethodMatcher("java.lang.StringBuilder toString()")); + new MethodMatcher("java.lang.StringBuilder toString()"), + new MethodMatcher("java.lang.CharSequence toString()")); @Override public String getDisplayName() { diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java index 883c16860b..636f830dc2 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 the original author or authors. + * Copyright 2023 the original author or authors. *

* Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -119,4 +119,35 @@ boolean foo(CharSequence cs, String str) { ) ); } + + @Test + void runsOnNonStringVariablesAlso() { + //language=java + rewriteRun( + java( + """ + class SomeClass { + boolean foo(String str) { + return str.equals(getMessage().toString()); + } + + StringBuilder getMessage() { + return new StringBuilder("message"); + } + } + """, + """ + class SomeClass { + boolean foo(String str) { + return str.contentEquals(getMessage()); + } + + StringBuilder getMessage() { + return new StringBuilder("message"); + } + } + """ + ) + ); + } } From 18287b2ff407b67bebbadac81a9f717ed39c12f5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 17 Jun 2023 20:53:59 +0200 Subject: [PATCH 13/14] Polish to minimize loops & logic --- .../staticanalysis/EqualsToContentEquals.java | 65 ++++++++----------- .../EqualsToContentEqualsTest.java | 6 +- 2 files changed, 30 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index 844614296b..9002b9e81e 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -22,32 +22,28 @@ import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.search.UsesType; -import org.openrewrite.java.tree.*; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.TypeUtils; -import java.util.Arrays; import java.util.Collections; -import java.util.List; -import java.util.stream.Stream; +import java.util.Set; public class EqualsToContentEquals extends Recipe { - private static final MethodMatcher equals_matcher = new MethodMatcher("java.lang.String equals(..)"); private static final TreeVisitor PRECONDITION = Preconditions.or( + new UsesType<>("java.lang.CharSequence", false), new UsesType<>("java.lang.StringBuffer", false), - new UsesType<>("java.lang.StringBuilder", false), - new UsesType<>("java.lang.CharSequence", false)); - private static final List TOSTRING_MATCHERS = Arrays.asList( - new MethodMatcher("java.lang.String toString()"), - new MethodMatcher("java.lang.StringBuffer toString()"), - new MethodMatcher("java.lang.StringBuilder toString()"), - new MethodMatcher("java.lang.CharSequence toString()")); + new UsesType<>("java.lang.StringBuilder", false) + ); @Override public String getDisplayName() { - return "Use contentEquals to compare StringBuilder to a String"; + return "Use `String.contentEquals(CharSequence)` instead of `String.equals(CharSequence.toString())`"; } + @Override public String getDescription() { - return "Use contentEquals to compare StringBuilder to a String."; + return "Use `String.contentEquals(CharSequence)` instead of `String.equals(CharSequence.toString())`."; } public TreeVisitor getVisitor() { @@ -55,34 +51,27 @@ public TreeVisitor getVisitor() { } private static class EqualsToContentEqualsVisitor extends JavaIsoVisitor { + private static final MethodMatcher EQUALS_MATCHER = new MethodMatcher("String equals(Object)"); + private static final MethodMatcher TOSTRING_MATCHER = new MethodMatcher("java.lang.* toString()"); + @Override public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) { J.MethodInvocation m = super.visitMethodInvocation(mi, ctx); - // create method matcher on equals(String) - if (equals_matcher.matches(m)) { - Expression argument = m.getArguments().get(0); - - // checks whether the argument is a toString() method call on a StringBuffer or CharSequence - if (TOSTRING_MATCHERS.stream().anyMatch(matcher -> matcher.matches(argument))) { - J.MethodInvocation inv = (J.MethodInvocation) argument; - Expression newArg = inv.getSelect(); - if (inv.getSelect() == null) { return m; } - - Stream TYPES = Stream.of( - JavaType.buildType("java.lang.StringBuilder"), - JavaType.buildType("java.lang.StringBuffer"), - JavaType.buildType("java.lang.CharSequence") - ); - - if (TYPES.anyMatch(type -> TypeUtils.isOfType(newArg.getType(), type))) { - // strip out the toString() on the argument - return m.withArguments(Collections.singletonList(newArg)) - .withName(m.getName().withSimpleName("contentEquals")); - } - } + if (!EQUALS_MATCHER.matches(m)) { + return m; } - - return m; + Expression equalsArgument = m.getArguments().get(0); + if (!TOSTRING_MATCHER.matches(equalsArgument)) { + return m; + } + J.MethodInvocation inv = (J.MethodInvocation) equalsArgument; + Expression toStringSelect = inv.getSelect(); + if (toStringSelect == null || !TypeUtils.isAssignableTo("java.lang.CharSequence", toStringSelect.getType())) { + return m; + } + // Strip out the toString() on the argument and replace with contentEquals + return m.withArguments(Collections.singletonList(toStringSelect)) + .withName(m.getName().withSimpleName("contentEquals")); } } } diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java index 636f830dc2..900203cc4c 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsToContentEqualsTest.java @@ -23,7 +23,7 @@ import static org.openrewrite.java.Assertions.java; -public class EqualsToContentEqualsTest implements RewriteTest { +class EqualsToContentEqualsTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { @@ -34,7 +34,7 @@ public void defaults(RecipeSpec spec) { @Test @DocumentExample - public void replaceStringBuilder() { + void replaceStringBuilder() { //language=java rewriteRun( java( @@ -59,7 +59,7 @@ boolean foo(StringBuilder sb) { } @Test - public void onlyRunsOnCorrectInvocations() { + void onlyRunsOnCorrectInvocations() { //language=java rewriteRun( java( From 8b939442c908076ec2b73837c01fd7b8db20a446 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 17 Jun 2023 20:59:57 +0200 Subject: [PATCH 14/14] Remove unused import --- .../org/openrewrite/staticanalysis/EqualsToContentEquals.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java index 9002b9e81e..a261c13f1a 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsToContentEquals.java @@ -27,7 +27,6 @@ import org.openrewrite.java.tree.TypeUtils; import java.util.Collections; -import java.util.Set; public class EqualsToContentEquals extends Recipe { private static final TreeVisitor PRECONDITION = Preconditions.or(