diff --git a/conf/config-schema.js b/conf/config-schema.js index d89bf0f58c5..164f0b4219f 100644 --- a/conf/config-schema.js +++ b/conf/config-schema.js @@ -21,6 +21,7 @@ const baseConfigProperties = { rules: { type: "object" }, settings: { type: "object" }, noInlineConfig: { type: "boolean" }, + reportUnusedDisableDirectives: { type: "boolean" }, ecmaFeatures: { type: "object" } // deprecated; logs a warning when used }; diff --git a/conf/default-cli-options.js b/conf/default-cli-options.js index 9670a14b00c..abbd9184e21 100644 --- a/conf/default-cli-options.js +++ b/conf/default-cli-options.js @@ -26,6 +26,6 @@ module.exports = { cacheFile: ".eslintcache", fix: false, allowInlineConfig: true, - reportUnusedDisableDirectives: false, + reportUnusedDisableDirectives: void 0, globInputPaths: true }; diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index aa1c82b9838..0818f85e2a5 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -530,6 +530,34 @@ To disable rules inside of a configuration file for a group of files, use the `o } ``` +## Configuring Inline Comment Behaviors + +### Disabling Inline Comments + +To disable all inline config comments, use `noInlineConfig` setting. For example: + +```json +{ + "rules": {...}, + "noInlineConfig": true +} +``` + +This setting is similar to [--no-inline-config](./command-line-interface.md#--no-inline-config) CLI option. + +### Report Unused `eslint-disable` Comments + +To report unused `eslint-disable` comments, use `reportUnusedDisableDirectives` setting. For example: + +```json +{ + "rules": {...}, + "reportUnusedDisableDirectives": true +} +``` + +This setting is similar to [--report-unused-disable-directives](./command-line-interface.md#--report-unused-disable-directives) CLI option, but doesn't fail linting (reports as `"warn"` severity). + ## Adding Shared Settings ESLint supports adding shared settings into configuration file. You can add `settings` object to ESLint configuration file and it will be supplied to every rule that will be executed. This may be useful if you are adding custom rules and want them to have access to the same information and be easily configurable. diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 0b2ed07b6a0..6e1ba1e02b9 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -531,6 +531,7 @@ class ConfigArrayFactory { parserOptions, plugins: pluginList, processor, + reportUnusedDisableDirectives, root, rules, settings, @@ -573,6 +574,7 @@ class ConfigArrayFactory { parserOptions, plugins, processor, + reportUnusedDisableDirectives, root, rules, settings diff --git a/lib/cli-engine/config-array/config-array.js b/lib/cli-engine/config-array/config-array.js index 0859868d824..6383c02115f 100644 --- a/lib/cli-engine/config-array/config-array.js +++ b/lib/cli-engine/config-array/config-array.js @@ -59,6 +59,7 @@ const { ExtractedConfig } = require("./extracted-config"); * @property {Object|undefined} parserOptions The parser options. * @property {Record|undefined} plugins The plugin loaders. * @property {string|undefined} processor The processor name to refer plugin's processor. + * @property {boolean|undefined} reportUnusedDisableDirectives The flag to report unused `eslint-disable` comments. * @property {boolean|undefined} root The flag to express root. * @property {Record|undefined} rules The rule settings * @property {Object|undefined} settings The shared settings. @@ -257,6 +258,11 @@ function createConfig(instance, indices) { config.configNameOfNoInlineConfig = element.name; } + // Adopt the reportUnusedDisableDirectives which was found at first. + if (config.reportUnusedDisableDirectives === void 0 && element.reportUnusedDisableDirectives !== void 0) { + config.reportUnusedDisableDirectives = element.reportUnusedDisableDirectives; + } + // Merge others. mergeWithoutOverwrite(config.env, element.env); mergeWithoutOverwrite(config.globals, element.globals); diff --git a/lib/cli-engine/config-array/extracted-config.js b/lib/cli-engine/config-array/extracted-config.js index 53208c16e46..66858313ba6 100644 --- a/lib/cli-engine/config-array/extracted-config.js +++ b/lib/cli-engine/config-array/extracted-config.js @@ -77,6 +77,12 @@ class ExtractedConfig { */ this.processor = null; + /** + * The flag that reports unused `eslint-disable` directive comments. + * @type {boolean|undefined} + */ + this.reportUnusedDisableDirectives = void 0; + /** * Rule settings. * @type {Record} diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index c764a9b7020..41d6934abba 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -93,7 +93,7 @@ function applyDirectives(options) { : "Unused eslint-disable directive (no problems were reported).", line: directive.unprocessedDirective.line, column: directive.unprocessedDirective.column, - severity: 2, + severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2, nodeType: null })); @@ -114,17 +114,17 @@ function applyDirectives(options) { * 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 {boolean} options.reportUnusedDisableDirectives If `true`, adds additional problems for unused directives + * @param {"off" | "warn" | "error"} options.reportUnusedDisableDirectives If `"warn"` or `"error"`, adds additional problems for unused directives * @returns {{ruleId: (string|null), line: number, column: number}[]} * A list of reported problems that were not disabled by the directive comments. */ -module.exports = options => { - const blockDirectives = options.directives +module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off" }) => { + const blockDirectives = directives .filter(directive => directive.type === "disable" || directive.type === "enable") .map(directive => Object.assign({}, directive, { unprocessedDirective: directive })) .sort(compareLocations); - const lineDirectives = lodash.flatMap(options.directives, directive => { + const lineDirectives = lodash.flatMap(directives, directive => { switch (directive.type) { case "disable": case "enable": @@ -147,10 +147,18 @@ module.exports = options => { } }).sort(compareLocations); - const blockDirectivesResult = applyDirectives({ problems: options.problems, directives: blockDirectives }); - const lineDirectivesResult = applyDirectives({ problems: blockDirectivesResult.problems, directives: lineDirectives }); - - return options.reportUnusedDisableDirectives + const blockDirectivesResult = applyDirectives({ + problems, + directives: blockDirectives, + reportUnusedDisableDirectives + }); + const lineDirectivesResult = applyDirectives({ + problems: blockDirectivesResult.problems, + directives: lineDirectives, + reportUnusedDisableDirectives + }); + + return reportUnusedDisableDirectives !== "off" ? lineDirectivesResult.problems .concat(blockDirectivesResult.unusedDisableDirectives) .concat(lineDirectivesResult.unusedDisableDirectives) diff --git a/lib/linter/linter.js b/lib/linter/linter.js index d367cef6cb4..7d1dc8c8c1c 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -54,6 +54,11 @@ const DEFAULT_ERROR_LOC = { start: { line: 1, column: 0 }, end: { line: 1, colum /** @typedef {import("../shared/types").Processor} Processor */ /** @typedef {import("../shared/types").Rule} Rule */ +/** + * @template T + * @typedef {{ [P in keyof T]-?: T[P] }} Required + */ + /** * @typedef {Object} DisableDirective * @property {("disable"|"enable"|"disable-line"|"disable-next-line")} type @@ -79,7 +84,7 @@ const DEFAULT_ERROR_LOC = { start: { line: 1, column: 0 }, end: { line: 1, colum * @property {boolean} [disableFixes] if `true` then the linter doesn't make `fix` * properties into the lint result. * @property {string} [filename] the filename of the source code. - * @property {boolean} [reportUnusedDisableDirectives] Adds reported errors for + * @property {boolean | "off" | "warn" | "error"} [reportUnusedDisableDirectives] Adds reported errors for * unused `eslint-disable` directives. */ @@ -103,6 +108,12 @@ const DEFAULT_ERROR_LOC = { start: { line: 1, column: 0 }, end: { line: 1, colum * whether fixes should be applied. */ +/** + * @typedef {Object} InternalOptions + * @property {string | null} warnInlineConfig The config name what `noInlineConfig` setting came from. If `noInlineConfig` setting didn't exist, this is null. If this is a config name, then the linter warns directive comments. + * @property {"off" | "warn" | "error"} reportUnusedDisableDirectives (boolean values were normalized) + */ + //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ @@ -467,7 +478,7 @@ function normalizeFilename(filename) { * consistent shape. * @param {VerifyOptions} providedOptions Options * @param {ConfigData} config Config. - * @returns {Required & { warnInlineConfig: string|null }} Normalized options + * @returns {Required & InternalOptions} Normalized options */ function normalizeVerifyOptions(providedOptions, config) { const disableInlineConfig = config.noInlineConfig === true; @@ -476,13 +487,22 @@ function normalizeVerifyOptions(providedOptions, config) { ? ` (${config.configNameOfNoInlineConfig})` : ""; + let reportUnusedDisableDirectives = providedOptions.reportUnusedDisableDirectives; + + if (typeof reportUnusedDisableDirectives === "boolean") { + reportUnusedDisableDirectives = reportUnusedDisableDirectives ? "error" : "off"; + } + if (typeof reportUnusedDisableDirectives !== "string") { + reportUnusedDisableDirectives = config.reportUnusedDisableDirectives ? "warn" : "off"; + } + return { filename: normalizeFilename(providedOptions.filename || ""), allowInlineConfig: !ignoreInlineConfig, warnInlineConfig: disableInlineConfig && !ignoreInlineConfig ? `your config${configNameOfNoInlineConfig}` : null, - reportUnusedDisableDirectives: Boolean(providedOptions.reportUnusedDisableDirectives), + reportUnusedDisableDirectives, disableFixes: Boolean(providedOptions.disableFixes) }; } diff --git a/lib/options.js b/lib/options.js index be4c09b8eab..440773a844b 100644 --- a/lib/options.js +++ b/lib/options.js @@ -192,7 +192,7 @@ module.exports = optionator({ { option: "report-unused-disable-directives", type: "Boolean", - default: false, + default: void 0, description: "Adds reported errors for unused eslint-disable directives" }, { diff --git a/lib/shared/types.js b/lib/shared/types.js index 8a889d21db9..12bd0aed8c6 100644 --- a/lib/shared/types.js +++ b/lib/shared/types.js @@ -36,6 +36,7 @@ module.exports = {}; * @property {ParserOptions} [parserOptions] The parser options. * @property {string[]} [plugins] The plugin specifiers. * @property {string} [processor] The processor specifier. + * @property {boolean|undefined} reportUnusedDisableDirectives The flag to report unused `eslint-disable` comments. * @property {boolean} [root] The root flag. * @property {Record} [rules] The rule settings. * @property {Object} [settings] The shared settings. @@ -54,6 +55,7 @@ module.exports = {}; * @property {ParserOptions} [parserOptions] The parser options. * @property {string[]} [plugins] The plugin specifiers. * @property {string} [processor] The processor specifier. + * @property {boolean|undefined} reportUnusedDisableDirectives The flag to report unused `eslint-disable` comments. * @property {Record} [rules] The rule settings. * @property {Object} [settings] The shared settings. */ diff --git a/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js index dc42cfa6828..d8591788f58 100644 --- a/tests/lib/cli-engine/cli-engine.js +++ b/tests/lib/cli-engine/cli-engine.js @@ -3534,6 +3534,64 @@ describe("CLIEngine", () => { assert.strictEqual(messages[0].message, "'/*globals*/' has no effect because you have 'noInlineConfig' setting in your config (.eslintrc.yml ยป eslint-config-foo)."); }); }); + + describe("with 'reportUnusedDisableDirectives' setting", () => { + const root = getFixturePath("cli-engine/reportUnusedDisableDirectives"); + + it("should warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives' was given.", () => { + CLIEngine = defineCLIEngineWithInMemoryFileSystem({ + cwd: () => root, + files: { + "test.js": "/* eslint-disable eqeqeq */", + ".eslintrc.yml": "reportUnusedDisableDirectives: true" + } + }).CLIEngine; + engine = new CLIEngine(); + + const { results } = engine.executeOnFiles(["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')."); + }); + + describe("the runtime option overrides config files.", () => { + it("should not warn unused 'eslint-disable' comments if 'reportUnusedDisableDirectives=off' was given in runtime.", () => { + CLIEngine = defineCLIEngineWithInMemoryFileSystem({ + cwd: () => root, + files: { + "test.js": "/* eslint-disable eqeqeq */", + ".eslintrc.yml": "reportUnusedDisableDirectives: true" + } + }).CLIEngine; + engine = new CLIEngine({ reportUnusedDisableDirectives: "off" }); + + const { results } = engine.executeOnFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 0); + }); + + it("should warn unused 'eslint-disable' comments as error if 'reportUnusedDisableDirectives=error' was given in runtime.", () => { + CLIEngine = defineCLIEngineWithInMemoryFileSystem({ + cwd: () => root, + files: { + "test.js": "/* eslint-disable eqeqeq */", + ".eslintrc.yml": "reportUnusedDisableDirectives: true" + } + }).CLIEngine; + engine = new CLIEngine({ reportUnusedDisableDirectives: "error" }); + + const { results } = engine.executeOnFiles(["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')."); + }); + }); + }); }); describe("getConfigForFile", () => { diff --git a/tests/lib/cli-engine/config-array-factory.js b/tests/lib/cli-engine/config-array-factory.js index b9f404dca1d..8b86b0f84ff 100644 --- a/tests/lib/cli-engine/config-array-factory.js +++ b/tests/lib/cli-engine/config-array-factory.js @@ -35,6 +35,7 @@ function assertConfigArrayElement(actual, providedExpected) { parserOptions: void 0, plugins: void 0, processor: void 0, + reportUnusedDisableDirectives: void 0, root: void 0, rules: void 0, settings: void 0, @@ -58,6 +59,7 @@ function assertConfig(actual, providedExpected) { parser: null, parserOptions: {}, plugins: [], + reportUnusedDisableDirectives: void 0, rules: {}, settings: {}, ...providedExpected diff --git a/tests/lib/cli-engine/config-array/config-array.js b/tests/lib/cli-engine/config-array/config-array.js index 82f3e750729..4e248af9f43 100644 --- a/tests/lib/cli-engine/config-array/config-array.js +++ b/tests/lib/cli-engine/config-array/config-array.js @@ -436,6 +436,7 @@ describe("ConfigArray", () => { }, plugins: {}, processor: null, + reportUnusedDisableDirectives: void 0, rules: {}, settings: {} }); @@ -466,6 +467,7 @@ describe("ConfigArray", () => { }, plugins: {}, processor: null, + reportUnusedDisableDirectives: void 0, rules: {}, settings: {} }); @@ -607,7 +609,8 @@ describe("ConfigArray", () => { }, settings: {}, processor: null, - noInlineConfig: void 0 + noInlineConfig: void 0, + reportUnusedDisableDirectives: void 0 }); assert.deepStrictEqual(config[0], { rules: { diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 440b2810f50..84dddcba4e6 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -431,7 +431,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 5 }], problems: [], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -451,7 +451,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 5, ruleId: null }], problems: [{ line: 2, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [] ); @@ -462,7 +462,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 5, ruleId: "foo" }], problems: [], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -482,7 +482,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 5, ruleId: "foo" }], problems: [{ line: 1, column: 20, ruleId: "not-foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -510,7 +510,7 @@ describe("apply-disable-directives", () => { { type: "enable", line: 1, column: 6, ruleId: "foo" } ], problems: [{ line: 1, column: 2, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -538,7 +538,7 @@ describe("apply-disable-directives", () => { { type: "enable", line: 1, column: 6, ruleId: null } ], problems: [], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -561,7 +561,7 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: null } ], problems: [], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -592,7 +592,7 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: null } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -615,7 +615,7 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: null } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -635,7 +635,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 5, ruleId: "foo" }], problems: [{ line: 1, column: 6, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [] ); @@ -649,7 +649,7 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: "foo" } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -672,7 +672,7 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: "foo" } ], problems: [{ line: 3, column: 1, ruleId: "bar" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -695,7 +695,7 @@ describe("apply-disable-directives", () => { { type: "enable", line: 1, column: 8, ruleId: "foo" } ], problems: [{ line: 1, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -723,7 +723,7 @@ describe("apply-disable-directives", () => { { type: "enable", line: 1, column: 8, ruleId: null } ], problems: [{ line: 1, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -752,7 +752,7 @@ describe("apply-disable-directives", () => { { type: "enable", line: 3, column: 1, ruleId: "foo" } ], problems: [{ line: 4, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -786,7 +786,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable-line", line: 1, column: 5, ruleId: null }], problems: [], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -807,7 +807,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable-line", line: 1, column: 5, ruleId: null }], problems: [{ line: 1, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [] ); @@ -818,7 +818,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 5, ruleId: null }], problems: [], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -838,7 +838,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 5, ruleId: null }], problems: [{ line: 2, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [] ); @@ -852,7 +852,7 @@ describe("apply-disable-directives", () => { { type: "disable-line", line: 1, column: 5, ruleId: null } ], problems: [], - reportUnusedDisableDirectives: true + reportUnusedDisableDirectives: "error" }), [ { @@ -875,12 +875,12 @@ describe("apply-disable-directives", () => { ); }); - it("Does not add problems when reportUnusedDisableDirectives: false is used", () => { + it("Does not add problems when reportUnusedDisableDirectives: \"off\" is used", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 5, ruleId: null }], problems: [], - reportUnusedDisableDirectives: false + reportUnusedDisableDirectives: "off" }), [] ); diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index b1b621935d7..baadf43cad1 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3211,6 +3211,54 @@ describe("Linter", () => { ] ); }); + + it("reports problems for unused eslint-disable comments (error)", () => { + assert.deepStrictEqual( + linter.verify("/* eslint-disable */", {}, { reportUnusedDisableDirectives: "error" }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + severity: 2, + nodeType: null + } + ] + ); + }); + + it("reports problems for unused eslint-disable comments (warn)", () => { + assert.deepStrictEqual( + linter.verify("/* eslint-disable */", {}, { reportUnusedDisableDirectives: "warn" }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + severity: 1, + nodeType: null + } + ] + ); + }); + + it("reports problems for unused eslint-disable comments (in config)", () => { + assert.deepStrictEqual( + linter.verify("/* eslint-disable */", { reportUnusedDisableDirectives: true }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + severity: 1, + nodeType: null + } + ] + ); + }); }); describe("when evaluating code with comments to change config when allowInlineConfig is disabled", () => {