SonarQube 7.9.1 LTS
SonarQube SonarScanner v2.11
Noticed a strange behavior while testing few things.
The rule squid:S2259 Null pointers should not be dereferenced triggered on one line but not on another (same class, same code).
See screenshot below. I expected the rule to fail on line “System.out.println(lastName22.length());” as well, but that’s not the case.
Can someone pls explain what’s going on here ?
Hello @ankurja.
I agree that, at first glance, it can be surprising, but I think the rule is right!
If you think about it carefully, in this piece of code, will this line ever throw a NPE?
No, a NPE happened before, the second length() will never be called, this is in fact dead code. Reporting the fact that a NPE could be thrown here would be incorrect.
If you change the code to
if (cond) {
String s1 = null;
System.out.println(s1.length());
} else {
String s2 = null;
System.out.println(s2.length());
}
Both line are reported.
I hope it clarifies the situation.
Best,
Quentin
Hello @Quentin,
I see your point but I don’t think this is how a static code testing tool should work. I would expect this behavior from probably a dynamic testing tool which actually tries to execute the code and might fail on the first occurrence of an error condition. SonarQube, however, does not try to execute the code and thus should not have stopped at the first instance of NPE.
If I go by the logic of dead code, then any line after NPE would be dead code. And if so, then why is the rule “Replace this use of System.out or System.err by a logger” reporting a problem after NPE occurred in previous lines ? If a NPE would actually happen on previous lines then this System.out will never be printed.
Thanks,
Ankur
You are raising interesting points, but also highly debatable… I bet you will find plenty of heated discussion about the subject out here. I’m not sure starting over here is a wise idea…
I just want to say that this behavior comes from the fact that we are using symbolic execution to detect such issue, if it might looks like a limitation in this simple case, it can also be seen as a benefit when the code is more complex, to potentially find more interesting case or report less false positive.
should not have stopped at the first instance of NPE.
I agree about this point: new issues appearing only once you fixed another one is annoying, but I expect bugs subject to this problem (the one using symbolic execution) to be critical enough to be rare, it will hardly ever happen in real code, and if it does, you will probably still be happy to discover it anyway.