javascript:S122 vs javascript:S1438

IMHO Sonarlint should not complain about S122 in the following line of JavaScript:

if (!x) { x = 1; }
// SonarLint: This line has 2 statements. Maximum allowed is 1.

Is this correct? Isn’t there obviously only one statement between the curly braces, not two – as Sonarlint suggests?

Certainly, the question is about validity. Either one-line if-conditions are generally a no-go, generally accepted, or conditionally :wink: accepted.

From my point of view the latter could be realized by checking whether there is only one statement. But this is counteracted by the assertion there are two statements already, anyway.

Trying to work around this by simply removing the semicolon (which I think triggers the assumption of two statements) results in Sonarlint complaining about S1438.

if (!x) { x = 1 }
// SonarLint: Missing semicolon

So even if this is not regarded as a bug: how would I allow short one-liners like this with Sonarlint? (Besides adding //NOSONAR or disabling rule S122 completly.)

Hi,

There are 2 statements on one line: if statement and expression statement (x = 1, with or without semicolon it’s still a statement). We believe this code is not perfectly readable in general.

Indeed in your case of super simple condition and super simple body statement you might want to allow it in your code. On the side on analyzer I’m not convinced in changing rule to allow such cases. On the side on SonarLint it seems like the two options you named (nosonar and disabling the rule) are the only one. If you use connected mode you can mark issue as won't fix on SonarQube/SonarCloud side and it will disappear in the IDE.

1 Like

ah sure! thanks for your explanation and patience. i really did not consider the if-statement being a statement, obviously… <softslaponmyforehead/>

nevertheless, what is the point of view about legibility of such short one-liners? i consider them easier to grasp and read than having three lines with curly braces stuff.

imho it also adds to the length of a code “page”, i.e. getting an overview of a whole code section gets diluted, don’t you think?

maybe you already discussed this already elsewhere, but what about providing special rules for if-statements? e.g. allowing simple assignment and of course return?

1 Like

No, I don’t :slight_smile: On opposite I find it more readable when condition and statement on different lines, even if statement is just 3 characters long.

So as I mentioned you can go for adding //NOSONAR or disabling rule S122 completely if you find more readable having them on one line.

1 Like

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