feat: Support custom severity when reporting unused disable directives (

#17212) * feat: Support custom severity when reporting unused disable directives * fix test * remove change to conf/default-cli-options.js * improve schema * wip * remove reportUnusedDisableDirectives from processOptions() in favor of overrideConfig * revert changes to lib/eslint/eslint.js and fix more tests * remove old type * more tests * fix comment for ParsedCLIOptions * remove unnecessary exception * revert comment * remove number severity * fix translateOptions and tests * revert comment * Update lib/cli.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update lib/options.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update docs/src/use/command-line-interface.md Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update lib/cli-engine/cli-engine.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update lib/cli-engine/cli-engine.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update lib/shared/types.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update lib/shared/types.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * add arg type to doc * mention enable * warn with old option * remove old comment * revert change to tests/lib/eslint/eslint.js * add test for removed option * revert copy change * switch several tests from boolean for reportUnusedDisableDirectives to string * improve tests * use reportUnusedDisableDirectives: error in eslint-config-eslint * support severity numbers * refactor * revert newline change * refactor and fix tests * refactor * cleanup * cleanup * use normalizeSeverityToString in another place * tweak * attempt to avoid test pollution * tweak test name * remove boolean handling from normalizeSeverityToString * Update docs/src/use/command-line-interface.md Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update docs/src/use/command-line-interface.md Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update docs/src/use/command-line-interface.md Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * cleanup * normalize flat config reportUnusedDisableDirectives to number * Update lib/options.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update lib/cli.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update lib/cli.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * remove unused reportUnusedDisableDirectives property * run CLI tests for both flat and eslintrc * Update tests/lib/eslint/flat-eslint.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update tests/lib/eslint/flat-eslint.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update docs/src/use/configure/rules.md Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * address pr feedback --------- Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
eslint · Dec 14, 2023 · 0dd9704 · 0dd9704
1 parent 31a7e3f
commit 0dd9704
Show file tree

Hide file tree

Showing 18 changed files with 509 additions and 67 deletions.
diff --git a/Makefile.js b/Makefile.js
@@ -68,7 +68,7 @@ const NODE = "node ", // intentional extra space
 
     // Utilities - intentional extra space at the end of each string
     MOCHA = `${NODE_MODULES}mocha/bin/_mocha `,
-    ESLINT = `${NODE} bin/eslint.js --report-unused-disable-directives `,
+    ESLINT = `${NODE} bin/eslint.js `,
 
     // Files
     RULE_FILES = glob.sync("lib/rules/*.js").filter(filePath => path.basename(filePath) !== "index.js"),

diff --git a/docs/src/use/command-line-interface.md b/docs/src/use/command-line-interface.md
@@ -98,6 +98,7 @@ Output:
 Inline configuration comments:
   --no-inline-config              Prevent comments from changing config or rules
   --report-unused-disable-directives  Adds reported errors for unused eslint-disable and eslint-enable directives
+  --report-unused-disable-directives-severity String  Chooses severity level for reporting unused eslint-disable and eslint-enable directives - either: off, warn, error, 0, 1, or 2
 
 Caching:
   --cache                         Only check changed files - default: false
@@ -582,7 +583,7 @@ This can be useful to prevent future errors from unexpectedly being suppressed,
 ::: warning
 When using this option, it is possible that new errors start being reported whenever ESLint or custom rules are upgraded.
 
-For example, suppose a rule has a bug that causes it to report a false positive, and an `eslint-disable` comment is added to suppress the incorrect report. If the bug is then fixed in a patch release of ESLint, the `eslint-disable` comment becomes unused since ESLint is no longer generating an incorrect report. This results in a new reported error for the unused directive if the `report-unused-disable-directives` option is used.
+For example, suppose a rule has a bug that causes it to report a false positive, and an `eslint-disable` comment is added to suppress the incorrect report. If the bug is then fixed in a patch release of ESLint, the `eslint-disable` comment becomes unused since ESLint is no longer generating an incorrect report. This results in a new reported error for the unused directive if the `--report-unused-disable-directives` option is used.
 :::
 
 ##### `--report-unused-disable-directives` example
@@ -591,6 +592,23 @@ For example, suppose a rule has a bug that causes it to report a false positive,
 npx eslint --report-unused-disable-directives file.js
 ```
 
+#### `--report-unused-disable-directives-severity`
+
+Same as [`--report-unused-disable-directives`](#--report-unused-disable-directives), but allows you to specify the severity level (`error`, `warn`, `off`) of the reported errors. Only one of these two options can be used at a time.
+
+* **Argument Type**: String. One of the following values:
+  1. `off` (or `0`)
+  1. `warn` (or `1`)
+  1. `error` (or `2`)
+* **Multiple Arguments**: No
+* **Default Value**: By default, `linterOptions.reportUnusedDisableDirectives` configuration setting is used.
+
+##### `--report-unused-disable-directives-severity` example
+
+```shell
+npx eslint --report-unused-disable-directives-severity warn file.js
+```
+
 ### Caching
 
 #### `--cache`

diff --git a/docs/src/use/configure/configuration-files-new.md b/docs/src/use/configure/configuration-files-new.md
@@ -76,7 +76,7 @@ Each configuration object contains all of the information ESLint needs to execut
     * `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.
 * `linterOptions` - An object containing settings related to the linting process.
     * `noInlineConfig` - A Boolean value indicating if inline configuration is allowed.
-    * `reportUnusedDisableDirectives` - A Boolean value indicating if unused disable and enable directives should be tracked and reported.
+    * `reportUnusedDisableDirectives` - A severity string indicating if and how unused disable and enable directives should be tracked and reported. For legacy compatibility, `true` is equivalent to `"warn"` and `false` is equivalent to `"off"`. (default: `"off"`)
 * `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"`).
 * `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.
 * `rules` - An object containing the configured rules. When `files` or `ignores` are specified, these rule configurations are only available to the matching files.
@@ -244,20 +244,22 @@ export default [
 
 #### Reporting unused disable directives
 
-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:
+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 a severity string, as in this example:
 
 ```js
 export default [
     {
         files: ["**/*.js"],
         linterOptions: {
-            reportUnusedDisableDirectives: true
+            reportUnusedDisableDirectives: "error"
         }
     }
 ];
 ```
 
-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.
+You can override this setting using the [`--report-unused-disable-directives`](../command-line-interface#--report-unused-disable-directives) or the [`--report-unused-disable-directives-severity`](../command-line-interface#--report-unused-disable-directives-severity) command line options.
+
+For legacy compatibility, `true` is equivalent to `"warn"` and `false` is equivalent to `"off"`.
 
 ### Configuring language options
 

diff --git a/docs/src/use/configure/migration-guide.md b/docs/src/use/configure/migration-guide.md
@@ -575,7 +575,7 @@ export default [
         // ...other config
         linterOptions: {
             noInlineConfig: true,
-            reportUnusedDisableDirectives: true
+            reportUnusedDisableDirectives: "warn"
         }
     }
 ];

diff --git a/docs/src/use/configure/rules.md b/docs/src/use/configure/rules.md
@@ -317,4 +317,4 @@ To report unused `eslint-disable` comments, use the `reportUnusedDisableDirectiv
 }
 ```
 
-This setting is similar to [--report-unused-disable-directives](../command-line-interface#--report-unused-disable-directives) CLI option, but doesn't fail linting (reports as `"warn"` severity).
+This setting is similar to [--report-unused-disable-directives](../command-line-interface#--report-unused-disable-directives) and [--report-unused-disable-directives-severity](../command-line-interface#--report-unused-disable-directives-severity) CLI options.
diff --git a/lib/cli-engine/cli-engine.js b/lib/cli-engine/cli-engine.js
@@ -83,7 +83,7 @@ const validFixTypes = new Set(["directive", "problem", "suggestion", "layout"]);
  * @property {string[]} [plugins] An array of plugins to load.
  * @property {Record<string,RuleConf>} [rules] An object of rules to use.
  * @property {string[]} [rulePaths] An array of directories to load custom rules from.
- * @property {boolean} [reportUnusedDisableDirectives] `true` adds reports for unused eslint-disable directives
+ * @property {boolean|string} [reportUnusedDisableDirectives] `true`, `"error"` or '"warn"' adds reports for unused eslint-disable directives
  * @property {boolean} [globInputPaths] Set to false to skip glob resolution of input file paths to lint (default: true). If false, each input file paths is assumed to be a non-glob path to an existing file.
  * @property {string} [resolvePluginsRelativeTo] The folder where plugins should be resolved from, defaulting to the CWD
  */
@@ -224,7 +224,7 @@ function calculateStatsPerRun(results) {
  * @param {ConfigArray} config.config The config.
  * @param {boolean} config.fix If `true` then it does fix.
  * @param {boolean} config.allowInlineConfig If `true` then it uses directive comments.
- * @param {boolean} config.reportUnusedDisableDirectives If `true` then it reports unused `eslint-disable` comments.
+ * @param {boolean|string} config.reportUnusedDisableDirectives If `true`, `"error"` or '"warn"', then it reports unused `eslint-disable` comments.
  * @param {FileEnumerator} config.fileEnumerator The file enumerator to check if a path is a target or not.
  * @param {Linter} config.linter The linter instance to verify.
  * @returns {LintResult} The result of linting.

diff --git a/lib/cli.js b/lib/cli.js
@@ -22,7 +22,8 @@ const fs = require("fs"),
     { FlatESLint, shouldUseFlatConfig } = require("./eslint/flat-eslint"),
     createCLIOptions = require("./options"),
     log = require("./shared/logging"),
-    RuntimeInfo = require("./shared/runtime-info");
+    RuntimeInfo = require("./shared/runtime-info"),
+    { normalizeSeverityToString } = require("./shared/severity");
 const { Legacy: { naming } } = require("@eslint/eslintrc");
 const { ModuleImporter } = require("@humanwhocodes/module-importer");
 
@@ -89,6 +90,7 @@ async function translateOptions({
     plugin,
     quiet,
     reportUnusedDisableDirectives,
+    reportUnusedDisableDirectivesSeverity,
     resolvePluginsRelativeTo,
     rule,
     rulesdir,
@@ -125,6 +127,14 @@ async function translateOptions({
             rules: rule ? rule : {}
         }];
 
+        if (reportUnusedDisableDirectives || reportUnusedDisableDirectivesSeverity !== void 0) {
+            overrideConfig[0].linterOptions = {
+                reportUnusedDisableDirectives: reportUnusedDisableDirectives
+                    ? "error"
+                    : normalizeSeverityToString(reportUnusedDisableDirectivesSeverity)
+            };
+        }
+
         if (parser) {
             overrideConfig[0].languageOptions.parser = await importer.import(parser);
         }
@@ -177,8 +187,7 @@ async function translateOptions({
         fixTypes: fixType,
         ignore,
         overrideConfig,
-        overrideConfigFile,
-        reportUnusedDisableDirectives: reportUnusedDisableDirectives ? "error" : void 0
+        overrideConfigFile
     };
 
     if (configType === "flat") {
@@ -190,6 +199,11 @@ async function translateOptions({
         options.useEslintrc = eslintrc;
         options.extensions = ext;
         options.ignorePath = ignorePath;
+        if (reportUnusedDisableDirectives || reportUnusedDisableDirectivesSeverity !== void 0) {
+            options.reportUnusedDisableDirectives = reportUnusedDisableDirectives
+                ? "error"
+                : normalizeSeverityToString(reportUnusedDisableDirectivesSeverity);
+        }
     }
 
     return options;
@@ -386,6 +400,11 @@ const cli = {
             return 2;
         }
 
+        if (options.reportUnusedDisableDirectives && options.reportUnusedDisableDirectivesSeverity !== void 0) {
+            log.error("The --report-unused-disable-directives option and the --report-unused-disable-directives-severity option cannot be used together.");
+            return 2;
+        }
+
         const ActiveESLint = usingFlatConfig ? FlatESLint : ESLint;
 
         const engine = new ActiveESLint(await translateOptions(options, usingFlatConfig ? "flat" : "eslintrc"));

diff --git a/lib/config/flat-config-schema.js b/lib/config/flat-config-schema.js
@@ -14,6 +14,7 @@
  * starting in Node.js v17.
  */
 const structuredClone = require("@ungap/structured-clone").default;
+const { normalizeSeverityToNumber } = require("../shared/severity");
 
 //-----------------------------------------------------------------------------
 // Type Definitions
@@ -262,6 +263,26 @@ const booleanSchema = {
     validate: "boolean"
 };
 
+const ALLOWED_SEVERITIES = new Set(["error", "warn", "off", 2, 1, 0]);
+
+/** @type {ObjectPropertySchema} */
+const disableDirectiveSeveritySchema = {
+    merge(first, second) {
+        const value = second === void 0 ? first : second;
+
+        if (typeof value === "boolean") {
+            return value ? "warn" : "off";
+        }
+
+        return normalizeSeverityToNumber(value);
+    },
+    validate(value) {
+        if (!(ALLOWED_SEVERITIES.has(value) || typeof value === "boolean")) {
+            throw new TypeError("Expected one of: \"error\", \"warn\", \"off\", 0, 1, 2, or a boolean.");
+        }
+    }
+};
+
 /** @type {ObjectPropertySchema} */
 const deepObjectAssignSchema = {
     merge(first = {}, second = {}) {
@@ -534,7 +555,7 @@ const flatConfigSchema = {
     linterOptions: {
         schema: {
             noInlineConfig: booleanSchema,
-            reportUnusedDisableDirectives: booleanSchema
+            reportUnusedDisableDirectives: disableDirectiveSeveritySchema
         }
     },
     languageOptions: {

diff --git a/lib/eslint/eslint-helpers.js b/lib/eslint/eslint-helpers.js
@@ -675,7 +675,6 @@ function processOptions({
     overrideConfig = null,
     overrideConfigFile = null,
     plugins = {},
-    reportUnusedDisableDirectives = null, // ← should be null by default because if it's a string then it overrides the 'reportUnusedDisableDirectives' setting in config files. And we cannot use `overrideConfig.reportUnusedDisableDirectives` instead because we cannot configure the `error` severity with that.
     warnIgnored = true,
     ...unknownOptions
 }) {
@@ -720,6 +719,9 @@ function processOptions({
         if (unknownOptionKeys.includes("rulePaths")) {
             errors.push("'rulePaths' has been removed. Please define your rules using plugins.");
         }
+        if (unknownOptionKeys.includes("reportUnusedDisableDirectives")) {
+            errors.push("'reportUnusedDisableDirectives' has been removed. Please use the 'overrideConfig.linterOptions.reportUnusedDisableDirectives' option instead.");
+        }
     }
     if (typeof allowInlineConfig !== "boolean") {
         errors.push("'allowInlineConfig' must be a boolean.");
@@ -774,14 +776,6 @@ function processOptions({
     if (Array.isArray(plugins)) {
         errors.push("'plugins' doesn't add plugins to configuration to load. Please use the 'overrideConfig.plugins' option instead.");
     }
-    if (
-        reportUnusedDisableDirectives !== "error" &&
-        reportUnusedDisableDirectives !== "warn" &&
-        reportUnusedDisableDirectives !== "off" &&
-        reportUnusedDisableDirectives !== null
-    ) {
-        errors.push("'reportUnusedDisableDirectives' must be any of \"error\", \"warn\", \"off\", and null.");
-    }
     if (typeof warnIgnored !== "boolean") {
         errors.push("'warnIgnored' must be a boolean.");
     }
@@ -806,7 +800,6 @@ function processOptions({
         globInputPaths,
         ignore,
         ignorePatterns,
-        reportUnusedDisableDirectives,
         warnIgnored
     };
 }

diff --git a/lib/eslint/flat-eslint.js b/lib/eslint/flat-eslint.js
@@ -84,7 +84,6 @@ const LintResultCache = require("../cli-engine/lint-result-cache");
  *      doesn't do any config file lookup when `true`; considered to be a config filename
  *      when a string.
  * @property {Record<string,Plugin>} [plugins] An array of plugin implementations.
- * @property {"error" | "warn" | "off"} [reportUnusedDisableDirectives] the severity to report unused eslint-disable directives.
  * @property {boolean} warnIgnored Show warnings when the file list includes ignored files
  */
 
@@ -449,7 +448,6 @@ async function calculateConfigArray(eslint, {
  * @param {FlatConfigArray} config.configs The config.
  * @param {boolean} config.fix If `true` then it does fix.
  * @param {boolean} config.allowInlineConfig If `true` then it uses directive comments.
- * @param {boolean} config.reportUnusedDisableDirectives If `true` then it reports unused `eslint-disable` comments.
  * @param {Linter} config.linter The linter instance to verify.
  * @returns {LintResult} The result of linting.
  * @private
@@ -461,7 +459,6 @@ function verifyText({
     configs,
     fix,
     allowInlineConfig,
-    reportUnusedDisableDirectives,
     linter
 }) {
     const filePath = providedFilePath || "<text>";
@@ -481,7 +478,6 @@ function verifyText({
             allowInlineConfig,
             filename: filePathToVerify,
             fix,
-            reportUnusedDisableDirectives,
 
             /**
              * Check if the linter should adopt a given code block or not.
@@ -749,7 +745,6 @@ class FlatESLint {
             cwd,
             fix,
             fixTypes,
-            reportUnusedDisableDirectives,
             globInputPaths,
             errorOnUnmatchedPattern,
             warnIgnored
@@ -859,7 +854,6 @@ class FlatESLint {
                             cwd,
                             fix: fixer,
                             allowInlineConfig,
-                            reportUnusedDisableDirectives,
                             linter
                         });
 
@@ -944,7 +938,6 @@ class FlatESLint {
             allowInlineConfig,
             cwd,
             fix,
-            reportUnusedDisableDirectives,
             warnIgnored: constructorWarnIgnored
         } = eslintOptions;
         const results = [];
@@ -968,7 +961,6 @@ class FlatESLint {
                 cwd,
                 fix,
                 allowInlineConfig,
-                reportUnusedDisableDirectives,
                 linter
             }));
         }

diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -44,6 +44,7 @@ const { getRuleFromConfig } = require("../config/flat-config-helpers");
 const { FlatConfigArray } = require("../config/flat-config-array");
 const { RuleValidator } = require("../config/rule-validator");
 const { assertIsRuleOptions, assertIsRuleSeverity } = require("../config/flat-config-schema");
+const { normalizeSeverityToString } = require("../shared/severity");
 const debug = require("debug")("eslint:linter");
 const MAX_AUTOFIX_PASSES = 10;
 const DEFAULT_PARSER_NAME = "espree";
@@ -653,9 +654,11 @@ function normalizeVerifyOptions(providedOptions, config) {
         reportUnusedDisableDirectives = reportUnusedDisableDirectives ? "error" : "off";
     }
     if (typeof reportUnusedDisableDirectives !== "string") {
-        reportUnusedDisableDirectives =
-            linterOptions.reportUnusedDisableDirectives
-                ? "warn" : "off";
+        if (typeof linterOptions.reportUnusedDisableDirectives === "boolean") {
+            reportUnusedDisableDirectives = linterOptions.reportUnusedDisableDirectives ? "warn" : "off";
+        } else {
+            reportUnusedDisableDirectives = linterOptions.reportUnusedDisableDirectives === void 0 ? "off" : normalizeSeverityToString(linterOptions.reportUnusedDisableDirectives);
+        }
     }
 
     return {

diff --git a/lib/options.js b/lib/options.js
@@ -48,6 +48,7 @@ const optionator = require("optionator");
  * @property {string[]} [plugin] Specify plugins
  * @property {string} [printConfig] Print the configuration for the given file
  * @property {boolean | undefined} reportUnusedDisableDirectives Adds reported errors for unused eslint-disable and eslint-enable directives
+ * @property {string | undefined} reportUnusedDisableDirectivesSeverity A severity string indicating if and how unused disable and enable directives should be tracked and reported.
  * @property {string} [resolvePluginsRelativeTo] A folder where plugins should be resolved from, CWD by default
  * @property {Object} [rule] Specify rules
  * @property {string[]} [rulesdir] Load additional rules from this directory. Deprecated: Use rules from plugins
@@ -306,6 +307,13 @@ module.exports = function(usingFlatConfig) {
                 default: void 0,
                 description: "Adds reported errors for unused eslint-disable and eslint-enable directives"
             },
+            {
+                option: "report-unused-disable-directives-severity",
+                type: "String",
+                default: void 0,
+                description: "Chooses severity level for reporting unused eslint-disable and eslint-enable directives",
+                enum: ["off", "warn", "error", "0", "1", "2"]
+            },
             {
                 heading: "Caching"
             },