keyword-spacing else ghost default

[LJNeon]

Tell us about your environment

• ESLint Version: 6.5.1
• Node Version: 10.16.3
• npm Version: 6.9.0

What parser (default, Babel-ESLint, etc.) are you using?
default
Please show your full configuration:

Configuration

rules: {
  keyword-spacing: ["error", {
    before: false,
    after: true,
    overrides: {
      else: {after: false},
      if: {after: false}
    }
  }],
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

if(false) {
  console.info("will not log");
}else{
  console.info("will log");
}

eslint index.js

What did you expect to happen?
before: false should set it so every keyword requires no spacing before it, as no overrides set it to true.

What actually happened? Please include the actual, raw output from ESLint.

/path/to/my/file/index.js
  3:2  error  Expected space(s) before "else"  keyword-spacing

Although the before property appears to work on every other keyword added to the overrides I've tried (if, for example, works fine using the above config), else for some reason sets it to true. Explicitly setting before: false in the else override appears to be a temporary solution to this bug.

Are you willing to submit a pull request to fix this bug?
I am unsure of how it would be fixed, but I would be willing to submit a PR if I did.

[g-plane]

Thanks for this issue.

It seems that you've set after: false in your configuration, while you're reporting about the before option.

[LJNeon]

I set before: false in the configuration as well, yet it's being treated as before: true for the else keyword, while working as intended for all other keywords I've tried.

[platinumazure]

I can reproduce (demo).

If I had to guess, it seems that we might be checking overrides and falling back to defaults incorrectly. The expected behavior would be: Check overrides for the given keyword and location; if not present, check the specified config option for the location; if that's not specified, fall back to the default option for the location. Instead, it looks like we're letting the override object completely replace the location-only config option and then falling back to the rule's default.

[tickct]

I will take a look

[yeonjuan]

I think thisPR (#12411) can fix it. Can I get a review? @platinumazure
I realize that the schemes default value overrides user's configuration

[mdjermanovic]

It does seem that default values are causing this.

const thisBefore = ("before" in override) ? override.before : before;
const thisAfter = ("after" in override) ? override.after : after;

parseOptions checks if the key ("before"/"after") is present in the overrides object, but after the default values were introduced in #11288 both keys are always there.

It looks that the code was made to support merging overrides with the common settings. Removing default values could be the only way to restore this feature.

Though, this might be the only rule that was supposed to work like that? An alternative is to require complete configuration in the overrides object.

[mdjermanovic]

Tested the original example in:

eslint@5.13 - no error
eslint@5.14 - error

Seems that #11288 changed behavior here, but tests didn't catch that.

I think that proposed change in PR #12411 is correct, and it would be also good to add tests to show that overrides can override just "before" or just "after" or both. Maybe also improve the docs.

The problem might be that this change can now produce more warnings in some cases.