Choose an unused name and rename references for underscore identifiers

[martinfrancois]

Suggested review order: 43 of 52 (Score: 1.5)
Review first: openrewrite/rewrite-static-analysis#991

What's changed?

RenameUnderscoreIdentifier now renames _ to a name that is not already in use, instead of always renaming it to __. When it renames a class, it now also renames the references to that class, not only its declaration.

The recipe picks the first name in the sequence __, ___, ____ and so on that is free. For a variable or a class, a name is free when no identifier anywhere in the source file being visited uses it, and no field or method declared on or inherited by a class that the same file declares uses it. Avoiding inherited names matters because such a rename would not collide visibly: JLS 6.4 says a declaration of a field, a method or a type named n shadows every other declaration of n in scope where it occurs, so the simple names in that class would silently rebind.

For a method the source file is not consulted. The name is derived from the root of the override chain, reached through TypeUtils.findOverriddenMethod, and from the method names of that root's declaring type, its supertypes and its interfaces. Field names are not consulted, since a field and a method may share a name in Java. Every source file that declares, overrides or calls the method reaches the same root and reads the same hierarchy, so they all compute the same name without seeing each other.

Renaming a top level class also renames its source file, so the recipe is now a ScanningRecipe<Set<Path>> that collects the source path of every file in the run. A candidate name is skipped when a source file already exists at <candidate>.java beside the file being renamed, so the renamed compilation unit is never written over an existing source file.

A method declaration, a method invocation, a method reference or a class declaration with no type attribution is now left unchanged, whereas main renamed it: the type is what the new code needs, both to compute a name that other source files agree on and to find the references to a renamed class. That deliberately reduces what the recipe does, since _ then survives into the output and the Java 9 keyword error with it, while main renamed the declaration to __ with no collision check of any kind. Variable renames do not use type attribution and are unaffected.

What's your motivation?

Recipe: org.openrewrite.java.migrate.lang.RenameUnderscoreIdentifier.

Before

int sum(int _, int __) { return _ + __; }

Actual after the recipe

int sum(int __, int __) { return __ + __; }

Expected after the recipe

int sum(int ___, int __) { return ___ + __; }

The output on main does not compile, in two different ways. A rename that collides with an existing declaration gives a duplicate declaration error, variable __ is already defined in method sum for the example above. A class declaration that main renames while leaving the references to it spelled _ gives as of release 9, '_' is a keyword at each of those references. In both cases the recipe reports the file as changed and raises no error, so a build sees a run that succeeded and source that does not compile. RenameUnderscoreIdentifier runs inside Java8toJava11, which both UpgradeToJava17 and UpgradeToJava21 include, so any of those three upgrades can produce it. Reproduced on 3.41.0 and on current main.

Affected code in real projects

• yinwang0/rubysonar _.java: a Java 7 static analyzer whose utility class is literally named _ and calls itself with qualified references such as _.die(..) at lines 99, 167, 217 and 530; the recipe from main renames the declaration to class __ while leaving those references spelled _, so each of them hits the release-9 keyword error and the file no longer compiles.

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

No existing test changed its expectation, and 905 of the 1137 added lines are the test file. One added and one deleted line are the regenerated recipes.csv row; the rest is the implementation.

Three limits:

• References to a renamed class from other source files are not followed, the same as on main. This limit is about class references only: method renames do agree across files, for the reason given under What's changed.
• A collision the recipe cannot see still produces a duplicate declaration error from javac on the output, the same as on main. The recipe knows the paths of the other source files in the run, but not the declarations inside them, so a class named __ declared in a file whose own name is not __.java, for example a second, package private top level class in another file of the same package, is invisible to the check.
• The recipe checks the whole source file rather than only the enclosing scope, so it picks ___ whenever another declaration anywhere in the source file already uses __, even when __ is free in the enclosing scope.

The accumulator is a flat Set<Path> rather than a map keyed by JavaProject, so a multi-project run can hold source paths belonging to another project. Such a path can only make the recipe skip a candidate name and move on to the next one; it can never make it choose a name that collides.

Have you considered any alternatives or workarounds?

When the file name that a class rename would need is already occupied by another source file, this pull request skips that candidate name and moves on to the next free one. The alternative is to leave that class unrenamed, which keeps the _ keyword error in the output. The choice lives in one place, availableName(JavaType.FullyQualified), so switching to the other behaviour is a few lines there.

Any additional context

Pre-existing tests changed: None.

This change adds 24 tests to RenameUnderscoreIdentifierTest. Without the code change in this pull request, 18 of them fail. They cover five shapes:

• a name taken by a declaration already in the file, for parameters, local variables, fields, methods and nested classes alike, with the search advancing until a free name is found, for example parameterCollision;
• a name that would shadow an inherited member, for example inheritedMemberNameIsNotShadowed;
• a renamed class whose references move with it, in every reference position, for example nestedTypeRenamedInEveryReferencePosition;
• a renamed method whose overloads, method references and callers in other files agree on one name, for example callersInAnotherSourceFileFollowTheDeclaration;
• a renamed top level class whose source file moves without landing on an existing file, for example classFileRenameDoesNotOverwriteACollidingClassFile.

The full list is the test file in this change. All 18 are new tests; the 11 that already existed keep their expectations. The other 6 new tests pass either way and pin down behaviour this change keeps.

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