Skip to content

Commit

Permalink
feat: implement rfc 2021-suppression-support (#15459)
Browse files Browse the repository at this point in the history 
* feat: implement rfc 2021-suppression-support

* fix --report-unused-disable-directives not working in some cases.

* modify the JSDoc comments of applyDirectives().

* fix not working with processors

* update the return type of _verifyXX methods

* fix the issue that only string after the first -- is treated as justification

* fix eslint-disable-next-line not working after /* eslint-disable */

* refactor applyDirectives() to process usedDisableDirectives properly

* fix another unused-disable-directive issue, and add more test cases to cover

* apply the simpler implementation of applyDirective()

* update doc

* filter out suppressedMessages in getErrorResults()

* add meta for rules in suppressedMessages for getRulesMetaForResults()

* update ESLint.getErrorResults() to keep the current behavior
  • Loading branch information
@Yiwei-Ding
Yiwei-Ding committed Jan 27, 2022
1 parent 5769cc2 commit 5d60812
Show file tree
Hide file tree
Showing 11 changed files with 2,277 additions and 307 deletions.
47 changes: 47 additions & 0 deletions docs/developer-guide/nodejs-api.md
View file
Expand Up @@ -19,6 +19,7 @@ While ESLint is designed to be run on the command line, it's possible to use ESL
* [static getErrorResults()][eslint-geterrorresults]
* [LintResult type][lintresult]
* [LintMessage type][lintmessage]
* [SuppressedLintMessage type][suppressedlintmessage]
* [EditInfo type][editinfo]
* [Formatter type][formatter]
* [SourceCode](#sourcecode)
Expand Down Expand Up @@ -347,6 +348,8 @@ The `LintResult` value is the information of the linting result of each file. Th
The absolute path to the file of this result. This is the string `"<text>"` if the file path is unknown (when you didn't pass the `options.filePath` option to the [`eslint.lintText()`][eslint-linttext] method).
* `messages` (`LintMessage[]`)<br>
The array of [LintMessage] objects.
* `suppressedMessages` (`SuppressedLintMessage[]`)<br>
The array of [SuppressedLintMessage] objects.
* `fixableErrorCount` (`number`)<br>
The number of errors that can be fixed automatically by the `fix` constructor option.
* `fixableWarningCount` (`number`)<br>
Expand Down Expand Up @@ -389,6 +392,33 @@ The `LintMessage` value is the information of each linting error. The `messages`
* `suggestions` (`{ desc: string; fix: EditInfo }[] | undefined`)<br>
The list of suggestions. Each suggestion is the pair of a description and an [EditInfo] object to fix code. API users such as editor integrations can choose one of them to fix the problem of this message. This property is undefined if this message doesn't have any suggestions.

### ◆ SuppressedLintMessage type

The `SuppressedLintMessage` value is the information of each suppressed linting error. The `suppressedMessages` property of the [LintResult] type contains it. It has the following properties:

* `ruleId` (`string` | `null`)<br>
Same as `ruleId` in [LintMessage] type.
* `severity` (`1 | 2`)<br>
Same as `severity` in [LintMessage] type.
* `fatal` (`boolean | undefined`)<br>
Same as `fatal` in [LintMessage] type.
* `message` (`string`)<br>
Same as `message` in [LintMessage] type.
* `line` (`number | undefined`)<br>
Same as `line` in [LintMessage] type.
* `column` (`number | undefined`)<br>
Same as `column` in [LintMessage] type.
* `endLine` (`number | undefined`)<br>
Same as `endLine` in [LintMessage] type.
* `endColumn` (`number | undefined`)<br>
Same as `endColumn` in [LintMessage] type.
* `fix` (`EditInfo | undefined`)<br>
Same as `fix` in [LintMessage] type.
* `suggestions` (`{ desc: string; fix: EditInfo }[] | undefined`)<br>
Same as `suggestions` in [LintMessage] type.
* `suppressions` (`{ kind: string; justification: string}[]`)<br>
The list of suppressions. Each suppression is the pair of a kind and a justification.

### ◆ EditInfo type

The `EditInfo` value is information to edit text. The `fix` and `suggestions` properties of [LintMessage] type contain it. It has following properties:
Expand Down Expand Up @@ -551,6 +581,22 @@ The information available for each linting message is:
* `fix` - an object describing the fix for the problem (this property is omitted if no fix is available).
* `suggestions` - an array of objects describing possible lint fixes for editors to programmatically enable (see details in the [Working with Rules docs](./working-with-rules.md#providing-suggestions)).

You can get the suppressed messages from the previous run by `getSuppressedMessages()` method. If there is not a previous run, `getSuppressedMessage()` will return an empty list.

```js
const Linter = require("eslint").Linter;
const linter = new Linter();

const messages = linter.verify("var foo = bar; // eslint-disable-line -- Need to suppress", {
rules: {
semi: ["error", "never"]
}
}, { filename: "foo.js" });
const suppressedMessages = linter.getSuppressedMessages();

console.log(suppressedMessages[0].suppressions); // [{ "kind": "directive", "justification": "Need to suppress" }]
```

Linting message objects have a deprecated `source` property. This property **will be removed** from linting messages in an upcoming breaking release. If you depend on this property, you should now use the `SourceCode` instance provided by the linter.

You can also get an instance of the `SourceCode` object used inside of `linter` by using the `getSourceCode()` method:
Expand Down Expand Up @@ -912,6 +958,7 @@ ruleTester.run("my-rule", myRule, {
[eslint-geterrorresults]: #-eslintgeterrorresultsresults
[lintresult]: #-lintresult-type
[lintmessage]: #-lintmessage-type
[suppressedlintmessage]: #-suppressedlintmessage-type
[editinfo]: #-editinfo-type
[formatter]: #-formatter-type
[linter]: #linter
6 changes: 6 additions & 0 deletions lib/cli-engine/cli-engine.js
View file
Expand Up @@ -51,6 +51,7 @@ const validFixTypes = new Set(["directive", "problem", "suggestion", "layout"]);
/** @typedef {import("../shared/types").ConfigData} ConfigData */
/** @typedef {import("../shared/types").DeprecatedRuleInfo} DeprecatedRuleInfo */
/** @typedef {import("../shared/types").LintMessage} LintMessage */
/** @typedef {import("../shared/types").SuppressedLintMessage} SuppressedLintMessage */
/** @typedef {import("../shared/types").ParserOptions} ParserOptions */
/** @typedef {import("../shared/types").Plugin} Plugin */
/** @typedef {import("../shared/types").RuleConf} RuleConf */
Expand Down Expand Up @@ -91,6 +92,7 @@ const validFixTypes = new Set(["directive", "problem", "suggestion", "layout"]);
* @typedef {Object} LintResult
* @property {string} filePath The path to the file that was linted.
* @property {LintMessage[]} messages All of the messages for the result.
* @property {SuppressedLintMessage[]} suppressedMessages All of the suppressed messages for the result.
* @property {number} errorCount Number of errors for the result.
* @property {number} fatalErrorCount Number of fatal errors for the result.
* @property {number} warningCount Number of warnings for the result.
Expand Down Expand Up @@ -263,6 +265,7 @@ function verifyText({
const result = {
filePath,
messages,
suppressedMessages: linter.getSuppressedMessages(),
...calculateStatsPerFile(messages)
};

Expand Down Expand Up @@ -309,6 +312,7 @@ function createIgnoreResult(filePath, baseDir) {
message
}
],
suppressedMessages: [],
errorCount: 0,
fatalErrorCount: 0,
warningCount: 1,
Expand Down Expand Up @@ -683,11 +687,13 @@ class CLIEngine {

results.forEach(result => {
const filteredMessages = result.messages.filter(isErrorMessage);
const filteredSuppressedMessages = result.suppressedMessages.filter(isErrorMessage);

if (filteredMessages.length > 0) {
filtered.push({
...result,
messages: filteredMessages,
suppressedMessages: filteredSuppressedMessages,
errorCount: filteredMessages.length,
warningCount: 0,
fixableErrorCount: result.fixableErrorCount,
Expand Down
5 changes: 5 additions & 0 deletions lib/eslint/eslint.js
View file
Expand Up @@ -32,6 +32,7 @@ const { version } = require("../../package.json");
/** @typedef {import("../shared/types").DeprecatedRuleInfo} DeprecatedRuleInfo */
/** @typedef {import("../shared/types").ConfigData} ConfigData */
/** @typedef {import("../shared/types").LintMessage} LintMessage */
/** @typedef {import("../shared/types").SuppressedLintMessage} SuppressedLintMessage */
/** @typedef {import("../shared/types").Plugin} Plugin */
/** @typedef {import("../shared/types").Rule} Rule */

Expand Down Expand Up @@ -78,6 +79,7 @@ const { version } = require("../../package.json");
* @typedef {Object} LintResult
* @property {string} filePath The path to the file that was linted.
* @property {LintMessage[]} messages All of the messages for the result.
* @property {SuppressedLintMessage[]} suppressedMessages All of the suppressed messages for the result.
* @property {number} errorCount Number of errors for the result.
* @property {number} fatalErrorCount Number of fatal errors for the result.
* @property {number} warningCount Number of warnings for the result.
Expand Down Expand Up @@ -526,6 +528,9 @@ class ESLint {
for (const { ruleId } of result.messages) {
resultRuleIds.add(ruleId);
}
for (const { ruleId } of result.suppressedMessages) {
resultRuleIds.add(ruleId);
}
}

// create a map of all rules in the results
Expand Down
79 changes: 35 additions & 44 deletions lib/linter/apply-disable-directives.js
View file
Expand Up @@ -197,62 +197,52 @@ function processUnusedDisableDirectives(allDirectives) {
* for the exported function, except that `reportUnusedDisableDirectives` is not supported
* (this function always reports unused disable directives).
* @returns {{problems: Problem[], unusedDisableDirectives: Problem[]}} An object with a list
* of filtered problems and unused eslint-disable directives
* of problems (including suppressed ones) and unused eslint-disable directives
*/
function applyDirectives(options) {
const problems = [];
let nextDirectiveIndex = 0;
let currentGlobalDisableDirective = null;
const disabledRuleMap = new Map();

// enabledRules is only used when there is a current global disable directive.
const enabledRules = new Set();
const usedDisableDirectives = new Set();

for (const problem of options.problems) {
let disableDirectivesForProblem = [];
let nextDirectiveIndex = 0;

while (
nextDirectiveIndex < options.directives.length &&
compareLocations(options.directives[nextDirectiveIndex], problem) <= 0
) {
const directive = options.directives[nextDirectiveIndex++];

switch (directive.type) {
case "disable":
if (directive.ruleId === null) {
currentGlobalDisableDirective = directive;
disabledRuleMap.clear();
enabledRules.clear();
} else if (currentGlobalDisableDirective) {
enabledRules.delete(directive.ruleId);
disabledRuleMap.set(directive.ruleId, directive);
} else {
disabledRuleMap.set(directive.ruleId, directive);
}
break;

case "enable":
if (directive.ruleId === null) {
currentGlobalDisableDirective = null;
disabledRuleMap.clear();
} else if (currentGlobalDisableDirective) {
enabledRules.add(directive.ruleId);
disabledRuleMap.delete(directive.ruleId);
} else {
disabledRuleMap.delete(directive.ruleId);
}
break;

// no default
if (directive.ruleId === null || directive.ruleId === problem.ruleId) {
switch (directive.type) {
case "disable":
disableDirectivesForProblem.push(directive);
break;

case "enable":
disableDirectivesForProblem = [];
break;

// no default
}
}
}

if (disabledRuleMap.has(problem.ruleId)) {
usedDisableDirectives.add(disabledRuleMap.get(problem.ruleId));
} else if (currentGlobalDisableDirective && !enabledRules.has(problem.ruleId)) {
usedDisableDirectives.add(currentGlobalDisableDirective);
} else {
problems.push(problem);
if (disableDirectivesForProblem.length > 0) {
const suppressions = disableDirectivesForProblem.map(directive => ({
kind: "directive",
justification: directive.unprocessedDirective.justification
}));

if (problem.suppressions) {
problem.suppressions = problem.suppressions.concat(suppressions);
} else {
problem.suppressions = suppressions;
usedDisableDirectives.add(disableDirectivesForProblem[disableDirectivesForProblem.length - 1]);
}
}

problems.push(problem);
}

const unusedDisableDirectivesToReport = options.directives
Expand Down Expand Up @@ -282,22 +272,23 @@ function applyDirectives(options) {

/**
* Given a list of directive comments (i.e. metadata about eslint-disable and eslint-enable comments) and a list
* of reported problems, determines which problems should be reported.
* of reported problems, adds the suppression information to the problems.
* @param {Object} options Information about directives and problems
* @param {{
* type: ("disable"|"enable"|"disable-line"|"disable-next-line"),
* ruleId: (string|null),
* line: number,
* column: number
* column: number,
* justification: string
* }} options.directives Directive comments found in the file, with one-based columns.
* Two directive comments can only have the same location if they also have the same type (e.g. a single eslint-disable
* comment for two different rules is represented as two directives).
* @param {{ruleId: (string|null), line: number, column: number}[]} options.problems
* A list of problems reported by rules, sorted by increasing location in the file, with one-based columns.
* @param {"off" | "warn" | "error"} options.reportUnusedDisableDirectives If `"warn"` or `"error"`, adds additional problems for unused directives
* @param {boolean} options.disableFixes If true, it doesn't make `fix` properties.
* @returns {{ruleId: (string|null), line: number, column: number}[]}
* A list of reported problems that were not disabled by the directive comments.
* @returns {{ruleId: (string|null), line: number, column: number, suppressions?: {kind: string, justification: string}}[]}
* An object with a list of reported problems, the suppressed of which contain the suppression information.
*/
module.exports = ({ directives, disableFixes, problems, reportUnusedDisableDirectives = "off" }) => {
const blockDirectives = directives
Expand Down

0 comments on commit 5d60812

Please sign in to comment.