Skip to content

Commit

Permalink
Update: Warn for deprecation in Node output (fixes #7443) (#10953)
Browse files Browse the repository at this point in the history
* Fix: Warn for deprecation in Node output (fixes #7443)

* Reduce complexity of createRuleWarnings()

* Specify the usedDeprecatedRules object properties in the docs

* Add safety checks for createRuleWarnings()

* Document replacedBy in working-with-rules

* Move meta.docs.replacedBy to meta.replacedBy
  • Loading branch information
calling authored and nzakas committed Oct 30, 2018
1 parent 9771442 commit 802e926
Show file tree
Hide file tree
Showing 14 changed files with 155 additions and 82 deletions.
2 changes: 1 addition & 1 deletion Makefile.js
Expand Up @@ -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 = {
Expand Down
10 changes: 8 additions & 2 deletions docs/developer-guide/nodejs-api.md
Expand Up @@ -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: []
}
```

Expand Down Expand Up @@ -482,6 +483,7 @@ var report = cli.executeOnFiles(["myfile.js", "lib/"]);
warningCount: 0,
fixableErrorCount: 1,
fixableWarningCount: 0,
usedDeprecatedRules: []
}
```

Expand Down Expand Up @@ -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: []
}
```

Expand All @@ -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).

Expand Down
2 changes: 2 additions & 0 deletions docs/developer-guide/working-with-rules.md
Expand Up @@ -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
Expand Down
35 changes: 34 additions & 1 deletion lib/cli-engine.js
Expand Up @@ -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"),
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -555,14 +585,17 @@ class CLIEngine {

const stats = calculateStatsPerRun(results);

const ruleWarnings = createRuleWarnings(this.options.rules, this.getRules());

debug(`Linting complete in: ${Date.now() - startTime}ms`);

return {
results,
errorCount: stats.errorCount,
warningCount: stats.warningCount,
fixableErrorCount: stats.fixableErrorCount,
fixableWarningCount: stats.fixableWarningCount
fixableWarningCount: stats.fixableWarningCount,
usedDeprecatedRules: ruleWarnings.usedDeprecatedRules
};
}

Expand Down
3 changes: 2 additions & 1 deletion lib/rules/indent-legacy.js
Expand Up @@ -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: [
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/lines-around-directive.js
Expand Up @@ -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: [{
Expand All @@ -42,7 +41,8 @@ module.exports = {
]
}],
fixable: "whitespace",
deprecated: true
deprecated: true,
replacedBy: ["padding-line-between-statements"]
},

create(context) {
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/newline-after-var.js
Expand Up @@ -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"
},

Expand All @@ -34,7 +33,9 @@ module.exports = {

fixable: "whitespace",

deprecated: true
deprecated: true,

replacedBy: ["padding-line-between-statements"]
},

create(context) {
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/newline-before-return.js
Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/no-catch-shadow.js
Expand Up @@ -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: {
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/no-native-reassign.js
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/no-negated-in-lhs.js
Expand Up @@ -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: []
},

Expand Down
3 changes: 2 additions & 1 deletion lib/rules/no-spaced-func.js
Expand Up @@ -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: []
},
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/prefer-reflect.js
Expand Up @@ -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",
Expand Down

0 comments on commit 802e926

Please sign in to comment.