New rule to log a violation when the NOSONAR tag is used without any additional comment

Hi Freddy,

I am not sure if I understand your question correctly.

NOSONAR shall work as it does currently.
But I want to have the option to create a violation when there is a //NOSONAR without a comment.

Example:

I have a code line that create a violation, e.g. a "Non-transient non-serializable instance field in serializable class" violation on the following code line:
private Logger logger = Factory.getLogger();

Additionally I have configured a CommentPatternMatcher rule that triggers if //NOSONAR has no comment (with regular expression), as you can see in the mailing list I added in the description.

Now what I want is that I can write
private Logger logger = Factory.getLogger(); // NOSONAR
and the violation "Non-transient non-serializable instance field in serializable class" does no longer occur.
But then I want in this case that the violation triggered by the NOSONAR-CommentPatternMatcher rule pops up.
The NOSONAR tags shall still be counted (e.g. for the taglist plugin).

If a developer writes
private Logger logger = Factory.getLogger(); // NOSONAR because that and that
the violation "Non-transient non-serializable instance field in serializable class" does no longer occur. And neither shall the NOSONAR-CommentPatternMatcher rule pop up. The NOSONAR tags shall still be counted (e.g. for the taglist plugin).

In our company we have the following Sonar-Setting:
All coding-guidelines have the severity BLOCKER and we have an alert that is "ERROR when >0 BLOCKER". Additionally we have the BuildBreakerPlugin that lets our Maven-Build fail when Errors are present.
This means I want to "force" our developers to comment (to reason) when they use NOSONAR, it shall be prohibited to use //NOSONAR without a reason.

I hope it is more clear now.

Regards,
Florian

That was my understanding Florian. This only point on which I disagree is that you can't use the CommentPatternMatcher to raise a violation for the reason I explained on the mailing list. But we just have to extend the rule "Avoid use of //NOSONAR marker" to cover your use case. Are you aware of this rule ?

I am aware of this rule. We have it also activated because it is needed for the Taglist-plugin (as far as I understand).

The question I have then:
How would this extension look like? Do you already have an idea?
The question that is popping up in my mind is that we want to collect all //NOSONARs (with or without comment) for the taglist-plugin. For this the "Avoid use of //NOSONAR marker" rule has severity "Info".
If you'd extend this rule, how can //NOSONAR's with comment be "Info" and //NOSONAR's without comment "Blocker".

Maybe I only understand a little bit to less of Sonar

Regards,
Florian

PS. Basically for me it does not matter how the feature is implemented as far as it works

Florian, for me things are pretty simple :

• There is already a rule which logs a violation when the tag NOSONAR is used (this core rule "Avoid use of //NOSONAR marker" doesn't depend at all on the taglist plugin)
• You'd like to get a violation "ONLY" when there isn't any comment after the NOSONAR tag

So we just need to update/extend this rule with a parameter ('onlyWhenNoComment' for instance) in order to let the Sonar user customize the behavior of this rule. By default the value of this parameter would be false but by setting it to true you'll get exactly what you want.

Freddy,

Great to hear that.
Would be great if this feature would be available in one of the next Sonar-releases.
I'd really appreciate that.

Regards,
Florian

Additionally,

would it also be possible to have two NOSONAR-"Reports":

• All NOSONAR occurences, with and without comment
• Only the NOSONAR-occurences without any comment

So it would be possible to have the taglist plugin to show all NOSONAR occurences (for the chart) and further it would be possible to break the build if at least one NOSONAR has no comment.

Regards,
Florian

Hi guys, my two cents :

IMO "nosonar" tag is a bad thing in any case - with or without comment, and here is some of my thoughts about why :

• Line has contained one violation and developer decided that this is a false-positive and marked it with "nosonar" and with comment, but then due to changes in other places of code two violations was detected for this line - this fact will be completely hidden and comment will be wrong and misleading. Thus it might be preferable to use Sonar UI to flag violations as false-positive.
• "You Can't Manage What You Don't Measure", so we forced to introduce new metrics like number of "nosonar" tags ( and as you proposed - in future : number of "nosonar" tags without comment ), whereas seems more logical and simply to rely on number of false-positives and in this case it's also possible to know exact rule without any additional comment. Thus "nosonar" tags adds unnecessary complexity - both for developers to support features around it ( e.g. by adding new parameter to detect comment, by introducing new metrics, ... ) and for users to manage debts associated with this (amount of information grows - number of metrics, number of configuration properties, ...)
• Trailing comments are bad practice by themselves - see http://checkstyle.sourceforge.net/config_misc.html#TrailingComment

Hi Evgeny,

Your hint with the false-positives sounds interesting.
I am going to have a look at it. The important points for me are

• When I run Sonar in Eclipse (with the plugin) then violations marked with false-positives must not occur
• Violations marked with false-positiv shall not occur in any statistics exception the false-positive statistics
• Reoccuring builds shall recognize earlier marks of false-positive so that I am not forced to recreate all false-positives for each build.

I am coming back here.

Regards,
Florian

Last point is already the case - we have an algorithm in place to track violations over the time (with some precision), so we able to keep false-positives as false-positives.

Sonar Eclipse is another story and probably you right, however in this case it should be another request for improvement, which is completely separated from this one.

Hi Evgeny,

Thanks for your response.
I thought about this issue and I think for my case it would be sufficient to work with false-positives when the three points I mentioned in my last post are fulfilled.

1. False-Positive-Recognition in Eclipse Sonar Plugin, to be requested in a separate request.
2. Violations marked with false-positive shall not occur in any statistics
3. Reoccurring builds shall keep the false-positive

So as I see the content of this issue could be changed to point number 2.
I tried a little bit with false-positive and when I mark a violation as false-positive it is still recognized e.g. as Critical. Now when I have an alert that lets the build break when there are e.g. 10 Critical violations the build will break even if all those 10 violations would be marked with false-positive.
Do you agree? If yes, can this behavior be changed? This would be a requirement so that I can achieve my goal.

Regards,
Florian

I agree on first point.

But correct me, if I'm wrong about second point:

1. perform first analysis of project => critical violation
2. mark this violation as false-positive in UI => number of critical violations not changed, because was not recomputed
3. perform second analysis of project => number of critical violations recomputed.

So I don't see problem, which prevents you from using alerts and sonar-build-breker-plugin.

Ah sorry,
I forgot to rebuild. It surely works.

I created a new issue for the SONAR Eclipse Plugin, see here: http://jira.codehaus.org/browse/SONARIDE-297

Regards,
Florian