padded-blocks: Change default behavior so that string config affects blocks, classes, and switch statements

[platinumazure]

What rule do you want to change?

padded-blocks

Does this change cause the rule to produce more or fewer warnings?

More in certain circumstances.

How will the change be implemented? (New option, new default behavior, etc.)?

Change default behavior so that a configuration of "always" or "never" will affect classes and switch statements, as well as blocks.

Currently "always" and "never" only affect blocks for backward compatibility reasons. This proposal is to change that behavior (making the rule more intuitive and consistent) in a major release.

Please provide some example code that this change will affect:

/* eslint padded-blocks: ["error", "always"] */

switch (foo) {
    case 1:
        doSomething();
        break;
}

class Foo {
    bar() {
        doSomething();
    }
}

What does the rule currently do for this code?

Does not warn, since these are not BlockStatements.

What will the rule do after it's changed?

Warn-- the switch and class lack appropriate padding. Analogous cases can be created for the "never" string configuration as well.

[alberto]

Previous discussions:
#5845 particularly #5845 (comment)

#5092

[platinumazure]

My thoughts:

• 5845: breaking changes should not be introduced unless harm of not doing so is greater than harm of doing so
  • For me, I'm on the fence on this. We've just fixed the documentation explaining how "always" and "never" work, so we can always point users to the docs. However, the inconsistent behavior of those options was always done only as a back-compat thing, and I think we're far away enough from that history that new users will just find the behavior confusing and we should fix it (and longtime users of this rule have had plenty of time to look at the rule docs and switch over to a more explicit configuration if desired). I don't think this change is worth deprecating this rule and creating a new one, but I also understand the potential pain of making this change.
• 5092: Intentionally decided to make a non-breaking change on this pre-2.0.0
  • That was then... this is now. I'm happy to let the team come to consensus on whether this change is worthwhile or not. Personally, I'd like to unify the behavior of "always" and "never" so it works for all the supported "block-like types".

Another option: If we wanted to deprecate the string option altogether, we could provide a config transform (maybe?) which would turn "always"/"never" into the appropriate object config. That way the configuration is more explicit, at the cost of not allowing a simple config for a common case.

[treybrisbane]

As a new ESLint user who has recently been bitten by this (see #7868), I'm in favour of this change (either as a new rule, or as a future breaking change to padded-blocks). Even though the docs have now been updated regarding this, I still find the existing behaviour to be unintuitive.

[not-an-aardvark]

This has 3 👍s from team members, so I'm labelling it as accepted.

[platinumazure]

This being a breaking change, does TSC need to review? (Currently we have two 👍s from TSC members and no opposition, so hopefully it'd be pretty smooth sailing.)

[platinumazure]

@alberto Reassigning this to you since you have a PR up for this. Thanks for writing a PR!