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

Suggest to use Double.compare or Float.compare when ObjectEqualsForPrimitives is triggered on floating point primitives #4392

Closed
findepi opened this issue May 10, 2024 · 0 comments

Comments

@findepi
Copy link
Contributor

findepi commented May 10, 2024

for a class like

public class DoubleLiteral
{
    private final double value;

    public DoubleLiteral(double value) { this.value = value; }

    @Override
    public boolean equals(Object o)
    {
        if (this == o) {
            return true;
        }
        if (o == null || getClass() != o.getClass()) {
            return false;
        }
        DoubleLiteral that = (DoubleLiteral) o;
        return Objects.equals(value, that.value);
    }

    @Override
    public int hashCode()
    {
        return Objects.hash(value);
    }
}

ObjectEqualsForPrimitives' current suggestion is:

DoubleLiteral.java:[37,30] [ObjectEqualsForPrimitives] Avoid unnecessary boxing by using plain == for primitive types.
[ERROR]     (see https://errorprone.info/bugpattern/ObjectEqualsForPrimitives)
[ERROR]   Did you mean 'return (value == that.value);'?

That changes semantics -- different result when both compared values are NaN. Object Equals should be value based and generally should compare NaNs as equal.
It's probably worth noting that equals generated for Java record classes also compares NaN values as equal (as if Objects.equals was used).

Therefore ObjectEqualsForPrimitives should not suggest ==, but it should suggest Double.compare(...) == 0 instead.

copybara-service bot pushed a commit that referenced this issue May 16, 2024
`Objects.equal(a_double, another_double)` and `a_double == another_double` have different semantics (different result when both compared values are NaN). This is especially relevant when implementing `equals` methods.

Fixes #4392

Fixes #4394

FUTURE_COPYBARA_INTEGRATE_REVIEW=#4394 from findepi:findepi/double-compare 603b9c6
PiperOrigin-RevId: 634390506
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant