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

Errorprone BoxedPrimitiveEquality error on generated code by jsonschema2pojo #1625

Open
juherr opened this issue Aug 15, 2024 · 6 comments
Open

Comments

@juherr
Copy link
Contributor

juherr commented Aug 15, 2024

When using ErrorProne on my project using jsonschema2pojo, it complains about the following errors:

[BoxedPrimitiveEquality] Comparison using reference equality instead of value equality. Reference equality of boxed primitive types is usually not useful, as they are value objects, and it is bug-prone, as instances are cached for some values but not others.

((this.myInteger == rhs.myInteger)||((this.myInteger!= null)&&this.myInteger.equals(rhs.myInteger)))

According to ErrorProne, the generated code should be:

(Objects.equals(this.myInteger, rhs.myInteger))
@unkish
Copy link
Collaborator

unkish commented Aug 16, 2024

Hi

I'd consider it as a false-positive and suggested change as incorrect.

Given following oversimplified schema:

{
  "type": "object",
  "javaType":"Example",
  "properties": {
    "myInteger": {
      "type": "integer"
    }
  }
}

And following code

System.out.println(new Example().equals(new Example()));

Code accoring to ErrorProne would yield false whereas one would expect it to be true.

@juherr
Copy link
Contributor Author

juherr commented Aug 16, 2024

@unkish Example is not a boxed primitive. The issue is about Integer, Double, Float, ...

Object equality must not change.

@unkish
Copy link
Collaborator

unkish commented Aug 16, 2024

"type": "integer" can be replaced with "existingJavaType": "Integer" yielding same outcome whilst having same expectations

@juherr
Copy link
Contributor Author

juherr commented Aug 16, 2024

Yes and

System.out.println(new Integer(10).equals(new Integer(10)));

will return true as expected.

I still don't understand your point.

@unkish
Copy link
Collaborator

unkish commented Aug 16, 2024

Schema above will generate code containing equals method

    @Override
    public boolean equals(Object other) {
        if (other == this) {
            return true;
        }
        if ((other instanceof Example) == false) {
            return false;
        }
        Example rhs = ((Example) other);
        return ((this.myInteger == rhs.myInteger)||((this.myInteger!= null)&&this.myInteger.equals(rhs.myInteger)));
    }

that contains "erroneous code" as reported by ErrorProne.
My point is that leaving out this.myInteger == rhs.myInteger produces result that would differ from expected one.

@juherr
Copy link
Contributor Author

juherr commented Aug 16, 2024

@unkish Sorry for overlooking the double null case. Thank you for pointing it out.
By the way, I have made the necessary updates to the request, and it is now functioning as expected thanks to Objects.equals.

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