prettier / eslint-config-prettier

Turns off all rules that are unnecessary or might conflict with Prettier.
MIT License
5.39k stars 255 forks source link

Don't disable `nonblock-statement-body-position` and `curly` #228

Closed alecmev closed 2 years ago

alecmev commented 2 years ago

Reference:

https://eslint.org/docs/latest/rules/nonblock-statement-body-position https://eslint.org/docs/latest/rules/curly

I understand that Prettier can turn something like this:

if (conditionA && conditionB && conditionC && conditionD && conditionE && conditionF && conditionG) return;

... into this:

if (
  conditionA &&
  conditionB &&
  conditionC &&
  conditionD &&
  conditionE &&
  conditionF &&
  conditionG
)
  return;

... triggering nonblock-statement-body-position (in the default beside mode), but that's actually desired! I want return; to be wrapped in curly braces here, that's what this rule is all about.

The below mode won't work, unfortunately, but does anybody actually use it? I've never seen any guides/configs that recommend such a style. Could it be left up to the user to disable it, perhaps? Otherwise this is a bit of a "baby & bathwater" situation.

curly doesn't interfere with Prettier at all, I believe, since it's 100% about the presence/absence of curly braces (something it officially isn't concerned with), and not line breaks, like nonblock-statement-body-position is.

Edit:

Sorry, I was wrong about my assessment of curly. As outlined in README, it does consider line breaks. I suppose then nonblock-statement-body-position just needs to change to 0 too, for consistency.

My argument about Prettier triggering these rules being desired still stands though. If I have curly set to multi-line then I would see this code:

if (cart.items && cart.items[0] && cart.items[0].quantity === 0) updateCart(cart);

... turn into:

if (cart.items && cart.items[0] && cart.items[0].quantity === 0)
  updateCart(cart);

... and then ESLint would tell me about the error, so I would go ahead and fix it myself:

if (cart.items && cart.items[0] && cart.items[0].quantity === 0) {
  updateCart(cart);
}

I don't think people would want anything else in this case, would they?

lydell commented 2 years ago

Hi! Am I understanding this correctly? Your code style is:

  1. Single statements inside if/for/while/do are allowed without braces, but must be placed beside not below and must fit on the line.
  2. Otherwise curly braces are required.

And one can enforce this using nonblock-statement-body-position and curly?

Then, yes, it seems like nonblock-statement-body-position could become a “special rule” too, possibly (probably?) with a validator that checks that you use "beside" (but then there’s the overrides thing that I haven’t fully thought of yet). And this should be compatible with Prettier.

It’s unfortunate that the error message is “Expected no linebreak before this statement”, while the real solution is to add curly braces. Takes a bit of learning for new contributors, it sounds like.

lydell commented 2 years ago

Closing because no response.

alecmev commented 2 years ago

Hi! Sorry for the delay, yes, that's the style I'm going for. Is this something you would be interested in adding?

lydell commented 2 years ago

I think this is the verdict:

So to me, it does not sound like it’s worth the trouble.

alecmev commented 2 years ago

This is certainly an edge casey issue, no argument there. I think just setting nonblock-statement-body-position to 0 should suffice. However, just in case, this style isn't exotic, it's what Airbnb's guide demands, for example.

lydell commented 2 years ago

I think only setting it to 0 would be a bit of a cop out, since it looks like it is validatable.

JounQin commented 2 years ago

I'd love to use curly with all option, but I just found it's not enabled at all util I viewed this issue. 😅😅

alecmev commented 2 years ago

@lydell Hmm, your call! What exactly are you concerned about with overrides?

lydell commented 2 years ago

I felt like it wasn’t clear to me how to handle it in a validator, and I wasn’t interested enough to figure it out.