Skip to content

Conversation

timtebeek
Copy link
Member

@timtebeek timtebeek commented Aug 29, 2025

As discovered on

Before we had

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Method method = (Method) o;
return Objects.equals(declaringType, method.declaringType) &&
name.equals(method.name) &&
Objects.equals(returnType, method.returnType) &&
Arrays.equals(parameterTypes, method.parameterTypes);
}

Which then led to calling equals on the declaringType first, which for FullyQualified.Class looks like:

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Class aClass = (Class) o;
return TypeUtils.fullyQualifiedNamesAreEqual(fullyQualifiedName, aClass.fullyQualifiedName) &&
(typeParameters == null && aClass.typeParameters == null || typeParameters != null && Arrays.equals(typeParameters, aClass.typeParameters));
}

Whereas the name is just a String that we can more cheaply compare and reject, making it unnecessary to do name comparisons first in
https://github.com/openrewrite/rewrite-static-analysis/blob/232339c63ac81a5e47275a093beb845154b6f745/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethods.java#L96-L98

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions could not be made:

  • rewrite-javascript/src/integTest/java/org/openrewrite/javascript/rpc/JavaScriptRewriteRpcTest.java
    • lines 36-36
    • lines 66-66

@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Aug 29, 2025
@timtebeek timtebeek merged commit d6a5c29 into main Aug 29, 2025
2 checks passed
@timtebeek timtebeek deleted the compare-name-before-declaringType branch August 29, 2025 10:26
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Aug 29, 2025
- org.openrewrite.java.recipes.migrate.RemoveTraitsUsageRecipes
- org.openrewrite.staticanalysis.NeedBraces
- org.openrewrite.staticanalysis.RemoveSystemOutPrintln
# - org.openrewrite.staticanalysis.RemoveSystemOutPrintln
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this intended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, didn't want to keep patching main when folks as print statements to test code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants