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

Reconsider special handling of static constants in BoxedPrimitiveEquality #4796

Closed
PBoddington opened this issue Jan 28, 2025 · 2 comments
Closed

Comments

@PBoddington
Copy link

PBoddington commented Jan 28, 2025

For BoxedPrimitiveEquality, if either side of the == is a static final constant, it's not considered a match.

For example, for the code

public void test(Boolean bool) {
    final Boolean bool2 = Boolean.FALSE;
    boolean result = bool == bool2;
}

there is a match: error: [BoxedPrimitiveEquality] Comparison using reference equality instead of value equality.

On the other hand, for

public void test(Boolean bool) {
    boolean result = bool == Boolean.FALSE;
}

there is no match.

Note that this special handling for constants does not occur for ReferenceEquality, so both of the above pieces of code match ReferenceEquality.

Would it be possible to reconsider this special handling? Shouldn't comparing boxed primitives with == be an error in every situation?

I'm using errorprone version 2.36.0.

@graememorgan
Copy link
Member

Thanks, we agree. We took a look over existing violations of the stricter form of the check, and none of them look like a good idea to allow.

copybara-service bot pushed a commit that referenced this issue Feb 3, 2025
I found myself agreeing with #4796. All the findings look bad: unknown commit

PiperOrigin-RevId: 721764250
copybara-service bot pushed a commit that referenced this issue Feb 3, 2025
I found myself agreeing with #4796. All the findings look bad: unknown commit

PiperOrigin-RevId: 722600371
@PBoddington
Copy link
Author

Thanks, we agree. We took a look over existing violations of the stricter form of the check, and none of them look like a good idea to allow.

Thank you. Really appreciate this fast response.

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

No branches or pull requests

2 participants