feat: Allows users to disable autofixes on a per-rule basis #143
feat: Allows users to disable autofixes on a per-rule basis #143amareshsm wants to merge 13 commits intoeslint:mainfrom
Conversation
…ed" to suggestions.
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
|
|
||
| The object format supports the same properties you're already familiar with from array format, plus a new one: | ||
|
|
||
| - **`severity`**: Same as the first element in array format (`"off"`, `"warn"`, `"error"`, `0`, `1`, or `2`) |
There was a problem hiding this comment.
What would be the defaults of severity (likely "error") and options (likely undefined) when they are not specified? The properties do not seem to be required as some earlier examples just set "autofix".
There was a problem hiding this comment.
Good catch! With the array format, we could be certain that a severity was always set, but in the object format, it's possible that it will be left off.
I see two options:
- We throw an error saying that a severity much be specified. This would have to be done during the config merging process because severity can be inherited.
- We default to "error".
|
|
||
| #### Step 2: Linter Integration (`lib/linter/linter.js`) | ||
|
|
||
| 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. |
There was a problem hiding this comment.
Can you specify what "suppress" means?
Will the fixer still create a fix but not applied or will it not run at all?
As side effects (e.g. creating files) is possible, I think the fixer should not run.
| #### Step 2: Linter Integration (`lib/linter/linter.js`) | ||
|
|
||
| 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. | ||
|
|
There was a problem hiding this comment.
We could also transform the fix into a suggestion, so it can still be applied manually but the problem would be what the message (desc) of the message should be (it could default to "Fix ${ruleId}").
|
|
||
| - **`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`) |
There was a problem hiding this comment.
Should ESLint raise an error when the rule does not set meta.fixable and autofix is set to true?
There was a problem hiding this comment.
Good question!
By default there's an implicit autofix: true for every rule, so there's really no reason to explicitly put autofix: true in the rule config as that's the default.
However, if someone was setting autofix via a variable, then it would make sense to do, and not make sense to throw an error.
So I think I've talked myself into not checking meta.fixable to preserve that use case.
6 participants
Summary
This RFC proposes extending ESLint's rule config to support an optional object format that allows users to disable autofixes on a per-rule basis while keeping the rule enabled for linting.
I’ve attempted to capture all relevant details. Please let me know if anything is missing or should be expanded.
Related Issues
eslint/eslint#18696
I used AI to make some minor adjustments to improve clarity and understanding.