Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider - forcing braces on loops and if statements. #930

Open
danfma opened this issue Aug 11, 2023 · 4 comments
Open

Consider - forcing braces on loops and if statements. #930

danfma opened this issue Aug 11, 2023 · 4 comments

Comments

@danfma
Copy link

danfma commented Aug 11, 2023

Hello,

It would be nice if we could enforce the curly brackets for conditions and loops. Probably, a strategy for that would be better, something like:

  • always => always use;
  • when_multiline => only when there are more than two statements;

What do you think?

@belav
Copy link
Owner

belav commented Aug 11, 2023

CSharpier is an opinionated formatter and adding an option goes against that.

I would consider always forcing braces, but I don't know how split developers are in the always braces vs no braces. There are also other tools to force braces, they just don't run on save.

@belav belav changed the title Define strategies for curly brackets on conditions and loops Consider - forcing braces on loops and if statements. Aug 11, 2023
@jods4
Copy link

jods4 commented Aug 16, 2023

I feel like this is unlikely to get strong consensus in community.

Personally, I like no-brace style for short single-line statements like if (!alive) return;.
I know other people who swear by always-use-braces.

I think it's probably simpler for CSharpier to stick to its "no-syntax change, whitespace formatting-only" general policy on this one.
Braces can be enforced by linters (even C# by itself can be configured through .editorconfig to error on if without braces).

(BTW in the same category: dropping brackets around single-argument, type-less lambdas? Prettier has an option to change (x) => true into x => true. I like this but I feel my comment applies just the same for this one.)

@danfma
Copy link
Author

danfma commented Sep 18, 2024

Yeah, I understand csharpier is opinionated and that adding more options will make the code harder to maintain, so that was just an idea.

@jake-carpenter
Copy link

jake-carpenter commented Oct 16, 2024

For what it's worth, I have strong opinions on the subject and think that the opinionated nature of CSharpier could implement a set of rules that work well.

Always forcing curly braces is the wrong opinion. Especially in C# and our obnoxious Allman standard of braces. However, if within the definition of "opinion" CSharpier will concede to one small stipulation under the following conditions, then I think everyone can be happy:

('block header' is used as a blanket definition for if (...) for[each], etc)

  1. The statement after the block header can only be one line long. If the return value wraps - braces. If the throw new Exception message wraps - braces.
  2. There can only be a single statement under the block header. If you execute a log statement then return; - braces.
  3. Only the following keywords can be allowed without wrapping braces:
    1. return
    2. yield return
    3. throw
    4. continue
    5. break
  4. If these conditions are met, but the code in question is already wrapped with braces, leave it alone. This is where CSharpier needs to bend.

Number 3 sets forth a limited set of rules for a reasonable standard of when no braces should be allowed. Guard statements are very common and it's really an indicator that the language should provide us with a better construct than if to accomplish our goal.

I believe these rules in number 3 satisfy the oft quoted argument of "what if you have to add a line of code inside the if?" by limiting the scope of when it's acceptable, and number 4 is a guard because I certainly wouldn't want CSharpier deleting my braces just because I'm following TDD and haven't added another line yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants