From 9267c022eaed95082933514cd1f80471cb789d2a Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 13:25:31 +0200 Subject: [PATCH 1/3] RenameMethodsNamedHashcodeEqualOrToString: do not rename onto an existing method Java method names are case sensitive, so a type may legally declare both a near-miss method and the correctly named Object method, each with its own behavior. The recipe renamed the near-miss declaration regardless, leaving two declarations with the same erased signature in one type, which does not compile. Renaming onto an inherited final method has the same defect, because the result is an illegal override. Guard the hashCode(), equals(Object) and toString() branches independently: skip the rename when the enclosing type already declares the target signature, or when a supertype declares it final. Neither declaration is deleted or merged, since their bodies and callers can represent distinct behavior. Unambiguous near-miss declarations are still renamed together with their call sites. The guard reads sibling declarations from the enclosing type body in the LST rather than from JavaType.Class#getMethods(). For records, and for annotation-processed classes such as Lombok @Data, the type model also carries generated equals/hashCode/toString members that an explicit declaration replaces rather than collides with, so consulting it would make the recipe a silent no-op on those types. Two limits remain, both pre-existing on main and unchanged here: two near-miss variants of the same target in one type still collide with each other after renaming, and renaming a package-private near-miss to an Object method name can produce a weaker-access-privileges error. --- ...meMethodsNamedHashcodeEqualOrToString.java | 37 ++- ...thodsNamedHashcodeEqualOrToStringTest.java | 241 +++++++++++++++++- 2 files changed, 274 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java index 466e0edbd..750db70ce 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java @@ -24,8 +24,10 @@ import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.search.DeclaresMethod; +import org.openrewrite.java.tree.Flag; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Statement; import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.staticanalysis.java.JavaFileChecker; @@ -63,17 +65,46 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex String sn = method.getSimpleName(); JavaType rte = method.getReturnTypeExpression().getType(); JavaType.Method t = method.getMethodType(); - if (equalsIgnoreCaseExclusive(sn, "hashCode") && JavaType.Primitive.Int == rte && NO_ARGS.matches(t)) { + if (equalsIgnoreCaseExclusive(sn, "hashCode") && JavaType.Primitive.Int == rte && NO_ARGS.matches(t) && canRenameTo(t, "hashCode", NO_ARGS)) { doAfterVisit(new ChangeMethodName(MethodMatcher.methodPattern(method), "hashCode", true, false).getVisitor()); - } else if ("equal".equalsIgnoreCase(sn) && JavaType.Primitive.Boolean == rte && OBJECT_ARG.matches(t)) { + } else if ("equal".equalsIgnoreCase(sn) && JavaType.Primitive.Boolean == rte && OBJECT_ARG.matches(t) && canRenameTo(t, "equals", OBJECT_ARG)) { doAfterVisit(new ChangeMethodName(MethodMatcher.methodPattern(method), "equals", true, false).getVisitor()); - } else if (equalsIgnoreCaseExclusive(sn, "toString") && TypeUtils.isString(rte) && NO_ARGS.matches(t)) { + } else if (equalsIgnoreCaseExclusive(sn, "toString") && TypeUtils.isString(rte) && NO_ARGS.matches(t) && canRenameTo(t, "toString", NO_ARGS)) { doAfterVisit(new ChangeMethodName(MethodMatcher.methodPattern(method), "toString", true, false).getVisitor()); } } return super.visitMethodDeclaration(method, ctx); } + /** + * Java method names are case sensitive, so a type may legally declare both the near-miss method and the + * correctly named one, each with its own behavior. Renaming would then emit a duplicate declaration, and + * renaming onto an inherited `final` method would emit an illegal override, so leave both cases alone + * rather than deleting or merging either implementation. + *

