eslint: Rule Change: Maybe prefer-const should not autofix

What rule do you want to change?

prefer-const

What change to do you want to make?

Generate more warnings

How do you think the change should be implemented?

Other

Example code

let foundStudent = null;
for (const student of students) {
    if (student.name === targetName) {
        // Bug: we forgot to do 'foundStudent = student' here!
        break;
    }
}
console.log(foundStudent);

What does the rule currently do for this code?

The autofix will change the let to a const. This fixes the symptom and further hides the true bug.

What will the rule do after it’s changed?

Maybe it’s better for prefer-const to not perform any autofixes?

Participation

I am willing to submit a pull request to implement this change.

Additional comments

The bug in the code is that the programmer forgot to set foundStudent.
The prefer-const rule sort of catches this bug.
The prefer-const rule’s autofix, if applied, would mask this bug!

I think it’s common for people to trust the autofixes and not check them, e.g. my team has our IDEs configured to apply all autofixes on save. The prefer-const autofix is dangerous in that it deals with the symptom and hides the root cause.

I noticed there’s a 3rd-party ESLint plugin that lets you disable autofixes for specific rules: eslint-plugin-disable-autofix. Their first example disables the autofix for the the prefer-const rule 😃

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 7
Comments: 19 (9 by maintainers)

Most upvoted comments

@mdjermanovic what are you looking for feedback on here?

Im 👎 to disabling autofix for this rule. There will always be edge cases but the utility of this autofix has been proven over time.

nzakas on Jun 8, 2022

The autofix will change the let to a const. This fixes the symptom and further hides the true bug.

This indeed looks like a case where autofix for a best practice / stylistic rule hides an actual bug in the code. I think that should be avoided in general.

Maybe it’s better for prefer-const to not perform any autofixes?

Given the above, this seems reasonable. However, this particular autofix is very useful when enabling this rule for the first time in a large codebase. The fix is technically correct as it doesn’t change the behavior of the code. If the user doesn’t want to manually verify each case, not having the autofix feature could be a blocker for enabling this rule.

I think it’s common for people to trust the autofixes and not check them, e.g. my team has our IDEs configured to apply all autofixes on save. The prefer-const autofix is dangerous in that it deals with the symptom and hides the root cause.

ESLint provides option --fix-type in CLI, fixTypes in the API.

If your IDE supports this option, you could configure it to autofix on save only layout problems. These are problems reported by formatting rules, mostly whitespace-related.

I also wouldn’t mind adding an option to disable autofixes (or provide them as suggestions) for specified rules in the eslint config file.

mdjermanovic on May 30, 2022

Provide a way in eslintrc to disable the autofixes for a rule.

I think there is a lot hidden complexity down this path. We will need to consider shareable configs closely in this, because if shareable configs turn in auto fixing for all rules, putting it on the consumer of a shareable config to set everything back to default will be a laborious task.

I’m willing to entertain an RFC to convince me otherwise but my initial opinion is that this granularity of control would be destructive for end users.

nzakas on Oct 3, 2022

I also don’t want to rehash points where we fully understand each other but simply disagree. But your reply of “we just can’t know every situation for everyone’s code” made me think maybe there’s still a misunderstanding?

To clarify where I’m coming from:

I don’t expect ESLint to work for every situation for everyone’s code. But that doesn’t mean there’s never room for improvement.
I’m not only thinking about my use case. I’ve already used eslint-plugin-no-autofix to disable the prefer-const autofix in my codebases. The point of filing this issue is to make ESLint better for everyone.

The problem with the prefer-const autofix is subtle. I’ve been using ESLint for years and only noticed it recently. And it surprised me because it seems more dangerous than any other autofix.

But I’m just a regular user, so I haven’t accumulated the broad experience that the ESLint maintainers have. That’s why I’m genuinely curious – are there any other autofixes that are similarly dangerous? Is prefer-const the only one so far?

cakoose on Jun 11, 2022

The problem is ESLint can’t know what code you forgot to write. 😄 That is part of why we don’t recommend using autofix in an editor, because ESLint always thinks the code is complete even though it won’t be as you’re writing it, and then the autofixes are more likely to cause problems than help them.

Instead of thinking about which autofixes to turn off, I think it’s more helpful to think about when you use autofix and when you don’t. Generally the best time to autofix is on precommit, so you know the code is as complete as possible. Other times, it’s best not to use autofix and instead just rely on the warnings to manually check your work.

nzakas on Jun 10, 2022