How to manage "new" issues, that are really "old" issues

[Ian Beaumont]

Using SonarQube 6.
We have the quality gate set as no new bugs/vulnerabilities. Often when a developer changes old source it will flag up an issue that was already in the code as being new.  Therefore the quality gate fails. For these issues, the only option seems to be:
1. They have to be fixed there and then (which isn't desirable, getting the developer to spend lots of time cleaning up old code).
2. They can be marked as false positive, or won't fix.  Neither are appropriate as they may be fixed in future.

So what I'm after is an option to say - the issue is really an old issue and should be added to the baseline project and not marked as a new issue.

Does anything like that exist, or how do other people deal with such situations?

Thanks
Ian

[G. Ann Campbell]

Hi Ian,

This doesn't directly respond to your question, but you may be interested in MMF-567, which is about keeping issues raised on old code as the result of activation of new rules out of the leak period.

Regarding your situation, it sounds like you have multiple old issues on a line, the developer works on that one line without fixing them, and they're all marked new? If I've interpreted it correctly, why wouldn't you want all the issues on the line fixed at once?

Ann

[Ian Beaumont]

Hi Ann,
Thanks for the reply.  Certainly I think MMF-567 would help, but unless the algorithm was really clever it wouldn't deal 100% with all issues.

The issue doesn't necessarily have to be on the same line - a developer may need to add some code, and in doing so he does a bit of refactoring.  This then leads to Sonar to incorrectly claim new issues have been added.

As for - "why wouldn't you want all the issues on the line fixed at once, " - some issues can run very deep and would need a lot of refactoring, and wanting to avoid full regression testing it can be better to leave them be.  Cleaning up old debt does tend to cause a spiral of new issues to appear, as you go from one to another, more and more "new" issues appear.  We are working on a code base that started over 15 years ago with Java 1.3 - so you can understand how many violations don't match the current Java standards.

I guess I'm just asking for a feature to allow us to control and use SonarQube in a way that would work for us, rather than being forced to always clean up issues when we touch old files.

Thanks
Ian

[G. Ann Campbell]

Hi Ian,

I'm still not entirely understanding your situation. Issues should only show up as "new" on LoC that have been modified...?

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

--
You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/KJzN-kG5zFE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/ac8920c9-9402-4c5e-a366-b7b7f4d9148a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Ian Beaumont]

