Start using @javax.annotation.Nullable

[surli]

@tmortagne

@vmassol

With @CheckForNull you’d either need to mark it as false positive or add useless code.

Sure. Now, I’m pretty sure the cases where you should check the return value is much more common than cases like this so IMO we should recommend @CheckForNull for most use cases and explain when @Nullable is enough, instead of only using @Nullable for all cases.

+1

[vmassol]

In summary:

• We could use only @Nullable. It just means that SonarQube will not check that calling code verifies that the return value used is not null. However SonarQube still throws an issue if it sees that the return value can be null if there’s no @Nullable annotation for it.
• We could use both @CheckForNull and @Nullable, with a recommendation to use @CheckForNull only when the calling code must always check for null. It would provide an additional check in SonarQube. And for the other cases, use @Nullable. My fear is that this makes it more complex and as a coder, I’m not 100% sure what it means to ask calling code to always check for null (since there’s no method that always return null, there are always conditions (IFs) that would warrant not checking for null if you know you’re not in the IF that would lead to returning null.

So I have a slight doubt in our ability to use @CheckForNull properly and in an objective way (ie not subjective, ie different depending on the dev).

[surli]

@vmassol

We could use both @CheckForNull and @Nullable, with a recommendation to use @CheckForNull only when the calling code must always check for null. It would provide an additional check in SonarQube. And for the other cases, use @Nullable. My fear is that this makes it more complex and as a coder, I’m not 100% sure what it means to ask calling code to always check for null (since there’s no method that always return null, there are always conditions (IFs) that would warrant not checking for null if you know you’re not in the IF that would lead to returning null.

I’d go for recommending to use always @CheckForNull by default, as it is the safest: most of the time we want to enforce checking for null value if we’re thinking about specifying that the method might be null. We could document that in the rare cases where we don’t need such check we could use @Nullable but frankly I’m not sure I see the point of using an annotation for such case: the javadoc could be enough then.

[tmortagne]

@surli

I’m not sure I see the point of using an annotation for such case: the javadoc could be enough then.

Just to let Sonar know that, yes, we know that method can return null, but it’s OK to not check for null in that specific case.