Support for @Nullable annotation for method return values

SonarQube Cloud
1

Given the code

  @Nullable
  public List<String> someLibraryMethod_ReturnCheck()
  {
    return Collections.emptyList();
  }

  public void someLibraryMethod_ParamCheck(@Nullable List<String> param)
  {
    assert param.size() > 1;
  }

  public void foo()
  {
    List<String> list = someLibraryMethod_ReturnCheck();
    
    assert list.size() > 1;
  }

When using sonarqube or sonarlint the foo methods assert line is not marked as a Bug against the sonar rule java:S2259. Even thought the return value of the method is not checked, the parameters of someLibraryMethod_ParamCheck is checked in that assert line.

I would like to know the reason why method return values annotated with @Nullable is not supported by sonarqube or sonarlint.

2

Hello @gayanper,

First, what annotation are you using? Depending on if we are supporting it or not, you might or might not see the expected behavior.

Then, the javax.annotation.Nullable documentation gives hints to answer your question:

In general, this means developers will have to read the documentation to determine when a null value is acceptable and whether it is necessary to check for a null value.

Static analysis tools should generally treat the annotated items as though they had no annotation, […]

Use CheckForNull to indicate that the element value should always be checked for a null value.

We sometimes refer to them as “Weak Nullable” (javax.annotation.Nullable), as opposed to “Strong nullable” (javax.annotation.CheckForNull).

Now, if we look at your example with this in mind, and assume you are using javax.annotation.Nullable, we have two situations:

  1. @Nullable parameter
  public void someLibraryMethod_ParamCheck(@Nullable List<String> param) {
    assert param.size() > 1; // One issue!
  }

This code will raise an issue about a possible NPE. And it makes sense, if you added the annotation on the parameter, it means that you expect someone to pass null to it, strong or weak do not matter. If you dereference it, we are sure that a NPE is possible, otherwise, you might want to simply remove the annotation.

  1. @Nullable return
  @Nullable
  public List<String> someLibraryMethod_ReturnCheck() {...}

  public void foo() {
    List<String> list = someLibraryMethod_ReturnCheck();
    assert list.size() > 1; // No issue!
  }

This case is not the same. From the citation above, there are cases where it is acceptable to not check the return value for null, because you determined that it was not necessary. We can not be sure statically that a NPE is possible, therefore we do not report an issue. If you want to enforce a null check, you should use CheckForNull, and an issue will appear!

Hope it clarifies the situation.

3

Hi Yes now i get how it is working, But what i do not understand is, when we have a return value from a method, if the value can be null sometime, we annotate it as @Nullable, But why do we treat the @Nullable in this situation as the check is not needed ? and if @CheckNotNull the check is needed. The same check can be done for @Nullable as well right as we do with method parameters. or do i missing some thing ?

4

The short answer to your question: this is how it is designed…
And just as a side note, we (Sonarsource) are not designing these annotations, we are using them to detect issues.

The long answer is that we need something between “never returns null” (no annotation) and “always check if return null” (@CheckNotNull). It is useful, among other things, for other rules to works correctly.

For example, let’s take this scenario:
I want to write a method: public List<String> f(String s).
I want this method to return null if the argument is null.

public List<String> f(String s) {
  if (s == null) {
    return null;
  }
  // More logic
}

However, our dear analyzer will trigger RSPEC-1168. For some curious reason, I really want to return null here. So I annotate the method with @CheckNotNull. The issue disappears because the annotation shows that this is not a careless mistake, I make explicit that this method can return null and the user will be aware of that.
Now, the problem is when I’m using my method:

f("abc").length(); // Issue java:S2259, a NPE is possible! Really?

I will always have to check the return value for nullness despite the fact that I’m sure it can not be null because the only cases where it will return null are explicit.

At this point, I want something that would show that I and users are perfectly aware that my method can return null, to comply with S1168, and to not impose a cumbersome check for null when it is obviously not. The solution is nothing else than @Nullable.