diff --git a/Makefile.js b/Makefile.js index 692168ad7dc..5e5352895bc 100644 --- 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 index d0100aae568..e07ff80ed04 100644 --- 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 index d3ba4cd2bcf..a79fc110a4c 100644 --- 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 index 89c72e5c827..2414ff25642 100644 --- 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 index 7fa0bde7f31..65277610e54 100644 --- 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 index 311dc61e81c..49c8902c161 100644 --- 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} [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 index f472659c20f..1d909ec1cf2 100644 --- 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 index 911d159d93b..3b6612b4984 100644 --- 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 index 72828363c3d..685826ac69c 100644 --- 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 index b44777d71bc..785f41edf8d 100644 --- 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} [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 || ""; @@ -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 index 2995df015b1..f74d0ecd13f 100644 --- 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 index 81c0fa60ab9..dd67c399e64 100644 --- 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" }, diff --git a/lib/shared/severity.js b/lib/shared/severity.js new file mode 100644 index 00000000000..6b21469a9aa --- /dev/null +++ b/lib/shared/severity.js @@ -0,0 +1,49 @@ +/** + * @fileoverview Helpers for severity values (e.g. normalizing different types). + * @author Bryan Mishkin + */ + +"use strict"; + +/** + * Convert severity value of different types to a string. + * @param {string|number} severity severity value + * @throws error if severity is invalid + * @returns {string} severity string + */ +function normalizeSeverityToString(severity) { + if ([2, "2", "error"].includes(severity)) { + return "error"; + } + if ([1, "1", "warn"].includes(severity)) { + return "warn"; + } + if ([0, "0", "off"].includes(severity)) { + return "off"; + } + throw new Error(`Invalid severity value: ${severity}`); +} + +/** + * Convert severity value of different types to a number. + * @param {string|number} severity severity value + * @throws error if severity is invalid + * @returns {number} severity number + */ +function normalizeSeverityToNumber(severity) { + if ([2, "2", "error"].includes(severity)) { + return 2; + } + if ([1, "1", "warn"].includes(severity)) { + return 1; + } + if ([0, "0", "off"].includes(severity)) { + return 0; + } + throw new Error(`Invalid severity value: ${severity}`); +} + +module.exports = { + normalizeSeverityToString, + normalizeSeverityToNumber +}; diff --git a/packages/eslint-config-eslint/base.js b/packages/eslint-config-eslint/base.js index 083336919e1..e7be7fc0de6 100644 --- a/packages/eslint-config-eslint/base.js +++ b/packages/eslint-config-eslint/base.js @@ -378,7 +378,7 @@ const eslintCommentsConfigs = [eslintComments.configs.recommended, { }]; module.exports = [ - { linterOptions: { reportUnusedDisableDirectives: true } }, + { linterOptions: { reportUnusedDisableDirectives: "error" } }, ...jsConfigs, ...unicornConfigs, ...jsdocConfigs, diff --git a/tests/lib/cli.js b/tests/lib/cli.js index 339b35e5f85..c69498231d0 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -1367,6 +1367,114 @@ describe("cli", () => { }); }); + describe("when passing --report-unused-disable-directives", () => { + describe(`config type: ${configType}`, () => { + it("errors when --report-unused-disable-directives", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives --rule "'no-console': 'error'"`, + "foo(); // eslint-disable-line no-console", + useFlatConfig); + + assert.strictEqual(log.error.callCount, 0, "log.error should not be called"); + assert.strictEqual(log.info.callCount, 1, "log.info is called once"); + assert.ok(log.info.firstCall.args[0].includes("Unused eslint-disable directive (no problems were reported from 'no-console')"), "has correct message about unused directives"); + assert.ok(log.info.firstCall.args[0].includes("1 error and 0 warning"), "has correct error and warning count"); + assert.strictEqual(exitCode, 1, "exit code should be 1"); + }); + + it("errors when --report-unused-disable-directives-severity error", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives-severity error --rule "'no-console': 'error'"`, + "foo(); // eslint-disable-line no-console", + useFlatConfig); + + assert.strictEqual(log.error.callCount, 0, "log.error should not be called"); + assert.strictEqual(log.info.callCount, 1, "log.info is called once"); + assert.ok(log.info.firstCall.args[0].includes("Unused eslint-disable directive (no problems were reported from 'no-console')"), "has correct message about unused directives"); + assert.ok(log.info.firstCall.args[0].includes("1 error and 0 warning"), "has correct error and warning count"); + assert.strictEqual(exitCode, 1, "exit code should be 1"); + }); + + it("errors when --report-unused-disable-directives-severity 2", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives-severity 2 --rule "'no-console': 'error'"`, + "foo(); // eslint-disable-line no-console", + useFlatConfig); + + assert.strictEqual(log.error.callCount, 0, "log.error should not be called"); + assert.strictEqual(log.info.callCount, 1, "log.info is called once"); + assert.ok(log.info.firstCall.args[0].includes("Unused eslint-disable directive (no problems were reported from 'no-console')"), "has correct message about unused directives"); + assert.ok(log.info.firstCall.args[0].includes("1 error and 0 warning"), "has correct error and warning count"); + assert.strictEqual(exitCode, 1, "exit code should be 1"); + }); + + it("warns when --report-unused-disable-directives-severity warn", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives-severity warn --rule "'no-console': 'error'""`, + "foo(); // eslint-disable-line no-console", + useFlatConfig); + + assert.strictEqual(log.error.callCount, 0, "log.error should not be called"); + assert.strictEqual(log.info.callCount, 1, "log.info is called once"); + assert.ok(log.info.firstCall.args[0].includes("Unused eslint-disable directive (no problems were reported from 'no-console')"), "has correct message about unused directives"); + assert.ok(log.info.firstCall.args[0].includes("0 errors and 1 warning"), "has correct error and warning count"); + assert.strictEqual(exitCode, 0, "exit code should be 0"); + }); + + it("warns when --report-unused-disable-directives-severity 1", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives-severity 1 --rule "'no-console': 'error'"`, + "foo(); // eslint-disable-line no-console", + useFlatConfig); + + assert.strictEqual(log.error.callCount, 0, "log.error should not be called"); + assert.strictEqual(log.info.callCount, 1, "log.info is called once"); + assert.ok(log.info.firstCall.args[0].includes("Unused eslint-disable directive (no problems were reported from 'no-console')"), "has correct message about unused directives"); + assert.ok(log.info.firstCall.args[0].includes("0 errors and 1 warning"), "has correct error and warning count"); + assert.strictEqual(exitCode, 0, "exit code should be 0"); + }); + + it("does not report when --report-unused-disable-directives-severity off", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives-severity off --rule "'no-console': 'error'"`, + "foo(); // eslint-disable-line no-console", + useFlatConfig); + + assert.strictEqual(log.error.callCount, 0, "log.error should not be called"); + assert.strictEqual(log.info.callCount, 0, "log.info should not be called"); + assert.strictEqual(exitCode, 0, "exit code should be 0"); + }); + + it("does not report when --report-unused-disable-directives-severity 0", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives-severity 0 --rule "'no-console': 'error'"`, + "foo(); // eslint-disable-line no-console", + useFlatConfig); + + assert.strictEqual(log.error.callCount, 0, "log.error should not be called"); + assert.strictEqual(log.info.callCount, 0, "log.info should not be called"); + assert.strictEqual(exitCode, 0, "exit code should be 0"); + }); + + it("fails when passing invalid string for --report-unused-disable-directives-severity", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives-severity foo`, null, useFlatConfig); + + assert.strictEqual(log.info.callCount, 0, "log.info should not be called"); + assert.strictEqual(log.error.callCount, 1, "log.error should be called once"); + + const lines = ["Option report-unused-disable-directives-severity: 'foo' not one of off, warn, error, 0, 1, or 2."]; + + if (useFlatConfig) { + lines.push("You're using eslint.config.js, some command line flags are no longer available. Please see https://eslint.org/docs/latest/use/command-line-interface for details."); + } + assert.deepStrictEqual(log.error.firstCall.args, [lines.join("\n")], "has the right text to log.error"); + assert.strictEqual(exitCode, 2, "exit code should be 2"); + }); + + it("fails when passing both --report-unused-disable-directives and --report-unused-disable-directives-severity", async () => { + const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --report-unused-disable-directives --report-unused-disable-directives-severity warn`, null, useFlatConfig); + + assert.strictEqual(log.info.callCount, 0, "log.info should not be called"); + assert.strictEqual(log.error.callCount, 1, "log.error should be called once"); + assert.deepStrictEqual(log.error.firstCall.args, ["The --report-unused-disable-directives option and the --report-unused-disable-directives-severity option cannot be used together."], "has the right text to log.error"); + assert.strictEqual(exitCode, 2, "exit code should be 2"); + }); + }); + }); + // --------- }); diff --git a/tests/lib/config/flat-config-array.js b/tests/lib/config/flat-config-array.js index b0dbfec93f7..79f80bce425 100644 --- a/tests/lib/config/flat-config-array.js +++ b/tests/lib/config/flat-config-array.js @@ -1036,28 +1036,28 @@ describe("FlatConfigArray", () => { await assertInvalidConfig([ { linterOptions: { - reportUnusedDisableDirectives: "true" + reportUnusedDisableDirectives: {} } } - ], /Expected a Boolean/u); + ], /Expected one of: "error", "warn", "off", 0, 1, 2, or a boolean./u); }); it("should merge two objects when second object has overrides", () => assertMergedResult([ { linterOptions: { - reportUnusedDisableDirectives: false + reportUnusedDisableDirectives: "off" } }, { linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" } } ], { plugins: baseConfig.plugins, linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: 1 } })); @@ -1065,14 +1065,14 @@ describe("FlatConfigArray", () => { {}, { linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" } } ], { plugins: baseConfig.plugins, linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: 1 } })); diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index 4311537387f..d71da6936e9 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -185,11 +185,12 @@ describe("FlatESLint", () => { parser: "", parserOptions: {}, rules: {}, - plugins: [] + plugins: [], + reportUnusedDisableDirectives: "error" }), new RegExp(escapeStringRegExp([ "Invalid Options:", - "- Unknown options: cacheFile, configFile, envs, globals, ignorePath, ignorePattern, parser, parserOptions, rules" + "- Unknown options: cacheFile, configFile, envs, globals, ignorePath, ignorePattern, parser, parserOptions, rules, reportUnusedDisableDirectives" ].join("\n")), "u") ); }); @@ -211,7 +212,6 @@ describe("FlatESLint", () => { overrideConfig: "", overrideConfigFile: "", plugins: "", - reportUnusedDisableDirectives: "", warnIgnored: "" }), new RegExp(escapeStringRegExp([ @@ -230,7 +230,6 @@ describe("FlatESLint", () => { "- 'overrideConfig' must be an object or null.", "- 'overrideConfigFile' must be a non-empty string, null, or true.", "- 'plugins' must be an object or null.", - "- 'reportUnusedDisableDirectives' must be any of \"error\", \"warn\", \"off\", and null.", "- 'warnIgnored' must be a boolean." ].join("\n")), "u") ); @@ -3866,16 +3865,110 @@ describe("FlatESLint", () => { const root = getFixturePath("cli-engine/reportUnusedDisableDirectives"); let cleanup; + let i = 0; beforeEach(() => { cleanup = () => { }; + i++; }); afterEach(() => cleanup()); - it("should warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives' was given.", async () => { + it("should error unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = error'.", async () => { const teardown = createCustomTeardown({ - cwd: root, + cwd: `${root}${i}`, + files: { + "test.js": "/* eslint-disable eqeqeq */", + "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: 'error' } }" + } + }); + + + await teardown.prepare(); + cleanup = teardown.cleanup; + eslint = new FlatESLint({ cwd: teardown.getPath() }); + + const results = await eslint.lintFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 1); + assert.strictEqual(messages[0].severity, 2); + assert.strictEqual(messages[0].message, "Unused eslint-disable directive (no problems were reported from 'eqeqeq')."); + assert.strictEqual(results[0].suppressedMessages.length, 0); + }); + + it("should error unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = 2'.", async () => { + const teardown = createCustomTeardown({ + cwd: `${root}${i}`, + files: { + "test.js": "/* eslint-disable eqeqeq */", + "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: 2 } }" + } + }); + + + await teardown.prepare(); + cleanup = teardown.cleanup; + eslint = new FlatESLint({ cwd: teardown.getPath() }); + + const results = await eslint.lintFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 1); + assert.strictEqual(messages[0].severity, 2); + assert.strictEqual(messages[0].message, "Unused eslint-disable directive (no problems were reported from 'eqeqeq')."); + assert.strictEqual(results[0].suppressedMessages.length, 0); + }); + + it("should warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = warn'.", async () => { + const teardown = createCustomTeardown({ + cwd: `${root}${i}`, + files: { + "test.js": "/* eslint-disable eqeqeq */", + "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: 'warn' } }" + } + }); + + + await teardown.prepare(); + cleanup = teardown.cleanup; + eslint = new FlatESLint({ cwd: teardown.getPath() }); + + const results = await eslint.lintFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 1); + assert.strictEqual(messages[0].severity, 1); + assert.strictEqual(messages[0].message, "Unused eslint-disable directive (no problems were reported from 'eqeqeq')."); + assert.strictEqual(results[0].suppressedMessages.length, 0); + }); + + it("should warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = 1'.", async () => { + const teardown = createCustomTeardown({ + cwd: `${root}${i}`, + files: { + "test.js": "/* eslint-disable eqeqeq */", + "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: 1 } }" + } + }); + + + await teardown.prepare(); + cleanup = teardown.cleanup; + eslint = new FlatESLint({ cwd: teardown.getPath() }); + + const results = await eslint.lintFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 1); + assert.strictEqual(messages[0].severity, 1); + assert.strictEqual(messages[0].message, "Unused eslint-disable directive (no problems were reported from 'eqeqeq')."); + assert.strictEqual(results[0].suppressedMessages.length, 0); + }); + + it("should warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = true'.", async () => { + const teardown = createCustomTeardown({ + cwd: `${root}${i}`, files: { "test.js": "/* eslint-disable eqeqeq */", "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: true } }" @@ -3896,13 +3989,73 @@ describe("FlatESLint", () => { assert.strictEqual(results[0].suppressedMessages.length, 0); }); + it("should not warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = false'.", async () => { + const teardown = createCustomTeardown({ + cwd: `${root}${i}`, + files: { + "test.js": "/* eslint-disable eqeqeq */", + "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: false } }" + } + }); + + + await teardown.prepare(); + cleanup = teardown.cleanup; + eslint = new FlatESLint({ cwd: teardown.getPath() }); + + const results = await eslint.lintFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 0); + }); + + it("should not warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = off'.", async () => { + const teardown = createCustomTeardown({ + cwd: `${root}${i}`, + files: { + "test.js": "/* eslint-disable eqeqeq */", + "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: 'off' } }" + } + }); + + + await teardown.prepare(); + cleanup = teardown.cleanup; + eslint = new FlatESLint({ cwd: teardown.getPath() }); + + const results = await eslint.lintFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 0); + }); + + it("should not warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives = 0'.", async () => { + const teardown = createCustomTeardown({ + cwd: `${root}${i}`, + files: { + "test.js": "/* eslint-disable eqeqeq */", + "eslint.config.js": "module.exports = { linterOptions: { reportUnusedDisableDirectives: 0 } }" + } + }); + + + await teardown.prepare(); + cleanup = teardown.cleanup; + eslint = new FlatESLint({ cwd: teardown.getPath() }); + + const results = await eslint.lintFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 0); + }); + describe("the runtime option overrides config files.", () => { it("should not warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives=off' was given in runtime.", async () => { const teardown = createCustomTeardown({ - cwd: root, + cwd: `${root}${i}`, files: { "test.js": "/* eslint-disable eqeqeq */", - ".eslintrc.yml": "reportUnusedDisableDirectives: true" + "eslint.config.js": "module.exports = [{ linterOptions: { reportUnusedDisableDirectives: true } }]" } }); @@ -3911,7 +4064,9 @@ describe("FlatESLint", () => { eslint = new FlatESLint({ cwd: teardown.getPath(), - reportUnusedDisableDirectives: "off" + overrideConfig: { + linterOptions: { reportUnusedDisableDirectives: "off" } + } }); const results = await eslint.lintFiles(["test.js"]); @@ -3922,10 +4077,10 @@ describe("FlatESLint", () => { it("should warn unused 'eslint-disable' comments as error if 'reportUnusedDisableDirectives=error' was given in runtime.", async () => { const teardown = createCustomTeardown({ - cwd: root, + cwd: `${root}${i}`, files: { "test.js": "/* eslint-disable eqeqeq */", - ".eslintrc.yml": "reportUnusedDisableDirectives: true" + "eslint.config.js": "module.exports = [{ linterOptions: { reportUnusedDisableDirectives: true } }]" } }); @@ -3934,7 +4089,9 @@ describe("FlatESLint", () => { eslint = new FlatESLint({ cwd: teardown.getPath(), - reportUnusedDisableDirectives: "error" + overrideConfig: { + linterOptions: { reportUnusedDisableDirectives: "error" } + } }); const results = await eslint.lintFiles(["test.js"]); @@ -4767,9 +4924,11 @@ describe("FlatESLint", () => { overrideConfig: { rules: { "no-var": "warn" + }, + linterOptions: { + reportUnusedDisableDirectives: "warn" } - }, - reportUnusedDisableDirectives: "warn" + } }); { @@ -4795,8 +4954,7 @@ describe("FlatESLint", () => { it("should return a non-empty value if some of the messages are related to a rule", async () => { const engine = new FlatESLint({ overrideConfigFile: true, - overrideConfig: { rules: { "no-var": "warn" } }, - reportUnusedDisableDirectives: "warn" + overrideConfig: { rules: { "no-var": "warn" }, linterOptions: { reportUnusedDisableDirectives: "warn" } } }); const results = await engine.lintText("// eslint-disable-line no-var\nvar foo;"); @@ -4971,7 +5129,7 @@ describe("FlatESLint", () => { describe("when evaluating code when reportUnusedDisableDirectives is enabled", () => { it("should report problems for unused eslint-disable directives", async () => { - const eslint = new FlatESLint({ overrideConfigFile: true, reportUnusedDisableDirectives: "error" }); + const eslint = new FlatESLint({ overrideConfigFile: true, overrideConfig: { linterOptions: { reportUnusedDisableDirectives: "error" } } }); assert.deepStrictEqual( await eslint.lintText("/* eslint-disable */"), diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 842d345f294..dec2aabb67d 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -15589,7 +15589,7 @@ var a = "test2"; assert.strictEqual(suppressedMessages.length, 0); }); - it("reports problems for unused eslint-disable comments (in config)", () => { + it("reports problems for unused eslint-disable comments (in config) (boolean value, true)", () => { const messages = linter.verify("/* eslint-disable */", { linterOptions: { reportUnusedDisableDirectives: true @@ -15618,11 +15618,82 @@ var a = "test2"; assert.strictEqual(suppressedMessages.length, 0); }); + it("does not report problems for unused eslint-disable comments (in config) (boolean value, false)", () => { + const messages = linter.verify("/* eslint-disable */", { + linterOptions: { + reportUnusedDisableDirectives: false + } + }); + const suppressedMessages = linter.getSuppressedMessages(); + + assert.deepStrictEqual(messages, []); + + assert.strictEqual(suppressedMessages.length, 0); + }); + + it("does not report problems for unused eslint-disable comments (in config) (string value, off)", () => { + const messages = linter.verify("/* eslint-disable */", { + linterOptions: { + reportUnusedDisableDirectives: "off" + } + }); + const suppressedMessages = linter.getSuppressedMessages(); + + assert.deepStrictEqual(messages, []); + + assert.strictEqual(suppressedMessages.length, 0); + }); + + it("reports problems for unused eslint-disable comments (in config) (string value, error)", () => { + const messages = linter.verify("/* eslint-disable */", { + linterOptions: { + reportUnusedDisableDirectives: "error" + } + }); + const suppressedMessages = linter.getSuppressedMessages(); + + assert.deepStrictEqual( + messages, + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + fix: { + range: [0, 20], + text: " " + }, + severity: 2, + nodeType: null + } + ] + ); + + assert.strictEqual(suppressedMessages.length, 0); + }); + + it("throws with invalid string for reportUnusedDisableDirectives in config", () => { + assert.throws(() => linter.verify("/* eslint-disable */", { + linterOptions: { + reportUnusedDisableDirectives: "foo" + } + }), 'Key "linterOptions": Key "reportUnusedDisableDirectives": Expected one of: "error", "warn", "off", 0, 1, 2, or a boolean.'); + }); + + it("throws with invalid type for reportUnusedDisableDirectives in config", () => { + assert.throws(() => linter.verify("/* eslint-disable */", { + linterOptions: { + reportUnusedDisableDirectives: {} + } + }), 'Key "linterOptions": Key "reportUnusedDisableDirectives": Expected one of: "error", "warn", "off", 0, 1, 2, or a boolean.'); + }); + it("reports problems for partially unused eslint-disable comments (in config)", () => { const code = "alert('test'); // eslint-disable-line no-alert, no-redeclare"; const config = { linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" }, rules: { "no-alert": 1, @@ -15662,7 +15733,7 @@ var a = "test2"; assert.deepStrictEqual( linter.verify("// eslint-disable-next-line", { linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" } }), [ @@ -15686,7 +15757,7 @@ var a = "test2"; assert.deepStrictEqual( linter.verify("/* \neslint-disable-next-line\n */", { linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" } }), [ @@ -15710,7 +15781,7 @@ var a = "test2"; const code = "// eslint-disable-next-line no-alert, no-redeclare \nalert('test');"; const config = { linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" }, rules: { "no-alert": 1, @@ -15752,7 +15823,7 @@ var a = "test2"; `; const config = { linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" }, rules: { "no-alert": 1, @@ -16109,7 +16180,7 @@ var a = "test2"; } }, linterOptions: { - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "warn" }, rules: { ...Object.fromEntries(usedRules.map(name => [`test/${name}`, "error"])),