Print deprecation warning instead of crash on unknown rule

[andrewdeandrade]

Recently the "space-unary-word-ops" rule was changed to "space-unary-ops". When the name of a rule is changed, eslint encounters an unknown key and crashes hard. Instead of crashing, printing an error like: "unknown rule: {{ error.ruleId }}. Please check to see if this rule exists or has been deprecated" to stderr would be more instructive.

Along with this message could be a link to a document that is nothing more than a rule changelog, that states what rules were added, removed or modified in each release, so that users can just check that document to see what's new, what they should remove from their config file and what additional options might now be available for rules they are currently using.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[nzakas]

100% agree. We need to come up with a strategy for deprecating rules as well as for dealing with unknown rules. At one point we assumed that rules would never change, but clearly we've disproved that theory.

[gcochard]

I would suggest aliasing the old rule name to the new rule, and if it's invoked with the old name try to mimic the behavior of the old rule. Somewhat similar in nature to how vim mimics vi if it is launched as vi, and it opens the file read only if launched as view. Print the deprecation warning and a suggestion to migrate to the new rule name, with the new rule's (hopefully expanded) options.

On the next major release (or the release called out in the deprecation warning) remove all the deprecated rules.

[nzakas]

Aliasing doesn't really work because old rules and new rules don't necessarily have a one-to-one mapping. Sometimes rules are merged, sometimes split apart.

I'm thinking the best strategy is:

1. If the old rule is on my default, turn it off. Also, turn on new rule by default with a setting that mimics the old rule.
2. Add a deprecation notice to the old rule. Not sure if this should be via context.report() or console.error().
3. Wait one minor release before removing the old rule.

[gcochard]

I'd be averse to removing rules (making a breaking change) in a minor release. Consider the way npm install --save handles the versions now. If I install a package at v1.2.3 it will save at ^1.2.3 in my package.json.

If a rule is deprecated at 1.2.4 and then removed at 1.3.0 then a subsequent npm install will install 1.3.0 and I am broken. If you're using semver like all npm packages do, you should stick to that contract and only remove rules at a major release.

[nzakas]

Chances are that we simply won't remove rules once we hit 1.0.0. Prior to that, I think two minor releases are more than enough time to fix such an issue in your project. Especially if we change the behavior of unknown rules so that it warns instead of crashes.

[gcochard]

With pre-1.x, I'd be completely OK with this behavior. npm fixes the minor release number when you install at 0.x. It will not install 0.11.x when the package.json specifies ^0.10.0.

[bedney]

Ran into this as well. Whichever way we jump on deprecating rules, can we fix the bug where eslint crashes, rather than produce a warning, on unknown rules? The problem is that different members of the development team may have an different versions of eslint that don't know about newer rules and so they experience crashes when they pull a new .eslint file referencing newer rules.

[andrewdeandrade]

I recently created a new rule and one of the really nice features of the test suite is the thoroughness that it guarantees

Sent from my iPhone

On May 2, 2015, at 1:21 PM, William J. Edney notifications@github.com wrote:

Ran into this as well. Whichever way we jump on deprecating rules, can we fix the bug where eslint crashes, rather than produce a warning, on unknown rules? The problem is that different members of the development team may have an different versions of eslint that don't know about newer rules and so they experience crashes when they pull a new .eslint file referencing newer rules.

—
Reply to this email directly or view it on GitHub.

[gyandeeps]

I was planning to work on it but before I actually started I wanted to know if it would be good to have this functionality inside config-validator. Since its already validating the rule.

[nzakas]

Config validator throws an error when it finds a problem, which would be the same as current behavior.

I think what we are talking about here is using a stub rule that just outputs "This rule had been removed in favor of x" as a normal rule error.

[gyandeeps]

Yes, what I meant was that it will not throw an error rather display the deprecation message.
My ask is that if we should add that logic inside config-validator functions?
Since its the gatekeeper for all the rules.

[nzakas]

Sorry, I wasn't clear. No, this should not be in config-validator, as validation always throws an error. @btmills do you agree?

[welwood08]

How would using stub rules solve the problem of older code encountering a newer unknown rule?

[nzakas]

@welwood08 any unknown rule would be assigned to the stub. The message can always be different depending on if the rule was deprecated, removed, or unknown.

[btmills]

No, this should not be in config-validator, as validation always throws an error.

@nzakas Yep, totally. Validation errors cause ESLint to bail before linting because any results would be bogus anyway if rules are configured anyway. I very much like the idea of a stub rule for this.

[IanVS]

Based on #1898 (comment), should this issue have a v1.0.0 milestone assignment?

[ilyavolodin]

Yes, thanks for noticing

[IanVS]

I'd be happy to take a crack at it. However, I'm not so familiar with the concept of a stub rule. Could someone give a basic overview of what needs to be done, so I know I'm going down the right path?

[nzakas]

Basically, you'd create something like this:

function createStubRule(warning) {
    return {
        Program: function(node) {
            context.report(node, warning);
        }
    };
}

// called as
var rule = createStubRule("Unknown rule");

And whenever a rule isn't found or is deprecated (probably need to add deprecated.json into /config that maps while rules are deprecated), create a stub rule in its place.

[IanVS]

@nzakas, thanks for the hints. If you have a chance, please review what I have so far, and then I will tackle deprecations (which will use much of the same infrastructure).