From af928bed35a78bad401ee1a13d85886d383deb9c Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 13:24:56 +0200 Subject: [PATCH 1/3] Qualify generated java.util.Arrays calls when the name is shadowed RemoveHashCodeCallsFromArrayInstances and RemoveToStringCallsFromArrayInstances both emitted a bare Arrays.hashCode(..) or Arrays.toString(..) and relied on maybeAddImport("java.util.Arrays"). Adding an import does not make a simple name unambiguous: when the compilation unit already declares or imports its own type named Arrays, the generated call resolves to that type instead of the JDK one, so the recipe emits code that binds to the wrong class or does not compile. Both recipes now emit a fully qualified java.util.Arrays reference and hand only the select they just generated to ImportService.shortenFullyQualifiedTypeReferencesIn. The simple name and its import come back wherever Arrays is free, and the qualified form stays wherever it is not. Scoping the shortener to that select leaves qualified names the caller wrote inside the array argument untouched. RemoveToStringCallsFromArrayInstances routes its direct, String.valueOf, Objects.toString, implicit-argument and concatenation paths through one helper so every emitting site gets the same treatment. Worth weighing on review: the shortener's conflict set is the compilation unit's own declarations and imports, not the full JLS scope, so an Arrays type inherited from a supertype, or a field or variable named Arrays, is still not detected and still receives the simple name. Those shapes were already wrong before this change; covering them needs scope-aware shortening in rewrite-java rather than a change here. --- ...RemoveHashCodeCallsFromArrayInstances.java | 14 +- ...RemoveToStringCallsFromArrayInstances.java | 29 ++- ...veHashCodeCallsFromArrayInstancesTest.java | 171 ++++++++++++++++++ ...veToStringCallsFromArrayInstancesTest.java | 159 ++++++++++++++++ 4 files changed, 361 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java b/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java index d1e485bb7..520dc25aa 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,17 @@ 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") + // Emit a fully qualified reference and let the import service shorten it back to the simple + // name when no type named `Arrays` is declared anywhere in this compilation unit or brought + // in by one of its imports. A type named `Arrays` that is only inherited from a supertype, + // and a field or variable named `Arrays`, are not detected; those shapes produced the wrong + // reference before this change and still do. The shortener is scoped to the `java.util.Arrays` + // select this template introduced, never to the user's argument expression. + 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..f9097380f 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,23 @@ 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); + } + + /** + * Emits a fully qualified {@code java.util.Arrays.toString(..)} call and lets the import service shorten it + * back to the simple name when no type named {@code Arrays} is declared anywhere in this compilation unit + * or brought in by one of its imports. A type named {@code Arrays} that is only inherited from a supertype, + * and a field or variable named {@code Arrays}, are not detected; those shapes produced the wrong reference + * before this change and still do. The shortener is scoped to the {@code java.util.Arrays} select this + * template introduced, never to the user's argument expression. + */ + private J 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 +137,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 (Expression) 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..123d1a53c 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java @@ -90,6 +90,177 @@ 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 { + } + } + """ + ), + //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 doNotChangeHashCodeOnNonArrayWhenArraysIsShadowed() { + //language=java + rewriteRun( + java( + """ + class Test { + static class Arrays { + } + + int hash(Object value) { + return value.hashCode(); + } + } + """ + ) + ); + } + @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..37a8df990 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java @@ -57,6 +57,165 @@ 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 doNotChangeToStringOnNonArrayWhenArraysIsShadowed() { + //language=java + rewriteRun( + java( + """ + import java.util.Objects; + + class Test { + static class Arrays { + } + + void render(Object value) { + String direct = value.toString(); + String valueOf = String.valueOf(value); + String objects = Objects.toString(value); + System.out.println(value); + String concat = "value=" + value; + } + } + """ + ) + ); + } + @Test void doesNotRunOnNonArrayInstances() { //language=java From 533fd71185aca72b0c33b9cd1afb67433eeb183d Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 11 Aug 2026 11:41:33 +0200 Subject: [PATCH 2/3] Review fixes: condense comments, narrow the helper return type, and tighten the tests --- ...RemoveHashCodeCallsFromArrayInstances.java | 8 ++---- ...RemoveToStringCallsFromArrayInstances.java | 12 +++------ ...veHashCodeCallsFromArrayInstancesTest.java | 20 +-------------- ...veToStringCallsFromArrayInstancesTest.java | 25 ------------------- 4 files changed, 7 insertions(+), 58 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java b/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java index 520dc25aa..940f0c924 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java @@ -62,12 +62,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat if (HASHCODE_MATCHER.matches(mi)) { Expression select = mi.getSelect(); if (select != null && select.getType() instanceof JavaType.Array) { - // Emit a fully qualified reference and let the import service shorten it back to the simple - // name when no type named `Arrays` is declared anywhere in this compilation unit or brought - // in by one of its imports. A type named `Arrays` that is only inherited from a supertype, - // and a field or variable named `Arrays`, are not detected; those shapes produced the wrong - // reference before this change and still do. The shortener is scoped to the `java.util.Arrays` - // select this template introduced, never to the user's argument expression. + // Emit `java.util.Arrays` fully qualified and shorten only the select the template introduced, + // so that 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); diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java b/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java index f9097380f..1563cf360 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java @@ -116,14 +116,10 @@ public J buildReplacement(Expression select, J.MethodInvocation mi) { } /** - * Emits a fully qualified {@code java.util.Arrays.toString(..)} call and lets the import service shorten it - * back to the simple name when no type named {@code Arrays} is declared anywhere in this compilation unit - * or brought in by one of its imports. A type named {@code Arrays} that is only inherited from a supertype, - * and a field or variable named {@code Arrays}, are not detected; those shapes produced the wrong reference - * before this change and still do. The shortener is scoped to the {@code java.util.Arrays} select this - * template introduced, never to the user's argument expression. + * Emits {@code java.util.Arrays} fully qualified and shortens only the select the template introduced, + * so that a competing {@code Arrays} in scope keeps the qualified form rather than binding wrongly. */ - private J arraysToString(JavaCoordinates coordinates, Expression array) { + private J.MethodInvocation arraysToString(JavaCoordinates coordinates, Expression array) { J.MethodInvocation replacement = JavaTemplate.builder("java.util.Arrays.toString(#{anyArray(java.lang.Object)})") .build() .apply(getCursor(), coordinates, array); @@ -137,7 +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) { - return (Expression) arraysToString(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 123d1a53c..90ebf5148 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstancesTest.java @@ -198,6 +198,7 @@ void doesNotShortenQualifiedNamesTheUserWroteInsideTheArgument() { public class Base { public static class Holder { + public static String[] ARR = {"base"}; } } """ @@ -242,25 +243,6 @@ int hash() { ); } - @Test - void doNotChangeHashCodeOnNonArrayWhenArraysIsShadowed() { - //language=java - rewriteRun( - java( - """ - class Test { - static class Arrays { - } - - int hash(Object value) { - return value.hashCode(); - } - } - """ - ) - ); - } - @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 37a8df990..7b8c6c03c 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstancesTest.java @@ -191,31 +191,6 @@ String render() { ); } - @Test - void doNotChangeToStringOnNonArrayWhenArraysIsShadowed() { - //language=java - rewriteRun( - java( - """ - import java.util.Objects; - - class Test { - static class Arrays { - } - - void render(Object value) { - String direct = value.toString(); - String valueOf = String.valueOf(value); - String objects = Objects.toString(value); - System.out.println(value); - String concat = "value=" + value; - } - } - """ - ) - ); - } - @Test void doesNotRunOnNonArrayInstances() { //language=java From 79ffafccdb875651be2c790ac65b95dea7e576e5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:27:24 +0200 Subject: [PATCH 3/3] Trim commentary --- .../staticanalysis/RemoveHashCodeCallsFromArrayInstances.java | 4 ++-- .../staticanalysis/RemoveToStringCallsFromArrayInstances.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java b/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java index 940f0c924..50a4b7796 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveHashCodeCallsFromArrayInstances.java @@ -62,8 +62,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat if (HASHCODE_MATCHER.matches(mi)) { Expression select = mi.getSelect(); if (select != null && select.getType() instanceof JavaType.Array) { - // Emit `java.util.Arrays` fully qualified and shorten only the select the template introduced, - // so that a competing `Arrays` in scope keeps the qualified form rather than binding wrongly. + // 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); diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java b/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java index 1563cf360..d00174317 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java @@ -116,8 +116,8 @@ public J buildReplacement(Expression select, J.MethodInvocation mi) { } /** - * Emits {@code java.util.Arrays} fully qualified and shortens only the select the template introduced, - * so that a competing {@code Arrays} in scope keeps the qualified form rather than binding wrongly. + * 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)})")