Skip to content
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

Fix: Warn for deprecation in Node output (fixes #7443) #10953

Merged
merged 8 commits into from Oct 30, 2018
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