Skip to content

Commit 1637b8e

Browse files
JoshuaKGoldbergnzakasmdjermanovic
authored
feat: add --report-unused-inline-configs (#19201)
* feat: add --report-unused-inline-configs / linterOptions.reportUnusedInlineConfigs * fix: unused severity normalization refactor * Update lib/linter/linter.js * Review feedback: reverting when in eslintrc mode * Apply suggestions from code review Co-authored-by: Nicholas C. Zakas <[email protected]> * Review feedback: removing boolean support * Review feedback: explain 'unused' * fix: getDirectiveComments call * Update unit tests to put logic within FlatConfigArray * Finish updating tests: cli.js * fix: any array size is a difference * fix: any object key mismatch is a difference too * git checkout main -- docs/src/use/configure/migration-guide.md * fix: remove migration notice from inline configs note in eslintrc-incompat.js * git checkout main -- messages/eslintrc-incompat.js * Update docs/src/integrate/nodejs-api.md Co-authored-by: Milos Djermanovic <[email protected]> * Apply suggestions from code review Co-authored-by: Milos Djermanovic <[email protected]> * rules.md touchups * Handle nullish values in containsDifferentProperty * Revert lib/shared/severity.js whitespace change * Apply suggestions from code review Co-authored-by: Milos Djermanovic <[email protected]> * Fix option-utils, and collectively update tests * Report already-disabled rules too * Ah, on second thought, one more unit test... * Update lib/linter/linter.js Co-authored-by: Nicholas C. Zakas <[email protected]> * Updated tests too * Apply suggestions from code review Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/linter/linter.js Co-authored-by: Milos Djermanovic <[email protected]> --------- Co-authored-by: Nicholas C. Zakas <[email protected]> Co-authored-by: Milos Djermanovic <[email protected]>
1 parent e39d3f2 commit 1637b8e

File tree

14 files changed

+593
-12
lines changed

14 files changed

+593
-12
lines changed

‎docs/src/use/command-line-interface.md

+21
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ Inline configuration comments:
128128
--report-unused-disable-directives Adds reported errors for unused eslint-disable and eslint-enable directives
129129
--report-unused-disable-directives-severity String Chooses severity level for reporting unused eslint-disable and
130130
eslint-enable directives - either: off, warn, error, 0, 1, or 2
131+
--report-unused-inline-configs String Adds reported errors for unused eslint inline config comments - either: off, warn, error, 0, 1, or 2
131132
132133
Caching:
133134
--cache Only check changed files - default: false
@@ -759,6 +760,26 @@ Same as [`--report-unused-disable-directives`](#--report-unused-disable-directiv
759760
args: ["--report-unused-disable-directives-severity", "warn", "file.js"]
760761
}) }}
761762

763+
#### `--report-unused-inline-configs`
764+
765+
This option causes ESLint to report inline config comments like `/* eslint rule-name: "error" */` whose rule severity and any options match what's already been configured.
766+
767+
* **Argument Type**: String. One of the following values:
768+
1. `off` (or `0`)
769+
1. `warn` (or `1`)
770+
1. `error` (or `2`)
771+
* **Multiple Arguments**: No
772+
* **Default Value**: By default, `linterOptions.reportUnusedInlineConfigs` configuration setting is used (which defaults to `"off"`).
773+
774+
This can be useful to keep files clean and devoid of misleading clutter.
775+
Inline config comments are meant to change ESLint's behavior in some way: if they change nothing, there is no reason to leave them in.
776+
777+
##### `--report-unused-inline-configs` example
778+
779+
```shell
780+
npx eslint --report-unused-inline-configs error file.js
781+
```
782+
762783
### Caching
763784

764785
#### `--cache`

‎docs/src/use/configure/configuration-files.md

