New: reportUnusedDisableDirectives in config (refs eslint/rfcs#22) (#…

…12151)

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
 };

conf/default-cli-options.js

@@ -26,6 +26,6 @@ module.exports = {
     cacheFile: ".eslintcache",
     fix: false,
     allowInlineConfig: true,
-    reportUnusedDisableDirectives: false,
+    reportUnusedDisableDirectives: void 0,
     globInputPaths: true
 };

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.

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

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<string, DependentPlugin>|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<string, RuleConf>|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);

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<string, [SeverityConf, ...any[]]>}

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)

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<VerifyOptions> & { warnInlineConfig: string|null }} Normalized options
+ * @returns {Required<VerifyOptions> & 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 || "<input>"),
         allowInlineConfig: !ignoreInlineConfig,
         warnInlineConfig: disableInlineConfig && !ignoreInlineConfig
             ? `your config${configNameOfNoInlineConfig}`
             : null,
-        reportUnusedDisableDirectives: Boolean(providedOptions.reportUnusedDisableDirectives),
+        reportUnusedDisableDirectives,
         disableFixes: Boolean(providedOptions.disableFixes)
     };
 }

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"
         },
         {

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<string, RuleConf>} [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<string, RuleConf>} [rules] The rule settings.
  * @property {Object} [settings] The shared settings.
  */

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", () => {

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

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: {