+ * Explicit declarations are read from the enclosing type's body in the LST rather than from the type + * model, because for records and annotation-processed classes (such as Lombok's `@Data`) the type model + * also contains generated `equals`/`hashCode`/`toString` members that an explicit declaration would + * replace rather than collide with. + */ + private boolean canRenameTo(JavaType.Method methodType, String targetName, MethodMatcher signature) { + J.Block body = getCursor().firstEnclosing(J.Block.class); + if (body != null) { + for (Statement statement : body.getStatements()) { + if (statement instanceof J.MethodDeclaration) { + J.MethodDeclaration sibling = (J.MethodDeclaration) statement; + if (targetName.equals(sibling.getSimpleName()) && + sibling.getMethodType() != null && signature.matches(sibling.getMethodType())) { + return false; + } + } + } + } + return !TypeUtils.findDeclaredMethod(methodType.getDeclaringType().getSupertype(), targetName, methodType.getParameterTypes()) + .filter(m -> m.getFlags().contains(Flag.Final)) + .isPresent(); + } + private boolean equalsIgnoreCaseExclusive(String inputToCheck, String targetToCheck) { return inputToCheck.equalsIgnoreCase(targetToCheck) && !inputToCheck.equals(targetToCheck); } diff --git a/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java b/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java index 34393129e..5800b1687 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java @@ -21,8 +21,9 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.version; -@SuppressWarnings({"MethodMayBeStatic", "MisspelledEquals", "BooleanMethodNameMustStartWithQuestion"}) +@SuppressWarnings({"MethodMayBeStatic", "MisspelledEquals", "BooleanMethodNameMustStartWithQuestion", "unused"}) class RenameMethodsNamedHashcodeEqualOrToStringTest implements RewriteTest { @Override @@ -113,4 +114,242 @@ public int hashcode(int a, int b) { ) ); } + + @Test + void doNotRenameWhenTargetIsAlreadyDeclared() { + rewriteRun( + //language=java + java( + """ + class Test { + int hashcode() { + return 1; + } + + public int hashCode() { + return 2; + } + + boolean equal(Object value) { + return false; + } + + public boolean equals(Object value) { + return true; + } + + String tostring() { + return "near"; + } + + public String toString() { + return "proper"; + } + } + """ + ) + ); + } + + @Test + void doNotRenameWhenInterfaceAlreadyDeclaresTarget() { + rewriteRun( + //language=java + java( + """ + interface ITest { + int hashcode(); + + int hashCode(); + + boolean equal(Object obj); + + boolean equals(Object obj); + + String tostring(); + + String toString(); + } + """ + ) + ); + } + + @Test + void doNotRenameWhenEnumAlreadyDeclaresTarget() { + rewriteRun( + //language=java + java( + """ + enum Test { + A; + + String tostring() { + return "near"; + } + + @Override + public String toString() { + return "proper"; + } + } + """ + ) + ); + } + + @Test + void doNotRenameWhenRecordAlreadyDeclaresTarget() { + rewriteRun( + version( + //language=java + java( + """ + record Test(int value) { + int hashcode() { + return 1; + } + + @Override + public int hashCode() { + return 2; + } + } + """ + ), 17) + ); + } + + @Test + void renameWhenRecordDoesNotExplicitlyDeclareTarget() { + rewriteRun( + version( + //language=java + java( + """ + record Test(int value) { + public int hashcode() { + return 1; + } + } + """, + """ + record Test(int value) { + public int hashCode() { + return 1; + } + } + """ + ), 17) + ); + } + + @Test + void doNotRenameWhenInheritedTargetIsFinal() { + rewriteRun( + //language=java + java( + """ + class Base { + @Override + public final int hashCode() { + return 1; + } + } + + class Test extends Base { + int hashcode() { + return 2; + } + } + """ + ) + ); + } + + @Test + void renameWhenExistingMethodIsAnOverloadWithDifferentParameters() { + rewriteRun( + //language=java + java( + """ + class Test { + public boolean equal(Object obj) { + return false; + } + + public boolean equals(String other) { + return true; + } + } + """, + """ + class Test { + public boolean equals(Object obj) { + return false; + } + + public boolean equals(String other) { + return true; + } + } + """ + ) + ); + } + + @Test + void renameUpdatesCallSitesWhenThereIsNoCollision() { + rewriteRun( + //language=java + java( + """ + import java.util.function.Supplier; + + class Test { + public String tostring() { + return ""; + } + + String local() { + return tostring(); + } + + Supplier reference() { + return this::tostring; + } + } + + class Caller { + String call(Test test) { + return test.tostring(); + } + } + """, + """ + import java.util.function.Supplier; + + class Test { + public String toString() { + return ""; + } + + String local() { + return toString(); + } + + Supplier reference() { + return this::toString; + } + } + + class Caller { + String call(Test test) { + return test.toString(); + } + } + """ + ) + ); + } } From 4d67238c22c5999dd7a590962f08acbdf32fe6a7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 11 Aug 2026 10:11:31 +0200 Subject: [PATCH 2/3] Review fixes: also reject same-file subtype collisions, and non-public or static near-miss methods --- ...meMethodsNamedHashcodeEqualOrToString.java | 51 +++++++----- ...thodsNamedHashcodeEqualOrToStringTest.java | 80 +++++++++++++++++-- 2 files changed, 104 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java index 750db70ce..d4fc34fdc 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java @@ -26,13 +26,14 @@ import org.openrewrite.java.search.DeclaresMethod; import org.openrewrite.java.tree.Flag; import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaSourceFile; import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.Statement; import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.staticanalysis.java.JavaFileChecker; import java.time.Duration; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Collections.singleton; @@ -78,31 +79,39 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex /** * Java method names are case sensitive, so a type may legally declare both the near-miss method and the - * correctly named one, each with its own behavior. Renaming would then emit a duplicate declaration, and - * renaming onto an inherited `final` method would emit an illegal override, so leave both cases alone - * rather than deleting or merging either implementation. + * correctly named one, each with its own behavior; renaming would then emit a duplicate declaration. + * The rename also applies to overrides, so any subtype declared in the same source file has to be free + * of the target method as well. Renaming onto an inherited `final` method, or renaming a non-public or + * static method onto one of `Object`'s public instance methods, would emit an illegal override. *

- * Explicit declarations are read from the enclosing type's body in the LST rather than from the type - * model, because for records and annotation-processed classes (such as Lombok's `@Data`) the type model - * also contains generated `equals`/`hashCode`/`toString` members that an explicit declaration would - * replace rather than collide with. + * Explicit declarations are read from the method declarations in the LST rather than from the type model, + * because for records and annotation-processed classes (such as Lombok's `@Data`) the type model also + * contains generated `equals`/`hashCode`/`toString` members that an explicit declaration would replace + * rather than collide with. */ private boolean canRenameTo(JavaType.Method methodType, String targetName, MethodMatcher signature) { - J.Block body = getCursor().firstEnclosing(J.Block.class); - if (body != null) { - for (Statement statement : body.getStatements()) { - if (statement instanceof J.MethodDeclaration) { - J.MethodDeclaration sibling = (J.MethodDeclaration) statement; - if (targetName.equals(sibling.getSimpleName()) && - sibling.getMethodType() != null && signature.matches(sibling.getMethodType())) { - return false; - } + Set flags = methodType.getFlags(); + if (!flags.contains(Flag.Public) || flags.contains(Flag.Static)) { + return false; + } + JavaType.FullyQualified declaringType = methodType.getDeclaringType(); + AtomicBoolean targetAlreadyDeclared = new AtomicBoolean(); + new JavaIsoVisitor() { + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration existing, AtomicBoolean declared) { + JavaType.Method existingType = existing.getMethodType(); + if (existingType != null && targetName.equals(existing.getSimpleName()) && + signature.matches(existingType) && + TypeUtils.isAssignableTo(declaringType, existingType.getDeclaringType())) { + declared.set(true); } + return super.visitMethodDeclaration(existing, declared); } - } - return !TypeUtils.findDeclaredMethod(methodType.getDeclaringType().getSupertype(), targetName, methodType.getParameterTypes()) - .filter(m -> m.getFlags().contains(Flag.Final)) - .isPresent(); + }.visit(getCursor().firstEnclosingOrThrow(JavaSourceFile.class), targetAlreadyDeclared); + return !targetAlreadyDeclared.get() && + !TypeUtils.findDeclaredMethod(declaringType.getSupertype(), targetName, methodType.getParameterTypes()) + .filter(m -> m.getFlags().contains(Flag.Final)) + .isPresent(); } private boolean equalsIgnoreCaseExclusive(String inputToCheck, String targetToCheck) { diff --git a/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java b/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java index 5800b1687..00a9af565 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToStringTest.java @@ -122,7 +122,7 @@ void doNotRenameWhenTargetIsAlreadyDeclared() { java( """ class Test { - int hashcode() { + public int hashcode() { return 1; } @@ -130,7 +130,7 @@ public int hashCode() { return 2; } - boolean equal(Object value) { + public boolean equal(Object value) { return false; } @@ -138,7 +138,7 @@ public boolean equals(Object value) { return true; } - String tostring() { + public String tostring() { return "near"; } @@ -184,7 +184,7 @@ void doNotRenameWhenEnumAlreadyDeclaresTarget() { enum Test { A; - String tostring() { + public String tostring() { return "near"; } @@ -206,7 +206,7 @@ void doNotRenameWhenRecordAlreadyDeclaresTarget() { java( """ record Test(int value) { - int hashcode() { + public int hashcode() { return 1; } @@ -258,9 +258,77 @@ public final int hashCode() { } class Test extends Base { - int hashcode() { + public int hashcode() { + return 2; + } + } + """ + ) + ); + } + + @Test + void doNotRenameWhenSubclassAlreadyDeclaresTarget() { + rewriteRun( + //language=java + java( + """ + class Base { + public int hashcode() { + return 1; + } + } + + class Sub extends Base { + @Override + public int hashcode() { return 2; } + + @Override + public int hashCode() { + return 3; + } + } + """ + ) + ); + } + + @Test + void doNotRenameNonPublicMethod() { + rewriteRun( + //language=java + java( + """ + class Test { + int hashcode() { + return 1; + } + + private String tostring() { + return ""; + } + + protected boolean equal(Object value) { + return false; + } + } + """ + ) + ); + } + + @Test + void doNotRenameStaticMethod() { + rewriteRun( + //language=java + java( + """ + class Test { + public static int hashcode() { + return 1; + } } """ ) From d287aa38c00827b5494f411d57d3dbbda932bc6f Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:24:24 +0200 Subject: [PATCH 3/3] Trim commentary --- ...RenameMethodsNamedHashcodeEqualOrToString.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java index d4fc34fdc..29e786867 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java @@ -78,16 +78,13 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex } /** - * Java method names are case sensitive, so a type may legally declare both the near-miss method and the - * correctly named one, each with its own behavior; renaming would then emit a duplicate declaration. - * The rename also applies to overrides, so any subtype declared in the same source file has to be free - * of the target method as well. Renaming onto an inherited `final` method, or renaming a non-public or - * static method onto one of `Object`'s public instance methods, would emit an illegal override. + * Method names are case sensitive, so a type may legally declare both the near-miss and the correctly + * named method; renaming would emit a duplicate. The rename reaches overrides too, so subtypes in the + * same file must be free of the target as well, and renaming onto an inherited `final` method or onto + * one of `Object`'s public instance methods would emit an illegal override. *

- * Explicit declarations are read from the method declarations in the LST rather than from the type model, - * because for records and annotation-processed classes (such as Lombok's `@Data`) the type model also - * contains generated `equals`/`hashCode`/`toString` members that an explicit declaration would replace - * rather than collide with. + * Declarations come from the LST rather than the type model, which for records and Lombok `@Data` + * classes also carries generated members an explicit declaration would replace rather than collide with. */ private boolean canRenameTo(JavaType.Method methodType, String targetName, MethodMatcher signature) { Set flags = methodType.getFlags();