[java:S5993] Lower severity

Qube: Community 9.9

The Java rule S5993 has a default severity of Major. I don’t think, this is correct. It should at most be Minor.

While there is no point in making the constructors public there is also no risk. It is more or less nice to have to make them protected.

Yes, it is a good thing to list this smell. But not as Major.

Qube: Community 9.9

The Java rule S2864 has a default severity of Major. The rule is about coding style and maybe a bit of performance. But nothing is really wrong iterating over keys and querying the value via get(key).

Severity should be lowered to Minor.

Qube: Community 9.9

The Java rule S1604 has a default severity of Major. The rule is purely about coding style and using new features. It is great, that this rule reminds me about places, where I did not yet migrate to lambdas. Though it is nothing wrong using old style closure.

Default severity should be no more than Minor.

Hi,

Thanks for these reports. Normally your one-thread-per-rule format is perfect, and absolutely what we want. However, I’ve combined these three threads because I have the same answer to all three of them:

We’ve deprecated severity, and the plan is that it will soon disappear from the product. So we aren’t likely to make this change.

You can read more here:

 
Ann

Hi Ann,

that’s fine for me. This sub categorization was a bit random anyways. But maybe it is worth for the current smells to get raised to bugs when severity is gone.

I know, it sounds odd. But it may be worth a thought.

Cheers
Marvin

1 Like

Hi Marvin,

Thanks for bringing this up. I think the points you bring up have merit and the Java squad could consider them, I’ll ping them.

How are you using severity in your workflow?
Do you have quality gates with conditions on severity?

Just to clarify, we’ve changed how severity works, it is not going away.

Of course I meant “…worth for some of the current smells…”, not “…worth for the current smells…”.

Yes, we’ve defined a quality gate. From our point of view some of the smells can be treated like what they are named and don’t necessarily have to be dealt with, if time is crucial. But others, which are more or less those with a severity of Major and above, really pay into the code cleannes when fixed. That’s why I try to notify about a few rules, which’s severities don’t fit into this net.

In the end we think, for existing code bases a different categorization would fit a lot better into what companies can actually effort to upgrade them and the current tags “Bug” and “Code Smell” don’t help a lot. Yes, it is worth a lot to know from that tag, if the issue does notify about something, that can break your code flow. But eventually we need something, that somehow forces us to actually fix the issue (because of a failed gate and bad vibes in reports) or on the other hand to just notify about something, that is ugly or unmodern or bad style/habit, but doesn’t need to be upgraded. The latter would not appear with more than a side note in any report.

As a result the current three tags are not that bad. And the severity is not a bad idea either. But I would change it as follows.

Tags:

  • Security Hotspot (as it is now)
  • Bug (will (potentially) break your code flow)
  • Drop the tag “Code Smell”. Findings would simply have none of the two above.

Severity (or maybe: Duty):

  • “Must be fixed” (would fail the gate and prominently be shown in the problems view of your IDE)
  • “Could use an upgrade” (Code, that can be written nicer maybe with the use of modern syntax. Would not be listed in the IDE other than “info”)
  • “Open Point” (Things like “remove deprecated code” etc.)

The concrete setup is a bit subjective. But the sonar defaults don’t used to be that bad.

These “new” tags would not necessarily lead to any gate to fail or to show something in the errors or warnings in the IDE. The “new” severity would control that.

I would also drop the estimated time to fix the sum of the issues. It is plain wrong and heavily depends on the developers skill and experience with fixing those findings. Normally it is in order of magnitudes too high and just scares develpers when there are many days or even weeks of potential work, which melt down to a lot less once the nobrainers are dealt with.