Skip to content

Commit

Permalink
feat: Support custom severity when reporting unused disable directives
Browse files Browse the repository at this point in the history
  • Loading branch information
bmish committed May 23, 2023
1 parent 7a2a0be commit bdd4a1a
Show file tree
Hide file tree
Showing 16 changed files with 181 additions and 23 deletions.
1 change: 1 addition & 0 deletions conf/default-cli-options.js
Expand Up @@ -28,5 +28,6 @@ module.exports = {
fix: false,
allowInlineConfig: true,
reportUnusedDisableDirectives: void 0,
reportUnusedDisableDirectivesSeverity: void 0,
globInputPaths: true
};
13 changes: 12 additions & 1 deletion docs/src/use/command-line-interface.md
Expand Up @@ -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 directives
--report-unused-disable-directives-severity String Choose what severity level that eslint-disable directives should be reported as
Caching:
--cache Only check changed files - default: false
Expand Down Expand Up @@ -581,7 +582,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
Expand All @@ -590,6 +591,16 @@ 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.

##### `--report-unused-disable-directives-severity` example

```shell
npx eslint --report-unused-disable-directives-severity warn file.js
```

### Caching

#### `--cache`
Expand Down
10 changes: 6 additions & 4 deletions docs/src/use/configure/configuration-files-new.md
Expand Up @@ -45,7 +45,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 directives should be tracked and reported.
* `reportUnusedDisableDirectives` - A severity string indicating if and how unused disable 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.
Expand Down Expand Up @@ -208,20 +208,22 @@ export default [

#### Reporting unused disable directives

Disable directives such as `/*eslint-disable*/` 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 directives such as `/*eslint-disable*/` 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 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.md#report-unused-disable-directives) or the [`--report-unused-disable-directives-severity`](../command-line-interface.md#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

Expand Down
4 changes: 2 additions & 2 deletions docs/src/use/configure/rules.md
Expand Up @@ -302,8 +302,8 @@ To report unused `eslint-disable` comments, use the `reportUnusedDisableDirectiv
```json
{
"rules": {...},
"reportUnusedDisableDirectives": true
"reportUnusedDisableDirectives": "error"
}
```

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.
4 changes: 2 additions & 2 deletions lib/cli-engine/cli-engine.js
Expand Up @@ -83,7 +83,7 @@ const validFixTypes = new Set(["directive", "problem", "suggestion", "layout"]);
* @property {string[]} [plugins] An array of plugins to load.
* @property {Record<string,RuleConf>} [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` or `error` 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
*/
Expand Down Expand Up @@ -215,7 +215,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` or `error`, 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.
Expand Down
8 changes: 7 additions & 1 deletion lib/cli.js
Expand Up @@ -89,6 +89,7 @@ async function translateOptions({
plugin,
quiet,
reportUnusedDisableDirectives,
reportUnusedDisableDirectivesSeverity,
resolvePluginsRelativeTo,
rule,
rulesdir
Expand Down Expand Up @@ -177,7 +178,7 @@ async function translateOptions({
ignore,
overrideConfig,
overrideConfigFile,
reportUnusedDisableDirectives: reportUnusedDisableDirectives ? "error" : void 0
reportUnusedDisableDirectives: reportUnusedDisableDirectivesSeverity || (reportUnusedDisableDirectives ? "error" : void 0)
};

if (configType === "flat") {
Expand Down Expand Up @@ -377,6 +378,11 @@ const cli = {
return 2;
}

if (options.reportUnusedDisableDirectives && options.reportUnusedDisableDirectivesSeverity) {
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"));
Expand Down
12 changes: 11 additions & 1 deletion lib/config/flat-config-schema.js
Expand Up @@ -222,6 +222,16 @@ const booleanSchema = {
validate: "boolean"
};

/** @type {ObjectPropertySchema} */
const booleanOrStringSchema = {
merge: "replace",
validate(value) {
if (typeof value !== "string" && typeof value !== "boolean") {
throw new TypeError("Expected a string or a boolean.");
}
}
};

/** @type {ObjectPropertySchema} */
const deepObjectAssignSchema = {
merge(first = {}, second = {}) {
Expand Down Expand Up @@ -447,7 +457,7 @@ exports.flatConfigSchema = {
linterOptions: {
schema: {
noInlineConfig: booleanSchema,
reportUnusedDisableDirectives: booleanSchema
reportUnusedDisableDirectives: booleanOrStringSchema
}
},
languageOptions: {
Expand Down
2 changes: 1 addition & 1 deletion lib/eslint/eslint-helpers.js
Expand Up @@ -675,7 +675,7 @@ 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.
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. TODO: should anything change based on this comment?
...unknownOptions
}) {
const errors = [];
Expand Down
2 changes: 1 addition & 1 deletion lib/eslint/eslint.js
Expand Up @@ -165,7 +165,7 @@ 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.
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. TODO: should anything change based on this comment?
resolvePluginsRelativeTo = null, // ← should be null by default because if it's a string then it suppresses RFC47 feature.
rulePaths = [],
useEslintrc = true,
Expand Down
2 changes: 1 addition & 1 deletion lib/eslint/flat-eslint.js
Expand Up @@ -466,7 +466,7 @@ 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 {boolean|string} config.reportUnusedDisableDirectives If `true` or `error`, then it reports unused `eslint-disable` comments.
* @param {Linter} config.linter The linter instance to verify.
* @returns {LintResult} The result of linting.
* @private
Expand Down
16 changes: 13 additions & 3 deletions lib/linter/linter.js
Expand Up @@ -613,6 +613,7 @@ function normalizeFilename(filename) {
* @param {VerifyOptions} providedOptions Options
* @param {ConfigData} config Config.
* @returns {Required<VerifyOptions> & InternalOptions} Normalized options
* @throws an error upon invalid options
*/
function normalizeVerifyOptions(providedOptions, config) {

Expand All @@ -631,9 +632,18 @@ function normalizeVerifyOptions(providedOptions, config) {
reportUnusedDisableDirectives = reportUnusedDisableDirectives ? "error" : "off";
}
if (typeof reportUnusedDisableDirectives !== "string") {
reportUnusedDisableDirectives =
linterOptions.reportUnusedDisableDirectives
? "warn" : "off";

// Handle either a boolean or severity string in linterOptions.reportUnusedDisableDirectives.
if (typeof linterOptions.reportUnusedDisableDirectives === "string") {
if (!["error", "warn", "off"].includes(linterOptions.reportUnusedDisableDirectives)) {
throw new Error("Invalid value for 'reportUnusedDisableDirectives' in config.");
}
reportUnusedDisableDirectives = linterOptions.reportUnusedDisableDirectives;
} else if (linterOptions.reportUnusedDisableDirectives === true) {
reportUnusedDisableDirectives = "warn";
} else {
reportUnusedDisableDirectives = "off";
}
}

return {
Expand Down
9 changes: 8 additions & 1 deletion lib/options.js
Expand Up @@ -47,7 +47,7 @@ const optionator = require("optionator");
* @property {Object} [parserOptions] Specify parser options
* @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 directives
* @property {boolean|string|undefined} reportUnusedDisableDirectives Adds reported errors for unused eslint-disable directives
* @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
Expand Down Expand Up @@ -294,6 +294,13 @@ module.exports = function(usingFlatConfig) {
default: void 0,
description: "Adds reported errors for unused eslint-disable directives"
},
{
option: "report-unused-disable-directives-severity",
type: "String",
default: void 0,
description: "Chooses severity level for reporting unused eslint-disable directives",
enum: ["off", "warn", "error"]
},
{
heading: "Caching"
},
Expand Down
4 changes: 2 additions & 2 deletions lib/shared/types.js
Expand Up @@ -47,7 +47,7 @@ module.exports = {};
* @property {ParserOptions} [parserOptions] The parser options.
* @property {string[]} [plugins] The plugin specifiers.
* @property {string} [processor] The processor specifier.
* @property {boolean} [reportUnusedDisableDirectives] The flag to report unused `eslint-disable` comments.
* @property {boolean|string} [reportUnusedDisableDirectives] The flag or severity 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.
Expand All @@ -66,7 +66,7 @@ module.exports = {};
* @property {ParserOptions} [parserOptions] The parser options.
* @property {string[]} [plugins] The plugin specifiers.
* @property {string} [processor] The processor specifier.
* @property {boolean} [reportUnusedDisableDirectives] The flag to report unused `eslint-disable` comments.
* @property {boolean|string} [reportUnusedDisableDirectives] The flag or severity to report unused `eslint-disable` comments.
* @property {Record<string, RuleConf>} [rules] The rule settings.
* @property {Object} [settings] The shared settings.
*/
Expand Down

0 comments on commit bdd4a1a

Please sign in to comment.