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

NullPointerTester should not require @Nullable on the param of an equals() override #1819

Closed
gissuebot opened this issue Oct 31, 2014 · 3 comments
Labels
Milestone

Comments

@gissuebot
Copy link

Original issue created by ogregoire on 2014-07-29 at 03:35 PM


When I call the following, the test case fails (see full test case in attachment).

  new ClassSanityTester().testNulls(MyObject.class);

I get the following error: No exception thrown for parameter at index 0 from MyObjectTest$MyObject.public boolean MyObjectTest$MyObject.equals(java.lang.Object)[null] for class MyObjectTest$MyObject

Clearly, this indicates that it expects a call to "equals(Object)" to throw a NullPointerException. That method should never throw an exception but rather return true or false. The test seems to skip this assertion and I'm surprised this hasn't been reported earlier.

Please make it so ClassSanityTester#testNulls(Class) doesn't fail when equals is behaving correctly (see the test case where testEquals(MyObject.class succeeds).

On a side note, the text returned should be adapted as it mentions three times MyObjectTest$MyObject. That's 2 too many, and it renders the whole error message nearly not understandable.

@gissuebot
Copy link
Author

Original comment posted by ogregoire on 2014-07-30 at 03:21 PM


Okay, I found one issue on my side: the lack of online javadoc available. I had to check in the code online to find out that it's playing nice only if @Nullable is present.

I checked the front-page of Guava and no, there is no link at all to the testlib javadoc. I played a bit with urls and couldn't find any url for the testlib javadoc.

@gissuebot gissuebot added type=defect Bug, not working as expected migrated package=testing labels Nov 1, 2014
@cgdecker cgdecker removed the migrated label Nov 1, 2014
@PhilippWendler
Copy link

While it is possible to add @Nullable to equals for own code, it may not always be possible to do this for generated code. For example, the AutoValue team decided to not do this (google/auto#73).

Thus I think that NullPointerTester should special-case equals and behave correctly whether the annotation is present or not. After all, the JavaDoc of equals specifies explicitly that the parameter can be null.

@kevinb9n
Copy link
Contributor

I agree that equals() is worth special-casing.

@kevinb9n kevinb9n changed the title ClassSanityTester.testNulls fails on equals NullPointerTester should not require @Nullable on the param of an equals() override Nov 10, 2015
@cgdecker cgdecker added this to the 20.0 milestone Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants