I’m getting a false positive saying “map() can return null”, demonstrated with the following code:
private static Long test() {
return Stream.of(1,2,3)
.findFirst()
.map(MyClass::test2)
.orElseThrow(() -> new IllegalStateException(""));
}
private static @Nullable Long test2(Integer value) {
if (value == 1) {
return null;
} else {
return value.longValue();
}
}
Removing the @Nullable annotation from the return value of test2() makes the issue go away, but the nullability of the return value of test2() should have no bearing on the perceived nullability of the return value of Optional.map().
EDIT: Further tinkering suggests the issue is specifically related to usage of org.jspecify.annotations.Nullable. Using jakarta.annotation.Nullable does not result in an issue being reported.
I observe the same issue in our code base using org.eclipse.jdt.annotation.Nullable .
Are these different annotations treated differently by SonarQube? We are open to move away from the eclipse annotation. But what alternative works best with SonarQube?
Thank you for reporting the issue and testing your setup with the Jakarta annotation for comparison. This is actually the right track to understanding how the analyzer comes to raise or not in seemingly identical situations.
However, it is not a good reason enough to raise it in this case since, as you mentioned, a null returned value, weak or strong, can be handled without any problem by Optional.map.
I created a ticket to document the issue.
Unfortunately, this ticket is unlikely to be tackled soon as we are no longer actively maintaining the symbolic execution for Java. It will however be used to benchmark our work on better dataflow analysis for a new version of S2259 or equivalent.
In the meantime, I suggest you mark this issue as a false positive and monitor the ticket to see the progress.
Are these different annotations treated differently by SonarQube? We are open to move away from the eclipse annotation. But what alternative works best with SonarQube?
It is difficult to give a generic answer to the question of which framework is best supported. We are interested in providing better support for jspecify annotations but as you can see in the answer above, the subtleties of the different frameworks can play nasty tricks on our existing rules.
My personal opinion (read not Sonar’s) is that jspecify annotations are worth investing in as popular frameworks such as Spring are themselves moving to use jspecify annotations.
Thanks for reporting this issue but without the details of which product and version you are experiencing this issue with, we will have a hard time helping you.
I suggest you create a new thread referencing this one using the template to share which product you are using.
This is especially important because we recently migrated our implementation of S2259 to a new engine in some products and it would help to know if you are experiencing the same issue with the new implementation still.
We’ve been facing the same problem and had hoped that waiting would bring a solution. Since that hasn’t been the case so far, we’d appreciate this issue to be picked up soon.
@Dorian_Burihabwa maybe now that Spring Framework 7 is out with a massive support of JSpecify it is the right time to finally get rid of these very distracting false positives?
In SonarQube Cloud and current versions of SonarQube Server, java:S2259 has been moved to javabugs:S2259 and its performance improved. No further work is anticipated on the version of java:S2259 that remains in SonarQube Community Build.
If you’re able to, please let us know if you still see this issue outside of Community Build!