+21
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ Each configuration object contains all of the information ESLint needs to execut
7474
* `linterOptions` - An object containing settings related to the linting process.
7575
* `noInlineConfig` - A Boolean value indicating if inline configuration is allowed.
7676
* `reportUnusedDisableDirectives` - A severity string indicating if and how unused disable and enable directives should be tracked and reported. For legacy compatibility, `true` is equivalent to `"warn"` and `false` is equivalent to `"off"`. (default: `"warn"`).
77+
* `reportUnusedInlineConfigs` - A severity string indicating if and how unused inline configs should be tracked and reported. (default: `"off"`)
7778
* `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"`).
7879
* `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.
7980
* `rules` - An object containing the configured rules. When `files` or `ignores` are specified, these rule configurations are only available to the matching files.
@@ -301,6 +302,26 @@ You can override this setting using the [`--report-unused-disable-directives`](.
301302

302303
For legacy compatibility, `true` is equivalent to `"warn"` and `false` is equivalent to `"off"`.
303304

305+
#### Reporting Unused Inline Configs
306+
307+
Inline config comments such as `/* eslint rule-name: "error" */` are used to change ESLint rule severity and/or options around certain portions of code.
308+
As a project's ESLint configuration file changes, it's possible for these directives to no longer be different from what was already set.
309+
You can enable reporting of these unused inline config comments by setting the `reportUnusedInlineConfigs` option to a severity string, as in this example:
310+
311+
```js
312+
// eslint.config.js
313+
export default [
314+
{
315+
files: ["**/*.js"],
316+
linterOptions: {
317+
reportUnusedInlineConfigs: "error"
318+
}
319+
}
320+
];
321+
```
322+
323+
You can override this setting using the [`--report-unused-inline-configs`](../command-line-interface#--report-unused-inline-configs) command line option.
324+
304325
### Configuring Rules
305326

306327
You can configure any number of rules in a configuration object by add a `rules` property containing an object with your rule configurations. The names in this object are the names of the rules and the values are the configurations for each of those rules. Here's an example:

‎docs/src/use/configure/rules.md

+20-1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,25 @@ Configuration comments can include descriptions to explain why the comment is ne
7373
*/
7474
```
7575

