From 41598f3a0d81f56ae6c75f59273269a54db50085 Mon Sep 17 00:00:00 2001 From: Samuel Therrien Date: Tue, 22 Oct 2024 13:50:04 -0400 Subject: [PATCH 01/13] 2024-per-rule-autofix-configuration initial commit --- .../README.md | 159 ++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 designs/2024-per-rule-autofix-configuration/README.md diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md new file mode 100644 index 00000000..79f2cbeb --- /dev/null +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -0,0 +1,159 @@ +- Repo: +- Start Date: 2024-10-22 +- RFC PR: +- Authors: [Samuel Therrien](https://github.com/Samuel-Therrien-Beslogic) (aka [@Avasam](https://github.com/Avasam)) + +# Per-rule autofix configuration + +## Summary + + +This feature aims to make it possible to control autofixes through shareable configuration on a per-rule basis. + +## Motivation + + +Some rules provide autofixing, which is great, but can sometimes be broken or otherwise simply unwanted for various reasons. +Unsafe autofixes should be suggestions, and broken fixes should be reported, *but* ESLint is a large ecosystem where some very useful plugins are not always actively maintained. Even then, wanting to disable an autofix for project-specific or personal reasons could still happen. + +## Detailed Design + + + +Similar to how Ruff () does it, a top-level key to specify which rules to not autofix would be in my opinion the least disruptive and forward/backwards compatible. It should be overridable (in the `overrides` section), and picked up when extending a configuration. + +Concretely, it could look like this: + +```js +export default [ + { + autofixes: { + // We don't want this to autofix, as a rule suddenly not failing should require human attention + "@eslint-community/eslint-comments/no-unused-disable": false, + }, + rules: { + '@eslint-community/eslint-comments/no-unused-disable': 'error', + } + overrides: [ + files: ["*.spec.js"], + autofixes: { + // Let's pretend we want this to be autofixed in tests, for the sake of the RFC + "@eslint-community/eslint-comments/no-unused-disable": true, + }, + ] + } +] +``` + +I think that disabling autofixes for a rule that doesn't have any or doesn't exist should be a no-op. Just like disabling a rule that doesn't exist. The reasoning being that this allows much more flexible shareable configurations. +It's still an open question whether *enabling* autofixes for a rule that doesn't exist should warn, error or be silent. + +## Documentation + + +I think that "Configuring autofixes" or "Disabling autofixes" could be documented as a subsection of [Configuring Rules](https://eslint.org/docs/latest/use/configure/rules). Or as a section on the same level (between "Configuring Rules" and "Configuring Plugins") + +## Drawbacks + + +A potential drawback I could see is that the configuration for autofixing a rule is not directly related with the rule itself. As a counter, I'd say this is already the case for plenty of rule-related settings, environment and parser configurations, etc. It's also less of a drawback than [Alternatives - Configure in the rule itself](#configure-in-the-rule-itself). + +## Backwards Compatibility Analysis + + +Given that this proposal adds a new optional configuration section, this feature should be fully backwards compatible. Users that don't want to use this feature should stay completely unaffected. (see [Alternatives - Configure in the rule itself](#configure-in-the-rule-itself)) + +## Alternatives + + + +### Configure in the rule itself + +Another approach I can think of is to encode that in the rule config itself. Something like `"my-plugin/my-rule": "[{severity: "error", autofix: False}, {...otherConfigs}]"` but it's harder to commit to such a change, and means that any config extension needs to reconfigure the rule correctly just to disable autofixing (which is already an issue when someone wants to set a pre-configured rule as warning for example) + +### Use of a 3rd-party plugin + + is a tool that exists to currently work around this limitation of ESLint, but it is not perfect. + +1. It is an extra third-party dependency, with its own potential maintenance issues (having to keep up with ESLint, separate dependencies that can fall out of date, obsolete, unsecure, etc.) +2. It may not work in all environments. For example, pre-commit.ci: +3. It may not work correctly with all third-party rules: + +## Open Questions + + +- Where exactly should the documentation go ? +- Should the value be more than a boolean ? (for example if we want to affect offering suggestions in editors) +- What should the key for the new configuration be ? +- What happens if we mark a rule as "should be autofixed" but there's no fix available? Warn? Silently ignore? +- Whether *enabling* autofixes for a rule that doesn't exist should warn, error or be silent. + +## Help Needed + + +My knowledge of ESLint's internals isn't that great. Whilst I think it's above the average user due to reading and configuring a lot, I haven't yet even learned how to write a plugin, and haven't migrated any project to ESLint 9 yet. +My free time both at work and personal, is currently also very limited (see how long it too me to just get to writing this RFC). +So I unfortunately don't think I can implement this feature myself, due to both a lack of time, personal motivation (I won't be able to use it for a while, but will push us towards ESLint 9 once implemented), and experience. + +## Frequently Asked Questions + + + +## Related Discussions + + + From 7497a524b5e04ded495d59aef22f33e6622d3e2d Mon Sep 17 00:00:00 2001 From: Samuel Therrien Date: Tue, 22 Oct 2024 14:42:48 -0400 Subject: [PATCH 02/13] Clarify that suggestions should not be disabled. Autofixes are "demoted" to suggestions. --- designs/2024-per-rule-autofix-configuration/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 79f2cbeb..8876ee50 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -53,6 +53,8 @@ export default [ ] ``` +The fix should still exist as a suggestion. Only autofixing (when running `eslint --fix` or editor action on save) should be disabled. + I think that disabling autofixes for a rule that doesn't have any or doesn't exist should be a no-op. Just like disabling a rule that doesn't exist. The reasoning being that this allows much more flexible shareable configurations. It's still an open question whether *enabling* autofixes for a rule that doesn't exist should warn, error or be silent. @@ -121,7 +123,6 @@ Another approach I can think of is to encode that in the rule config itself. Som you can remove this section. --> - Where exactly should the documentation go ? -- Should the value be more than a boolean ? (for example if we want to affect offering suggestions in editors) - What should the key for the new configuration be ? - What happens if we mark a rule as "should be autofixed" but there's no fix available? Warn? Silently ignore? - Whether *enabling* autofixes for a rule that doesn't exist should warn, error or be silent. From ef18a67e2ff80b213f21910bca81cdb7ce662010 Mon Sep 17 00:00:00 2001 From: Samuel Therrien Date: Thu, 24 Oct 2024 11:19:02 -0400 Subject: [PATCH 03/13] Update configuration example, addressing michaelfaith comments --- .../README.md | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 8876ee50..9547c48d 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -28,35 +28,34 @@ Unsafe autofixes should be suggestions, and broken fixes should be reported, *bu used. Be sure to define any new terms in this section. --> -Similar to how Ruff () does it, a top-level key to specify which rules to not autofix would be in my opinion the least disruptive and forward/backwards compatible. It should be overridable (in the `overrides` section), and picked up when extending a configuration. +Similar to how Ruff () does it, a top-level key to specify which rules to not autofix would be in my opinion the least disruptive and forward/backwards compatible. It should be overridable in per-file configurations, and picked up when extending a configuration. Concretely, it could look like this: ```js export default [ { - autofixes: { + disableAutofixes: { // We don't want this to autofix, as a rule suddenly not failing should require human attention - "@eslint-community/eslint-comments/no-unused-disable": false, + "@eslint-community/eslint-comments/no-unused-disable": true, }, rules: { '@eslint-community/eslint-comments/no-unused-disable': 'error', } - overrides: [ - files: ["*.spec.js"], - autofixes: { - // Let's pretend we want this to be autofixed in tests, for the sake of the RFC - "@eslint-community/eslint-comments/no-unused-disable": true, - }, - ] - } + }, + { + files: ["*.spec.js"], + disableAutofixes: { + // Let's pretend we want this to be autofixed in tests, for the sake of the RFC + "@eslint-community/eslint-comments/no-unused-disable": false, + }, + }, ] ``` The fix should still exist as a suggestion. Only autofixing (when running `eslint --fix` or editor action on save) should be disabled. -I think that disabling autofixes for a rule that doesn't have any or doesn't exist should be a no-op. Just like disabling a rule that doesn't exist. The reasoning being that this allows much more flexible shareable configurations. -It's still an open question whether *enabling* autofixes for a rule that doesn't exist should warn, error or be silent. +The chosen key name `disableAutofixes` aims to remove the concern about "turning on" an autofix that doesn't exist. Disabling autofixes for a rule that doesn't have any or doesn't exist should be a no-op. Just like turning `off` a rule that doesn't exist. The reasoning being that this allows much more flexible shareable configurations. ## Documentation @@ -123,9 +122,6 @@ Another approach I can think of is to encode that in the rule config itself. Som you can remove this section. --> - Where exactly should the documentation go ? -- What should the key for the new configuration be ? -- What happens if we mark a rule as "should be autofixed" but there's no fix available? Warn? Silently ignore? -- Whether *enabling* autofixes for a rule that doesn't exist should warn, error or be silent. ## Help Needed @@ -157,4 +153,6 @@ So I unfortunately don't think I can implement this feature myself, due to both If there is an issue, pull request, or other URL that provides useful context for this proposal, please include those links here. --> - +- +- +- From e4440f9cfe20e1b192e898a3753d87f4331dc3d7 Mon Sep 17 00:00:00 2001 From: Samuel Therrien Date: Thu, 24 Oct 2024 11:28:37 -0400 Subject: [PATCH 04/13] Add FAQ entry about array vs record --- designs/2024-per-rule-autofix-configuration/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 9547c48d..9a2f9fcf 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -145,6 +145,9 @@ So I unfortunately don't think I can implement this feature myself, due to both in this section. --> +Q: Could `disableAutofixes` be an array of autofixes to disable? +A: `disableAutofixes` is a record to allow re-enabling autofixes in downstream configurations and on a per-file basis. We could allow a shorthand to `disableAutofixes` to accept an array of rules to disable the autofix for, but that would result in additional complexity on the implementation side with marginal benefits to the user. + ## Related Discussions I think that "Configuring autofixes" or "Disabling autofixes" could be documented as a subsection of [Configuring Rules](https://eslint.org/docs/latest/use/configure/rules). Or as a section on the same level (between "Configuring Rules" and "Configuring Plugins") +As a new top-level property added to configuration objects, `disableAutofixes` should be documented in [Configuration Files > Configuration Objects](https://eslint.org/docs/latest/use/configure/configuration-files#configuration-objects) section. Additionally we may want to add a note to [Custom Rules](https://eslint.org/docs/latest/extend/custom-rulesd) to mention that some autofixes will be converted automatically into suggestions when the new feature is used. + ## Drawbacks - Where exactly should the documentation go ? +- Where this needs to be implemented in code. Those familiar with ESLint's codebase are welcome to provide this information ## Help Needed From 903c57e975af01d91326da3f6b0284ff8c037790 Mon Sep 17 00:00:00 2001 From: "Samuel T." Date: Tue, 21 Jan 2025 14:33:09 -0500 Subject: [PATCH 06/13] Add related discussion about `@eslint-community/eslint-comments/no-unused-disable` https://github.com/eslint-community/eslint-plugin-eslint-comments/issues/249#issuecomment-2605557271 --- designs/2024-per-rule-autofix-configuration/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 1451cdcd..6d173fcd 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -177,3 +177,4 @@ A: `disableAutofixes` is a record to allow re-enabling autofixes in downstream c - - - +- From aa8bad83e4567a96dd6f9f14a8f01cbce96ae4f2 Mon Sep 17 00:00:00 2001 From: "Samuel T." Date: Tue, 21 Jan 2025 14:39:14 -0500 Subject: [PATCH 07/13] Fix typo in link --- designs/2024-per-rule-autofix-configuration/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 6d173fcd..60e1872e 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -80,7 +80,7 @@ The chosen key name `disableAutofixes` aims to remove the concern about "turning --> I think that "Configuring autofixes" or "Disabling autofixes" could be documented as a subsection of [Configuring Rules](https://eslint.org/docs/latest/use/configure/rules). Or as a section on the same level (between "Configuring Rules" and "Configuring Plugins") -As a new top-level property added to configuration objects, `disableAutofixes` should be documented in [Configuration Files > Configuration Objects](https://eslint.org/docs/latest/use/configure/configuration-files#configuration-objects) section. Additionally we may want to add a note to [Custom Rules](https://eslint.org/docs/latest/extend/custom-rulesd) to mention that some autofixes will be converted automatically into suggestions when the new feature is used. +As a new top-level property added to configuration objects, `disableAutofixes` should be documented in [Configuration Files > Configuration Objects](https://eslint.org/docs/latest/use/configure/configuration-files#configuration-objects) section. Additionally we may want to add a note to [Custom Rules](https://eslint.org/docs/latest/extend/custom-rules) to mention that some autofixes will be converted automatically into suggestions when the new feature is used. ## Drawbacks From 72a6cd56795072d8521944c7ab08cb84a65a9f8a Mon Sep 17 00:00:00 2001 From: MorevM Date: Wed, 21 May 2025 20:01:44 +0300 Subject: [PATCH 08/13] feat: Update the RFC --- .../README.md | 322 +++++++++++++++--- 1 file changed, 274 insertions(+), 48 deletions(-) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 60e1872e..cde81d03 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -1,21 +1,30 @@ - Repo: - Start Date: 2024-10-22 - RFC PR: -- Authors: [Samuel Therrien](https://github.com/Samuel-Therrien-Beslogic) (aka [@Avasam](https://github.com/Avasam)) +- Authors: + - [Samuel Therrien](https://github.com/Samuel-Therrien-Beslogic) (aka [@Avasam](https://github.com/Avasam)) + - [Maxim Morev](https://github.com/MorevM) # Per-rule autofix configuration ## Summary -This feature aims to make it possible to control autofixes through shareable configuration on a per-rule basis. + +This RFC proposes adding support for controlling autofix behavior on a per-rule basis through shareable configuration. ## Motivation -Some rules provide autofixing, which is great, but can sometimes be broken or otherwise simply unwanted for various reasons. -Unsafe autofixes should be suggestions, and broken fixes should be reported, *but* ESLint is a large ecosystem where some very useful plugins are not always actively maintained. Even then, wanting to disable an autofix for project-specific or personal reasons could still happen. + +Some rules support autofixing, which is often convenient, but in certain cases +the fixes may be broken, unsafe, or simply undesirable. \ +Ideally, unsafe autofixes should be treated as suggestions, and broken fixes should be reported. + +However, ESLint is a large ecosystem, and some useful plugins are no longer actively maintained. +Even in actively maintained projects, users may want to disable autofixing for specific rules +due to project-specific or personal preferences. ## Detailed Design @@ -28,49 +37,234 @@ Unsafe autofixes should be suggestions, and broken fixes should be reported, *bu used. Be sure to define any new terms in this section. --> -Similar to how Ruff () does it, a top-level key to specify which rules to not autofix would be in my opinion the least disruptive and forward/backwards compatible. It should be overridable in per-file configurations, and picked up when extending a configuration. +### Abstract + +We introduce an *extended rule configuration format*: + +```ts +type RuleEntry = Partial<{ + /** + * The rule severity. + * + * @default 2 + */ + severity: 0 | 1 | 2 | 'off' | 'warn' | 'error'; + + /** + * Array of options passed to the rule. + * + * @default [] + */ + options: any[] + + /** + * Whether to allow a rule to perform autofixes. + * + * @default true + */ + autofix: boolean; +}>; +``` + +This format enables explicit control over a rule's autofix behavior, +and opens the door to further extension with other meta-properties +([example](https://github.com/eslint/eslint/issues/19342)). + +> [!IMPORTANT] +> This is an additional configuration format; existing configurations remain valid and do not need to be rewritten. + +#### Autofix details + +* The `autofix` option defaults to `true`, maintaining current behavior + (if a rule provides a fixer, it will run when `--fix` is used); +* If autofix is disabled for a rule, its fix becomes a suggestion labeled + "Apply disabled autofix" and can be applied manually through editor hints; +* If the rule does not define a fixer, the `autofix` key has no effect. -Concretely, it could look like this: +#### Additional extensions + +We extend `linterOptions.reportUnusedDisableDirectives` +and `linterOptions.reportUnusedInlineConfigs`(?) to support new extended format: ```js export default [ { - disableAutofixes: { - // We don't want this to autofix, as a rule suddenly not failing should require human attention - "@eslint-community/eslint-comments/no-unused-disable": true, + linterOptions: { + reportUnusedDisableDirectives: { + severity: 'error', + autofix: false, + }, }, - rules: { - '@eslint-community/eslint-comments/no-unused-disable': 'error', - } }, +]; +``` + +We extend CLI engine to support new format as an additional option to existing ones: + +```bash +npx eslint --fix --rule 'no-var: { autofix: false }' +``` + +We extend directive parser engine to support new format as well: + +```js +/* eslint eqeqeq: "off", curly: { severity: "warn", autofix: false } */ +``` + +### Example + +```js +export default [ { - files: ["*.spec.js"], - disableAutofixes: { - // Let's pretend we want this to be autofixed in tests, for the sake of the RFC - "@eslint-community/eslint-comments/no-unused-disable": false, + rules: { + // Full rule entry with disabled autofix + 'eqeqeq': { + severity: 'error', + options: ['always'], + autofix: false, + }, + // Options can be omitted if not needed + 'no-var': { + severity: 'error', + autofix: false, + }, + // Severity can be omitted if not needed + 'camelcase': { + options: [{ properties: 'never' }], + autofix: false, + }, + // Autofix can be omitted if not needed + 'yoda': { + options: ['never'], + }, + // Old format remains valid and will be normalized internally + 'no-regex-spaces': 'error', + 'no-unneeded-ternary': ['error', { + defaultAssignment: false, + }] }, }, -] +]; ``` -The fix should still exist as a suggestion. Only autofixing (when running `eslint --fix` or editor action on save) should be disabled. +> [!NOTE] +> In practice, users only need the full form when disabling autofix +> or when being explicitly descriptive. + +
+ Backward compatibility: existing configurations + + ```js + export default [ + { + linterOptions: { + reportUnusedDisableDirectives: 'error', + }, + rules: { + 'eqeqeq': 'error', + 'no-var': ['warn', 'always'], + '@scope/some-rule': ['warn', 'always', { option: true }], + }, + }, + ]; + ``` + + Will be normalized as: + + ```js + export default [ + { + linterOptions: { + reportUnusedDisableDirectives: { + severity: 'error', + autofix: true, + }, + }, + rules: { + 'eqeqeq': { + severity: 'error', + options: [], + autofix: true, + }, + 'no-var': { + severity: 'warn', + options: ['always'], + autofix: true, + }, + 'some-plugin/some-rule': { + severity: 'warn', + options: ['always', { option: true }], + autofix: true, + }, + }, + }, + ]; + ``` +
-This means removing the `LintMessage.fix` object into the `LintMessage.suggestions` array ([`LintMessage` API](https://eslint.org/docs/latest/integrate/nodejs-api#-lintmessage-type)) +### Implementation plan -```js -const newSuggestion = { - desc: 'Apply the disabled autofix', - fix: lintMessage.fix, -} -if (!lintMessage.suggestions) { - lintMessage.suggestions = [newSuggestion] -} else { - lintMessage.suggestions.push(newSuggestion) -} -lintMessage.fix = undefined -``` +#### 1. Normalize rule entries and configuration objects to the extended format + +> [!NOTE] +> This is actually not fully related to the problem of this RFC, just a step to implementation and future extensibility. + +As a first step, we normalize rule entries and `linterOptions.reportUnusedDisableDirectives` to the new extended format. + +Relevant starting points: + +* [@eslint/core > types.ts > LinterOptionsConfig](https://github.com/eslint/rewrite/blob/2020d38f9dbaabb9923b8f2116e7bcbafa530c85/packages/core/src/types.ts#L659) +* [@eslint/core > types.ts > RuleConfig](https://github.com/eslint/rewrite/blob/2020d38f9dbaabb9923b8f2116e7bcbafa530c85/packages/core/src/types.ts#L679) +* [types/index.d.ts > RuleEntry](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/types/index.d.ts#L1377) +* [types/index.d.ts > LinterOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/types/index.d.ts#L1876) +* --- +* [flat-config-schema.js > rulesSchema](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L450) +* [flat-config-schema.js > disableDirectiveSeveritySchema](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L298) +* [flat-config-schema.js > normalizeRuleOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L132) +* [config.js > #normalizeRulesConfig](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/config.js#L515) +* [config.js > validateRulesConfig](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/config.js#L553) +* [linter/apply-disable-directives.js](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/apply-disable-directives.js#L474) should take a full form of the entry. -The chosen key name `disableAutofixes` aims to remove the concern about "turning on" an autofix that doesn't exist. Disabling autofixes for a rule that doesn't have any or doesn't exist should be a no-op. Just like turning `off` a rule that doesn't exist. The reasoning being that this allows much more flexible shareable configurations. +Some of static methods / individual functions also need to be considered, +like [config.js > getRuleNumericSeverity](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/config.js#L634) +or [linter.js > getRuleOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L1086). + +There are likely additional functions that operate on non-normalized rule entries; tests should help identify them. \ +There's also an open question here regarding support for the legacy config format, as the current code relies on `@eslint/eslintrc` in some places. + +Next, the CLI engine and directive parser should be verified to support object-form rule entries. + +--- + +Next, all consumers of normalized rule entries and `reportUnusedDisableDirectives` must be updated accordingly. + +For directives, updating [apply-disable-directives.js](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/apply-disable-directives.js#L466) may be sufficient. + +For rules usage is more widespread, there are a lot of usage like [this](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L699), [here](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L2140) and [there](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L1086). \ +I'm not sure it's possible to identify them all and that it's necessary to list them here, hopefully the tests will help. + +Once normalization is in place and consistent, we can safely introduce autofix disabling logic. + +#### 2. Disabling autofixes + +##### Disabling for rules + +I think the earliest place where we can account for disabled autofix is [here](https://github.com/eslint/eslint/blob/52f5b7a0af48a2f143f0bccfd4e036025b08280d/lib/linter/linter.js#L1249), +when creating the `ReportTranslator`. \ +We have the context of the rule and its extended configuration, +and we can pass that into `createReportTranslator` as a separate boolean property +(since the current implementation turns off suggestions when `disableFixes: false`, +but we need them if the fixer is disabled). + +Inside `ReportTranslator` we can also convert the fix into a suggestion. + +##### Disabling for directives + +I think all the work in this regard can be done in the +[linter/apply-disabled-directives.js](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/apply-disable-directives.js#L466) +assuming the full entry of `reportUnusedDisableDirectives` configuration is available. + +It's unclear whether suggestions are currently supported for directives - please advise if they are, and where that logic resides. ## Documentation @@ -78,9 +272,30 @@ The chosen key name `disableAutofixes` aims to remove the concern about "turning How will this RFC be documented? Does it need a formal announcement on the ESLint blog to explain the motivation? --> -I think that "Configuring autofixes" or "Disabling autofixes" could be documented as a subsection of [Configuring Rules](https://eslint.org/docs/latest/use/configure/rules). Or as a section on the same level (between "Configuring Rules" and "Configuring Plugins") -As a new top-level property added to configuration objects, `disableAutofixes` should be documented in [Configuration Files > Configuration Objects](https://eslint.org/docs/latest/use/configure/configuration-files#configuration-objects) section. Additionally we may want to add a note to [Custom Rules](https://eslint.org/docs/latest/extend/custom-rules) to mention that some autofixes will be converted automatically into suggestions when the new feature is used. +### Autofix on rules + +I think it is necessary to demonstrate the use of the full form of the record +in the [Configuration Files > Configuring rules](https://eslint.org/docs/latest/use/configure/configuration-files#configuring-rules) section, +that way we show all the available options literally on one screen. + +I think there should be an explanatory section in the [Configure rules](https://eslint.org/docs/latest/use/configure/rules) section, +and maybe it should be the first one on the page to show all the available options at once so that the user is not intimidated +when they see multiple options on the same screen at the same time like: + +```js +/* eslint eqeqeq: "off", curly: { severity: "error", autofix: false } */ +``` + +I also believe that there should be a separate "Disabling autofix" section, +probably located before the [Disabling rules](https://eslint.org/docs/latest/use/configure/rules#disabling-rules) +section, so that it can be directly referenced. + +### `linterOptions.reportUnusedDisableDirectives` + +A note should be added in the +[Configuration Files > Reporting Unused Disable Directives](https://eslint.org/docs/latest/use/configure/configuration-files#reporting-unused-disable-directives) +section that autofix behavior can be configured using the extended format. ## Drawbacks @@ -94,7 +309,11 @@ As a new top-level property added to configuration objects, `disableAutofixes` s experience, etc. Try to identify as many potential problems with implementing this RFC as possible. --> -A potential drawback I could see is that the configuration for autofixing a rule is not directly related with the rule itself. As a counter, I'd say this is already the case for plenty of rule-related settings, environment and parser configurations, etc. It's also less of a drawback than [Alternatives - Configure in the rule itself](#configure-in-the-rule-itself). + +The primary drawback is the complexity of normalizing configuration formats +across the codebase, which requires careful implementation across multiple packages (?). + +It's important to ensure nothing breaks during this transition. ## Backwards Compatibility Analysis @@ -103,7 +322,11 @@ A potential drawback I could see is that the configuration for autofixing a rule change for them? If so, how are you going to minimize the disruption to existing users? --> -Given that this proposal adds a new optional configuration section, this feature should be fully backwards compatible. Users that don't want to use this feature should stay completely unaffected. (see [Alternatives - Configure in the rule itself](#configure-in-the-rule-itself)) + +This feature is fully backwards compatible: + +* `autofix` defaults to `true`, matching current behavior; +* Internal normalization does not affect user configurations or existing workflows. ## Alternatives @@ -114,10 +337,6 @@ Given that this proposal adds a new optional configuration section, this feature projects have already implemented a similar feature. --> -### Configure in the rule itself - -Another approach I can think of is to encode that in the rule config itself. Something like `"my-plugin/my-rule": "[{severity: "error", autofix: False}, {...otherConfigs}]"` but it's harder to commit to such a change, and means that any config extension needs to reconfigure the rule correctly just to disable autofixing (which is already an issue when someone wants to set a pre-configured rule as warning for example) - ### Use of a 3rd-party plugin is a tool that exists to currently work around this limitation of ESLint, but it is not perfect. @@ -138,8 +357,13 @@ Another approach I can think of is to encode that in the rule config itself. Som you've received the answers and updated the design to reflect them, you can remove this section. --> -- Where exactly should the documentation go ? -- Where this needs to be implemented in code. Those familiar with ESLint's codebase are welcome to provide this information + +- **Should there be legacy format support?** \ + I'd say no, but there's a call to [getRuleSeverity](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/eslint/eslint.js#L20) + coming [from @eslint/eslintc](https://github.com/eslint/eslintrc/blob/556e80029f01d07758ab1f5801bc9421bca4b072/lib/shared/config-ops.js#L30) repeatedly in the code, + and it would obviously stop working if a new record form was passed in. + +- **Should `linterOptions.reportUnusedInlineConfigs` support configurable autofix?** ## Help Needed @@ -149,9 +373,10 @@ Another approach I can think of is to encode that in the rule config itself. Som Are you able to implement this RFC on your own? If not, what kind of help would you need from the team? --> -My knowledge of ESLint's internals isn't that great. Whilst I think it's above the average user due to reading and configuring a lot, I haven't yet even learned how to write a plugin, and haven't migrated any project to ESLint 9 yet. -My free time both at work and personal, is currently also very limited (see how long it too me to just get to writing this RFC). -So I unfortunately don't think I can implement this feature myself, due to both a lack of time, personal motivation (I won't be able to use it for a while, but will push us towards ESLint 9 once implemented), and experience. + +While I am confident in implementing this RFC, I would greatly appreciate help +reviewing the structure and phrasing of this document +to ensure clarity and correctness in English. ## Frequently Asked Questions @@ -163,8 +388,8 @@ So I unfortunately don't think I can implement this feature myself, due to both in this section. --> -Q: Could `disableAutofixes` be an array of autofixes to disable? -A: `disableAutofixes` is a record to allow re-enabling autofixes in downstream configurations and on a per-file basis. We could allow a shorthand to `disableAutofixes` to accept an array of rules to disable the autofix for, but that would result in additional complexity on the implementation side with marginal benefits to the user. +Currently, most key concepts are covered above. \ +Additional FAQs can be added as needed during review. ## Related Discussions @@ -174,7 +399,8 @@ A: `disableAutofixes` is a record to allow re-enabling autofixes in downstream c If there is an issue, pull request, or other URL that provides useful context for this proposal, please include those links here. --> -- +- Original issue: +- Previous RFC: - - - From 59e0942aa43484c71d0fd20f8a61c6df9f8f2765 Mon Sep 17 00:00:00 2001 From: Maxim Morev Date: Thu, 22 May 2025 19:02:00 +0300 Subject: [PATCH 09/13] Add link to RFC PR Co-authored-by: Nicholas C. Zakas --- designs/2024-per-rule-autofix-configuration/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index cde81d03..8d9a15b5 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -1,6 +1,6 @@ - Repo: - Start Date: 2024-10-22 -- RFC PR: +- RFC PR: https://github.com/eslint/rfcs/pull/134 - Authors: - [Samuel Therrien](https://github.com/Samuel-Therrien-Beslogic) (aka [@Avasam](https://github.com/Avasam)) - [Maxim Morev](https://github.com/MorevM) From 2e892d31db92d234c3605e23b496e720ebd3994f Mon Sep 17 00:00:00 2001 From: MorevM Date: Fri, 6 Jun 2025 21:23:34 +0300 Subject: [PATCH 10/13] feat: Add `@eslint/plugin-kit` to starting points list --- designs/2024-per-rule-autofix-configuration/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 8d9a15b5..e5fbd486 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -218,6 +218,7 @@ Relevant starting points: * [types/index.d.ts > RuleEntry](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/types/index.d.ts#L1377) * [types/index.d.ts > LinterOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/types/index.d.ts#L1876) * --- +* [@eslint/plugin-kit > isSeverityValid](https://github.com/eslint/rewrite/blob/88525272b717efea7cae1d5ea2429d6343a7e066/packages/plugin-kit/src/config-comment-parser.js#L30-L37) * [flat-config-schema.js > rulesSchema](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L450) * [flat-config-schema.js > disableDirectiveSeveritySchema](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L298) * [flat-config-schema.js > normalizeRuleOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L132) From f309fb66f01fc7506d303ffd633c982e5da4098f Mon Sep 17 00:00:00 2001 From: MorevM Date: Fri, 6 Jun 2025 21:32:25 +0300 Subject: [PATCH 11/13] Add a note about backwards compatibility --- designs/2024-per-rule-autofix-configuration/README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index e5fbd486..869ec85c 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -324,11 +324,15 @@ It's important to ensure nothing breaks during this transition. to existing users? --> -This feature is fully backwards compatible: +This feature is fully backwards compatible for end users: * `autofix` defaults to `true`, matching current behavior; * Internal normalization does not affect user configurations or existing workflows. +However, this can be a breaking change for authors of solutions on top of ESLint +(e.g. a wrapper function for configuration(s), since the new config format is just JS), +or those that rely on ESLint's public API (like [`eslint.calculateConfigForFile()`](https://eslint.org/docs/latest/integrate/nodejs-api#-eslintcalculateconfigforfilefilepath)) + ## Alternatives +This RFC proposes extending ESLint's rule configuration to support an optional object format, allowing users to disable autofixes on a per-rule basis. -This RFC proposes adding support for controlling autofix behavior on a per-rule basis through shareable configuration. +The key point is that we can add this feature without affecting anything that already works — the current "error" string and ["error", options] array formats will keep working exactly the same. + +This proposal builds on the consensus reached in [RFC #125](https://github.com/eslint/rfcs/pull/125), where the team agreed that an object-based approach provides the best balance of flexibility and backward compatibility. ## Motivation - +### Problem + +As discussed in [issue #18696](https://github.com/eslint/eslint/issues/18696), there's a real need for per-rule autofix control. While ESLint's autofix feature is incredibly useful, there are legitimate scenarios where you want a rule enabled (to catch issues) but don't want its autofix applied automatically: + +1. **Unsafe autofixes**: Some rules have autofixes that might change code behavior in edge cases. You want to catch these issues but review fixes manually. + +2. **Unmaintained plugins**: When using plugins that are no longer actively maintained, you might encounter broken autofixes. Disabling the entire rule means losing valuable linting, but applying broken fixes is worse. + +3. **Manual review requirements**: Some teams have policies requiring human review of certain types of changes, even if technically "safe" to autofix. + +4. **CI/CD workflows**: You might want different behavior in CI (report only) vs local development (autofix enabled). + +### Available workarounds: + +Right now, users work around this limitation using [`eslint-plugin-no-autofix`](https://www.npmjs.com/package/eslint-plugin-no-autofix). While this plugin has been helpful, it comes with real downsides: + +- **Extra dependency**: Another package to maintain and keep updated +- **Environment issues**: Doesn't work in all environments (e.g., [pre-commit.ci crashes](https://github.com/aladdin-add/eslint-plugin/issues/98)) +- **Compatibility problems**: May not work correctly with all third-party rules (see [eslint-community/eslint-plugin-eslint-comments#234](https://github.com/eslint-community/eslint-plugin-eslint-comments/issues/234)) +- **Cumbersome setup**: Rules must use a no-autofix/ prefix, and autofix control is separate from the rule itself + +A native solution would eliminate these issues entirely. + +### Real-World Use Cases + +Here are some concrete scenarios where this feature would be valuable: -Some rules support autofixing, which is often convenient, but in certain cases -the fixes may be broken, unsafe, or simply undesirable. \ -Ideally, unsafe autofixes should be treated as suggestions, and broken fixes should be reported. +1. **Disable unsafe autofixes**: The `no-implicit-coercion` rule is great for catching issues, but its autofix can occasionally change behavior in subtle ways. You want the warnings, but prefer to review and apply fixes manually. + ```javascript + rules: { + "no-implicit-coercion": { severity: "error", autofix: false } + } + ``` -However, ESLint is a large ecosystem, and some useful plugins are no longer actively maintained. -Even in actively maintained projects, users may want to disable autofixing for specific rules -due to project-specific or personal preferences. +2. **Preserve imports during development**: As described in [issue #18696](https://github.com/eslint/eslint/issues/18696#issuecomment-2429108285), teams using `eslint-plugin-unused-imports` face a frustrating workflow issue - when you comment out code during development, autofixes immediately remove the imports. When you uncomment the code, you have to manually re-add all the imports. The solution is to keep the rule enabled (to catch actual unused imports) but disable its autofix. + ```javascript + rules: { + "unused-imports/no-unused-imports": { severity: "error", autofix: false } + } + ``` + +3. **Override extended configs**: As mentioned in [RFC #125](https://github.com/eslint/rfcs/pull/125#issuecomment-2437891333), you might want `@eslint-community/eslint-comments/no-unused-disable` to report issues but not autofix them, since a rule suddenly not failing should require human attention. + ```javascript + import recommended from 'eslint-config-recommended'; + export default [ + recommended, + { + rules: { + "@eslint-community/eslint-comments/no-unused-disable": { autofix: false } + } + } + ]; + ``` + +4. **Environment-specific behavior**: Different autofix settings for CI vs local development . + ```javascript + const isCI = process.env.CI === 'true'; + export default [{ + rules: { + "semi": { severity: "error", autofix: !isCI } + } + }]; + ``` ## Detailed Design - - -### Abstract - -We introduce an *extended rule configuration format*: - -```ts -type RuleEntry = Partial<{ - /** - * The rule severity. - * - * @default 2 - */ - severity: 0 | 1 | 2 | 'off' | 'warn' | 'error'; - - /** - * Array of options passed to the rule. - * - * @default [] - */ - options: any[] - - /** - * Whether to allow a rule to perform autofixes. - * - * @default true - */ - autofix: boolean; -}>; +### Rule Configuration Format + +The core idea is simple: extend ESLint's rule configuration to accept an **optional object format** alongside the current string and array formats. This is just adition - nothing breaks, nothing changes unless you opt in. + +As discussed in [RFC #125](https://github.com/eslint/rfcs/pull/125#discussion_r1929041731), this approach was chosen over alternatives (like a top-level `nofix` key) because it keeps all rule settings in one place and allows for additional options for each rule (as discussed in [#19342](https://github.com/eslint/eslint/issues/19342)). + +```javascript +export default [{ + rules: { + // Existing formats (continue to work) + "semi": "error", // String format + "semi": ["error"], // Array format + "quotes": ["error", "double"], // Array format + + // New object format (opt-in) + "no-unused-vars": { + severity: "error", + options: [{ "vars": "all" }], + autofix: false // New property + } + } +}]; ``` -This format enables explicit control over a rule's autofix behavior, -and opens the door to further extension with other meta-properties -([example](https://github.com/eslint/eslint/issues/19342)). +### Object Properties -> [!IMPORTANT] -> This is an additional configuration format; existing configurations remain valid and do not need to be rewritten. +The object format supports the same properties you're already familiar with from array format, plus a new one: -#### Autofix details +- **`severity`**: Same as the first element in array format (`"off"`, `"warn"`, `"error"`, `0`, `1`, or `2`) +- **`options`**: Same as the remaining elements in array format (rule-specific options) +- **`autofix`** *(new)*: Boolean to control whether autofixes are applied for this rule (default: `true`) -* The `autofix` option defaults to `true`, maintaining current behavior - (if a rule provides a fixer, it will run when `--fix` is used); -* If autofix is disabled for a rule, its fix becomes a suggestion labeled - "Apply disabled autofix" and can be applied manually through editor hints; -* If the rule does not define a fixer, the `autofix` key has no effect. +### Partial Object Merging -#### Additional extensions +This is where things get really useful. As [@nzakas pointed out in RFC #125](https://github.com/eslint/rfcs/pull/125#issuecomment-2582026498), we can support **partial object merging** - you only need to specify the properties you want to change: -We extend `linterOptions.reportUnusedDisableDirectives` -and `linterOptions.reportUnusedInlineConfigs`(?) to support new extended format: +> "I think we can stipulate that when merging, we'll just merge the keys that are specified and keep everything else." -```js +This means you don't need to know or repeat the full rule configuration just to disable autofix: + +```javascript +// base.config.js +export default [{ + rules: { + "semi": { + severity: "error", + options: [], + autofix: true + } + } +}]; + +// extended.config.js +import base from './base.config.js'; export default [ + base, { - linterOptions: { - reportUnusedDisableDirectives: { - severity: 'error', - autofix: false, - }, - }, - }, + rules: { + "semi": { autofix: false } // Only changes autofix, inherits severity & options + } + } ]; -``` -We extend CLI engine to support new format as an additional option to existing ones: - -```bash -npx eslint --fix --rule 'no-var: { autofix: false }' +// Result: { severity: "error", options: [], autofix: false } ``` -We extend directive parser engine to support new format as well: +Merge priority: Later configs override earlier ones. In the example above: +- `base.config.js` sets `autofix: true` +- `extended.config.js` sets `autofix: false` +- **Result**: `autofix: false` wins (later config takes priority) +- Properties not specified in the later config (`severity`, `options`) are inherited from the earlier config -```js -/* eslint eqeqeq: "off", curly: { severity: "warn", autofix: false } */ -``` +### File-Specific Overrides -### Example +One of the concerns raised in [RFC #125](https://github.com/eslint/rfcs/pull/125#issuecomment-2437891333) was the ability to re-enable autofixes in specific contexts. The object format handles this naturally through ESLint's existing file-based configuration: -```js +```javascript export default [ { rules: { - // Full rule entry with disabled autofix - 'eqeqeq': { - severity: 'error', - options: ['always'], - autofix: false, - }, - // Options can be omitted if not needed - 'no-var': { - severity: 'error', - autofix: false, - }, - // Severity can be omitted if not needed - 'camelcase': { - options: [{ properties: 'never' }], - autofix: false, - }, - // Autofix can be omitted if not needed - 'yoda': { - options: ['never'], - }, - // Old format remains valid and will be normalized internally - 'no-regex-spaces': 'error', - 'no-unneeded-ternary': ['error', { - defaultAssignment: false, - }] - }, + "semi": { + severity: "error", + autofix: false // Disabled globally + } + } }, + { + files: ["**/*.test.js"], + rules: { + "semi": { autofix: true } // Re-enabled for test files + } + } ]; ``` -> [!NOTE] -> In practice, users only need the full form when disabling autofix -> or when being explicitly descriptive. - -
- Backward compatibility: existing configurations - - ```js - export default [ - { - linterOptions: { - reportUnusedDisableDirectives: 'error', - }, - rules: { - 'eqeqeq': 'error', - 'no-var': ['warn', 'always'], - '@scope/some-rule': ['warn', 'always', { option: true }], - }, - }, - ]; - ``` - - Will be normalized as: - - ```js - export default [ - { - linterOptions: { - reportUnusedDisableDirectives: { - severity: 'error', - autofix: true, - }, - }, - rules: { - 'eqeqeq': { - severity: 'error', - options: [], - autofix: true, - }, - 'no-var': { - severity: 'warn', - options: ['always'], - autofix: true, - }, - 'some-plugin/some-rule': { - severity: 'warn', - options: ['always', { option: true }], - autofix: true, - }, - }, - }, - ]; - ``` -
+### CLI and Inline Comment Support + +As [@fasttime noted in RFC #125](https://github.com/eslint/rfcs/pull/125#discussion_r1835013306), it's important that rules can be configured consistently across different methods - config files, CLI, and inline comments. The beauty of the object-based approach is that it works everywhere with **no new syntax**: + +**CLI** (existing `--rule` syntax): +```bash +eslint --rule 'semi: { "autofix": false }' src/ +``` + +**Inline comments** (existing `/* eslint */` syntax): +```javascript +/* eslint semi: { "autofix": false } */ +function foo() { + return 1 +} +``` + +### Implementation Details + +#### Step 1: Config Schema (`lib/config/flat-config-schema.js`) + +We need to update the `rules` schema to handle object-based configurations. The key is the `normalizeRuleConfig()` helper that converts all formats to a common internal representation: + +```javascript +rules: { + merge(first = {}, second = {}) { + const result = { ...first }; + + for (const [ruleId, ruleConfig] of Object.entries(second)) { + const firstConfig = first[ruleId]; + const secondConfig = ruleConfig; + + // Normalize both configs to objects for merging + const firstObj = normalizeRuleConfig(firstConfig); + const secondObj = normalizeRuleConfig(secondConfig); + + // Merge: second overrides first (partial merge) + result[ruleId] = { ...firstObj, ...secondObj }; + } + + return result; + }, -### Implementation plan + validate(value) { + // Existing validation for string/array formats + // ... existing code ... + + // Add validation for object format + // Why we need this: Currently, ESLint's validation (lib/config/flat-config-schema.js:184-192) + // REJECTS objects via assertIsRuleOptions(), which only allows string/number/array. + // The object format is a completely new code path that needs its own validation. + for (const [ruleId, ruleConfig] of Object.entries(value)) { + if (typeof ruleConfig === 'object' && !Array.isArray(ruleConfig)) { + // Validate object format + const validKeys = ['severity', 'options', 'autofix']; + const configKeys = Object.keys(ruleConfig); + + for (const key of configKeys) { + if (!validKeys.includes(key)) { + throw new TypeError( + `Rule "${ruleId}": Unknown property "${key}". Valid properties: ${validKeys.join(', ')}` + ); + } + } + + // Validate severity (same validation as array format, but for object property) + if ('severity' in ruleConfig) { + validateSeverity(ruleConfig.severity); + } + + // Validate options (must be array, same as array format's remaining elements) + if ('options' in ruleConfig) { + if (!Array.isArray(ruleConfig.options)) { + throw new TypeError(`Rule "${ruleId}": 'options' must be an array`); + } + } + + // Validate autofix (NEW property introduced by this RFC) + if ('autofix' in ruleConfig) { + if (typeof ruleConfig.autofix !== 'boolean') { + throw new TypeError(`Rule "${ruleId}": 'autofix' must be a boolean`); + } + } + } + } + } +} + +// Helper function to normalize rule configs +// This is the key to maintaining backward compatibility - we convert +// all formats (string, array, object) to a common internal representation +function normalizeRuleConfig(config) { + // Handle edge case: null/undefined configs + if (config === null || config === undefined) { + return {}; + } + + // String format: "error" -> { severity: "error" } + if (typeof config === 'string' || typeof config === 'number') { + return { severity: config }; + } + + // Array format: ["error", options] -> { severity: "error", options: [options] } + if (Array.isArray(config)) { + return { + severity: config[0], + options: config.slice(1) + }; + } + + // Object format: already normalized + if (typeof config === 'object') { + return config; + } + + return {}; +} +``` + +#### Example transformations: + +```javascript +// Input: String format +rules: { "semi": "error" } +// After normalization: +{ "semi": { severity: "error" } } + +// Input: Array format +rules: { "quotes": ["error", "double"] } +// After normalization: +{ "quotes": { severity: "error", options: ["double"] } } + +// Input: Array format (severity only) +rules: { "no-console": ["warn"] } +// After normalization: +{ "no-console": { severity: "warn", options: [] } } + +// Input: Object format (new) +rules: { "semi": { severity: "error", autofix: false } } +// After normalization: +{ "semi": { severity: "error", autofix: false } } + +// Input: Object format (partial - only autofix) +rules: { "semi": { autofix: false } } +// After normalization (when merged with base config): +{ "semi": { severity: "error", options: [], autofix: false } } +// (severity and options inherited from earlier config) +``` -#### 1. Normalize rule entries and configuration objects to the extended format +#### Step 2: Linter Integration (`lib/linter/linter.js`) -> [!NOTE] -> This is actually not fully related to the problem of this RFC, just a step to implementation and future extensibility. +The linter needs to check the `autofix` property and suppress fixes when it's `false`. The important part here is that we **only suppress fixes, not suggestions** - this was clarified in the discussion, as suggestions require explicit user action and shouldn't be affected by autofix settings. -As a first step, we normalize rule entries and `linterOptions.reportUnusedDisableDirectives` to the new extended format. +```javascript +// In runRules function, when setting up rule context: +Object.keys(configuredRules).forEach(ruleId => { + const ruleConfig = configuredRules[ruleId]; -Relevant starting points: + // Normalize to object format to access autofix property + const normalizedConfig = normalizeRuleConfig(ruleConfig); -* [@eslint/core > types.ts > LinterOptionsConfig](https://github.com/eslint/rewrite/blob/2020d38f9dbaabb9923b8f2116e7bcbafa530c85/packages/core/src/types.ts#L659) -* [@eslint/core > types.ts > RuleConfig](https://github.com/eslint/rewrite/blob/2020d38f9dbaabb9923b8f2116e7bcbafa530c85/packages/core/src/types.ts#L679) -* [types/index.d.ts > RuleEntry](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/types/index.d.ts#L1377) -* [types/index.d.ts > LinterOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/types/index.d.ts#L1876) -* --- -* [@eslint/plugin-kit > isSeverityValid](https://github.com/eslint/rewrite/blob/88525272b717efea7cae1d5ea2429d6343a7e066/packages/plugin-kit/src/config-comment-parser.js#L30-L37) -* [flat-config-schema.js > rulesSchema](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L450) -* [flat-config-schema.js > disableDirectiveSeveritySchema](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L298) -* [flat-config-schema.js > normalizeRuleOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/flat-config-schema.js#L132) -* [config.js > #normalizeRulesConfig](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/config.js#L515) -* [config.js > validateRulesConfig](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/config.js#L553) -* [linter/apply-disable-directives.js](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/apply-disable-directives.js#L474) should take a full form of the entry. + // NEW: Check if autofix is disabled for this rule + const autofixDisabled = normalizedConfig.autofix === false; -Some of static methods / individual functions also need to be considered, -like [config.js > getRuleNumericSeverity](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/config/config.js#L634) -or [linter.js > getRuleOptions](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L1086). + const ruleContext = fileContext.extend({ + id: ruleId, + options: normalizedConfig.options || [], + report(...args) { + const problem = report.addRuleMessage(ruleId, severity, ...args); -There are likely additional functions that operate on non-normalized rule entries; tests should help identify them. \ -There's also an open question here regarding support for the legacy config format, as the current code relies on `@eslint/eslintrc` in some places. + // NEW: Suppress fix if autofix is disabled for this rule + if (autofixDisabled && problem.fix) { + problem.fix = null; + } -Next, the CLI engine and directive parser should be verified to support object-form rule entries. + // Suggestions remain enabled - they require explicit user action + // and shouldn't be affected by autofix settings + }, + }); ---- + // ... rest of rule execution +}); +``` -Next, all consumers of normalized rule entries and `reportUnusedDisableDirectives` must be updated accordingly. +**What this does:** +- Reads the `autofix` property from the normalized config +- When a rule reports a problem with a fix, checks if `autofixDisabled` is true +- If so, nulls out the fix before the problem is recorded +- Leaves suggestions untouched so they remain available to the user -For directives, updating [apply-disable-directives.js](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/apply-disable-directives.js#L466) may be sufficient. +#### Step 3: Inline Config Comment Support -For rules usage is more widespread, there are a lot of usage like [this](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L699), [here](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L2140) and [there](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/linter.js#L1086). \ -I'm not sure it's possible to identify them all and that it's necessary to list them here, hopefully the tests will help. +Inline config comments (like `/* eslint semi: { "autofix": false } */`) will work automatically once Steps 1 and 2 are implemented. Here's why: -Once normalization is in place and consistent, we can safely introduce autofix disabling logic. +- Inline config parsing happens in `SourceCode.applyInlineConfig()` (`lib/languages/js/source-code/source-code.js:981`) +- It uses `ConfigCommentParser.parseJSONLikeConfig()` from `@eslint/plugin-kit` to parse the comment value +- The parsed config is added to the `rules` object, which then goes through the same validation and normalization as file-based configs +- Since we're updating the `rulesSchema` in Step 1, inline configs will automatically support the object format -#### 2. Disabling autofixes +**No additional code changes needed** - the existing inline config infrastructure will handle object format once the schema is updated. -##### Disabling for rules +### Behavior Specifications -I think the earliest place where we can account for disabled autofix is [here](https://github.com/eslint/eslint/blob/52f5b7a0af48a2f143f0bccfd4e036025b08280d/lib/linter/linter.js#L1249), -when creating the `ReportTranslator`. \ -We have the context of the rule and its extended configuration, -and we can pass that into `createReportTranslator` as a separate boolean property -(since the current implementation turns off suggestions when `disableFixes: false`, -but we need them if the fixer is disabled). +1. **Default value**: When `autofix` is not specified, it defaults to `true` (maintains current behavior - no changes for existing configs) -Inside `ReportTranslator` we can also convert the fix into a suggestion. +2. **Suggestions preserved**: When `autofix: false`, suggestions remain available. This is intentional - suggestions require explicit user action -##### Disabling for directives +3. **Rule still reports**: Setting `autofix: false` does NOT disable the rule. Violations are still detected and reported. This is the whole point - you want the linting, just not the automatic fixing. -I think all the work in this regard can be done in the -[linter/apply-disabled-directives.js](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/linter/apply-disable-directives.js#L466) -assuming the full entry of `reportUnusedDisableDirectives` configuration is available. +4. **Merging behavior**: Later configs override only the properties they specify. If you specify `{ autofix: false }`, it inherits `severity` and `options` from earlier configs. -It's unclear whether suggestions are currently supported for directives - please advise if they are, and where that logic resides. +5. **CLI precedence**: CLI `--rule` flags override config file settings (this is existing ESLint behavior, unchanged). ## Documentation - +### User Documentation Updates + +1. **Configuration Guide** (`docs/src/use/configure/rules.md`): + - Add new section "Rule Configuration Formats" in the "Using Configuration Files" section (after line 113) + - Document the three formats: string, array, and object + - Explain the new `autofix` property and its behavior + - Show partial object merging with examples + - Provide use cases for disabling autofix while keeping rule enabled -### Autofix on rules +2. **Node.js API Reference** (`docs/src/integrate/nodejs-api.md`): + - Update the rule configuration documentation to mention the `autofix` property + - Note that it's available in all config sources (files, CLI, inline comments) + - Document default value is `true` (maintains backward compatibility) -I think it is necessary to demonstrate the use of the full form of the record -in the [Configuration Files > Configuring rules](https://eslint.org/docs/latest/use/configure/configuration-files#configuring-rules) section, -that way we show all the available options literally on one screen. +### Example Documentation -I think there should be an explanatory section in the [Configure rules](https://eslint.org/docs/latest/use/configure/rules) section, -and maybe it should be the first one on the page to show all the available options at once so that the user is not intimidated -when they see multiple options on the same screen at the same time like: +Rule Configuration Formats - ESLint supports three formats for configuring rules: -```js -/* eslint eqeqeq: "off", curly: { severity: "error", autofix: false } */ +#### String Format +```javascript +rules: { + "semi": "error" +} ``` -I also believe that there should be a separate "Disabling autofix" section, -probably located before the [Disabling rules](https://eslint.org/docs/latest/use/configure/rules#disabling-rules) -section, so that it can be directly referenced. +#### Array Format +```javascript +rules: { + "quotes": ["error", "double"] +} +``` -### `linterOptions.reportUnusedDisableDirectives` +#### Object Format +```javascript +rules: { + "no-unused-vars": { + severity: "error", + options: [{ "vars": "all" }], + autofix: false + } +} +``` -A note should be added in the -[Configuration Files > Reporting Unused Disable Directives](https://eslint.org/docs/latest/use/configure/configuration-files#reporting-unused-disable-directives) -section that autofix behavior can be configured using the extended format. +#### Disabling Autofixes -## Drawbacks +To disable autofixes for a specific rule while keeping it enabled: - +## Drawbacks -The primary drawback is the complexity of normalizing configuration formats -across the codebase, which requires careful implementation across multiple packages (?). +**No significant drawbacks identified.** -It's important to ensure nothing breaks during this transition. +This proposal adds an optional configuration format. Users who don't need per-rule autofix control will never encounter it - their existing string and array configurations continue to work unchanged. Users who do need this functionality will find it intuitive since it follows the same patterns as existing ESLint configuration. ## Backwards Compatibility Analysis - +This proposal is **100% backward compatible** + +### No Breaking Changes -This feature is fully backwards compatible for end users: +1. **Existing string format**: `"error"` continues to work +2. **Existing array format**: `["error", options]` continues to work +3. **No forced migration**: Object format is opt-in only +4. **All formats coexist**: Can mix string, array, and object formats in same config +5. **Default behavior unchanged**: When `autofix` is not specified, autofixes work as they do today -* `autofix` defaults to `true`, matching current behavior; -* Internal normalization does not affect user configurations or existing workflows. +### Migration Path -However, this can be a breaking change for authors of solutions on top of ESLint -(e.g. a wrapper function for configuration(s), since the new config format is just JS), -or those that rely on ESLint's public API (like [`eslint.calculateConfigForFile()`](https://eslint.org/docs/latest/integrate/nodejs-api#-eslintcalculateconfigforfilefilepath)) +Users can adopt the new format gradually: + +```javascript +// Before (using eslint-plugin-no-autofix) +import noAutofix from 'eslint-plugin-no-autofix'; + +export default [{ + plugins: { "no-autofix": noAutofix }, + rules: { + "no-autofix/semi": "error" + } +}]; +// After (native support) +export default [{ + rules: { + "semi": { severity: "error", autofix: false } + } +}]; + +// Or with partial object format +import baseConfig from './base.config.js'; +export default [ + baseConfig, + { + rules: { + "semi": { autofix: false } // Inherits severity from base + } + } +]; +``` ## Alternatives - +We could add a separate top-level `nofix` key to disable autofixes: -### Use of a 3rd-party plugin +```javascript +export default [{ + rules: { "semi": "error" }, + nofix: ["semi"] +}]; +``` - is a tool that exists to currently work around this limitation of ESLint, but it is not perfect. +This was actually my initial approach, Ref from [Ruff's `unfixable` setting](https://docs.astral.sh/ruff/settings/#lint_unfixable). -1. It is an extra third-party dependency, with its own potential maintenance issues (having to keep up with ESLint, separate dependencies that can fall out of date, obsolete, unsecure, etc.) -2. It may not work in all environments. For example, pre-commit.ci: -3. It may not work correctly with all third-party rules: +**Why we're not doing this**: +- Separates autofix control from rule configuration (less intuitive) +- Would require new CLI syntax (`--no-fix-rule` or similar) +- Would require new inline comment syntax +- Less discoverable - users would need to know about a separate config key +- Doesn't enable future per-rule meta options (like per-rule `reportUnusedDisableDirectives`) -## Open Questions +The object-based approach keeps everything in one place and works with existing syntax everywhere. - +I might need help in identifying: +- Edge cases from multi-file config merging combined with complex configs/patterns +- Plugin-specific behaviors that might be affected -- **Should there be legacy format support?** \ - I'd say no, but there's a call to [getRuleSeverity](https://github.com/eslint/eslint/blob/0f49329b4a7f91714f2cd1e9ce532d32202c47f4/lib/eslint/eslint.js#L20) - coming [from @eslint/eslintc](https://github.com/eslint/eslintrc/blob/556e80029f01d07758ab1f5801bc9421bca4b072/lib/shared/config-ops.js#L30) repeatedly in the code, - and it would obviously stop working if a new record form was passed in. +## Frequently Asked Questions -- **Should `linterOptions.reportUnusedInlineConfigs` support configurable autofix?** +### Why not just disable the rule? -## Help Needed +This is probably the most common question. The answer: disabling the rule (`"off"`) means violations won't be reported at all. You lose the linting entirely. - +### What happens to suggestions? -While I am confident in implementing this RFC, I would greatly appreciate help -reviewing the structure and phrasing of this document -to ensure clarity and correctness in English. +Suggestions remain available even when `autofix: false`. This is intentional. -## Frequently Asked Questions +Suggestions require explicit user action (e.g., clicking in your IDE), so they're fundamentally different from automatic fixes. If you've disabled autofix because you want manual control, suggestions are exactly what you want - they give you the option to apply a fix with one click, but only when you choose to. + +### Can I re-enable autofixes in overrides? + +Yes! This was actually a key concern raised in [RFC #125](https://github.com/eslint/rfcs/pull/125#issuecomment-2437891333). The object format handles this naturally: + +```javascript +export default [ + { + rules: { "semi": { autofix: false } } // Disabled globally + }, + { + files: ["**/*.test.js"], + rules: { "semi": { autofix: true } } // Enabled for tests + } +]; +``` - +This is one of the main use cases - being able to use a shareable config but disable specific autofixes: -Currently, most key concepts are covered above. \ -Additional FAQs can be added as needed during review. +```javascript +import recommended from 'eslint-config-recommended'; + +export default [ + recommended, + { + rules: { + "semi": { autofix: false } // Override from recommended + } + } +]; +``` + +### What's the default value for `autofix`? + +When not specified, `autofix` defaults to `true` (current behavior). Autofixes are enabled by default. + +### Can I use this with the `--fix` flag? + +Yes. When you run `eslint --fix`, rules with `autofix: false` will report violations but won't apply fixes. Rules without the `autofix` property will apply fixes normally. + +### How does this differ from `meta.fixable`? + +- **`meta.fixable`**: Set by rule authors to indicate if a rule *can* provide fixes. This is part of the rule's definition. +- **`autofix`**: Set by users to control whether fixes *should* be applied in their project. This is part of the configuration. ## Related Discussions - -- Original issue: -- Previous RFC: -- -- -- +This RFC builds on prior community discussions: + +- **[eslint/eslint#18696](https://github.com/eslint/eslint/issues/18696)**: Original feature request for per-rule autofix control +- **[eslint/rfcs#125](https://github.com/eslint/rfcs/pull/125)**: RFC discussion where the object-based approach reached consensus +- **[eslint/rfcs#134](https://github.com/eslint/rfcs/pull/134)**: Alternative RFC (closed due to breaking changes) +- **[eslint/eslint#19342](https://github.com/eslint/eslint/issues/19342)**: Related discussion on extensible per-rule meta options +- **[eslint-community/eslint-plugin-eslint-comments#234](https://github.com/eslint-community/eslint-plugin-eslint-comments/issues/234)**: Related discussion on per-rule autofix control From c82478b1dc2396e816329a8187624f71fa99af85 Mon Sep 17 00:00:00 2001 From: Amaresh S M Date: Fri, 30 Jan 2026 02:02:14 +0530 Subject: [PATCH 13/13] update pr link --- designs/2024-per-rule-autofix-configuration/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-per-rule-autofix-configuration/README.md b/designs/2024-per-rule-autofix-configuration/README.md index 56e21148..fded1ecb 100644 --- a/designs/2024-per-rule-autofix-configuration/README.md +++ b/designs/2024-per-rule-autofix-configuration/README.md @@ -1,6 +1,6 @@ - Repo: - Start Date: 2024-10-22 -- RFC PR: Leave blank until PR is created +- RFC PR: [Leave blank until PR is created](https://github.com/eslint/rfcs/pull/143) - Authors: - [Samuel Therrien](https://github.com/Samuel-Therrien-Beslogic) (aka [@Avasam](https://github.com/Avasam)) - [Maxim Morev](https://github.com/MorevM)