There is also a 2nd scenario, where the feature would be useful...
So when the analysis has gone wrong (not because of SonarQube, but some outside factor such as bytecode not being available, see https://groups.google.com/d/msgid/sonarqube/c762e614-207c-4082-8fd0-10ce58f30096%40googlegroups.com.) then being able to put the project back into the correct state would be very useful.  This again would rely on being able to flag issues as being "old".

[Ian Beaumont]

So I think the feature I really want is to flag something as “Won’t fix against this baseline”.  When the project is then re-baselined, these issues would become “unresolved” issues. They are issues, but we haven’t got time to deal with them in this release, but in future we might deal with them. Currently we get so much “noise” as 100’s of new issues are flagged up, it becomes impossible to manage.

So scenarios where I would use this:
1.    We fix a critical issue in some old code, but that then introduces a lower level issue which we aren’t concerned about at the moment.  For example,

Statement s = new Statement(“Some SQL”);
s.execute();

this code flags up a critical issue that we aren’t closing resources.  So we want to deal with that.

So it gets changed to:

try (Statement s = new Statement(“Some SQL”)) {
s.execute();
}

So the resource is now being auto closed and the issue has gone.  But this change can then introduce a violation “Refactor this code to not nest more than 3 if/for/while/switch/try statements.”.  This we aren’t worried about and I don’t want it flagged up on dashboard.

2.    Someone refactors some old code, as they need to refactor old common functionality.  E.g.

Before
myMethod1() {
 
    doSomething();

    print(“a”);
print(“b”);
print(“c”);
}

After, the refactoring

myMethod1() {
    doSomething();

    doPrinting();
}

doPrinting() {
    print(“a”);
print(“b”);
print(“c”);
}

Any violations in the doPrinting() are flagged up as new, whereas they really are old.

3.    Source was refactored, and a whole file removed.  Later it was realised the file was still needed so was added back in.  I’m assuming (haven’t tried it) – all rules against this file would be flagged as new.

4.    A situation I had where byte code was not available during the analysis. Rules get closed down, and then get marked as new again.

5.    Another real world case… removing “serializable” from a base class to try to fix cases of “Make "XXX" transient or serializable”.  Then realising the base class has to be serializable, so adding it back in.  All the “Make "XXX" transient or serializable” rules re-appear as new violations.  They aren’t, they are just old violations re-appearing.

[G. Ann Campbell]

Hi Ian,

Thanks for taking the time to provide such a thoughtful response. I've added specific responses inline, below.

On Fri, Nov 18, 2016 at 9:41 AM, Ian Beaumont <ian.be...@gmail.com> wrote:

So I think the feature I really want is to flag something as “Won’t fix against this baseline”.  When the project is then re-baselined, these issues would become “unresolved” issues. They are issues, but we haven’t got time to deal with them in this release, but in future we might deal with them. Currently we get so much “noise” as 100’s of new issues are flagged up, it becomes impossible to manage.

So scenarios where I would use this:
1.    We fix a critical issue in some old code, but that then introduces a lower level issue which we aren’t concerned about at the moment.  For example,

Statement s = new Statement(“Some SQL”);
s.execute();

this code flags up a critical issue that we aren’t closing resources.  So we want to deal with that.

So it gets changed to:

try (Statement s = new Statement(“Some SQL”)) {
s.execute();
}

So the resource is now being auto closed and the issue has gone.  But this change can then introduce a violation “Refactor this code to not nest more than 3 if/for/while/switch/try statements.”.  This we aren’t worried about and I don’t want it flagged up on dashboard.

I'm still skeptical of this use case. IMO, the use of SonarLint or pull request analysis should help head off this sort of thing. Even if they didn't, IMO, when you're refactoring code you should address the existing issues and avoid adding new ones. After all, there's no better time to get that piece of code really clean than when you're already working on it. 

But thanks for sharing. :-)

 

2.    Someone refactors some old code, as they need to refactor old common functionality.  E.g.

Before
myMethod1() {
 
    doSomething();

    print(“a”);
print(“b”);
print(“c”);
}

After, the refactoring

myMethod1() {
    doSomething();

    doPrinting();
}

doPrinting() {
    print(“a”);
print(“b”);
print(“c”);
}

Any violations in the doPrinting() are flagged up as new, whereas they really are old.

Ideally, this should be addressed by MMF-567. (And if not, then it's a question of how your versioning system is dating/crediting code changes.

 

3.    Source was refactored, and a whole file removed.  Later it was realised the file was still needed so was added back in.  I’m assuming (haven’t tried it) – all rules against this file would be flagged as new.

You're probably right. I've added a comment on this to MMF-567

4.    A situation I had where byte code was not available during the analysis. Rules get closed down, and then get marked as new again.

MMF-567 should fix this.

 

5.    Another real world case… removing “serializable” from a base class to try to fix cases of “Make "XXX" transient or serializable”.  Then realising the base class has to be serializable, so adding it back in.  All the “Make "XXX" transient or serializable” rules re-appear as new violations.  They aren’t, they are just old violations re-appearing.

ditto.

[pga...@gmail.com]

Hi Ann,

We have similar issue (old issues intermittently appear in leak period without changing relevant lines of code) and working on Version 5.6.7. 
Just wondering in which version MMF-567 has been fixed?

thanks,

Pawel

[G. Ann Campbell]

Hi Pawel,

I don't remember exactly, but the current LTS, 6.7.2, certainly includes it. 

:-D

Ann

---

G. Ann Campbell | SonarSource

Product Manager

@GAnnCampbell

https://sonarsource.com

--

You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/KJzN-kG5zFE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/843b02ec-b9f0-4166-b68b-da29480f1a91%40googlegroups.com.