Avoid existing methods when renaming near-miss Object methods

[martinfrancois]

Suggested review order: 36 of 52 (Score: 2.5)
Review first: #973

What's changed?

RenameMethodsNamedHashcodeEqualOrToString renames a near-miss method name such as hashcode, equal or tostring into the correctly spelled hashCode, equals or toString. With this change it makes no change at all in two cases:

• the type that declares the misspelled method already declares one with the correct name and the signature the rename would produce;
• a supertype declares a method with the correct name and the same parameter types, and that inherited method is final.

Each of the visitor's three branches, one per target name, now also calls the new canRenameTo(...), which permits the rename only when both of its checks allow it.

The first check looks for the already declared method in the enclosing J.Block, the body of the type as it was actually written, and deliberately not in the type model, JavaType.FullyQualified#getMethods(): for records, and for classes handled by an annotation processor such as Lombok's @Data, that model also lists members nobody wrote, for example a generated hashCode(), and an explicit declaration replaces such a member rather than colliding with it. renameWhenRecordDoesNotExplicitlyDeclareTarget covers that case: a record declaring only hashcode() is still renamed.

The second check does use the type model, calling TypeUtils.findDeclaredMethod with the direct supertype of the declaring type, the target name and the parameter types of the misspelled method.

What's your motivation?

Recipe: org.openrewrite.staticanalysis.RenameMethodsNamedHashcodeEqualOrToString.

Before

int hashcode() { return 1; }
public int hashCode() { return 2; }

Actual after the recipe

int hashCode() { return 1; }
public int hashCode() { return 2; }

Expected after the recipe

(unchanged)

Java method names are case sensitive, so one type may legally declare both hashcode() and hashCode(), each with its own body and its own callers. On main the recipe checks the misspelled name, the parameters and the return type and then renames, without checking whether the type already declares the correctly spelled method, so the source it produces fails to compile. The recipe is listed in common-static-analysis.yml, so anyone running org.openrewrite.staticanalysis.CommonStaticAnalysis can get source that no longer builds.

The rename is carried out by ChangeMethodName, which rewrites call sites as well as the declaration. That is correct in general, but in this collision it spreads the damage: every call to tostring() in the sources being rewritten becomes a call to toString(), the already existing method with a different body, and deleting the duplicate declaration by hand afterwards does not put those calls back. Reproduced on 2.39.0, 2.40.0 and 2.41.0-SNAPSHOT built from main.

Affected code in real projects

• jython/jython PyArray.java: PyArray implements the Python array.tostring() API in a public String tostring() method and separately overrides Object.toString() at line 698 of the same class. The recipe from main renames tostring() and its five call sites to toString(), so the class declares two no-arg toString() methods and no longer compiles.
• checkstyle/checkstyle InputMagicNumberDefault3.java: this MagicNumber test input deliberately declares both public int hashCode() and public int hashcode() in the same class. The recipe from main renames hashcode() to hashCode(), producing a second no-arg hashCode() in the same class, which does not compile.

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

No existing test expectation changed: noncompliantMethodNames and compliantWhenHasMismatchingTypeInformation keep their input and their expected output. The file is otherwise edited only to add "unused" to the class level @SuppressWarnings, for methods declared in the new test sources and never called from them, and to import org.openrewrite.java.Assertions.version for the two record tests, which wrap their source in version(..., 17) because records need Java 17.

Some cases stay wrong. Main produces the same result as this branch for each, so this change neither introduces them nor makes them worse:

• When one type declares two differently misspelled variants of the same correct name, such as hashcode() and hashCODE(), both are renamed to hashCode() and the two renamed declarations then collide: the new check looks for a sibling already named exactly hashCode, and at the moment either variant is examined the type declares no such method.
• A misspelled method whose access is weaker than the access of the method it would end up overriding, for example a package-private tostring() on a type that declares no toString() of its own, is still renamed. The result overrides the public Object.toString() with weaker access, which does not compile.
• A static misspelled method is still renamed, and so is a method declared as an element of an annotation type. Neither result compiles: a static method carrying the signature of an inherited instance method is a case of hiding rather than overriding (JLS 8.4.8.2), which Java does not permit between a static and an instance method, and an @interface member may not carry the name of a method of Object.

Have you considered any alternatives or workarounds?

The inherited half of the check stops a rename only when the inherited method is final. An inherited non-final hashCode() remains a valid target and the rename goes ahead, because renaming onto it produces a normal override: legal Java and the transformation this recipe is designed to make. The cost is that calls made through the supertype then run the renamed body. Stopping whenever the supertype lookup finds the target method at all, final or not, is the one line change of removing the .filter(m -> m.getFlags().contains(Flag.Final)) call from canRenameTo, but it turns the recipe off: java.lang.Object declares hashCode(), equals(Object) and toString(), so the lookup finds the target for every class and no rename is left. With that call removed, every test in this class that expects a rename fails, including noncompliantMethodNames.

Any additional context

This change adds 8 tests to RenameMethodsNamedHashcodeEqualOrToStringTest, taking it from 2 tests to 10. Without the code change in this pull request, these 5 tests fail:

• doNotRenameWhenEnumAlreadyDeclaresTarget
• doNotRenameWhenInheritedTargetIsFinal
• doNotRenameWhenInterfaceAlreadyDeclaresTarget
• doNotRenameWhenRecordAlreadyDeclaresTarget
• doNotRenameWhenTargetIsAlreadyDeclared

These 3 tests show that correct renames still happen:

• renameUpdatesCallSitesWhenThereIsNoCollision
• renameWhenExistingMethodIsAnOverloadWithDifferentParameters
• renameWhenRecordDoesNotExplicitlyDeclareTarget

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

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