Nested statements without braces

[belav]

Currently statements that have optional braces will format onto a single line if they fit

if (someCondition) return 12;

If statements are nested, and still fit onto a single line they still don't break.

for (var i = 0; i < count; i++) if (comparer.Equals(this[i], item)) return i;
// and also
if (behaviors != null) foreach (var behavior in behaviors) AddBehavior(behavior);

Should statements without braces always break + indent? Should they break + indent if they contain another statement that could use braces but doesn't?

[loraderon]

I think that statements should always break + indent to be consistent.

[respel]

Don't see any issues per se with keeping everything on the same line. Emulating prettier is a pretty safe approach and prettier keeps them on the same line.

On a tangential note, I find allowing if/for statements without braces to be problematic in the first place and spitting in the face of consistency. Anyhoo, can't do much about that 🤷‍♀️

[belav]

I personally prefer braces to always be there. However if an auto formatter would always break + indent for me, I think I might consider not using braces.

If most c# developers that don't use braces on if statements always break + indent them, then I think it makes sense to follow that standard and not emulate prettier. I'm not clear how to really determine that.

From my not very accurate search of the roslyn source, if statements without braces are a mix between breaking and non-breaking. I could theoretically write something that analayzes some public repos to figure out the actual stats.

[respel]

Just did some quick searches and reached the same conclusion as Bela. Both are used often, with the one using newline being more popular.

However, this means that there isn't a clear consensus among the community and we should be free to choose an option solely on merit rather than being forced to maintain backwards compatibility with the community.

(Yes denotes at least one usage in the repo)

Spreadsheet Link

Search without newline

Search with newline

[belav]

Your regex search was definitely more accurate than mine. I'm not sure if you could get numbers using it, but I did tweak csharpier this morning to run some actual numbers. This is running against the code in https://github.com/belav/csharpier-repos which contains 11 repositories, some of the same ones you searched. There were 434,910 total IfStatements, of those 155,580 did not use braces. Of those 155,580 without braces, 110,900 of them did break. Which means ~71% of the time there are no braces, there is a line break + indent.

Although I just realized running the numbers like this is a little flawed. It doesn't take into account how long the lines are. For the IfStatements that break, are they something like

if (SomeLongCondition && SomeOtherCondition && AnotherCondition)
    CallSomeMethod(withParameters, thatMakesItLong);

Or are they

if (condition)
    CallMethod();

Your search results do show that some of them are the later, but we don't really know how many.

[respel]

I guess I could get some hard numbers locally. Do you have a commit in csharpier-repos where everything is unformatted?

[belav]

No formatting is ever committed to csharpier-repos. An action runs against csharpier-repos to validate that csharpier won't throw exceptions or produce code that fails the validation check but the action doesn't commit anything. I have separate forks of repos where I commit code so I can review how each new version or individual tickets are going to affect formatting. https://github.com/belav/aspnetcore for example is currently formatted with 0.9.5

[respel]

@belav Just searched the repo locally with Sublime Text and I arrived at an 80-20 ratio with the one having newlines taking the greater share.

[loraderon]

I often use an exit early strategy with statements without braces and I think that the readability get lost when no line breaks are present.

Tested on my code and found some cases that are affected, especially the last case gets a bit weird.

// before
if (evnt.Tenant?.IsSandboxed == true)
    return;
// after
if (evnt.Tenant?.IsSandboxed == true) return;

// before
if (client.TheatreId == 0)
    continue;
// after
if (client.TheatreId == 0) continue;

// before
if (maybeExisting != null)
    client.TheatreId = maybeExisting.Id;
else
    client.TheatreId = await theatreApi.Create(name);

// after
if (maybeExisting != null) client.TheatreId = maybeExisting.Id;
else
    client.TheatreId = await theatreApi.Create(name);

[belav]

Tested on my code and found some cases that are affected, especially the last case gets a bit weird. There is an open issue for that - #245

I'm inclined to say that we should break statements without braces. It seems like a majority of existing code does it that way. And it's also an easy fix for the nested statements problem.