Qualify generated java.util.Arrays calls when the name is shadowed

[martinfrancois]

Suggested review order: 33 of 52 (Score: 2.75)
Review first: openrewrite/rewrite-migrate-java#1198

What's changed?

RemoveHashCodeCallsFromArrayInstances and RemoveToStringCallsFromArrayInstances now write a reference to java.util.Arrays that resolves correctly when the file already binds the simple name Arrays to something else.

Both recipes now emit the call fully qualified and pass only the receiver of the generated call, the java.util.Arrays part, to ImportService.shortenFullyQualifiedTypeReferencesIn. That service rewrites the receiver back to the simple name and adds the import, but only when the compilation unit neither declares nor imports another type called Arrays. Passing the receiver rather than the whole call keeps the service away from any qualified name the user wrote inside the argument.

In a file where nothing else is called Arrays the output is unchanged from current main, except that the import now comes from the shortening service rather than from the maybeAddImport("java.util.Arrays") call this change deletes from both recipes.

RemoveToStringCallsFromArrayInstances built the same template twice, in buildReplacement and in visitExpression. Both now go through one private helper, arraysToString, so all five rewrite paths emit the same call: array.toString(), Objects.toString(array), String.valueOf(array), an array passed to a method such as print or format, and string concatenation.

What's your motivation?

Recipe: RemoveHashCodeCallsFromArrayInstances and RemoveToStringCallsFromArrayInstances.

Before

return values.hashCode();

Actual after the recipe

return Arrays.hashCode(values); // binds to Test.Arrays

Expected after the recipe

return java.util.Arrays.hashCode(values);

On current main both recipes call maybeAddImport("java.util.Arrays") and then print the simple name. maybeAddImport declines only when an existing import already binds the simple name Arrays, and it never looks at the types the file itself declares. A type declaration shadows an import of the same simple name throughout its scope (JLS 6.4), so javac resolves the Arrays.hashCode(values) above to the user's own nested type and rejects it with method hashCode in class Object cannot be applied to given types.

The silent variant is worse: if that user type declares a static method matching the generated call, the output compiles and calls it instead of the JDK one. Both shapes reproduce on v2.40.0 and on main 5785534.

The one collision current main does handle is a conflicting import. In a file containing import other.Arrays;, maybeAddImport refuses to add a second import for that simple name and the generated reference is left fully qualified, so that shape already compiles.

Anything in particular you'd like reviewers to focus on?

No existing test expectation changed; both test files are additions only.

Three limits:

• A type named Arrays inherited from a supertype, and a variable named Arrays, are still not detected, on current main and on this branch.
• The output can be more verbose than it needs to be. The shortening service treats Arrays as taken as soon as a type of that name is declared anywhere in the compilation unit, even one not in scope at the call site, for example a static class Arrays nested inside a different top level class of the same file. Every call these recipes write in that file then stays fully qualified. Only that one name is affected, and the qualified call is correct, so the cost is verbosity, not behaviour.
• One shape stays broken here and was already broken on current main: a file with both a variable named java in scope at the call site and its own type named Arrays. There, java.util.Arrays.hashCode(values) does not compile, because a variable is chosen over a package name of the same spelling (JLS 6.5.2). Current main writes Arrays.hashCode(values) there, which binds to the file's own type, so it does not compile either. Only the way it fails changes.

Have you considered any alternatives or workarounds?

One alternative is to detect the conflict inside each recipe instead of in the import service. That is more exact and would fix the first limit above, but each recipe would then reimplement Java's name resolution rules. Each recipe has one doAfterVisit line, the one that passes the generated java.util.Arrays receiver to the shortening service; switching means replacing that line with a walk over the enclosing scopes looking for a binding of the simple name Arrays, roughly ten lines per recipe. This branch does not contain that check. Say the word in review and I will write it.

Any additional context

This change adds 5 tests to RemoveHashCodeCallsFromArrayInstancesTest and 4 to RemoveToStringCallsFromArrayInstancesTest. Without the code change in this pull request, these 5 tests fail:

• doesNotShortenQualifiedNamesTheUserWroteInsideTheArgument in both classes
• qualifyArraysWhenMemberTypeShadowsIt in both classes
• qualifyArraysWhenSameFileTopLevelTypeShadowsIt in the hashCode class

The other 4 pass either way. Two are qualifyArraysWhenAnotherArraysTypeIsImported, covering the conflicting import above, which I am happy to drop. The other two, doNotChangeHashCodeOnNonArrayWhenArraysIsShadowed and doNotChangeToStringOnNonArrayWhenArraysIsShadowed, declare a nested static class Arrays, call on a plain Object receiver and assert that nothing is rewritten.

The two classes hold 8 and 22 tests on this branch.

This change was prepared with AI assistance (Claude Code). I reviewed the code, the tests and this description.

I ran the formatter with the repository's .editorconfig. It also wanted to re-indent lines that this change does not touch, so I left those alone and kept the diff limited to this change.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've run ./gradlew build locally, and committed any resulting changes to recipes.csv
• I've formatted the lines I changed, without reformatting code I didn't touch