eslint: should throw an error when disabling a non-existent rule
Tell us about your environment
- **ESLint Version:**master
- Node Version:
- npm Version:
What parser (default, Babel-ESLint, etc.) are you using? default
What did you do? Please include the actual source code causing the issue.
/* eslint foo: 0 */
// edit: or in a config file
{
"rules": { "foo": 0}
}
What did you expect to happen?
throw an error, since there is no rule named foo.
What actually happened? Please include the actual, raw output from ESLint.
no error thrown.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Comments: 33 (33 by maintainers)
Commits related to this issue
- Fix: throw an error when disable a non-existent rule (fixes #9505) — committed to aladdin-add/eslint by aladdin-add 6 years ago
- Fix: throw an error when disable a non-existent rule (fixes #9505) — committed to aladdin-add/eslint by aladdin-add 6 years ago
- Fix: throw an error when disable a non-existent rule (fixes #9505) — committed to aladdin-add/eslint by aladdin-add 6 years ago
- Fix: throw an error when disable a non-existent rule (fixes #9505) — committed to aladdin-add/eslint by aladdin-add 6 years ago
- Fix: throw an error when disable a non-existent rule (fixes #9505) — committed to aladdin-add/eslint by aladdin-add 6 years ago
- Breaking: throw an error when disable a non-existent rule (refs #9505) — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: throw an error if disables a non-existent rule in a configuration comment (fixes#9505) — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: throw an error if disables a non-existent rule in a configuration comment (fixes#9505) — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: throw an error if disables a non-existent rule in a configuration comment (fixes#9505) — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: more stricter rule config validating(fixes#9505) to fix issue #9505, it made a refactor and introduced a few changes to make validating more consistent: * will report a linting error when e... — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: stricter rule config validating(fixes#9505) to fix issue #9505, it made a refactor and introduced a few changes to make validating more consistent: * will report a linting error when enable... — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: stricter rule config validating (fixes #9505) to fix issue #9505, it made a refactor and introduced a few changes to make validating more consistent: * will report a linting error when enab... — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: stricter rule config validating (fixes #9505) to fix issue #9505, it made a refactor and introduced a few changes to make validating more consistent: * will report a linting error when enab... — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: stricter rule config validating (fixes #9505) to fix issue #9505, it made a refactor and introduced a few changes to make validating more consistent: * will report a linting error when enab... — committed to aladdin-add/eslint by aladdin-add 5 years ago
- Breaking: stricter rule config validating (fixes #9505) (#11742) * Breaking: stricter rule config validating (fixes #9505) to fix issue #9505, it made a refactor and introduced a few changes to ma... — committed to eslint/eslint by aladdin-add 5 years ago
TSC Summary: The main issue is that if a rule is disabled, either in a config file or in an inline comment, the disable would silently do nothing and the user would never know. We thought this would be a problem for end users, but Jordan Harband pointed out that this has a large negative effect on plugin maintainers that want to support multiple versions of ESLint. This issue was originally created and accepted as a bug and I don’t believe sufficient discussion has occurred that takes into account the negative side effects of the original design. I believe this should not be in 6.0.0 and that this needs more discussion, possibly even an RFC.
TSC Question: Should we remove this from 6.0.0 and ask for further design discussion? As an ancillary question, should we require an RFC for this change?
thanks @kaicataldo Ill hold off. Plenty of other stuff I can get done.
We discussed this in today’s TSC meeting, and the conclusion was that we should be safe throwing an error if someone disables a non-existent rule in a configuration comment in 6.0.0. We’ll leave configuration files (and plugins) alone for this version and see if there’s a solution that works for end users and plugin authors to be defined later.
alternatively, only throw on first-party config or in-line comments, if that’s feasible.
@ljharb After reading your latest, I’m rethinking my support for this issue.
At the very least, I think it’s safe to say that the potential harm for plugin and configuration authors if we make this a default setting is at least as great as the potential benefit to end users who might be making typos in their project configuration or inline disable comments.
I’m hoping to have a new proposal in a few days, but greatly appreciate any further discussion in the meantime.
@platinumazure why is that not ideal? If the user’s intention in disabling “foo” is “i don’t want any errors from foo” then they’ve achieved that, whether foo exists or not. What actual end user problem is this solving?
@platinumazure I’ve created a PR #11432 a while ago, but it seems has too many conflicts.
I will create a new PR, when #11546 get landed. 😄
@sstern6 Though you can feel free to make a PR, we’re most likely quite a ways out from our next major release. Since this is a breaking change, we wouldn’t be able to merge it until we start working on that release in earnest. My recommendation would be to hold off for now until that process starts.