New: add option to report unused eslint-disable directives (fixes #9249…

…) (#9250)

conf/default-cli-options.js

@@ -23,5 +23,6 @@ module.exports = {
     cacheLocation: "",
     cacheFile: ".eslintcache",
     fix: false,
-    allowInlineConfig: true
+    allowInlineConfig: true,
+    reportUnusedDisableDirectives: false
 };

docs/developer-guide/nodejs-api.md

@@ -70,6 +70,7 @@ The most important method on `Linter` is `verify()`, which initiates linting of
     * `preprocess` - (optional) A function that accepts a string containing source text, and returns an array of strings containing blocks of code to lint. Also see: [Processors in Plugins](/docs/developer-guide/working-with-plugins#processors-in-plugins)
     * `postprocess` - (optional) A function that accepts an array of problem lists (one list of problems for each block of code from `preprocess`), and returns a one-dimensional array of problems containing problems for the original, unprocessed text. Also see: [Processors in Plugins](/docs/developer-guide/working-with-plugins#processors-in-plugins)
     * `allowInlineConfig` - (optional) set to `false` to disable inline comments from changing eslint rules.
+    * `reportUnusedDisableDirectives` - (optional) when set to `true`, adds reported errors for unused `eslint-disable` directives when no problems would be reported in the disabled area anyway.
 
 If the third argument is a string, it is interpreted as the `filename`.
 
@@ -297,6 +298,7 @@ The `CLIEngine` is a constructor, and you can create a new instance by passing i
 * `parser` - Specify the parser to be used (default: `espree`). Corresponds to `--parser`.
 * `parserOptions` - An object containing parser options (default: empty object). Corresponds to `--parser-options`.
 * `plugins` - An array of plugins to load (default: empty array). Corresponds to `--plugin`.
+* `reportUnusedDisableDirectives` - When set to `true`, adds reported errors for unused `eslint-disable` directives when no problems would be reported in the disabled area anyway (default: false). Corresponds to `--report-unused-disable-directives`.
 * `rulePaths` - An array of directories to load custom rules from (default: empty array). Corresponds to `--rulesdir`.
 * `rules` - An object of rules to use (default: null). Corresponds to `--rule`.
 * `useEslintrc` - Set to false to disable use of `.eslintrc` files (default: true). Corresponds to `--no-eslintrc`.

docs/user-guide/command-line-interface.md

@@ -30,50 +30,51 @@ The command line utility has several options. You can view the options by runnin
 eslint [options] file.js [file.js] [dir]
 
 Basic configuration:
-  -c, --config path::String    Use configuration from this file or shareable config
-  --no-eslintrc                Disable use of configuration from .eslintrc
-  --env [String]               Specify environments
-  --ext [String]               Specify JavaScript file extensions - default: .js
-  --global [String]            Define global variables
-  --parser String              Specify the parser to be used
-  --parser-options Object      Specify parser options
+  -c, --config path::String      Use configuration from this file or shareable config
+  --no-eslintrc                  Disable use of configuration from .eslintrc
+  --env [String]                 Specify environments
+  --ext [String]                 Specify JavaScript file extensions - default: .js
+  --global [String]              Define global variables
+  --parser String                Specify the parser to be used
+  --parser-options Object        Specify parser options
 
 Caching:
-  --cache                      Only check changed files - default: false
-  --cache-file path::String    Path to the cache file. Deprecated: use --cache-location - default: .eslintcache
+  --cache                        Only check changed files - default: false
+  --cache-file path::String      Path to the cache file. Deprecated: use --cache-location - default: .eslintcache
   --cache-location path::String  Path to the cache file or directory
 
 Specifying rules and plugins:
-  --rulesdir [path::String]    Use additional rules from this directory
-  --plugin [String]            Specify plugins
-  --rule Object                Specify rules
+  --rulesdir [path::String]      Use additional rules from this directory
+  --plugin [String]              Specify plugins
+  --rule Object                  Specify rules
 
 Ignoring files:
-  --ignore-path path::String   Specify path of ignore file
-  --no-ignore                  Disable use of ignore files and patterns
-  --ignore-pattern [String]    Pattern of files to ignore (in addition to those in .eslintignore)
+  --ignore-path path::String     Specify path of ignore file
+  --no-ignore                    Disable use of ignore files and patterns
+  --ignore-pattern [String]      Pattern of files to ignore (in addition to those in .eslintignore)
 
 Using stdin:
-  --stdin                      Lint code provided on <STDIN> - default: false
-  --stdin-filename String      Specify filename to process STDIN as
+  --stdin                        Lint code provided on <STDIN> - default: false
+  --stdin-filename String        Specify filename to process STDIN as
 
 Handling warnings:
-  --quiet                      Report errors only - default: false
-  --max-warnings Int           Number of warnings to trigger nonzero exit code - default: -1
+  --quiet                        Report errors only - default: false
+  --max-warnings Int             Number of warnings to trigger nonzero exit code - default: -1
 
 Output:
   -o, --output-file path::String  Specify file to write report to
-  -f, --format String          Use a specific output format - default: stylish
-  --color, --no-color          Force enabling/disabling of color
+  -f, --format String            Use a specific output format - default: stylish
+  --color, --no-color            Force enabling/disabling of color
 
 Miscellaneous:
-  --init                       Run config initialization wizard - default: false
-  --fix                        Automatically fix problems
-  --debug                      Output debugging information
-  -h, --help                   Show help
-  -v, --version                Output the version number
-  --no-inline-config           Prevent comments from changing config or rules
-  --print-config path::String  Print the configuration for the given file
+  --init                         Run config initialization wizard - default: false
+  --fix                          Automatically fix problems
+  --debug                        Output debugging information
+  -h, --help                     Show help
+  -v, --version                  Output the version number
+  --no-inline-config             Prevent comments from changing config or rules
+  --report-unused-disable-directives  Adds reported errors for unused eslint-disable directives
+  --print-config path::String    Print the configuration for the given file
 ```
 
 Options that accept array values can be specified by repeating the option or with a comma-delimited list (other than `--ignore-pattern` which does not allow the second style).
@@ -391,6 +392,16 @@ Example:
 
     eslint --no-inline-config file.js
 
+#### `--report-unused-disable-directives`
+
+This option causes ESLint to report directive comments like `// eslint-disable-line` when no errors would have been reported on that line anyway. This can be useful to prevent future errors from unexpectedly being suppressed, by cleaning up old `eslint-disable` comments which are no longer applicable.
+
+**Warning**: When using this option, it is possible that new errors will 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 will become unused since ESLint is no longer generating an incorrect report. This will result in a new reported error for the unused directive if the `report-unused-disable-directives` option is used.
+
+Example:
+
+    eslint --report-unused-disable-directives file.js
+
 #### `--print-config`
 
 This option outputs the configuration to be used for the file passed. When present, no linting is performed and only config-related options are valid.

lib/cli-engine.js

@@ -56,6 +56,7 @@ const debug = require("debug")("eslint:cli-engine");
  * @property {string[]} plugins An array of plugins to load.
  * @property {Object<string,*>} 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
  */
 
 /**
@@ -137,11 +138,12 @@ function calculateStatsPerRun(results) {
  * @param {string} filename An optional string representing the texts filename.
  * @param {boolean|Function} fix Indicates if fixes should be processed.
  * @param {boolean} allowInlineConfig Allow/ignore comments that change config.
+ * @param {boolean} reportUnusedDisableDirectives Allow/ignore comments that change config.
  * @param {Linter} linter Linter context
  * @returns {LintResult} The results for linting on this text.
  * @private
  */
-function processText(text, configHelper, filename, fix, allowInlineConfig, linter) {
+function processText(text, configHelper, filename, fix, allowInlineConfig, reportUnusedDisableDirectives, linter) {
     let filePath,
         fileExtension,
         processor;
@@ -173,6 +175,7 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, linte
     const fixedResult = linter.verifyAndFix(text, config, {
         filename,
         allowInlineConfig,
+        reportUnusedDisableDirectives,
         fix: !!autofixingEnabled && fix,
         preprocess: processor && (rawText => processor.preprocess(rawText, filename)),
         postprocess: processor && (problemLists => processor.postprocess(problemLists, filename))
@@ -213,7 +216,15 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, linte
 function processFile(filename, configHelper, options, linter) {
 
     const text = fs.readFileSync(path.resolve(filename), "utf8"),
-        result = processText(text, configHelper, filename, options.fix, options.allowInlineConfig, linter);
+        result = processText(
+            text,
+            configHelper,
+            filename,
+            options.fix,
+            options.allowInlineConfig,
+            options.reportUnusedDisableDirectives,
+            linter
+        );
 
     return result;
 
@@ -591,7 +602,17 @@ class CLIEngine {
                 results.push(createIgnoreResult(filename, options.cwd));
             }
         } else {
-            results.push(processText(text, configHelper, filename, options.fix, options.allowInlineConfig, this.linter));
+            results.push(
+                processText(
+                    text,
+                    configHelper,
+                    filename,
+                    options.fix,
+                    options.allowInlineConfig,
+                    options.reportUnusedDisableDirectives,
+                    this.linter
+                )
+            );
         }
 
         const stats = calculateStatsPerRun(results);

lib/cli.js

@@ -64,7 +64,8 @@ function translateOptions(cliOptions) {
         cacheFile: cliOptions.cacheFile,
         cacheLocation: cliOptions.cacheLocation,
         fix: cliOptions.fix && (cliOptions.quiet ? quietFixPredicate : true),
-        allowInlineConfig: cliOptions.inlineConfig
+        allowInlineConfig: cliOptions.inlineConfig,
+        reportUnusedDisableDirectives: cliOptions.reportUnusedDisableDirectives
     };
 }
 

lib/linter.js

@@ -735,20 +735,24 @@ module.exports = class Linter {
      * @param {(string|Object)} [filenameOrOptions] The optional filename of the file being checked.
      *      If this is not set, the filename will default to '<input>' in the rule context. If
      *      an object, then it has "filename", "saveState", and "allowInlineConfig" properties.
-     * @param {boolean} [filenameOrOptions.allowInlineConfig] Allow/disallow inline comments' ability to change config once it is set. Defaults to true if not supplied.
+     * @param {boolean} [filenameOrOptions.allowInlineConfig=true] Allow/disallow inline comments' ability to change config once it is set. Defaults to true if not supplied.
      *      Useful if you want to validate JS without comments overriding rules.
+     * @param {boolean} [filenameOrOptions.reportUnusedDisableDirectives=false] Adds reported errors for unused
+     *      eslint-disable directives
      * @returns {Object[]} The results as an array of messages or null if no messages.
      */
     _verifyWithoutProcessors(textOrSourceCode, config, filenameOrOptions) {
         let text,
             parserServices,
             allowInlineConfig,
-            providedFilename;
+            providedFilename,
+            reportUnusedDisableDirectives;
 
         // evaluate arguments
         if (typeof filenameOrOptions === "object") {
             providedFilename = filenameOrOptions.filename;
             allowInlineConfig = filenameOrOptions.allowInlineConfig;
+            reportUnusedDisableDirectives = filenameOrOptions.reportUnusedDisableDirectives;
         } else {
             providedFilename = filenameOrOptions;
         }
@@ -969,7 +973,8 @@ module.exports = class Linter {
 
         return applyDisableDirectives({
             directives: disableDirectives,
-            problems: problems.sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column)
+            problems: problems.sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column),
+            reportUnusedDisableDirectives
         });
     }
 

lib/options.js

@@ -214,6 +214,12 @@ module.exports = optionator({
             default: "true",
             description: "Prevent comments from changing config or rules"
         },
+        {
+            option: "report-unused-disable-directives",
+            type: "Boolean",
+            default: false,
+            description: "Adds reported errors for unused eslint-disable directives"
+        },
         {
             option: "print-config",
             type: "path::String",