Consider adding exception to S122 for one-line conditionals

Versions: SQ 8.3, scanner 4.3.0, css plugin 1.2.0

I’d like to invite some discussion of this older post which has since been closed. The gist is that a user posted that they like to have short one-line conditionals (if(!a) { b = c; }) on a single line to improve readability, and the response from the Sonar team was that they find one-line conditionals to be less readable, and there the conversation ended.

This is clearly a matter of personal opinion, on which reasonable people can disagree. I could find a dozen well-respected style guides to argue on both sides of the issue. I’m not here to re-litigate a religious argument, I just want to cut down the noise in my Sonar results. The simple fact is that when this rule showed up, my “medium sized” project flagged over a thousand violations. I’m sure almost all of them are for single-line conditionals (which are acceptable under our style guide) but maybe once or twice, I wrote foo.bar();baz(); instead of foo.bar().baz(); – this rule is a great opportunity to catch errors like that!

Now, I could review those thousand instances to see if one or two are a real error, but that would take ages. That’s why I’m writing this post, asking you to reconsider your stance. I’d like to see either a property on the rule that lets single-line conditionals pass, or I’d like to see the rule split into two, leaving the current one to check for actual multi-statement lines (treating conditions and body-blocks as separate “lines”), and one to check for single-line conditionals, so I could configure my project to ignore the latter. (I think there’s already a rule to flag conditionals that don’t use braces in JS/TS; certainly if there’s not, there should be.)

I think this change is worth pursuing because the rule as written today achieves two goals, which should be treated separately on their merits. On the one hand, it’s a readability issue, but those are relatively low severity and I don’t think Sonar is otherwise too opinionated about style-guide stuff. On the other hand, it can catch errors of the class I described above, which should be treated as fairly severe.

(/cc @p3k)

As further support for this suggestion, take a look at the docs for rule 1105:

if(condition) {doSomething();} //Compliant

Even the Sonar team seems to agree that one-line conditionals are “compliant”.