diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java b/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java index d1e485bb7..50a4b7796 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java @@ -24,6 +24,7 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.search.UsesMethod; +import org.openrewrite.java.service.ImportService; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; @@ -31,6 +32,7 @@ import java.util.Set; import static java.util.Collections.singleton; +import static java.util.Objects.requireNonNull; public class RemoveHashCodeCallsFromArrayInstances extends Recipe { private static final MethodMatcher HASHCODE_MATCHER = new MethodMatcher("java.lang.Object hashCode()"); @@ -60,11 +62,13 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat if (HASHCODE_MATCHER.matches(mi)) { Expression select = mi.getSelect(); if (select != null && select.getType() instanceof JavaType.Array) { - maybeAddImport("java.util.Arrays"); - return JavaTemplate.builder("Arrays.hashCode(#{anyArray(java.lang.Object)})") - .imports("java.util.Arrays") + // Shorten only the select the template introduced, so a competing `Arrays` in scope keeps the + // qualified form rather than binding wrongly + J.MethodInvocation replacement = JavaTemplate.builder("java.util.Arrays.hashCode(#{anyArray(java.lang.Object)})") .build() .apply(getCursor(), mi.getCoordinates().replace(), select); + doAfterVisit(service(ImportService.class).shortenFullyQualifiedTypeReferencesIn(requireNonNull(replacement.getSelect()))); + return replacement; } } diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java b/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java index de4257ddf..d00174317 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java @@ -20,8 +20,10 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.service.ImportService; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaCoordinates; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypedTree; @@ -30,6 +32,7 @@ import java.util.Set; import static java.util.Collections.singleton; +import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toList; public class RemoveToStringCallsFromArrayInstances extends Recipe { @@ -109,11 +112,19 @@ public J buildReplacement(Expression select, J.MethodInvocation mi) { return mi; } - maybeAddImport("java.util.Arrays"); - return JavaTemplate.builder("Arrays.toString(#{anyArray(java.lang.Object)})") - .imports("java.util.Arrays") + return arraysToString(mi.getCoordinates().replace(), select); + } + + /** + * Shortens only the select the template introduced, so a competing {@code Arrays} in scope keeps the + * qualified form rather than binding wrongly. + */ + private J.MethodInvocation arraysToString(JavaCoordinates coordinates, Expression array) { + J.MethodInvocation replacement = JavaTemplate.builder("java.util.Arrays.toString(#{anyArray(java.lang.Object)})") .build() - .apply(getCursor(), mi.getCoordinates().replace(), select); + .apply(getCursor(), coordinates, array); + doAfterVisit(service(ImportService.class).shortenFullyQualifiedTypeReferencesIn(requireNonNull(replacement.getSelect()))); + return replacement; } @Override @@ -122,11 +133,7 @@ public Expression visitExpression(Expression exp, ExecutionContext ctx) { if (e instanceof TypedTree && e.getType() instanceof JavaType.Array) { 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 arraysToString(e.getCoordinates().replace(), e); } } diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java index 8ed4dac82..90ebf5148 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java @@ -90,6 +90,159 @@ public int[] getArr() { ); } + @Test + void qualifyArraysWhenMemberTypeShadowsIt() { + //language=java + rewriteRun( + java( + """ + class Test { + static class Arrays { + } + + int hash(String[] values) { + return values.hashCode(); + } + } + """, + """ + class Test { + static class Arrays { + } + + int hash(String[] values) { + return java.util.Arrays.hashCode(values); + } + } + """ + ) + ); + } + + @Test + void qualifyArraysWhenSameFileTopLevelTypeShadowsIt() { + //language=java + rewriteRun( + java( + """ + class Test { + int hash(String[] values) { + return values.hashCode(); + } + } + + class Arrays { + } + """, + """ + class Test { + int hash(String[] values) { + return java.util.Arrays.hashCode(values); + } + } + + class Arrays { + } + """ + ) + ); + } + + @Test + void qualifyArraysWhenAnotherArraysTypeIsImported() { + rewriteRun( + //language=java + java( + """ + package other; + + public class Arrays { + } + """ + ), + //language=java + java( + """ + import other.Arrays; + + class Test { + Arrays arrays; + + int hash(String[] values) { + return values.hashCode(); + } + } + """, + """ + import other.Arrays; + + class Test { + Arrays arrays; + + int hash(String[] values) { + return java.util.Arrays.hashCode(values); + } + } + """ + ) + ); + } + + @Test + void doesNotShortenQualifiedNamesTheUserWroteInsideTheArgument() { + rewriteRun( + //language=java + java( + """ + package com.example; + + public class Base { + public static class Holder { + public static String[] ARR = {"base"}; + } + } + """ + ), + //language=java + java( + """ + package other; + + public class Holder { + public static String[] ARR = {"other"}; + } + """ + ), + //language=java + java( + """ + package com.example; + + class Test extends Base { + static class Arrays { + } + + int hash() { + return other.Holder.ARR.hashCode(); + } + } + """, + """ + package com.example; + + class Test extends Base { + static class Arrays { + } + + int hash() { + return java.util.Arrays.hashCode(other.Holder.ARR); + } + } + """ + ) + ); + } + @Test void onlyRunOnArrayInstances() { //language=java diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java index cb4107246..7b8c6c03c 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java @@ -57,6 +57,140 @@ public static void main(String[] args) { ); } + @Test + void qualifyArraysWhenMemberTypeShadowsIt() { + //language=java + rewriteRun( + java( + """ + import java.util.Objects; + + class Test { + static class Arrays { + } + + void render(String[] values) { + String direct = values.toString(); + String valueOf = String.valueOf(values); + String objects = Objects.toString(values); + System.out.println(values); + String concat = "values=" + values; + } + } + """, + """ + class Test { + static class Arrays { + } + + void render(String[] values) { + String direct = java.util.Arrays.toString(values); + String valueOf = java.util.Arrays.toString(values); + String objects = java.util.Arrays.toString(values); + System.out.println(java.util.Arrays.toString(values)); + String concat = "values=" + java.util.Arrays.toString(values); + } + } + """ + ) + ); + } + + @Test + void qualifyArraysWhenAnotherArraysTypeIsImported() { + rewriteRun( + //language=java + java( + """ + package other; + + public class Arrays { + } + """ + ), + //language=java + java( + """ + import other.Arrays; + + class Test { + Arrays arrays; + + String render(String[] values) { + return values.toString(); + } + } + """, + """ + import other.Arrays; + + class Test { + Arrays arrays; + + String render(String[] values) { + return java.util.Arrays.toString(values); + } + } + """ + ) + ); + } + + @Test + void doesNotShortenQualifiedNamesTheUserWroteInsideTheArgument() { + rewriteRun( + //language=java + java( + """ + package com.example; + + public class Base { + public static class Holder { + public static String[] ARR = {"base"}; + } + } + """ + ), + //language=java + java( + """ + package other; + + public class Holder { + public static String[] ARR = {"other"}; + } + """ + ), + //language=java + java( + """ + package com.example; + + class Test extends Base { + static class Arrays { + } + + String render() { + return other.Holder.ARR.toString(); + } + } + """, + """ + package com.example; + + class Test extends Base { + static class Arrays { + } + + String render() { + return java.util.Arrays.toString(other.Holder.ARR); + } + } + """ + ) + ); + } + @Test void doesNotRunOnNonArrayInstances() { //language=java