Skip to content

Commit 70648ee

Browse files
ota-meshimdjermanovicnzakas
authored
feat: report-unused-disable-directive to report unused eslint-enable (#17611)
* feat: report-unused-disable-directive to report unused eslint-enable * test: fix test case * chore: refactor * test: add test cases * chore: update order * chore: fix test case order * fix: revert test case * docs: update docs * chore: minor refactor * fix: false positive * chore: update comment * Update lib/linter/apply-disable-directives.js Co-authored-by: Milos Djermanovic <[email protected]> * chore: refactored collectUsedEnableDirectives to return a Set * Update lib/linter/apply-disable-directives.js Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/linter/apply-disable-directives.js Co-authored-by: Nicholas C. Zakas <[email protected]> * fix: doc comments * test: fix test cases * fix: remove withoutRuleIdKey and fix jsdoc * chore: refactor jsdoc * Update lib/linter/apply-disable-directives.js Co-authored-by: Milos Djermanovic <[email protected]> --------- Co-authored-by: Milos Djermanovic <[email protected]> Co-authored-by: Nicholas C. Zakas <[email protected]>
1 parent dcfe573 commit 70648ee

7 files changed

Lines changed: 2847 additions & 368 deletions

File tree

‎docs/src/integrate/nodejs-api.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ The `ESLint` constructor takes an `options` object. If you omit the `options` ob
152152
* `options.plugins` (`Record<string, Plugin> | null`)<br>
153153
Default is `null`. The plugin implementations that ESLint uses for the `plugins` setting of your configuration. This is a map-like object. Those keys are plugin IDs and each value is implementation.
154154
* `options.reportUnusedDisableDirectives` (`"error" | "warn" | "off" | null`)<br>
155-
Default is `null`. The severity to report unused eslint-disable directives. If this option is a severity, it overrides the `reportUnusedDisableDirectives` setting in your configurations.
155+
Default is `null`. The severity to report unused eslint-disable and eslint-enable directives. If this option is a severity, it overrides the `reportUnusedDisableDirectives` setting in your configurations.
156156
* `options.resolvePluginsRelativeTo` (`string` | `null`)<br>
157157
Default is `null`. The path to a directory where plugins should be resolved from. If `null` is present, ESLint loads plugins from the location of the configuration file that contains the plugin setting. If a path is present, ESLint loads all plugins from there.
158158
* `options.rulePaths` (`string[]`)<br>
@@ -537,7 +537,7 @@ The most important method on `Linter` is `verify()`, which initiates linting of
537537
* `filterCodeBlock` - (optional) A function that decides which code blocks the linter should adopt. The function receives two arguments. The first argument is the virtual filename of a code block. The second argument is the text of the code block. If the function returned `true` then the linter adopts the code block. If the function was omitted, the linter adopts only `*.js` code blocks. If you provided a `filterCodeBlock` function, it overrides this default behavior, so the linter doesn't adopt `*.js` code blocks automatically.
538538
* `disableFixes` - (optional) when set to `true`, the linter doesn't make either the `fix` or `suggestions` property of the lint result.
539539
* `allowInlineConfig` - (optional) set to `false` to disable inline comments from changing ESLint rules.
540-
* `reportUnusedDisableDirectives` - (optional) when set to `true`, adds reported errors for unused `eslint-disable` directives when no problems would be reported in the disabled area anyway.
540+
* `reportUnusedDisableDirectives` - (optional) when set to `true`, adds reported errors for unused `eslint-disable` and `eslint-enable` directives when no problems would be reported in the disabled area anyway.
541541

542542
If the third argument is a string, it is interpreted as the `filename`.
543543

‎docs/src/use/command-line-interface.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ Output:
9797
9898
Inline configuration comments:
9999
--no-inline-config Prevent comments from changing config or rules
100-
--report-unused-disable-directives Adds reported errors for unused eslint-disable directives
100+
--report-unused-disable-directives Adds reported errors for unused eslint-disable and eslint-enable directives
101101
102102
Caching:
103103
--cache Only check changed files - default: false
@@ -577,7 +577,7 @@ This option causes ESLint to report directive comments like `// eslint-disable-l
577577

578578
* **Argument Type**: No argument.
579579

580-
This can be useful to prevent future errors from unexpectedly being suppressed, by cleaning up old `eslint-disable` comments which are no longer applicable.
580+
This can be useful to prevent future errors from unexpectedly being suppressed, by cleaning up old `eslint-disable` and `eslint-enable` comments which are no longer applicable.
581581

582582
::: warning
583583
When using this option, it is possible that new errors start being reported whenever ESLint or custom rules are upgraded.

‎docs/src/use/configure/configuration-files-new.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ Each configuration object contains all of the information ESLint needs to execut
7676
* `parserOptions` - An object specifying additional options that are passed directly to the `parse()` or `parseForESLint()` method on the parser. The available options are parser-dependent.
7777
* `linterOptions` - An object containing settings related to the linting process.
7878
* `noInlineConfig` - A Boolean value indicating if inline configuration is allowed.
79-
* `reportUnusedDisableDirectives` - A Boolean value indicating if unused disable directives should be tracked and reported.
79+
* `reportUnusedDisableDirectives` - A Boolean value indicating if unused disable and enable directives should be tracked and reported.
8080
* `processor` - Either an object containing `preprocess()` and `postprocess()` methods or a string indicating the name of a processor inside of a plugin (i.e., `"pluginName/processorName"`).
8181
* `plugins` - An object containing a name-value mapping of plugin names to plugin objects. When `files` is specified, these plugins are only available to the matching files.
8282
* `rules` - An object containing the configured rules. When `files` or `ignores` are specified, these rule configurations are only available to the matching files.
@@ -244,7 +244,7 @@ export default [
244244

245245
#### Reporting unused disable directives
246246

247-
Disable directives such as `/*eslint-disable*/` and `/*eslint-disable-next-line*/` are used to disable ESLint rules around certain portions of code. As code changes, it's possible for these directives to no longer be needed because the code has changed in such a way that the rule is no longer triggered. You can enable reporting of these unused disable directives by setting the `reportUnusedDisableDirectives` option to `true`, as in this example:
247+
Disable and enable directives such as `/*eslint-disable*/`, `/*eslint-enable*/` and `/*eslint-disable-next-line*/` are used to disable ESLint rules around certain portions of code. As code changes, it's possible for these directives to no longer be needed because the code has changed in such a way that the rule is no longer triggered. You can enable reporting of these unused disable directives by setting the `reportUnusedDisableDirectives` option to `true`, as in this example:
248248

249249
```js
250250
export default [
@@ -257,7 +257,7 @@ export default [
257257
];
258258
```
259259

260-
By default, unused disable directives are reported as warnings. You can change this setting using the `--report-unused-disable-directives` command line option.
260+
By default, unused disable and enable directives are reported as warnings. You can change this setting using the `--report-unused-disable-directives` command line option.
261261

262262
### Configuring language options
263263

‎lib/linter/apply-disable-directives.js‎

Lines changed: 126 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ function compareLocations(itemA, itemB) {
3030

3131
/**
3232
* Groups a set of directives into sub-arrays by their parent comment.
33-
* @param {Directive[]} directives Unused directives to be removed.
33+
* @param {Iterable<Directive>} directives Unused directives to be removed.
3434
* @returns {Directive[][]} Directives grouped by their parent comment.
3535
*/
3636
function groupByParentComment(directives) {
@@ -177,10 +177,10 @@ function createCommentRemoval(directives, commentToken) {
177177

178178
/**
179179
* Parses details from directives to create output Problems.
180-
* @param {Directive[]} allDirectives Unused directives to be removed.
180+
* @param {Iterable<Directive>} allDirectives Unused directives to be removed.
181181
* @returns {{ description, fix, unprocessedDirective }[]} Details for later creation of output Problems.
182182
*/
183-
function processUnusedDisableDirectives(allDirectives) {
183+
function processUnusedDirectives(allDirectives) {
184184
const directiveGroups = groupByParentComment(allDirectives);
185185

186186
return directiveGroups.flatMap(
@@ -199,14 +199,103 @@ function processUnusedDisableDirectives(allDirectives) {
199199
);
200200
}
201201

202+
/**
203+
* Collect eslint-enable comments that are removing suppressions by eslint-disable comments.
204+
* @param {Directive[]} directives The directives to check.
205+
* @returns {Set<Directive>} The used eslint-enable comments
206+
*/
207+
function collectUsedEnableDirectives(directives) {
208+
209+
/**
210+
* A Map of `eslint-enable` keyed by ruleIds that may be marked as used.
211+
* If `eslint-enable` does not have a ruleId, the key will be `null`.
212+
* @type {Map<string|null, Directive>}
213+
*/
214+
const enabledRules = new Map();
215+
216+
/**
217+
* A Set of `eslint-enable` marked as used.
218+
* It is also the return value of `collectUsedEnableDirectives` function.
219+
* @type {Set<Directive>}
220+
*/
221+
const usedEnableDirectives = new Set();
222+
223+
/*
224+
* Checks the directives backwards to see if the encountered `eslint-enable` is used by the previous `eslint-disable`,
225+
* and if so, stores the `eslint-enable` in `usedEnableDirectives`.
226+
*/
227+
for (let index = directives.length - 1; index >= 0; index--) {
228+
const directive = directives[index];
229+
230+
if (directive.type === "disable") {
231+
if (enabledRules.size === 0) {
232+
continue;
233+
}
234+
if (directive.ruleId === null) {
235+
236+
// If encounter `eslint-disable` without ruleId,
237+
// mark all `eslint-enable` currently held in enabledRules as used.
238+
// e.g.
239+
// /* eslint-disable */ <- current directive
240+
// /* eslint-enable rule-id1 */ <- used
241+
// /* eslint-enable rule-id2 */ <- used
242+
// /* eslint-enable */ <- used
243+
for (const enableDirective of enabledRules.values()) {
244+
usedEnableDirectives.add(enableDirective);
245+
}
246+
enabledRules.clear();
247+
} else {
248+
const enableDirective = enabledRules.get(directive.ruleId);
249+
250+
if (enableDirective) {
251+
252+
// If encounter `eslint-disable` with ruleId, and there is an `eslint-enable` with the same ruleId in enabledRules,
253+
// mark `eslint-enable` with ruleId as used.
254+
// e.g.
255+
// /* eslint-disable rule-id */ <- current directive
256+
// /* eslint-enable rule-id */ <- used
257+
usedEnableDirectives.add(enableDirective);
258+
} else {
259+
const enabledDirectiveWithoutRuleId = enabledRules.get(null);
260+
261+
if (enabledDirectiveWithoutRuleId) {
262+
263+
// If encounter `eslint-disable` with ruleId, and there is no `eslint-enable` with the same ruleId in enabledRules,
264+
// mark `eslint-enable` without ruleId as used.
265+
// e.g.
266+
// /* eslint-disable rule-id */ <- current directive
267+
// /* eslint-enable */ <- used
268+
usedEnableDirectives.add(enabledDirectiveWithoutRuleId);
269+
}
270+
}
271+
}
272+
} else if (directive.type === "enable") {
273+
if (directive.ruleId === null) {
274+
275+
// If encounter `eslint-enable` without ruleId, the `eslint-enable` that follows it are unused.
276+
// So clear enabledRules.
277+
// e.g.
278+
// /* eslint-enable */ <- current directive
279+
// /* eslint-enable rule-id *// <- unused
280+
// /* eslint-enable */ <- unused
281+
enabledRules.clear();
282+
enabledRules.set(null, directive);
283+
} else {
284+
enabledRules.set(directive.ruleId, directive);
285+
}
286+
}
287+
}
288+
return usedEnableDirectives;
289+
}
290+
202291
/**
203292
* This is the same as the exported function, except that it
204293
* doesn't handle disable-line and disable-next-line directives, and it always reports unused
205294
* disable directives.
206295
* @param {Object} options options for applying directives. This is the same as the options
207296
* for the exported function, except that `reportUnusedDisableDirectives` is not supported
208297
* (this function always reports unused disable directives).
209-
* @returns {{problems: LintMessage[], unusedDisableDirectives: LintMessage[]}} An object with a list
298+
* @returns {{problems: LintMessage[], unusedDirectives: LintMessage[]}} An object with a list
210299
* of problems (including suppressed ones) and unused eslint-disable directives
211300
*/
212301
function applyDirectives(options) {
@@ -258,17 +347,42 @@ function applyDirectives(options) {
258347
const unusedDisableDirectivesToReport = options.directives
259348
.filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive));
260349

261-
const processed = processUnusedDisableDirectives(unusedDisableDirectivesToReport);
262350

263-
const unusedDisableDirectives = processed
351+
const unusedEnableDirectivesToReport = new Set(
352+
options.directives.filter(directive => directive.unprocessedDirective.type === "enable")
353+
);
354+
355+
/*
356+
* If directives has the eslint-enable directive,
357+
* check whether the eslint-enable comment is used.
358+
*/
359+
if (unusedEnableDirectivesToReport.size > 0) {
360+
for (const directive of collectUsedEnableDirectives(options.directives)) {
361+
unusedEnableDirectivesToReport.delete(directive);
362+
}
363+
}
364+
365+
const processed = processUnusedDirectives(unusedDisableDirectivesToReport)
366+
.concat(processUnusedDirectives(unusedEnableDirectivesToReport));
367+
368+
const unusedDirectives = processed
264369
.map(({ description, fix, unprocessedDirective }) => {
265370
const { parentComment, type, line, column } = unprocessedDirective;
266371

372+
let message;
373+
374+
if (type === "enable") {
375+
message = description
376+
? `Unused eslint-enable directive (no matching eslint-disable directives were found for ${description}).`
377+
: "Unused eslint-enable directive (no matching eslint-disable directives were found).";
378+
} else {
379+
message = description
380+
? `Unused eslint-disable directive (no problems were reported from ${description}).`
381+
: "Unused eslint-disable directive (no problems were reported).";
382+
}
267383
return {
268384
ruleId: null,
269-
message: description
270-
? `Unused eslint-disable directive (no problems were reported from ${description}).`
271-
: "Unused eslint-disable directive (no problems were reported).",
385+
message,
272386
line: type === "disable-next-line" ? parentComment.commentToken.loc.start.line : line,
273387
column: type === "disable-next-line" ? parentComment.commentToken.loc.start.column + 1 : column,
274388
severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2,
@@ -277,7 +391,7 @@ function applyDirectives(options) {
277391
};
278392
});
279393

280-
return { problems, unusedDisableDirectives };
394+
return { problems, unusedDirectives };
281395
}
282396

283397
/**
@@ -344,8 +458,8 @@ module.exports = ({ directives, disableFixes, problems, reportUnusedDisableDirec
344458

345459
return reportUnusedDisableDirectives !== "off"
346460
? lineDirectivesResult.problems
347-
.concat(blockDirectivesResult.unusedDisableDirectives)
348-
.concat(lineDirectivesResult.unusedDisableDirectives)
461+
.concat(blockDirectivesResult.unusedDirectives)
462+
.concat(lineDirectivesResult.unusedDirectives)
349463
.sort(compareLocations)
350464
: lineDirectivesResult.problems;
351465
};

‎lib/options.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ const optionator = require("optionator");
4747
* @property {Object} [parserOptions] Specify parser options
4848
* @property {string[]} [plugin] Specify plugins
4949
* @property {string} [printConfig] Print the configuration for the given file
50-
* @property {boolean | undefined} reportUnusedDisableDirectives Adds reported errors for unused eslint-disable directives
50+
* @property {boolean | undefined} reportUnusedDisableDirectives Adds reported errors for unused eslint-disable and eslint-enable directives
5151
* @property {string} [resolvePluginsRelativeTo] A folder where plugins should be resolved from, CWD by default
5252
* @property {Object} [rule] Specify rules
5353
* @property {string[]} [rulesdir] Load additional rules from this directory. Deprecated: Use rules from plugins
@@ -304,7 +304,7 @@ module.exports = function(usingFlatConfig) {
304304
option: "report-unused-disable-directives",
305305
type: "Boolean",
306306
default: void 0,
307-
description: "Adds reported errors for unused eslint-disable directives"
307+
description: "Adds reported errors for unused eslint-disable and eslint-enable directives"
308308
},
309309
{
310310
heading: "Caching"

0 commit comments

Comments
 (0)