Don't replace a useful rule with a useless one

Not sure how to tag this as I’m asking not to delete an old rule.

Rule java:S1697 checks for incorrect polarity in null checks (using && when you should use ||, or vice versa). This rule seems to have close to a 100% hit rate. But it’s deprecated; the rule description says use S2259 instead.

Rule S2259 is a generic null pointer check. At least for us, this rule has a very high percentage of false positives. This isn’t a knock on SQ; it’s an intractable problem in general, and SQ seems to be casting a very wide net. But the effect of this is that there is a lot of pushback from developers on examining issues that are most likely non-problems. Furthermore, S2259 doesn’t even catch some of the cases that S1697 identifies.

So the net effect of deprecating S1697 and steering users toward S2259 is that some legitimate errors get missed.

This observation could apply to some of the other “deprecated” rules as well. They look for specific patterns which have a high probability of marking code that is buggy, or at least smelly. The general null pointer checker is taking on a more difficult problem, and is therefore usually going to have a higher rate of false positives. It would be more useful to keep the pattern-based rules, and have some kind of filter in SQ so that, for instance, if both S2259 and S1697 are activated in the quality profile, then code which triggers S1697 won’t also trigger S2259.

Hi Mister Py,

Thank you for your messages and recommendation.

We created a ticket to make relive S1697, and we would investigate what improvements we could propose in terms of user experience.

Regards,
Richard

1 Like

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.