76+
#### Report unused `eslint` inline config comments
77+
78+
To report unused `eslint` inline config comments (those that don't change anything from what was already configured), use the `reportUnusedInlineConfigs` setting. For example:
79+
80+
```js
81+
// eslint.config.js
82+
export default [
83+
{
84+
linterOptions: {
85+
reportUnusedInlineConfigs: "error"
86+
}
87+
}
88+
];
89+
```
90+
91+
This setting defaults to `"off"`.
92+
93+
This setting is similar to the [`--report-unused-inline-configs`](../command-line-interface#--report-unused-inline-configs) CLI option.
94+
7695
### Using Configuration Files
7796

7897
To configure rules inside of a [configuration file](./configuration-files#configuration-file), use the `rules` key along with an error level and any options you want to use. For example:
@@ -345,7 +364,7 @@ You can also use the [`--no-inline-config`](../command-line-interface#--no-inlin
345364

346365
#### Report unused `eslint-disable` comments
347366

348-
To report unused `eslint-disable` comments, use the `reportUnusedDisableDirectives` setting. For example:
367+
To report unused `eslint-disable` comments (those that disable rules which would not report on the disabled line), use the `reportUnusedDisableDirectives` setting. For example:
349368

350369
```js
351370
// eslint.config.js

‎lib/cli.js

+8
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ async function translateOptions({
128128
quiet,
129129
reportUnusedDisableDirectives,
130130
reportUnusedDisableDirectivesSeverity,
131+
reportUnusedInlineConfigs,
131132
resolvePluginsRelativeTo,
132133
rule,
133134
rulesdir,
@@ -180,6 +181,13 @@ async function translateOptions({
180181
};
181182
}
182183

184+
if (reportUnusedInlineConfigs !== void 0) {
185+
overrideConfig[0].linterOptions = {
186+
...overrideConfig[0].linterOptions,
187+
reportUnusedInlineConfigs: normalizeSeverityToString(reportUnusedInlineConfigs)
188+
};
189+
}
190+
183191
if (plugin) {
184192
overrideConfig[0].plugins = await loadPlugins(importer, plugin);
185193
}

‎lib/config/flat-config-schema.js

+16-1
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,20 @@ const disableDirectiveSeveritySchema = {
304304
}
305305
};
306306

307+
/** @type {ObjectPropertySchema} */
308+
const unusedInlineConfigsSeveritySchema = {
309+
merge(first, second) {
310+
const value = second === void 0 ? first : second;
311+
312+
return normalizeSeverityToNumber(value);
313+
},
314+
validate(value) {
315+
if (!ALLOWED_SEVERITIES.has(value)) {
316+
throw new TypeError("Expected one of: \"error\", \"warn\", \"off\", 0, 1, or 2.");
317+
}
318+
}
319+
};
320+
307321
/** @type {ObjectPropertySchema} */
308322
const deepObjectAssignSchema = {
309323
merge(first = {}, second = {}) {
@@ -555,7 +569,8 @@ const flatConfigSchema = {
555569
linterOptions: {
556570
schema: {
557571
noInlineConfig: booleanSchema,
558-
reportUnusedDisableDirectives: disableDirectiveSeveritySchema
572+
reportUnusedDisableDirectives: disableDirectiveSeveritySchema,
573+
reportUnusedInlineConfigs: unusedInlineConfigsSeveritySchema
559574
}
560575
},
561576
language: languageSchema,

‎lib/linter/linter.js

+74-5
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ const { FlatConfigArray } = require("../config/flat-config-array");
4040
const { startTime, endTime } = require("../shared/stats");
4141
const { RuleValidator } = require("../config/rule-validator");
4242
const { assertIsRuleSeverity } = require("../config/flat-config-schema");
43-
const { normalizeSeverityToString } = require("../shared/severity");
43+
const { normalizeSeverityToString, normalizeSeverityToNumber } = require("../shared/severity");
4444
const { deepMergeArrays } = require("../shared/deep-merge-arrays");
4545
const jslang = require("../languages/js");
4646
const { activeFlags, inactiveFlags } = require("../shared/flags");
@@ -56,6 +56,7 @@ const { VFile } = require("./vfile");
5656
const { ParserService } = require("../services/parser-service");
5757
const { FileContext } = require("./file-context");
5858
const { ProcessorService } = require("../services/processor-service");
59+
const { containsDifferentProperty } = require("../shared/option-utils");
5960
const STEP_KIND_VISIT = 1;
6061
const STEP_KIND_CALL = 2;
6162

@@ -76,6 +77,7 @@ const STEP_KIND_CALL = 2;
7677
/** @typedef {import("@eslint/core").Language} Language */
7778
/** @typedef {import("@eslint/core").RuleSeverity} RuleSeverity */
7879
/** @typedef {import("@eslint/core").RuleConfig} RuleConfig */
80+
/** @typedef {import("../types").Linter.StringSeverity} StringSeverity */
7981

8082

8183
/* eslint-disable jsdoc/valid-types -- https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/4#issuecomment-778805577 */
@@ -141,7 +143,8 @@ const STEP_KIND_CALL = 2;
141143
/**
142144
* @typedef {Object} InternalOptions
143145
* @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.
144-
* @property {"off" | "warn" | "error"} reportUnusedDisableDirectives (boolean values were normalized)
146+
* @property {StringSeverity} reportUnusedDisableDirectives Severity to report unused disable directives, if not "off" (boolean values were normalized).
147+
* @property {StringSeverity} reportUnusedInlineConfigs Severity to report unused inline configs, if not "off".
145148
*/
146149

147150
//------------------------------------------------------------------------------
@@ -314,6 +317,62 @@ function createLintingProblem(options) {
314317
};
315318
}
316319

320+
/**
321+
* Wraps the value in an Array if it isn't already one.
322+
* @template T
323+
* @param {T|T[]} value Value to be wrapped.
324+
* @returns {Array} The value as an array.
325+
*/
326+
function asArray(value) {
327+
return Array.isArray(value) ? value : [value];
328+
}
329+
330+
/**
331+
* Pushes a problem to inlineConfigProblems if ruleOptions are redundant.
332+
* @param {ConfigData} config Provided config.
333+
* @param {Object} loc A line/column location
334+
* @param {Array} problems Problems that may be added to.
335+
* @param {string} ruleId The rule ID.
336+
* @param {Array} ruleOptions The rule options, merged with the config's.
337+
* @param {Array} ruleOptionsInline The rule options from the comment.
338+
* @param {"error"|"warn"} severity The severity to report.
339+
* @returns {void}
340+
*/
341+
function addProblemIfSameSeverityAndOptions(config, loc, problems, ruleId, ruleOptions, ruleOptionsInline, severity) {
342+
const existingConfigRaw = config.rules?.[ruleId];
343+
const existingConfig = existingConfigRaw ? asArray(existingConfigRaw) : ["off"];
344+
const existingSeverity = normalizeSeverityToString(existingConfig[0]);
345+
const inlineSeverity = normalizeSeverityToString(ruleOptions[0]);
346+
const sameSeverity = existingSeverity === inlineSeverity;
347+
348+
if (!sameSeverity) {
349+
return;
350+
}
351+
352+
const alreadyConfigured = existingConfigRaw
353+
? `is already configured to '${existingSeverity}'`
354+
: "is not enabled so can't be turned off";
355+
let message;
356+
357+
if ((existingConfig.length === 1 && ruleOptions.length === 1) || existingSeverity === "off") {
358+
message = `Unused inline config ('${ruleId}' ${alreadyConfigured}).`;
359+
} else if (!containsDifferentProperty(ruleOptions.slice(1), existingConfig.slice(1))) {
360+
message = ruleOptionsInline.length === 1
361+
? `Unused inline config ('${ruleId}' ${alreadyConfigured}).`
362+
: `Unused inline config ('${ruleId}' ${alreadyConfigured} with the same options).`;
363+
}
364+
365+
if (message) {
366+
problems.push(createLintingProblem({
367+
ruleId: null,
368+
message,
369+
loc,
370+
language: config.language,
371+
severity: normalizeSeverityToNumber(severity)
372+
}));
373+
}
374+
}
375+
317376
/**
318377
* Creates a collection of disable directives from a comment
319378
* @param {Object} options to create disable directives
@@ -517,7 +576,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
517576
return;
518577
}
519578

520-
let ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];
579+
let ruleOptions = asArray(ruleValue);
521580

522581
/*
523582
* If the rule was already configured, inline rule configuration that
@@ -555,7 +614,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
555614
*/
556615
ruleOptions = [
557616
ruleOptions[0], // severity from the inline config
558-
...Array.isArray(config.rules[name]) ? config.rules[name].slice(1) : [] // options from the provided config
617+
...asArray(config.rules[name]).slice(1) // options from the provided config
559618
];
560619
}
561620

@@ -774,6 +833,8 @@ function normalizeVerifyOptions(providedOptions, config) {
774833
}
775834
}
776835

836+
const reportUnusedInlineConfigs = linterOptions.reportUnusedInlineConfigs === void 0 ? "off" : normalizeSeverityToString(linterOptions.reportUnusedInlineConfigs);
837+
777838
let ruleFilter = providedOptions.ruleFilter;
778839

779840
if (typeof ruleFilter !== "function") {
@@ -787,6 +848,7 @@ function normalizeVerifyOptions(providedOptions, config) {
787848
? `your config${configNameOfNoInlineConfig}`
788849
: null,
789850
reportUnusedDisableDirectives,
851+
reportUnusedInlineConfigs,
790852
disableFixes: Boolean(providedOptions.disableFixes),
791853
stats: providedOptions.stats,
792854
ruleFilter
@@ -1787,7 +1849,8 @@ class Linter {
17871849

17881850
try {
17891851

1790-
let ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];
1852+
const ruleOptionsInline = asArray(ruleValue);
1853+
let ruleOptions = ruleOptionsInline;
17911854

17921855
assertIsRuleSeverity(ruleId, ruleOptions[0]);
17931856

@@ -1850,6 +1913,12 @@ class Linter {
18501913
}
18511914
}
18521915

1916+
if (options.reportUnusedInlineConfigs !== "off") {
1917+
addProblemIfSameSeverityAndOptions(
1918+
config, loc, inlineConfigProblems, ruleId, ruleOptions, ruleOptionsInline, options.reportUnusedInlineConfigs
1919+
);
1920+
}
1921+
18531922
if (shouldValidateOptions) {
18541923
ruleValidator.validate({
18551924
plugins: config.plugins,

‎lib/options.js

+13
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,18 @@ module.exports = function(usingFlatConfig) {
187187
};
188188
}
189189

190+
let reportUnusedInlineConfigsFlag;
191+
192+
if (usingFlatConfig) {
193+
reportUnusedInlineConfigsFlag = {
194+
option: "report-unused-inline-configs",
195+
type: "String",
196+
default: void 0,
197+
description: "Adds reported errors for unused eslint inline config comments",
198+
enum: ["off", "warn", "error", "0", "1", "2"]
199+
};
200+
}
201+
190202
return optionator({
191203
prepend: "eslint [options] file.js [file.js] [dir]",
192204
defaults: {
@@ -350,6 +362,7 @@ module.exports = function(usingFlatConfig) {
350362
description: "Chooses severity level for reporting unused eslint-disable and eslint-enable directives",
351363
enum: ["off", "warn", "error", "0", "1", "2"]
352364
},
365+
reportUnusedInlineConfigsFlag,
353366
{
354367
heading: "Caching"
355368
},

‎lib/shared/option-utils.js

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/**
2+
* @fileoverview Utilities to operate on option objects.
3+
* @author Josh Goldberg
4+
*/
5+
6+
"use strict";
7+
8+
/**
9+
* Determines whether any of input's properties are different
10+
* from values that already exist in original.
11+
* @template T
12+
* @param {Partial<T>} input New value.
13+
* @param {T} original Original value.
14+
* @returns {boolean} Whether input includes an explicit difference.
15+
*/
16+
function containsDifferentProperty(input, original) {
17+
if (input === original) {
18+
return false;
19+
}
20+
21+
if (
22+
typeof input !== typeof original ||
23+
Array.isArray(input) !== Array.isArray(original)
24+
) {
25+
return true;
26+
}
27+
28+
if (Array.isArray(input)) {
29+
return (
30+
input.length !== original.length ||
31+
input.some((value, i) =>
32+
containsDifferentProperty(value, original[i]))
33+
);
34+
}
35+
36+
if (typeof input === "object") {
37+
if (input === null || original === null) {
38+
return true;
39+
}
40+
41+
const inputKeys = Object.keys(input);
42+
const originalKeys = Object.keys(original);
43+
44+
return inputKeys.length !== originalKeys.length || inputKeys.some(
45+
inputKey =>
46+
!Object.hasOwn(original, inputKey) ||
47+
containsDifferentProperty(input[inputKey], original[inputKey])
48+
);
49+
}
50+
51+
return true;
52+
}
53+
54+
module.exports = {
55+
containsDifferentProperty
56+
};

0 commit comments

Comments
 (0)