Update: "off" options for "space-before-blocks" (refs #10906)

[pineapplemachine]

What is the purpose of this pull request? (put an "X" next to item)

[X] Changes an existing rule (template)

What rule do you want to change?

This PR modifies the behavior of the space-before-blocks rule.

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

This PR does not affect the number of warnings for existing configurations of the space-before-blocks rule. It allows a new configuration option which ignores whitespace for functions, classes, and/or keywords, no longer warning regardless of whether whitespace was included in the specified cases.

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

The PR adds a new accepted value to existing options. Where previously only "always" and "never" were accepted, a third option "off" is now also accepted. "off" means that neither style of whitespace should be enforced for a certain category of blocks. Please refer to the diff for details.

Please provide some example code that this change will affect:

function MyFunction() { ... }
class MyClass { ... }
if( ... ) { ... }

What does the rule currently do for this code?

The space-before-blocks rule enforces that there is either "always" or "never" whitespace between the opening brace '{' and the previous non-whitespace character, with the option to have separate enforcement for functions, classes, and keywords.

What will the rule do after it's changed?

The rule is able to enforce "always" and/or "never" rules for one or more of functions, classes, and keywords, and not enforce either style for the remaining cases. This is not possible with the current behavior and options.

More info:

Please see this issue: #10906

The behavior of the "space-before-blocks" is affected. New functionality is added. All prior tests still pass without changes, which implies that the changes are fully backwards-compatible with prior eslint configurations.

Where "always" enforces space and "never" enforces no-space, "off" does not enforce any space preference. This can be useful, for example, when enforcing one whitespace setting for "functions" but not enforcing any setting for other kinds of blocks.

It may be useful to add a similar "off" option to other rules currently only accepting "always" and "never", too, but that is outside the scope of this PR. (This is the only such change that would have an impact on my own usage of eslint.)

TODO:

I added to the documentation, but not in as much detail as is probably warranted. Since I don't know that something like this exists elsewhere in the docs, I was unsure with how to proceed and describe the feature in more detail. I would really appreciate some input here.

[DONE] Two tests fail and I do not understand why. I don't think this is related to the implementation changes; I think that I have misunderstood something in regards to writing tests which use the "class" keyword. (Why do the tests on L457 and L464 pass but two of the tests that I added do not??)

  18549 passing (48s)
  2 failing

  1) space-before-blocks
       invalid
         class test { constructor(){} }:

      AssertionError [ERR_ASSERTION]: A fatal parsing error occurred: Parsing error: The keyword 'class' is reserved
      + expected - actual

      -false
      +true

      at testInvalidTemplate (lib/testers/rule-tester.js:9:13617)
      at Context.RuleTester.it (lib/testers/rule-tester.js:9:20143)

  2) space-before-blocks
       invalid
         class test { constructor() {} }:

      AssertionError [ERR_ASSERTION]: A fatal parsing error occurred: Parsing error: The keyword 'class' is reserved
      + expected - actual

      -false
      +true

      at testInvalidTemplate (lib/testers/rule-tester.js:9:13617)
      at Context.RuleTester.it (lib/testers/rule-tester.js:9:20143)

[jsf-clabot]

All committers have signed the CLA.

[platinumazure]

I've identified the test failure issues. Please make the suggested changes and hopefully your tests will pass.

[pineapplemachine]

Me and the commit-message bot are not getting along very well

It would be really nice and helpful if when I clicked "details" it could tell me specifically what was wrong with the title or commit message instead of just screaming a red "X" and "RTFM" at me

[nzakas]

Can you please also update this PR description using the template specified?

[pineapplemachine]

@nzakas I made the requested changes.

[platinumazure]

Thanks @pineapplemachine for updating the initial post.

I'll champion this.

FYI: We won't be able to merge this in until the proposal you laid out in your original message is accepted. As champion, it's my responsibility to obtain support from the team, although you are welcome to help keep discussion going as well.

[platinumazure]

LGTM, thanks! (Sorry for the delay in coming back to this.) Personally, I'm okay with the documentation changes as they are, but maybe other team members will have some suggestions.

[platinumazure]

FYI, the PR title had a trailing space, which was upsetting our commit-message bot. I've fixed it.

[platinumazure]

@eslint/eslint-team Please take a look at the proposal here and (hopefully) give it a 👍. Thanks!

[nzakas]

Thanks @pineapplemachine!