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
keyword-spacing else ghost default #12369
keyword-spacing else ghost default #12369
Comments
Thanks for this issue. It seems that you've set |
I set |
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. |
I will take a look |
I think thisPR (#12411) can fix it. Can I get a review? @platinumazure |
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;
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. |
Tested the original example in: eslint@5.13 - no 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 The problem might be that this change can now produce more warnings in some cases. |
Tell us about your environment
What parser (default, Babel-ESLint, etc.) are you using?
default
Please show your full configuration:
Configuration
What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.
What did you expect to happen?
before: false
should set it so every keyword requires no spacing before it, as no overrides set it totrue
.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 settingbefore: false
in theelse
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.
The text was updated successfully, but these errors were encountered: