From 5c1b771ce006086989c3d0497027f4e3d079f500 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 7 Jul 2024 17:53:51 -0400 Subject: [PATCH 01/15] feat: Reporting unused inline configs --- .../README.md | 244 ++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 designs/2024-report-unused-inline-configs/README.md diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md new file mode 100644 index 00000000..49c21faa --- /dev/null +++ b/designs/2024-report-unused-inline-configs/README.md @@ -0,0 +1,244 @@ +- Repo: eslint/eslint +- Start Date: 2024-07-08 +- RFC PR: +- Authors: [Josh Goldberg](https://github.com/JoshuaKGoldberg) + +# Reporting unused inline configs + +## Summary + +Add an option to report `/* eslint ... */` comments that don't change any settings. + +## Motivation + +Right now, nothing in ESLint core stops [inline configs (configuration comments)](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments) from redundantly repeating the same configuration _option_ and/or _severity_ as the file's existing computed configuration. +Unused inline configs suffer from the same drawbacks as [unused disable directives](https://eslint.org/docs/latest/use/configure/rules#report-unused-eslint-disable-comments): they take up space and can be misleading. + +For example: + +- [Playground of an inline config setting the same severity as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogXCJlcnJvclwiICovXG5cIuKdjCBkb2VzIG5vdGhpbmc6IGV4aXN0aW5nIHNldmVyaXR5IHZzLiBmaWxlXCJcbiIsIm9wdGlvbnMiOnsicnVsZXMiOnt9LCJsYW5ndWFnZU9wdGlvbnMiOnsiZWNtYVZlcnNpb24iOiJsYXRlc3QiLCJzb3VyY2VUeXBlIjoibW9kdWxlIiwicGFyc2VyT3B0aW9ucyI6eyJlY21hRmVhdHVyZXMiOnt9fX19fQ==) +- [Playground of an inline config setting the same options as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogW1wiZXJyb3JcIiwgeyBcImFsbG93U2hvcnRDaXJjdWl0XCI6IHRydWUgfV0gKi9cblwi4p2MIGRvZXMgbm90aGluZzogZXhpc3Rpbmcgc2V2ZXJpdHkgYW5kIG9wdGlvbnMgdnMuIGZpbGVcIlxuIiwib3B0aW9ucyI6eyJydWxlcyI6eyJuby11bnVzZWQtZXhwcmVzc2lvbnMiOlsiZXJyb3IiLHsiYWxsb3dTaG9ydENpcmN1aXQiOnRydWUsImFsbG93VGVybmFyeSI6ZmFsc2UsImFsbG93VGFnZ2VkVGVtcGxhdGVzIjpmYWxzZSwiZW5mb3JjZUZvckpTWCI6ZmFsc2V9XX0sImxhbmd1YWdlT3B0aW9ucyI6eyJlY21hVmVyc2lvbiI6ImxhdGVzdCIsInNvdXJjZVR5cGUiOiJtb2R1bGUiLCJwYXJzZXJPcHRpb25zIjp7ImVjbWFGZWF0dXJlcyI6e319fX19) + +This RFC proposes adding the ability for ESLint to report on those unused inline configs: + +- `--report-unused-inline-configs` CLI option +- `linterOptions.reportUnusedDisableDirectives` configuration file option + +```shell +npx eslint --report-unused-inline-configs error +``` + +```js +{ + linterOptions: { + reportUnusedDisableDirectives: "error", + } +} +``` + +These new options would be similar to the existing [`--report-unused-disable-directives(-severity)`](https://eslint.org/docs/latest/use/command-line-interface#--report-unused-disable-directives) and [`linterOptions.reportUnusedDisableDirectives`](https://eslint.org/docs/latest/use/configure/configuration-files#reporting-unused-disable-directives) options. + +### Examples + +The following table uses [`accessor-pairs`](https://eslint.org/docs/latest/rules/accessor-pairs) as an example with inline configs like `/* eslint accessor-pairs: "off" */`. +It assumes the `accessor-pairs` default options from [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) of: + +```json +{ + "enforceForClassMembers": true, + "getWithoutSet": false, + "setWithoutGet": true +} +``` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Original Config SettingInline Config SettingsReport?
+ "error" +
+ 2 +
+ ["error", { "enforceForClassMembers": true }] +
+ ["error", { "getWithoutSet": false }] + 		+ "off" +
+ 0 + 		No
+ "warn" +
+ 1 + 		No
+ "error" +
+ 2 + 		Yes
+ ["error", { "enforceForClassMembers": false }] +
+ [2, { "enforceForClassMembers": false }] + 		No
+ ["error", { "getWithoutSet": true }] +
+ [2, { "getWithoutSet": true }] + 		No
+ ["error", { "enforceForClassMembers": true }] +
+ [2, { "enforceForClassMembers": true }] + 		Yes
+ ["error", { "getWithoutSet": false }] +
+ [2, { "getWithoutSet": false }] + 		Yes
+ +## Detailed Design + +Additional logic can be added to the existing code points in `Linter` that validate inline config options: + +- [`_verifyWithFlatConfigArrayAndWithoutProcessors`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L1650): called in the current ("flat") config system +- [`getDirectiveComments`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L387): called when `ESLINT_USE_FLAT_CONFIG=true` + +For a rough code reference, see [`poc: reporting unused inline configs`](https://github.com/JoshuaKGoldberg/eslint/commit/e14e404ed93e6238bdee817923a449f5215eecd8). + +This proposed design intentionally not involve any language-specific code changes. +How a specific language computes its configuration comments is irrelevant to this proposed feature. + +### Computing Option Differences + +Each inline config comment will be compared against the existing configuration value(s) it attempts to override: + +- If the config comment only specifies a severity, then only the severity will be checked for redundancy + - The new logic will normalize options: `"off"` will be considered equivalent to `0` +- If the config comment also specifies rule options, they will be compared for deep equality to the existing rule options + +This RFC should wait to begin work until after [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) is merged into ESLint. +That way, a rule's `meta.defaultOptions` can be factored into computing whether an inline config's rule options differ from the previously configured options. + +### Default Values + +This RFC proposes a two-step approach to introducing unused inline config reporting: + +1. In the current major version upon introduction, don't enable unused inline config reporting by default +2. In the next major version, enable unused inline config reporting by default + +Note that the default value in the next major version should be the same as reporting unused disable directives. +See [Change Request: error by default for unused disable directives](https://github.com/eslint/eslint/issues/18665) for an issue on changing that from `"warn"` to `"error"`. + +## Documentation + +The new settings will be documented similarly to reporting unused disable directives: + +- [Configuration Files](https://eslint.org/docs/latest/use/configure/configuration-files): + - List item for `reportUnusedInlineConfig` under _[Configuration Objects](https://eslint.org/docs/latest/use/configure/configuration-files#configuration-objects)_ > `linterOptions` + - Sub-heading alongside _[Reporting Unused Disable Directives](https://eslint.org/docs/latest/use/configure/configuration-files#reporting-unused-disable-directives)_ +- [Configure Rules](https://eslint.org/docs/latest/use/configure/rules): + - Sub-heading alongside _[Report unused eslint-disable comments](https://eslint.org/docs/latest/use/configure/rules#report-unused-eslint-disable-comments)_ +- [Command Line Interface Reference](https://eslint.org/docs/latest/use/command-line-interface): + - List item under the _[Options](https://eslint.org/docs/latest/use/command-line-interface#options)_ code block, under `Inline Configuration comments:` + - Corresponding sub-headings under _[Inline Configuration Comments](https://eslint.org/docs/latest/use/command-line-interface#inline-configuration-comments)_ + +## Drawbacks + +Any added options come with an added tax on project maintenance and user comprehension. +This RFC believes the flagging of unused inline configs is worth that tax. + +See [Omitting Legacy Config Support](#omitting-legacy-config-support) for a possible reduction in cost. + +## Backwards Compatibility Analysis + +The proposed two-step approach introduces the options in a fully backwards-compatible way. +No new warnings or errors will be reported in the current major version without the user explicitly opting into them. + +## Alternatives + +### Omitting Legacy Config Support + +One way to reduce costs could be to wait until ESLint completely removes support for legacy configs. +That way, only the new ("flat") config system would need to be tested with this change. + +However, it is unclear when the legacy config system will be removed from ESLint core. +This RFC prefers taking action sooner rather than later. + +### Separate CLI Option + +Unlike the changes discussed in [Change Request: Enable reportUnusedDisableDirectives config option by default](https://github.com/eslint/eslint/issues/15466) -> [feat!: flexible config + default reporting of unused disable directives](https://github.com/eslint/rfcs/pull/100), reporting unused inline configs does not have legacy behavior to keep to. +The existing `--report-unused-disable-directives` (enabling) and `--report-unused-disable-directives-severity` (severity) options were kept separate for backwards compatibility. + +Adding a sole `--report-unused-inline-configs` CLI option presents a discrepency between the two sets of options. +An alternative could be to instead add `--report-unused-inline-configs` an `--report-unused-inline-configs-severity` options for consistency's sake. + +This RFC's opinion is that the consistency of adding two new options is not worth the excess options logic. +It would instead be preferable to, in a future ESLint major version, deprecate `--report-unused-disable-directives-severity` and merge its logic setting into `--report-unused-disable-directives`. + +### Superset Behavior: Unused Disable Directive Reporting + +Disable directives can be thought as a subset of inline configs in general. +Reporting on unused disable directives could be thought of as a subset of reporting on unused inline configs. + +An additional direction this RFC could propose would be to have the new unused inline config reporting act as a superset of unused disable directive reporting. + +However: + +- Deprecating `reportUnusedDisableDirectives` would be a distruptive breaking change +- This RFC proposes enabling `reportUnusedInlineConfigs` by default in a subsequent major version anyway: most users will not be manually configuring it + +## Help Needed + +I would like to implement this RFC. + +## Frequently Asked Questions + +### Why so many references to reporting unused disable directives? + +This RFC's proposed behavior is similar on the surface to the existing behavior around reporting unused disable directives. +It's beneficial for users to have similar behaviors between similar options. + +### Will inline configs be compared to previous inline configs in the same file? + +As of [Change Request: Disallow multiple configuration comments for the same rule](https://github.com/eslint/eslint/issues/18132) -> [feat!: disallow multiple configuration comments for same rule](https://github.com/eslint/eslint/pull/18157), the same rule cannot be configured by an inline config more than once per file. + +## Related Discussions + +- : the issue triggering this RFC + - : an older issue that #18230 duplicated +- : previous issue for enabling `reportUnusedDisableDirectives` config option by default + - : the RFC discussion flexible config + default reporting of unused disable directives + - : the PR implementing custom severity when reporting unused disable directives +- : issue suggesting erroring by default for unused disable directives +- : issue suggesting merging `--report-unused-disable-directives-severity` into `--report-unused-disable-directives` From 8175c4085b393cd7b90db5ed242ef3cd57291483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Mon, 8 Jul 2024 08:04:21 -0400 Subject: [PATCH 02/15] Update designs/2024-report-unused-inline-configs/README.md Co-authored-by: Francesco Trotta --- designs/2024-report-unused-inline-configs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 49c21faa..2f29b6fd 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -136,7 +136,7 @@ Additional logic can be added to the existing code points in `Linter` that valid For a rough code reference, see [`poc: reporting unused inline configs`](https://github.com/JoshuaKGoldberg/eslint/commit/e14e404ed93e6238bdee817923a449f5215eecd8). -This proposed design intentionally not involve any language-specific code changes. +This proposed design does intentionally not involve any language-specific code changes. How a specific language computes its configuration comments is irrelevant to this proposed feature. ### Computing Option Differences From 00ed2295e96acbc2ea74a11056ad573105b82eac Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 8 Jul 2024 08:04:53 -0400 Subject: [PATCH 03/15] fix: typo of option name in main section --- designs/2024-report-unused-inline-configs/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 2f29b6fd..bcb32454 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -22,7 +22,7 @@ For example: This RFC proposes adding the ability for ESLint to report on those unused inline configs: - `--report-unused-inline-configs` CLI option -- `linterOptions.reportUnusedDisableDirectives` configuration file option +- `linterOptions.reportUnusedInlineConfigs` configuration file option ```shell npx eslint --report-unused-inline-configs error @@ -31,7 +31,7 @@ npx eslint --report-unused-inline-configs error ```js { linterOptions: { - reportUnusedDisableDirectives: "error", + reportUnusedInlineConfigs: "error", } } ``` From a0d0e07454714fb6f42f26a8a4c1ec7a4608f78d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 6 Aug 2024 10:48:19 -0400 Subject: [PATCH 04/15] Update designs/2024-report-unused-inline-configs/README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 唯然 --- designs/2024-report-unused-inline-configs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index bcb32454..9fc29478 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -1,6 +1,6 @@ - Repo: eslint/eslint - Start Date: 2024-07-08 -- RFC PR: +- RFC PR: https://github.com/eslint/rfcs/pull/121 - Authors: [Josh Goldberg](https://github.com/JoshuaKGoldberg) # Reporting unused inline configs From c696d918cde6f8614bcd5155c7772916e931849b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 6 Aug 2024 10:48:30 -0400 Subject: [PATCH 05/15] Update designs/2024-report-unused-inline-configs/README.md Co-authored-by: Francesco Trotta --- designs/2024-report-unused-inline-configs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 9fc29478..c9e48352 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -201,7 +201,7 @@ Unlike the changes discussed in [Change Request: Enable reportUnusedDisableDirec The existing `--report-unused-disable-directives` (enabling) and `--report-unused-disable-directives-severity` (severity) options were kept separate for backwards compatibility. Adding a sole `--report-unused-inline-configs` CLI option presents a discrepency between the two sets of options. -An alternative could be to instead add `--report-unused-inline-configs` an `--report-unused-inline-configs-severity` options for consistency's sake. +An alternative could be to instead add `--report-unused-inline-configs` and `--report-unused-inline-configs-severity` options for consistency's sake. This RFC's opinion is that the consistency of adding two new options is not worth the excess options logic. It would instead be preferable to, in a future ESLint major version, deprecate `--report-unused-disable-directives-severity` and merge its logic setting into `--report-unused-disable-directives`. From c339a59bfaec0c48817a012c2f5e92a242a1b1e6 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 13 Aug 2024 22:16:53 -0400 Subject: [PATCH 06/15] Reference: RFC version targeting both config files and inline configs --- .../README.md | 87 ++++++++++++------- 1 file changed, 57 insertions(+), 30 deletions(-) rename designs/{2024-report-unused-inline-configs => 2024-report-unused-configs}/README.md (70%) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-configs/README.md similarity index 70% rename from designs/2024-report-unused-inline-configs/README.md rename to designs/2024-report-unused-configs/README.md index c9e48352..7004e489 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-configs/README.md @@ -3,26 +3,28 @@ - RFC PR: https://github.com/eslint/rfcs/pull/121 - Authors: [Josh Goldberg](https://github.com/JoshuaKGoldberg) -# Reporting unused inline configs +# Reporting inline configs ## Summary -Add an option to report `/* eslint ... */` comments that don't change any settings. +Add an option to report config file `rules` values and inline `/* eslint ... */` config comments that don't change any settings. ## Motivation -Right now, nothing in ESLint core stops [inline configs (configuration comments)](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments) from redundantly repeating the same configuration _option_ and/or _severity_ as the file's existing computed configuration. -Unused inline configs suffer from the same drawbacks as [unused disable directives](https://eslint.org/docs/latest/use/configure/rules#report-unused-eslint-disable-comments): they take up space and can be misleading. +Right now, nothing in ESLint core stops [config files `rule` entries](https://eslint.org/docs/latest/use/configure/configuration-files#configuring-rules) or [inline configs (configuration comments)](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments) from redundantly repeating the same configuration _option_ and/or _severity_ as the existing computed configuration. +Both forms of configuration are undesirable when redundant: they take up space and can be misleading. For example: -- [Playground of an inline config setting the same severity as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogXCJlcnJvclwiICovXG5cIuKdjCBkb2VzIG5vdGhpbmc6IGV4aXN0aW5nIHNldmVyaXR5IHZzLiBmaWxlXCJcbiIsIm9wdGlvbnMiOnsicnVsZXMiOnt9LCJsYW5ndWFnZU9wdGlvbnMiOnsiZWNtYVZlcnNpb24iOiJsYXRlc3QiLCJzb3VyY2VUeXBlIjoibW9kdWxlIiwicGFyc2VyT3B0aW9ucyI6eyJlY21hRmVhdHVyZXMiOnt9fX19fQ==) +- [Stackblitz of an `eslint.config.js` setting a rule twice with the same severity](https://stackblitz.com/edit/stackblitz-starters-qcrxqr?file=eslint.config.js) +- [Stackblitz of an `eslint.config.js` setting a rule twice with the same options](https://stackblitz.com/edit/stackblitz-starters-san7bj?file=eslint.config.js) +- [Playground of an inline config setting the same severity as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogXCJlcnJvclwiICovXG5cIuKdjCBkb2VzIG5vdGhpbmc6IGV4aXN0aW5nIHNldmVyaXR5IHZzLiBmaWxlXCJcbiIsIm9wdGlvbnMiOnsicnVsZXMiOnsibm8tdW51c2VkLWV4cHJlc3Npb25zIjpbImVycm9yIl19LCJsYW5ndWFnZU9wdGlvbnMiOnsiZWNtYVZlcnNpb24iOiJsYXRlc3QiLCJzb3VyY2VUeXBlIjoibW9kdWxlIiwicGFyc2VyT3B0aW9ucyI6eyJlY21hRmVhdHVyZXMiOnt9fX19fQ==) - [Playground of an inline config setting the same options as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogW1wiZXJyb3JcIiwgeyBcImFsbG93U2hvcnRDaXJjdWl0XCI6IHRydWUgfV0gKi9cblwi4p2MIGRvZXMgbm90aGluZzogZXhpc3Rpbmcgc2V2ZXJpdHkgYW5kIG9wdGlvbnMgdnMuIGZpbGVcIlxuIiwib3B0aW9ucyI6eyJydWxlcyI6eyJuby11bnVzZWQtZXhwcmVzc2lvbnMiOlsiZXJyb3IiLHsiYWxsb3dTaG9ydENpcmN1aXQiOnRydWUsImFsbG93VGVybmFyeSI6ZmFsc2UsImFsbG93VGFnZ2VkVGVtcGxhdGVzIjpmYWxzZSwiZW5mb3JjZUZvckpTWCI6ZmFsc2V9XX0sImxhbmd1YWdlT3B0aW9ucyI6eyJlY21hVmVyc2lvbiI6ImxhdGVzdCIsInNvdXJjZVR5cGUiOiJtb2R1bGUiLCJwYXJzZXJPcHRpb25zIjp7ImVjbWFGZWF0dXJlcyI6e319fX19) -This RFC proposes adding the ability for ESLint to report on those unused inline configs: +This RFC proposes adding the ability for ESLint to report on both of those kinds of unused configs: -- `--report-unused-inline-configs` CLI option -- `linterOptions.reportUnusedInlineConfigs` configuration file option +- `--report-unused-configs` CLI option +- `linterOptions.reportUnusedConfigs` configuration file option ```shell npx eslint --report-unused-inline-configs error @@ -31,7 +33,7 @@ npx eslint --report-unused-inline-configs error ```js { linterOptions: { - reportUnusedInlineConfigs: "error", + reportUnusedConfigs: "error", } } ``` @@ -40,7 +42,7 @@ These new options would be similar to the existing [`--report-unused-disable-dir ### Examples -The following table uses [`accessor-pairs`](https://eslint.org/docs/latest/rules/accessor-pairs) as an example with inline configs like `/* eslint accessor-pairs: "off" */`. +The following table uses [`accessor-pairs`](https://eslint.org/docs/latest/rules/accessor-pairs) as an example. It assumes the `accessor-pairs` default options from [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) of: ```json @@ -51,11 +53,13 @@ It assumes the `accessor-pairs` default options from [feat: add meta.defaultOpti } ``` +Values apply both for config files like `rules: { "accessor-pairs": "off" }` and inline configs like `/* eslint accessor-pairs: "off" */`. + - - + + @@ -129,36 +133,55 @@ It assumes the `accessor-pairs` default options from [feat: add meta.defaultOpti ## Detailed Design -Additional logic can be added to the existing code points in `Linter` that validate inline config options: - -- [`_verifyWithFlatConfigArrayAndWithoutProcessors`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L1650): called in the current ("flat") config system -- [`getDirectiveComments`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L387): called when `ESLINT_USE_FLAT_CONFIG=true` - -For a rough code reference, see [`poc: reporting unused inline configs`](https://github.com/JoshuaKGoldberg/eslint/commit/e14e404ed93e6238bdee817923a449f5215eecd8). +This RFC proposes changing logical entry points only for the current ("flat") config system. +Legacy ("eslintrc") configs are omitted to reduce maintenance costs. This proposed design does intentionally not involve any language-specific code changes. How a specific language computes its configuration comments is irrelevant to this proposed feature. +### Glossary + +For the sake of specificity, the following terms are used intentionally: + +- **Options**: The options passed to a rule, as described by its `meta.schema` +- **Setting**: A configuration entry specifying a rule name, its severity, and optionally its _options_ +- **Config file entry**: In an `eslint.config.js` file `rules` object, a key-value pair representing a single rule's _settings_ +- **Inline config**: In a source file, a comment representing a single rule's _settings_ + +### Logic Entry Points + +For rough code references, see: + +- Config file entries: _(todo)_ +- Inline config comments: [`poc: reporting unused inline configs`](https://github.com/JoshuaKGoldberg/eslint/commit/e14e404ed93e6238bdee817923a449f5215eecd8) + + ### Computing Option Differences -Each inline config comment will be compared against the existing configuration value(s) it attempts to override: +Each setting will be compared against the existing configuration value(s) it attempts to override: -- If the config comment only specifies a severity, then only the severity will be checked for redundancy +- If the setting only specifies a severity, then only the severity will be checked for redundancy - The new logic will normalize options: `"off"` will be considered equivalent to `0` -- If the config comment also specifies rule options, they will be compared for deep equality to the existing rule options +- If the setting also specifies rule options, they will be compared for deep equality to the existing rule options This RFC should wait to begin work until after [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) is merged into ESLint. -That way, a rule's `meta.defaultOptions` can be factored into computing whether an inline config's rule options differ from the previously configured options. +That way, a rule's `meta.defaultOptions` can be factored into computing whether an setting's rule options differ from the previously configured options. + +### Config Files + +Only configs in the user's code will be checked. +Third-party shareable configs will not be validated. + +For config files, that means only `rules` entries in the user's specified config file will be checked. ### Default Values -This RFC proposes a two-step approach to introducing unused inline config reporting: +This RFC proposes a two-step approach to introducing unused config reporting: -1. In the current major version upon introduction, don't enable unused inline config reporting by default -2. In the next major version, enable unused inline config reporting by default +1. In the current major version upon introduction, don't enable unused config reporting by default +2. In the next major version, enable unused config reporting by default -Note that the default value in the next major version should be the same as reporting unused disable directives. -See [Change Request: error by default for unused disable directives](https://github.com/eslint/eslint/issues/18665) for an issue on changing that from `"warn"` to `"error"`. +Note that the default value in the next major version should be the same as reporting unused disable directives, `"warn"`. ## Documentation @@ -187,6 +210,10 @@ No new warnings or errors will be reported in the current major version without ## Alternatives +### Deferring to the Config Inspector + +TODO + ### Omitting Legacy Config Support One way to reduce costs could be to wait until ESLint completely removes support for legacy configs. @@ -216,7 +243,7 @@ An additional direction this RFC could propose would be to have the new unused i However: - Deprecating `reportUnusedDisableDirectives` would be a distruptive breaking change -- This RFC proposes enabling `reportUnusedInlineConfigs` by default in a subsequent major version anyway: most users will not be manually configuring it +- This RFC proposes enabling `reportUnusedConfigs` by default in a subsequent major version anyway: most users will not be manually configuring it ## Help Needed @@ -235,8 +262,8 @@ As of [Change Request: Disallow multiple configuration comments for the same rul ## Related Discussions -- : the issue triggering this RFC - - : an older issue that #18230 duplicated +- : issue suggesting to report on unused config file comments +- : issue suggesting to report on unused inline config comments - : previous issue for enabling `reportUnusedDisableDirectives` config option by default - : the RFC discussion flexible config + default reporting of unused disable directives - : the PR implementing custom severity when reporting unused disable directives From ece1353677597579759173983fe0f1412db29de1 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 13 Aug 2024 22:16:59 -0400 Subject: [PATCH 07/15] Revert "Reference: RFC version targeting both config files and inline configs" This reverts commit c339a59bfaec0c48817a012c2f5e92a242a1b1e6. --- .../README.md | 87 +++++++------------ 1 file changed, 30 insertions(+), 57 deletions(-) rename designs/{2024-report-unused-configs => 2024-report-unused-inline-configs}/README.md (70%) diff --git a/designs/2024-report-unused-configs/README.md b/designs/2024-report-unused-inline-configs/README.md similarity index 70% rename from designs/2024-report-unused-configs/README.md rename to designs/2024-report-unused-inline-configs/README.md index 7004e489..c9e48352 100644 --- a/designs/2024-report-unused-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -3,28 +3,26 @@ - RFC PR: https://github.com/eslint/rfcs/pull/121 - Authors: [Josh Goldberg](https://github.com/JoshuaKGoldberg) -# Reporting inline configs +# Reporting unused inline configs ## Summary -Add an option to report config file `rules` values and inline `/* eslint ... */` config comments that don't change any settings. +Add an option to report `/* eslint ... */` comments that don't change any settings. ## Motivation -Right now, nothing in ESLint core stops [config files `rule` entries](https://eslint.org/docs/latest/use/configure/configuration-files#configuring-rules) or [inline configs (configuration comments)](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments) from redundantly repeating the same configuration _option_ and/or _severity_ as the existing computed configuration. -Both forms of configuration are undesirable when redundant: they take up space and can be misleading. +Right now, nothing in ESLint core stops [inline configs (configuration comments)](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments) from redundantly repeating the same configuration _option_ and/or _severity_ as the file's existing computed configuration. +Unused inline configs suffer from the same drawbacks as [unused disable directives](https://eslint.org/docs/latest/use/configure/rules#report-unused-eslint-disable-comments): they take up space and can be misleading. For example: -- [Stackblitz of an `eslint.config.js` setting a rule twice with the same severity](https://stackblitz.com/edit/stackblitz-starters-qcrxqr?file=eslint.config.js) -- [Stackblitz of an `eslint.config.js` setting a rule twice with the same options](https://stackblitz.com/edit/stackblitz-starters-san7bj?file=eslint.config.js) -- [Playground of an inline config setting the same severity as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogXCJlcnJvclwiICovXG5cIuKdjCBkb2VzIG5vdGhpbmc6IGV4aXN0aW5nIHNldmVyaXR5IHZzLiBmaWxlXCJcbiIsIm9wdGlvbnMiOnsicnVsZXMiOnsibm8tdW51c2VkLWV4cHJlc3Npb25zIjpbImVycm9yIl19LCJsYW5ndWFnZU9wdGlvbnMiOnsiZWNtYVZlcnNpb24iOiJsYXRlc3QiLCJzb3VyY2VUeXBlIjoibW9kdWxlIiwicGFyc2VyT3B0aW9ucyI6eyJlY21hRmVhdHVyZXMiOnt9fX19fQ==) +- [Playground of an inline config setting the same severity as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogXCJlcnJvclwiICovXG5cIuKdjCBkb2VzIG5vdGhpbmc6IGV4aXN0aW5nIHNldmVyaXR5IHZzLiBmaWxlXCJcbiIsIm9wdGlvbnMiOnsicnVsZXMiOnt9LCJsYW5ndWFnZU9wdGlvbnMiOnsiZWNtYVZlcnNpb24iOiJsYXRlc3QiLCJzb3VyY2VUeXBlIjoibW9kdWxlIiwicGFyc2VyT3B0aW9ucyI6eyJlY21hRmVhdHVyZXMiOnt9fX19fQ==) - [Playground of an inline config setting the same options as the config file](https://eslint.org/play/#eyJ0ZXh0IjoiLyogZXNsaW50IG5vLXVudXNlZC1leHByZXNzaW9uczogW1wiZXJyb3JcIiwgeyBcImFsbG93U2hvcnRDaXJjdWl0XCI6IHRydWUgfV0gKi9cblwi4p2MIGRvZXMgbm90aGluZzogZXhpc3Rpbmcgc2V2ZXJpdHkgYW5kIG9wdGlvbnMgdnMuIGZpbGVcIlxuIiwib3B0aW9ucyI6eyJydWxlcyI6eyJuby11bnVzZWQtZXhwcmVzc2lvbnMiOlsiZXJyb3IiLHsiYWxsb3dTaG9ydENpcmN1aXQiOnRydWUsImFsbG93VGVybmFyeSI6ZmFsc2UsImFsbG93VGFnZ2VkVGVtcGxhdGVzIjpmYWxzZSwiZW5mb3JjZUZvckpTWCI6ZmFsc2V9XX0sImxhbmd1YWdlT3B0aW9ucyI6eyJlY21hVmVyc2lvbiI6ImxhdGVzdCIsInNvdXJjZVR5cGUiOiJtb2R1bGUiLCJwYXJzZXJPcHRpb25zIjp7ImVjbWFGZWF0dXJlcyI6e319fX19) -This RFC proposes adding the ability for ESLint to report on both of those kinds of unused configs: +This RFC proposes adding the ability for ESLint to report on those unused inline configs: -- `--report-unused-configs` CLI option -- `linterOptions.reportUnusedConfigs` configuration file option +- `--report-unused-inline-configs` CLI option +- `linterOptions.reportUnusedInlineConfigs` configuration file option ```shell npx eslint --report-unused-inline-configs error @@ -33,7 +31,7 @@ npx eslint --report-unused-inline-configs error ```js { linterOptions: { - reportUnusedConfigs: "error", + reportUnusedInlineConfigs: "error", } } ``` @@ -42,7 +40,7 @@ These new options would be similar to the existing [`--report-unused-disable-dir ### Examples -The following table uses [`accessor-pairs`](https://eslint.org/docs/latest/rules/accessor-pairs) as an example. +The following table uses [`accessor-pairs`](https://eslint.org/docs/latest/rules/accessor-pairs) as an example with inline configs like `/* eslint accessor-pairs: "off" */`. It assumes the `accessor-pairs` default options from [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) of: ```json @@ -53,13 +51,11 @@ It assumes the `accessor-pairs` default options from [feat: add meta.defaultOpti } ``` -Values apply both for config files like `rules: { "accessor-pairs": "off" }` and inline configs like `/* eslint accessor-pairs: "off" */`. -
Original Config SettingInline Config SettingsFirst SettingsSecond Settings Report?
- - + + @@ -133,55 +129,36 @@ Values apply both for config files like `rules: { "accessor-pairs": "off" }` and ## Detailed Design -This RFC proposes changing logical entry points only for the current ("flat") config system. -Legacy ("eslintrc") configs are omitted to reduce maintenance costs. - -This proposed design does intentionally not involve any language-specific code changes. -How a specific language computes its configuration comments is irrelevant to this proposed feature. - -### Glossary - -For the sake of specificity, the following terms are used intentionally: - -- **Options**: The options passed to a rule, as described by its `meta.schema` -- **Setting**: A configuration entry specifying a rule name, its severity, and optionally its _options_ -- **Config file entry**: In an `eslint.config.js` file `rules` object, a key-value pair representing a single rule's _settings_ -- **Inline config**: In a source file, a comment representing a single rule's _settings_ - -### Logic Entry Points +Additional logic can be added to the existing code points in `Linter` that validate inline config options: -For rough code references, see: +- [`_verifyWithFlatConfigArrayAndWithoutProcessors`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L1650): called in the current ("flat") config system +- [`getDirectiveComments`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L387): called when `ESLINT_USE_FLAT_CONFIG=true` -- Config file entries: _(todo)_ -- Inline config comments: [`poc: reporting unused inline configs`](https://github.com/JoshuaKGoldberg/eslint/commit/e14e404ed93e6238bdee817923a449f5215eecd8) +For a rough code reference, see [`poc: reporting unused inline configs`](https://github.com/JoshuaKGoldberg/eslint/commit/e14e404ed93e6238bdee817923a449f5215eecd8). +This proposed design does intentionally not involve any language-specific code changes. +How a specific language computes its configuration comments is irrelevant to this proposed feature. ### Computing Option Differences -Each setting will be compared against the existing configuration value(s) it attempts to override: +Each inline config comment will be compared against the existing configuration value(s) it attempts to override: -- If the setting only specifies a severity, then only the severity will be checked for redundancy +- If the config comment only specifies a severity, then only the severity will be checked for redundancy - The new logic will normalize options: `"off"` will be considered equivalent to `0` -- If the setting also specifies rule options, they will be compared for deep equality to the existing rule options +- If the config comment also specifies rule options, they will be compared for deep equality to the existing rule options This RFC should wait to begin work until after [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) is merged into ESLint. -That way, a rule's `meta.defaultOptions` can be factored into computing whether an setting's rule options differ from the previously configured options. - -### Config Files - -Only configs in the user's code will be checked. -Third-party shareable configs will not be validated. - -For config files, that means only `rules` entries in the user's specified config file will be checked. +That way, a rule's `meta.defaultOptions` can be factored into computing whether an inline config's rule options differ from the previously configured options. ### Default Values -This RFC proposes a two-step approach to introducing unused config reporting: +This RFC proposes a two-step approach to introducing unused inline config reporting: -1. In the current major version upon introduction, don't enable unused config reporting by default -2. In the next major version, enable unused config reporting by default +1. In the current major version upon introduction, don't enable unused inline config reporting by default +2. In the next major version, enable unused inline config reporting by default -Note that the default value in the next major version should be the same as reporting unused disable directives, `"warn"`. +Note that the default value in the next major version should be the same as reporting unused disable directives. +See [Change Request: error by default for unused disable directives](https://github.com/eslint/eslint/issues/18665) for an issue on changing that from `"warn"` to `"error"`. ## Documentation @@ -210,10 +187,6 @@ No new warnings or errors will be reported in the current major version without ## Alternatives -### Deferring to the Config Inspector - -TODO - ### Omitting Legacy Config Support One way to reduce costs could be to wait until ESLint completely removes support for legacy configs. @@ -243,7 +216,7 @@ An additional direction this RFC could propose would be to have the new unused i However: - Deprecating `reportUnusedDisableDirectives` would be a distruptive breaking change -- This RFC proposes enabling `reportUnusedConfigs` by default in a subsequent major version anyway: most users will not be manually configuring it +- This RFC proposes enabling `reportUnusedInlineConfigs` by default in a subsequent major version anyway: most users will not be manually configuring it ## Help Needed @@ -262,8 +235,8 @@ As of [Change Request: Disallow multiple configuration comments for the same rul ## Related Discussions -- : issue suggesting to report on unused config file comments -- : issue suggesting to report on unused inline config comments +- : the issue triggering this RFC + - : an older issue that #18230 duplicated - : previous issue for enabling `reportUnusedDisableDirectives` config option by default - : the RFC discussion flexible config + default reporting of unused disable directives - : the PR implementing custom severity when reporting unused disable directives From 8cf920b3c8ea5586afbd2002d9f69fe5f3ec7675 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 13 Aug 2024 22:25:27 -0400 Subject: [PATCH 08/15] Mention c339a59bfaec0c48817a012c2f5e92a242a1b1e6 --- .../README.md | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index c9e48352..8f11b873 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -129,16 +129,9 @@ It assumes the `accessor-pairs` default options from [feat: add meta.defaultOpti ## Detailed Design -Additional logic can be added to the existing code points in `Linter` that validate inline config options: - -- [`_verifyWithFlatConfigArrayAndWithoutProcessors`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L1650): called in the current ("flat") config system -- [`getDirectiveComments`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L387): called when `ESLINT_USE_FLAT_CONFIG=true` - +Additional logic can be added to the existing code points in `Linter` that validate inline config options: [`_verifyWithFlatConfigArrayAndWithoutProcessors`](https://github.com/eslint/eslint/blob/7c78ad9d9f896354d557f24e2d37710cf79a27bf/lib/linter/linter.js#L1636). For a rough code reference, see [`poc: reporting unused inline configs`](https://github.com/JoshuaKGoldberg/eslint/commit/e14e404ed93e6238bdee817923a449f5215eecd8). -This proposed design does intentionally not involve any language-specific code changes. -How a specific language computes its configuration comments is irrelevant to this proposed feature. - ### Computing Option Differences Each inline config comment will be compared against the existing configuration value(s) it attempts to override: @@ -178,22 +171,39 @@ The new settings will be documented similarly to reporting unused disable direct Any added options come with an added tax on project maintenance and user comprehension. This RFC believes the flagging of unused inline configs is worth that tax. -See [Omitting Legacy Config Support](#omitting-legacy-config-support) for a possible reduction in cost. - ## Backwards Compatibility Analysis The proposed two-step approach introduces the options in a fully backwards-compatible way. No new warnings or errors will be reported in the current major version without the user explicitly opting into them. +## Out of Scope + +### Language-Specific Changes + +This proposed design does intentionally not involve any language-specific code changes. +How a specific language computes its configuration comments is irrelevant to this proposed feature. + +### Legacy ("eslintrc") Configs + +ESLint v9 is released and flat configs are the default for users. +The legacy config system will likely be removed in the next major version of ESLint. +This RFC prefers to avoid the added maintenance cost of supporting the legacy config system. + +If this RFC's new CLI flag or config file entry are enabled with using a legacy config, ESLint should throw an error. + ## Alternatives -### Omitting Legacy Config Support +### Checking Config File Values + +[eslint/eslint#15476 Change Request: report unnecessary config overrides](https://github.com/eslint/eslint/issues/15476) previously suggested also checking config files (`eslint.config.*`). +Doing so could be beneficial to flag values that become unnecessary in config files over time. +However, because flat config purely involves spreading objects, there's no way to know what objects originate from shared configs or shared packages. -One way to reduce costs could be to wait until ESLint completely removes support for legacy configs. -That way, only the new ("flat") config system would need to be tested with this change. +c339a59bfaec0c48817a012c2f5e92a242a1b1e6 is a reference commit of how this RFC might look with that support added in. -However, it is unclear when the legacy config system will be removed from ESLint core. -This RFC prefers taking action sooner rather than later. +An alternative could have been to require users provide metadata along with their shared configs, and/or wrap them in some function provided by ESLint. +That could be a future requirement opted into ESLint. +It's too late now to add that to the flat config system, and as such is out of scope for this RFC. ### Separate CLI Option @@ -236,7 +246,7 @@ As of [Change Request: Disallow multiple configuration comments for the same rul ## Related Discussions - : the issue triggering this RFC - - : an older issue that #18230 duplicated + - : previous issue suggesting reporting unnecessary config overrides - : previous issue for enabling `reportUnusedDisableDirectives` config option by default - : the RFC discussion flexible config + default reporting of unused disable directives - : the PR implementing custom severity when reporting unused disable directives From 5004de85dfed8b73eb652cf54b1bd02a8a5c117c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 13 Aug 2024 22:26:31 -0400 Subject: [PATCH 09/15] Make c339a5 a link --- designs/2024-report-unused-inline-configs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 8f11b873..279ff0c9 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -199,7 +199,7 @@ If this RFC's new CLI flag or config file entry are enabled with using a legacy Doing so could be beneficial to flag values that become unnecessary in config files over time. However, because flat config purely involves spreading objects, there's no way to know what objects originate from shared configs or shared packages. -c339a59bfaec0c48817a012c2f5e92a242a1b1e6 is a reference commit of how this RFC might look with that support added in. +[c339a5](https://github.com/eslint/rfcs/pull/121/commits/c339a59bfaec0c48817a012c2f5e92a242a1b1e6) is a reference commit of how this RFC might look with that support added in. An alternative could have been to require users provide metadata along with their shared configs, and/or wrap them in some function provided by ESLint. That could be a future requirement opted into ESLint. From 85a9b6bdcf8a9450ebcb64989f9565cb1cb11947 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 13 Aug 2024 23:29:41 -0400 Subject: [PATCH 10/15] A bit of clarification on options --- designs/2024-report-unused-inline-configs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 279ff0c9..08100800 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -37,6 +37,7 @@ npx eslint --report-unused-inline-configs error ``` These new options would be similar to the existing [`--report-unused-disable-directives(-severity)`](https://eslint.org/docs/latest/use/command-line-interface#--report-unused-disable-directives) and [`linterOptions.reportUnusedDisableDirectives`](https://eslint.org/docs/latest/use/configure/configuration-files#reporting-unused-disable-directives) options. +However, this RFC proposes a single option to both enable the report and configure its severity, rather than two separate options. ### Examples @@ -214,7 +215,6 @@ Adding a sole `--report-unused-inline-configs` CLI option presents a discrepency An alternative could be to instead add `--report-unused-inline-configs` and `--report-unused-inline-configs-severity` options for consistency's sake. This RFC's opinion is that the consistency of adding two new options is not worth the excess options logic. -It would instead be preferable to, in a future ESLint major version, deprecate `--report-unused-disable-directives-severity` and merge its logic setting into `--report-unused-disable-directives`. ### Superset Behavior: Unused Disable Directive Reporting From f15fcdace87f0d806f7f06977b80add0601ef76a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 13 Aug 2024 23:35:08 -0400 Subject: [PATCH 11/15] 'distruptive' --- designs/2024-report-unused-inline-configs/README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 08100800..c2bf7f24 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -223,10 +223,9 @@ Reporting on unused disable directives could be thought of as a subset of report An additional direction this RFC could propose would be to have the new unused inline config reporting act as a superset of unused disable directive reporting. -However: - -- Deprecating `reportUnusedDisableDirectives` would be a distruptive breaking change -- This RFC proposes enabling `reportUnusedInlineConfigs` by default in a subsequent major version anyway: most users will not be manually configuring it +However, deprecating `reportUnusedDisableDirectives` would be a disruptive breaking change. +This RFC prefers keeping away from larger changes like that. +A future change in a subsequent major version could take that on separately. ## Help Needed From cc46547e630697c3a68513f6568fd3c2c9368089 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 20 Aug 2024 11:19:13 -0400 Subject: [PATCH 12/15] Rephrases: aladdin-add --- designs/2024-report-unused-inline-configs/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index c2bf7f24..586a2547 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -142,7 +142,7 @@ Each inline config comment will be compared against the existing configuration v - If the config comment also specifies rule options, they will be compared for deep equality to the existing rule options This RFC should wait to begin work until after [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) is merged into ESLint. -That way, a rule's `meta.defaultOptions` can be factored into computing whether an inline config's rule options differ from the previously configured options. +That way, if a rule defines `meta.defaultOptions`, those default options can be factored into computing whether an inline config's rule options differ from the previously configured options. ### Default Values @@ -223,7 +223,7 @@ Reporting on unused disable directives could be thought of as a subset of report An additional direction this RFC could propose would be to have the new unused inline config reporting act as a superset of unused disable directive reporting. -However, deprecating `reportUnusedDisableDirectives` would be a disruptive breaking change. +However, deprecating `reportUnusedDisableDirectives` would be a disruptive change and eventual disruptive breaking change. This RFC prefers keeping away from larger changes like that. A future change in a subsequent major version could take that on separately. From 8536d68ad95cfaa916f8cbdbf331239b15f8c867 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 20 Aug 2024 12:07:17 -0400 Subject: [PATCH 13/15] Feedback: nzakas --- designs/2024-report-unused-inline-configs/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 586a2547..701042cd 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -177,6 +177,11 @@ This RFC believes the flagging of unused inline configs is worth that tax. The proposed two-step approach introduces the options in a fully backwards-compatible way. No new warnings or errors will be reported in the current major version without the user explicitly opting into them. +## Performance Analysis + +This RFC believes there will be no nontrivial performance impact from this change. +All rule lookups from inline comments are O(1) compared to the existing computed rules for a file. + ## Out of Scope ### Language-Specific Changes @@ -202,6 +207,8 @@ However, because flat config purely involves spreading objects, there's no way t [c339a5](https://github.com/eslint/rfcs/pull/121/commits/c339a59bfaec0c48817a012c2f5e92a242a1b1e6) is a reference commit of how this RFC might look with that support added in. +Instead, this RFC suggests that the [config inspector](https://github.com/eslint/config-inspector) would be a more natural place to see these conflicts. + An alternative could have been to require users provide metadata along with their shared configs, and/or wrap them in some function provided by ESLint. That could be a future requirement opted into ESLint. It's too late now to add that to the flat config system, and as such is out of scope for this RFC. From 63b48f2653deffe1668cdee3235f0f9e7f95f628 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 20 Aug 2024 12:07:54 -0400 Subject: [PATCH 14/15] Feedback: nzakas --- designs/2024-report-unused-inline-configs/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index 701042cd..d9a0295a 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -180,7 +180,8 @@ No new warnings or errors will be reported in the current major version without ## Performance Analysis This RFC believes there will be no nontrivial performance impact from this change. -All rule lookups from inline comments are O(1) compared to the existing computed rules for a file. +All rule lookups from inline configs are O(1) compared to the existing computed rules for a file. +It's rare in userland code to have more than a single digit number of inline configs in any file. ## Out of Scope From dbd33bc403a6ea6deaa8af99d2823083c7c1e436 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 2 Sep 2024 13:05:03 -0400 Subject: [PATCH 15/15] Clarify when to report --- designs/2024-report-unused-inline-configs/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/designs/2024-report-unused-inline-configs/README.md b/designs/2024-report-unused-inline-configs/README.md index d9a0295a..1be35e72 100644 --- a/designs/2024-report-unused-inline-configs/README.md +++ b/designs/2024-report-unused-inline-configs/README.md @@ -141,6 +141,9 @@ Each inline config comment will be compared against the existing configuration v - The new logic will normalize options: `"off"` will be considered equivalent to `0` - If the config comment also specifies rule options, they will be compared for deep equality to the existing rule options +There will be a report if both the severity and rule options are the same. +If there is a difference in the severity and/or rule options, then there will be no report. + This RFC should wait to begin work until after [feat: add meta.defaultOptions](https://github.com/eslint/eslint/pull/17656) is merged into ESLint. That way, if a rule defines `meta.defaultOptions`, those default options can be factored into computing whether an inline config's rule options differ from the previously configured options.
First SettingsSecond SettingsOriginal Config SettingInline Config Settings Report?