In a short living branch: new issues in old code


(Ute Liebermann) #1

We use Sonar (SonarQube, Enterprise Edition, Version 7.3) for the analysis of our java-projects and we use the new branching-feature.

Some days ago we did an analysis for one of our java-projects for Git-tag v_2017_08_22_MJ in a short lived branch (SLB).
The last analysis on the main-branch (and the sonar.branch.target for the SLB-analysis is set to the main-branch) was done about 2 years ago for version v_2015_09_19_PR. Since analysis of the main-branch and now we introduced 47 new rules into the quality-profile.

The result of the SLB-analysis is 11 issues, but all these 11 issues occur in old code, code that didn’t change between version v_2015_09_19_PR and v_2017_08_22_MJ. All these 11 issues affect the 47 new rules that didn’t yet exist 2 years ago. That’s why these issues were not found in the old analysis of the main-branch.

So on the one hand the issues are discovered for the first time (because of introducing new rules), so they are “new”, but on the other hand they are discovered in old code.

For the purpose of preventing new issues in old code Sonar introduced the means of issues-backdating (see for example https://blog.sonarsource.com/sonarqube-6-3-in-screenshots , “Backdating issues raised by new rules on old code…”).

In my example the backdating works in the SLB, the 11 issues are backdated (“5 years ago” and “3 years ago”), but nevertheless they are treated as “new” so that the QualityGate is failed/Status=ERROR.

Does Sonar work here as designed or is it perhaps a bug? If it works as designed, couldn’t the behavior be changed? I’d like not to get issues for code that I didn’t change, no matter if I do the analysis per SLB or LLB.


(G Ann Campbell) #2

Hi,

It’s actually working as designed here although your situation is a bit of a corner case, so let’s look at some examples of the more usual case:

  • the SLB deprecates a method that’s widely called. Issues about using deprecated code are now raised on all the untouched lines that include calls to your method

  • in multiple places you unconditionally de-reference the return value of a method. Your colleague’s SLB adds a path to the method that returns null, leaving you with null-pointer dereferences

In both of these cases, you probably do want to see the new issues in untouched code, so we’re not likely to change this behavior.

And you do have a way to flip this SLB’s status to green: move the issues out of Open status by either confirming or dismissing (False Positive / Won’t Fix) them.

At the same time, its incumbent upon me to point out that you have an apparently active project that’s not being analyzed regularly, which is not great practice. At the very least, you should analyze after each QP change and SQ or SonarJava update. </lecture>

 
Ann


(Ute Liebermann) #3

Hi,

Thank you for the answer, but I’m not sure if you really got the core of my question.

Yes, there are cases when one should get issues in old, untouched code. But in my case I don’t want to get the issues because they only arise because of the introduction of new rules.

And with version 6.3 of SonarQube you explicitly introduced the possibility to ignore issues in old code by the means of backdating issues for the special case that new rules have been introduced.
In your examples (deprecated methods and null-pointer dereferences) I cannot recognize that you talk of newly introduced rules.

Regarding “backdating issues” see for example https://blog.sonarsource.com/sonarqube-6-3-in-screenshots :

The SonarSource team is proud to announce the release of SonarQube 6.3, which brings both interface and analysis improvements:….Backdating issues raised by new rules on old code…If you’re living by the Leak Period you know the pain of adding new rules to your quality profile: suddenly code you haven’t touched in months or even years has “new” issues - valid issues you need to silence somehow, either by marking them Won’t Fix, or by editing code you previously had no plan to touch. Because we dogfood new rules at SonarSource we felt this pain acutely.
Well, help is here. Starting with 6.3, SonarQube backdates issues raised by newly activated rules on old code to the line’s last commit date. No longer will you be forced to excavate old code to clean up a specious leak. Instead, you can activate new rules with abandon, knowing that the only issues that show up in the leak period will be the ones that actually belong there.”

My question especially concerns the case of issues that arise only because of newly introduced rules whereby the analysis is done in a short lived branch.

In my example I did the analysis of sourcecode-version v_2017_08_22_MJ in a SLB (with sourcecode-version v_2015_09_19_PR as last analysis on the main-branch) and only because of the newly introduced rules I got 11 issues (all of them are backdated), the status of the SLB is ERROR.

But if I do the analysis of the same sourcecode-version v_2017_08_22_MJ not in a SLB but on the main-branch (with previous_version = sourcecode-version v_2015_09_19_PR) I do not get these 11 issues as new issues, the Quality Gate is not failed, but passed!!

And that’s the point: I’d like the analysis in the SLB to behave in this case like the analysis in the main-branch: Also in the SLB the backdating of issues (the backdating itself works) should prevent that the issues are regarded as new and thereby that the status is error.


(G Ann Campbell) #4

Hi,

The recently-released 7.4 offers some improvements in this area, so if you’re using Git or SVN (the only two SCM plugins with the requisite updates, as far as I know) you should try it out.

That said, not analyzing your master branch for multiple years is just not a use case we’re going to put a lot of effort into supporting, and your case of adding rules and analyzing a branch of an unanalyzed project is not a scenario we tested. Since the issues from the new rules are on code not touched in the SLB they likely won’t show up in it anymore. We recognize that as a corner case we probably need to come back to, since my arguments above still stand.

 
Ann