Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PreferJavaUtilObjectsEquals does not check for existing equals method #434

Open
timo-abele opened this issue Mar 20, 2024 · 2 comments
Open
Labels
bug Something isn't working

Comments

@timo-abele
Copy link

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

                <plugin>
                    <groupId>org.openrewrite.maven</groupId>
                    <artifactId>rewrite-maven-plugin</artifactId>
                    <version>5.23.1</version>
                    <configuration>
                        <activeRecipes>
                            <recipe>org.openrewrite.java.migrate.guava.NoGuavaJava11</recipe>
                        </activeRecipes>
                        <failOnDryRunResults>true</failOnDryRunResults>
                    </configuration>
                    <dependencies>
                        <dependency>
                            <groupId>org.openrewrite.recipe</groupId>
                            <artifactId>rewrite-migrate-java</artifactId>
                            <version>2.10.0</version>
                        </dependency>
                    </dependencies>
                </plugin>

What is the smallest, simplest way to reproduce the problem?

I haven't compiled this, but I hope the point gets across: the unconditional static import of java.util.Objects.equals is shadowed by the existing equals method

import static com.google.common.base.Objects.equal;

class A {
    int foo;
    int ba;

    @Override
    boolean equals(A other) {
        //omitting the nullchecks
        return equal(foo, other.foo) && equal(ba, other.ba);
    }
}

What did you expect to see?

import java.util.Objects;

class A {
    int foo;
    int ba;

    @Override
    boolean equals(A other) {
        //omitting the nullchecks
        return Objects.equals(foo, other.foo) && Objects.equals(ba, other.ba);
    }
}

What did you see instead?

import static java.util.Objects.equals; //unused!

class A {
    int foo;
    int ba;

    @Override
    boolean equals(A other) {
        //omitting the nullchecks
        return equals(foo, other.foo) && equals(ba, other.ba);
    }
}

Are you interested in contributing a fix to OpenRewrite?

No, just letting you know.

@timtebeek
Copy link
Contributor

Hi! Thanks for the report; I'm looking at your samples and how we've implemented this recipe

- org.openrewrite.java.ChangeMethodName:
methodPattern: com.google.common.base.Objects equal(Object, Object)
newMethodName: equals
- org.openrewrite.java.ChangeMethodTargetToStatic:
methodPattern: com.google.common.base.Objects equals(Object, Object)
fullyQualifiedTargetTypeName: java.util.Objects

With the example above I see one equals(Object) method and an import of an equals(Object, Object) method, that I think would not clash here because of the differing number of arguments; Does your experience there differ?

@timtebeek timtebeek added the question Further information is requested label May 6, 2024
@timo-abele
Copy link
Author

timo-abele commented May 6, 2024

I found the original code again! The number of arguments does seem to be an issue for Java, the (IntelliJ) error message is "Expected 1 arguments but found 2", so I assume java cannot handle the same method name, defined in a class and statically imported from somewhere else with a different number of parameters.
Maven error is:

[ERROR] /C:/Users/.../A.java:[63,16] method equals in class com.myorg.A cannot be applied to given types;
  required: java.lang.Object
  found: com.myorg.Foo,com.myorg.Foo
  reason: actual and formal argument lists differ in length

I've observed this in the original code with a custom type (Foo) and Strings. I hope this helps, let me know if you can't reproduce this.

@timtebeek timtebeek removed the question Further information is requested label Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

2 participants