Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion conf/default-cli-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ module.exports = {
cacheLocation: "",
cacheFile: ".eslintcache",
fix: false,
allowInlineConfig: true
allowInlineConfig: true,
reportUnusedDisableDirectives: false
};
2 changes: 2 additions & 0 deletions docs/developer-guide/nodejs-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down Expand Up @@ -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`.
Expand Down
67 changes: 39 additions & 28 deletions docs/user-guide/command-line-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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.
Expand Down
27 changes: 24 additions & 3 deletions lib/cli-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}

Expand Down
11 changes: 8 additions & 3 deletions lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -734,20 +734,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;
}
Expand Down Expand Up @@ -968,7 +972,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
});
}

Expand Down
6 changes: 6 additions & 0 deletions lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down