@Nullable and SonarQube 'Conditionally executed blocks should be reachable' warning

Question

Package has following package-info.java:

@ParametersAreNonnullByDefault
package foo;
import javax.annotation.ParametersAreNonnullByDefault;

Class has the following method:

private static String toIsoString(@Nullable Instant dateTime) {
  return dateTime == null ? null : dateTime.toString();
}

On which SonarQube (Version 6.2, SonarJava 4.14.0.11784) gives the following warning (squid:S2583):

How can I convince SonarQube that the code is actually correct?

Interestingly, SonarLint plugin (3.0.0.2041) in Idea doesn't generate the same warning.

May be check from where this is being called? It might be detecting that you are always passing null into the method. — matt helliwell, Oct 19 '17 at 18:26

score 8 · Accepted Answer · answered Nov 07 '17 at 15:42

8

Apparently, this problem was caused by us using sonar-scanner without specifying sonar.java.libraries. Since it's multimodule maven project it wasn't clear to us how to specify sonar.java.libraries correctly.

Nicolas Peru, from SonarSource, suggested that we should use sonar maven plugin, instead of sonar-scanner, as the plugin has access to build classpath of the project. Indeed that solved this problem for us.

answered Nov 07 '17 at 15:42

maximdim

8,041
3
33
48

Adding link to ggroup thread relevant to this conclusion : https://groups.google.com/forum/#!topic/sonarqube/yHH8IQ6Hhcw – benzonico Nov 07 '17 at 16:42

score -2 · Answer 2 · answered Oct 20 '17 at 11:50

-2

The JavaDoc of @Nullable says (emphasis mine)

This annotation is useful mostly for overriding a Nonnull annotation. Static analysis tools should generally treat the annotated items as though they had no annotation, unless they are configured to minimize false negatives.

Correspondingly, SonarJava ignores the annotation.

If you'd like to challenge the course of action taken in SonarJava, please open a thread :-)

answered Oct 20 '17 at 11:50

G. Ann - SonarSource Team

22,346
4
40
76

I would agree that if just 'Nullable' annotation is present it could be ignored, as by default methods could take null arguments in Java. However, in this case the package-level annotation indicates that by default arguments cannot be null and 'Nullable' on particular argument overrides that default. Please look at javadoc for 'javax.annotation.ParametersAreNonnullByDefault', which says: "This annotation can be applied to a package, class or method to indicate that the method parameters in that element are nonnull by default unless there is...An explicit nullness annotation" – maximdim Oct 20 '17 at 13:03
@maximdim if you'd like to challenge the course of action taken in SonarJava, please [open a thread](https://groups.google.com/forum/#!forum/sonarqube) – G. Ann - SonarSource Team Oct 20 '17 at 13:04
Done. Thanks. https://groups.google.com/forum/#!topic/sonarqube/yHH8IQ6Hhcw – maximdim Oct 20 '17 at 13:37
2

You have quoted the Javadoc for FindBugs's version of `@Nullable`. FindBugs has a [crazy definition of `@Nullable` that is different from every other tool](https://checkerframework.org/manual/#findbugs-nullable). FindBugs has no right to specify what other "static analysis tools should" do, so one should disregard the bolded part of your quote. – mernst Nov 07 '17 at 17:45
Downvoters: I've stated what the rule does. If you'd like to challenge the course of action taken in SonarJava, please [open a thread](https://groups.google.com/forum/#!forum/sonarqube) in the SonarQube Google Group. – G. Ann - SonarSource Team Nov 14 '17 at 12:42

@Nullable and SonarQube 'Conditionally executed blocks should be reachable' warning

2 Answers2

Linked