Control structures should use curly braces - except for single-line statements

Clean Code New rules / language support
1
  • More a variation of the existing rule (S121) than a new one
  • Allow single-line structures without curly braces.
  • snippet of Noncompliant Code
if (condition)
   DoSomething();
  • snippet of Compilant Code (fixing the above noncompliant code)
if (condition) DoSomething();

preferred.
At the moment this would be required:

if (condition) { DoSomething(); }

or

if (condition)
{
   DoSomething();
}
2

Hi,

It’s not obvious to me what language this is (other than that it’s C-ish)…?

 
Thx,
Ann

3

C# in my specific case but really any C-ish language that allows you to have single line statements in this style

1 Like
5

Hi,

Thanks. I get that the idea is somewhat generic, but new rules ideas are evaluated by language-specific teams (and propagated to other languages as appropriate) & we don’t have a “C-ish” team. :laughing:

I’ve added the csharp tag and the right folks should be along.

 
:smiley:
Ann

6

Without curly braces control structures deal with single-line statements. So, if that is your preferred way, why not just disable the rule?!

7

@ganncamp Thanks for tagging the issue - I am still a n00b here :wink:

@Corniel I want these to be valid:

if (condition) DoSomething();
if (condition)
{
    DoSomething();
}
if (condition) 
{
    DoSomething();
    DoSomethingElse();
}

What I do not want to allow is:

if (condition)
    DoSomething();

I thought for this last case I need curly braces control?

8

No, the last one is - from the compiler’s perspective - equal to if (condition) DoSomething();

Sonar has a rule by the way that allows:

if (condition) DoSomething(); // Compliant

if (condition)
    DoSomething(); // Compliant

if (condition)
DoSomething(); // Noncompliant

E.a.w, if the indenting gives the wrong impression, it will raise a warning. That is Rule S2681, and there is a PR pending to extend the coverage.

9

@Corniel so basically I should turn off csharpsquid:S121 and I will get closer to what I want.
But

if (condition)
    DoSomething();

will still be allowed, which I don’t really want…because of exactly the problem your PR is trying to address - I don’t want people to use this style because they could accidentally execute unconditional code. I guess with your PR in then it would be okay to have this style since the accidental indentation case will be caught

10

That might be an option, or you write your own. The thing is, off course, that rules that are more mainly about style (like these ones) are always up for debate. You can’t make everybody happy.

I personally only allow if statements without curly braces once on a single line when reviewing code, but I would also not advise Sonar to change the behavior of any of these rules.

12

Hello @tbutler!

I don’t have more things to add here, I would just like to summarize.

S121 does not allow single-line structures without curly braces.
S2681 however, does allow it.

You can choose the style that you would like to enforce by enabling one and disabling the other.

Best Regards,
Mary

14

Ok thanks @Mary_Georgiou
Do you have a feel for when you might be prepared to bring in the PR from @Corniel ?
I see unfortunately that it is conflicting again so I guess he needs to do another rebase/merge

thanks

Tony

16

Hello @tbutler,

I will try to look into it soon!

Thanks for your patience.

Cheers,
Mary

2 Likes
18

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