Repository: eslint/eslint (js)

Issue:
Warn on deprecated rules
As of v3.8.1, ESLint has a number of deprecated rules:
- [no-native-reassign](https://github.com/eslint/eslint/blob/v3.8.1/lib/rules/no-native-reassign.js)
- [no-negated-in-lhs](https://github.com/eslint/eslint/blob/v3.8.1/lib/rules/no-negated-in-lhs.js)
- [no-spaced-func](https://github.com/eslint/eslint/blob/v3.8.1/lib/rules/no-spaced-func.js)

These rules are marked as `deprecated` in `meta`.

However, there's no easy way for users to know that these rules are deprecated without explicitly following ESLint development.

There _is_ a capacity for warning about the use of rules that were previously removed and replaced by other rules, but this only applies to first-party rules, and only to ones that have actually been removed: https://github.com/eslint/eslint/blob/v3.8.1/lib/eslint.js#L818-L827

I believe it would be helpful to check `ruleCreator.meta.deprecated` at that location, then emit a warning if that rule is deprecated.

This would be helpful for third-party plugins as well; the impetus for this is my wanting to deprecate some rules in eslint-plugin-babel that now have their functionality covered by rules in ESLint proper.


Pull request:
Fix: Warn for deprecation in Node output (fixes #7443)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[ ] Documentation update
[x] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->
https://github.com/eslint/eslint/issues/7443
<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**
Added a property `usedDeprecatedRules` to the output of the Node API that flags usage of deprecated rules.

**Is there anything you'd like reviewers to focus on?**
The issue touched on other possible work (i.e. formatter changes, `usedRemovedRules`, `usedNotFoundRules`). From reading the issue, I think the group settled on only implementing `usedDeprecatedRules`, but please check my understanding.



Patch:
diff --git a/Makefile.js b/Makefile.js
index 473ffdf58..2a5c7ab8d 100644
--- a/Makefile.js
+++ b/Makefile.js
@@ -226,7 +226,7 @@ function generateRuleIndexPage(basedir) {
             if (rule.meta.deprecated) {
                 categoriesData.deprecated.rules.push({
                     name: basename,
-                    replacedBy: rule.meta.docs.replacedBy || []
+                    replacedBy: rule.meta.replacedBy || []
                 });
             } else {
                 const output = {
diff --git a/docs/developer-guide/nodejs-api.md b/docs/developer-guide/nodejs-api.md
index 8dbccc7af..0ff1e04ef 100644
--- a/docs/developer-guide/nodejs-api.md
+++ b/docs/developer-guide/nodejs-api.md
@@ -424,7 +424,8 @@ The return value is an object containing the results of the linting operation. H
     errorCount: 1,
     warningCount: 0,
     fixableErrorCount: 1,
-    fixableWarningCount: 0
+    fixableWarningCount: 0,
+    usedDeprecatedRules: []
 }
 ```
 
@@ -482,6 +483,7 @@ var report = cli.executeOnFiles(["myfile.js", "lib/"]);
     warningCount: 0,
     fixableErrorCount: 1,
     fixableWarningCount: 0,
+    usedDeprecatedRules: []
 }
 ```
 
@@ -513,6 +515,7 @@ If the operation ends with a parsing error, you will get a single message for th
     warningCount: 0,
     fixableErrorCount: 0,
     fixableWarningCount: 0,
+    usedDeprecatedRules: []
 }
 ```
 
@@ -524,7 +527,10 @@ The top-level report object has a `results` array containing all linting results
 * `source` - The source code for the given file. This property is omitted if this file has no errors/warnings or if the `output` property is present.
 * `output` - The source code for the given file with as many fixes applied as possible, so you can use that to rewrite the files if necessary. This property is omitted if no fix is available.
 
-The top-level report object also has `errorCount` and `warningCount` which give the exact number of errors and warnings respectively on all the files.
+The top-level report object also has `errorCount` and `warningCount` which give the exact number of errors and warnings respectively on all the files. Additionally, `usedDeprecatedRules` signals any deprecated rules used and their replacement (if available). Specifically, it is array of objects with properties like so:
+
+* `ruleId` - The name of the rule (e.g. `indent-legacy`).
+* `replacedBy` - An array of rules that replace the deprecated rule (e.g. `["indent"]`).
 
 Once you get a report object, it's up to you to determine how to output the results. Fixes will not be automatically applied to the files, even if you set `fix: true` when constructing the `CLIEngine` instance. To apply fixes to the files, call [`outputFixes`](#cliengineoutputfixes).
 
diff --git a/docs/developer-guide/working-with-rules.md b/docs/developer-guide/working-with-rules.md
index 8b5c0d5ac..85b8c25ea 100644
--- a/docs/developer-guide/working-with-rules.md
+++ b/docs/developer-guide/working-with-rules.md
@@ -66,6 +66,8 @@ The source file for a rule exports an object with the following properties.
 
 * `deprecated` (boolean) indicates whether the rule has been deprecated.  You may omit the `deprecated` property if the rule has not been deprecated.
 
+* `replacedBy` (array) in the case of a deprecated rule, specifies replacement rule(s)
+
 `create` (function) returns an object with methods that ESLint calls to "visit" nodes while traversing the abstract syntax tree (AST as defined by [ESTree](https://github.com/estree/estree)) of JavaScript code:
 
 * if a key is a node type or a [selector](./selectors.md), ESLint calls that **visitor** function while going **down** the tree
diff --git a/lib/cli-engine.js b/lib/cli-engine.js
index 5b52459ac..1545bf4c0 100644
--- a/lib/cli-engine.js
+++ b/lib/cli-engine.js
@@ -19,6 +19,7 @@ const fs = require("fs"),
     path = require("path"),
     defaultOptions = require("../conf/default-cli-options"),
     Linter = require("./linter"),
+    lodash = require("lodash"),
     IgnoredPaths = require("./ignored-paths"),
     Config = require("./config"),
     LintResultCache = require("./util/lint-result-cache"),
@@ -272,6 +273,35 @@ function createIgnoreResult(filePath, baseDir) {
     };
 }
 
+/**
+ * Produces rule warnings (i.e. deprecation) from configured rules
+ * @param {Object} rules - Rules configured
+ * @param {Map} loadedRules - Map of loaded rules
+ * @returns {Object} Contains rule warnings
+ * @private
+ */
+function createRuleWarnings(rules, loadedRules) {
+    const ruleWarnings = { usedDeprecatedRules: [] };
+
+    if (rules) {
+        Object.keys(rules).forEach(name => {
+            const loadedRule = loadedRules.get(name);
+
+            if (loadedRule.meta && loadedRule.meta.deprecated) {
+                const deprecatedRule = { ruleId: name };
+                const replacedBy = lodash.get(loadedRule, "meta.replacedBy", []);
+
+                if (replacedBy.every(newRule => lodash.isString(newRule))) {
+                    deprecatedRule.replacedBy = replacedBy;
+                }
+
+                ruleWarnings.usedDeprecatedRules.push(deprecatedRule);
+            }
+        });
+    }
+
+    return ruleWarnings;
+}
 
 /**
  * Checks if the given message is an error message.
@@ -555,6 +585,8 @@ class CLIEngine {
 
         const stats = calculateStatsPerRun(results);
 
+        const ruleWarnings = createRuleWarnings(this.options.rules, this.getRules());
+
         debug(`Linting complete in: ${Date.now() - startTime}ms`);
 
         return {
@@ -562,7 +594,8 @@ class CLIEngine {
             errorCount: stats.errorCount,
             warningCount: stats.warningCount,
             fixableErrorCount: stats.fixableErrorCount,
-            fixableWarningCount: stats.fixableWarningCount
+            fixableWarningCount: stats.fixableWarningCount,
+            usedDeprecatedRules: ruleWarnings.usedDeprecatedRules
         };
     }
 
diff --git a/lib/rules/indent-legacy.js b/lib/rules/indent-legacy.js
index e6dc92163..c3858ba75 100644
--- a/lib/rules/indent-legacy.js
+++ b/lib/rules/indent-legacy.js
@@ -25,12 +25,13 @@ module.exports = {
             description: "enforce consistent indentation",
             category: "Stylistic Issues",
             recommended: false,
-            replacedBy: ["indent"],
             url: "https://eslint.org/docs/rules/indent-legacy"
         },
 
         deprecated: true,
 
+        replacedBy: ["indent"],
+
         fixable: "whitespace",
 
         schema: [
diff --git a/lib/rules/lines-around-directive.js b/lib/rules/lines-around-directive.js
index e8d613a41..2ed72942d 100644
--- a/lib/rules/lines-around-directive.js
+++ b/lib/rules/lines-around-directive.js
@@ -18,7 +18,6 @@ module.exports = {
             description: "require or disallow newlines around directives",
             category: "Stylistic Issues",
             recommended: false,
-            replacedBy: ["padding-line-between-statements"],
             url: "https://eslint.org/docs/rules/lines-around-directive"
         },
         schema: [{
@@ -42,7 +41,8 @@ module.exports = {
             ]
         }],
         fixable: "whitespace",
-        deprecated: true
+        deprecated: true,
+        replacedBy: ["padding-line-between-statements"]
     },
 
     create(context) {
diff --git a/lib/rules/newline-after-var.js b/lib/rules/newline-after-var.js
index 62ccb3db2..df2ad7620 100644
--- a/lib/rules/newline-after-var.js
+++ b/lib/rules/newline-after-var.js
@@ -22,7 +22,6 @@ module.exports = {
             description: "require or disallow an empty line after variable declarations",
             category: "Stylistic Issues",
             recommended: false,
-            replacedBy: ["padding-line-between-statements"],
             url: "https://eslint.org/docs/rules/newline-after-var"
         },
 
@@ -34,7 +33,9 @@ module.exports = {
 
         fixable: "whitespace",
 
-        deprecated: true
+        deprecated: true,
+
+        replacedBy: ["padding-line-between-statements"]
     },
 
     create(context) {
diff --git a/lib/rules/newline-before-return.js b/lib/rules/newline-before-return.js
index 5bc1f7031..e9de4bb14 100644
--- a/lib/rules/newline-before-return.js
+++ b/lib/rules/newline-before-return.js
@@ -15,12 +15,12 @@ module.exports = {
             description: "require an empty line before `return` statements",
             category: "Stylistic Issues",
             recommended: false,
-            replacedBy: ["padding-line-between-statements"],
             url: "https://eslint.org/docs/rules/newline-before-return"
         },
         fixable: "whitespace",
         schema: [],
-        deprecated: true
+        deprecated: true,
+        replacedBy: ["padding-line-between-statements"]
     },
 
     create(context) {
diff --git a/lib/rules/no-catch-shadow.js b/lib/rules/no-catch-shadow.js
index f749490f3..a9253dde0 100644
--- a/lib/rules/no-catch-shadow.js
+++ b/lib/rules/no-catch-shadow.js
@@ -22,11 +22,12 @@ module.exports = {
             description: "disallow `catch` clause parameters from shadowing variables in the outer scope",
             category: "Variables",
             recommended: false,
-            url: "https://eslint.org/docs/rules/no-catch-shadow",
-            replacedBy: ["no-shadow"]
+            url: "https://eslint.org/docs/rules/no-catch-shadow"
         },
         deprecated: true,
 
+        replacedBy: ["no-shadow"],
+
         schema: [],
 
         messages: {
diff --git a/lib/rules/no-native-reassign.js b/lib/rules/no-native-reassign.js
index b1064b0bb..f24499b14 100644
--- a/lib/rules/no-native-reassign.js
+++ b/lib/rules/no-native-reassign.js
@@ -16,12 +16,13 @@ module.exports = {
             description: "disallow assignments to native objects or read-only global variables",
             category: "Best Practices",
             recommended: false,
-            replacedBy: ["no-global-assign"],
             url: "https://eslint.org/docs/rules/no-native-reassign"
         },
 
         deprecated: true,
 
+        replacedBy: ["no-global-assign"],
+
         schema: [
             {
                 type: "object",
diff --git a/lib/rules/no-negated-in-lhs.js b/lib/rules/no-negated-in-lhs.js
index 7f08814c9..54252cdbc 100644
--- a/lib/rules/no-negated-in-lhs.js
+++ b/lib/rules/no-negated-in-lhs.js
@@ -16,11 +16,12 @@ module.exports = {
             description: "disallow negating the left operand in `in` expressions",
             category: "Possible Errors",
             recommended: false,
-            replacedBy: ["no-unsafe-negation"],
             url: "https://eslint.org/docs/rules/no-negated-in-lhs"
         },
         deprecated: true,
 
+        replacedBy: ["no-unsafe-negation"],
+
         schema: []
     },
 
diff --git a/lib/rules/no-spaced-func.js b/lib/rules/no-spaced-func.js
index 42d1e4b24..1f584f88b 100644
--- a/lib/rules/no-spaced-func.js
+++ b/lib/rules/no-spaced-func.js
@@ -16,12 +16,13 @@ module.exports = {
             description: "disallow spacing between function identifiers and their applications (deprecated)",
             category: "Stylistic Issues",
             recommended: false,
-            replacedBy: ["func-call-spacing"],
             url: "https://eslint.org/docs/rules/no-spaced-func"
         },
 
         deprecated: true,
 
+        replacedBy: ["func-call-spacing"],
+
         fixable: "whitespace",
         schema: []
     },
diff --git a/lib/rules/prefer-reflect.js b/lib/rules/prefer-reflect.js
index 765163e0e..bda827cb7 100644
--- a/lib/rules/prefer-reflect.js
+++ b/lib/rules/prefer-reflect.js
@@ -15,12 +15,13 @@ module.exports = {
             description: "require `Reflect` methods where applicable",
             category: "ECMAScript 6",
             recommended: false,
-            replacedBy: [],
             url: "https://eslint.org/docs/rules/prefer-reflect"
         },
 
         deprecated: true,
 
+        replacedBy: [],
+
         schema: [
             {
                 type: "object",


Test patch:
diff --git a/tests/lib/cli-engine.js b/tests/lib/cli-engine.js
index cac7671b1..92458b59a 100644
--- a/tests/lib/cli-engine.js
+++ b/tests/lib/cli-engine.js
@@ -1375,6 +1375,33 @@ describe("CLIEngine", () => {
             assert.strictEqual(report.results[0].messages.length, 0);
         });
 
+        it("should warn when deprecated rules are configured", () => {
+            engine = new CLIEngine({
+                cwd: originalDir,
+                configFile: ".eslintrc.js",
+                rules: { "indent-legacy": 1 }
+            });
+
+            const report = engine.executeOnFiles(["lib/cli*.js"]);
+
+            assert.deepStrictEqual(
+                report.usedDeprecatedRules,
+                [{ ruleId: "indent-legacy", replacedBy: ["indent"] }]
+            );
+        });
+
+        it("should not warn when deprecated rules are not configured", () => {
+            engine = new CLIEngine({
+                cwd: originalDir,
+                configFile: ".eslintrc.js",
+                rules: { indent: 1 }
+            });
+
+            const report = engine.executeOnFiles(["lib/cli*.js"]);
+
+            assert.deepStrictEqual(report.usedDeprecatedRules, []);
+        });
+
         describe("Fix Mode", () => {
 
             it("should return fixed text on multiple files when in fix mode", () => {
@@ -1407,70 +1434,68 @@ describe("CLIEngine", () => {
                 const report = engine.executeOnFiles([path.resolve(fixtureDir, `${fixtureDir}/fixmode`)]);
 
                 report.results.forEach(convertCRLF);
-                assert.deepStrictEqual(report, {
-                    results: [
-                        {
-                            filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/multipass.js")),
-                            messages: [],
-                            errorCount: 0,
-                            warningCount: 0,
-                            fixableErrorCount: 0,
-                            fixableWarningCount: 0,
-                            output: "true ? \"yes\" : \"no\";\n"
-                        },
-                        {
-                            filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/ok.js")),
-                            messages: [],
-                            errorCount: 0,
-                            warningCount: 0,
-                            fixableErrorCount: 0,
-                            fixableWarningCount: 0
-                        },
-                        {
-                            filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/quotes-semi-eqeqeq.js")),
-                            messages: [
-                                {
-                                    column: 9,
-                                    line: 2,
-                                    message: "Expected '===' and instead saw '=='.",
-                                    messageId: "unexpected",
-                                    nodeType: "BinaryExpression",
-                                    ruleId: "eqeqeq",
-                                    severity: 2
-                                }
-                            ],
-                            errorCount: 1,
-                            warningCount: 0,
-                            fixableErrorCount: 0,
-                            fixableWarningCount: 0,
-                            output: "var msg = \"hi\";\nif (msg == \"hi\") {\n\n}\n"
-                        },
-                        {
-                            filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/quotes.js")),
-                            messages: [
-                                {
-                                    column: 18,
-                                    line: 1,
-                                    endColumn: 21,
-                                    endLine: 1,
-                                    message: "'foo' is not defined.",
-                                    nodeType: "Identifier",
-                                    ruleId: "no-undef",
-                                    severity: 2
-                                }
-                            ],
-                            errorCount: 1,
-                            warningCount: 0,
-                            fixableErrorCount: 0,
-                            fixableWarningCount: 0,
-                            output: "var msg = \"hi\" + foo;\n"
-                        }
-                    ],
-                    errorCount: 2,
-                    warningCount: 0,
-                    fixableErrorCount: 0,
-                    fixableWarningCount: 0
-                });
+                assert.deepStrictEqual(report.results, [
+                    {
+                        filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/multipass.js")),
+                        messages: [],
+                        errorCount: 0,
+                        warningCount: 0,
+                        fixableErrorCount: 0,
+                        fixableWarningCount: 0,
+                        output: "true ? \"yes\" : \"no\";\n"
+                    },
+                    {
+                        filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/ok.js")),
+                        messages: [],
+                        errorCount: 0,
+                        warningCount: 0,
+                        fixableErrorCount: 0,
+                        fixableWarningCount: 0
+                    },
+                    {
+                        filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/quotes-semi-eqeqeq.js")),
+                        messages: [
+                            {
+                                column: 9,
+                                line: 2,
+                                message: "Expected '===' and instead saw '=='.",
+                                messageId: "unexpected",
+                                nodeType: "BinaryExpression",
+                                ruleId: "eqeqeq",
+                                severity: 2
+                            }
+                        ],
+                        errorCount: 1,
+                        warningCount: 0,
+                        fixableErrorCount: 0,
+                        fixableWarningCount: 0,
+                        output: "var msg = \"hi\";\nif (msg == \"hi\") {\n\n}\n"
+                    },
+                    {
+                        filePath: fs.realpathSync(path.resolve(fixtureDir, "fixmode/quotes.js")),
+                        messages: [
+                            {
+                                column: 18,
+                                line: 1,
+                                endColumn: 21,
+                                endLine: 1,
+                                message: "'foo' is not defined.",
+                                nodeType: "Identifier",
+                                ruleId: "no-undef",
+                                severity: 2
+                            }
+                        ],
+                        errorCount: 1,
+                        warningCount: 0,
+                        fixableErrorCount: 0,
+                        fixableWarningCount: 0,
+                        output: "var msg = \"hi\" + foo;\n"
+                    }
+                ]);
+                assert.strictEqual(report.errorCount, 2);
+                assert.strictEqual(report.warningCount, 0);
+                assert.strictEqual(report.fixableErrorCount, 0);
+                assert.strictEqual(report.fixableWarningCount, 0);
             });
 
             it("should run autofix even if files are cached without autofix results", () => {
@@ -2117,7 +2142,7 @@ describe("CLIEngine", () => {
                 assert.deepStrictEqual(result, cachedResult, "the result is the same regardless of using cache or not");
 
                 // assert the file was not processed because the cache was used
-                assert.isFalse(spy.called, "the file was not loaded because it used the cache");
+                assert.isFalse(spy.calledWith(file), "the file was not loaded because it used the cache");
             });
 
             it("should remember the files from a previous run and do not operate on then if not changed", () => {