We have an occasional annoyance in which the following sequence occurs:
SQ thinks it found a null pointer (java:S2259)
We analyze the code and mark it FP
For the next few scans, SQ looks at the same code and reaches the OPPOSITE conclusion, so it thinks the bug is fixed and marks it CLOSED
At some scan in the future, SQ changes its mind and flags it again
If #4 happens before enough scans have occurred to cause the closed issue to be removed entirely, then SQ will notice the issue had previously been in the RESOLVED state, and it will put it back into that state (RESOLVED). BUT if too many scans have failed to flag that issue, the issue is removed entirely, which means the FP resolution is also lost. So now it thinks it’s an entirely new issue and we have to mark it FP again.
We wouldn’t want to have a bunch of old closed issues sitting around forever. But is there a way to change the number of negative scans before removing a closed issue, but just for a specific issue or set of issues?
The root of the issue sounds like SonarQube is reaching different conclusions based on the same code – is that the case, or has code actually changed that influences whether the issue is raised (at which point, I have to tell you to reproduce on a supported version of SonarQube).
If the code is changing… it makes sense that the issue is raised as new and needs to be reevaluated as to whether it is still a false positive.
However, to answer your question about
Housekeeping (specifically the Delete closed issues after) setting influences when closed issues are removed, but it’s not possible to isolate this to specific issues (at its most granular, it’s possible to set this at a project level),
From what I can tell, the code isn’t being touched, or even the whole file, in many cases. I haven’t looked into it foo much, but I assume there’s heuristics in the implementation of that rule (given that it’s trying to do the inherently impossible) and maybe something in the interprocedural analysis is on the threshold of being tickled or not tickled.
Anyway, I think I can kludge a workaround in a script … but thought I’d check first in case I missed a feature in the manual.
OK, my workaround didn’t work, because the web_api endpoint issues/do_transition only takes 10 possible transition values, and all of them fail, with messages like:
“Transition from state CLOSED does not exist: reopen”
You can’t reopen closed issues – as the system itself marked it as closed (when it noted the issue was no longer present). You can only reopen issues that a user marked as FP/WF.
But we DID mark them FP. A subsequent scan changed the resolution field to FIXED, and also cleared out the transitions field. Comments we added (as to why they’re FP) are still there, as is the custom tag we added for those issues. The history of the issue is visible on both the web portal and through issues/changelog. But we are unable to mark it FP through either the web portal or the web_api.
I’ve noticed that if the null pointer issue was marked FIXED by the system, and then rediscovered, it is placed back into the RESOLVED state with the same resolution (FP) we had given it before. Apparently it’s going into the changelog to get this information.
BTW, when this rule is checking for NPE, does it follow variables back to the method calls that produce their values?
I’m seeing FP in code that looks something like this (which I’ve simplified here):
1 Foo foobar = foo.bar();
2 if (foobar != null) {
3 // do something
4 }
5 // some lines, with no entry or exit points
6 x = foobar.field;
SQ flags the right side of line 6, putting a “2” on it with a “1” on line 2. It says that since line 2 implies that foobar could be null, the dereference in line 6 can cause NPE.
It turns out that foo.bar() can’t return null. The method calls a constructor, populates some fields in the newly-constructed object, and then returns that object. Since constructors can’t return null, foo.bar() is guaranteed to return a non-null object. Of course, line 2 is therefore superfluous.
I’m just wondering if SQ is SUPPOSED to look down into foo.bar(), and isn’t for some reason, or if its analysis only goes as far as saying line 2 implies foobar COULD be null.