Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider remove autofixing unicorn/prevent-abbreviations #2266

Open
jpolo opened this issue Jan 29, 2024 · 5 comments
Open

Consider remove autofixing unicorn/prevent-abbreviations #2266

jpolo opened this issue Jan 29, 2024 · 5 comments

Comments

@jpolo
Copy link

jpolo commented Jan 29, 2024

When using prevent-abbreviations with VSCode fix on save, some variable is automagically renamed.
This could look at first sight but when the code was not written by yourself this is problematic...

The autofixer also takes a risk of breaking business logic "just because of formatting issue", which IMHO does not worth the tradeoff.

unicorn/prevent-abbreviations

I did some digging in github and it seems that many people (including me) are disabling this rule so it indicates this rule has some usage issue at the moment...

@sindresorhus
Copy link
Owner

When using prevent-abbreviations with VSCode fix on save, some variable is automagically renamed.
This could look at first sight but when the code was not written by yourself this is problematic...

The problem here is reformatting other people's code with your own rules, not this rule in particular. You should only have "format on save" enabled for your own projects.

@sindresorhus
Copy link
Owner

I find the rule very useful, and it would be a chore without the auto-fixing, so we are unlikely to disable that.

It also does not make sense for every rule to have a setting to turn off auto-fixing, but you could maybe try to get ESLint to add a way to disable auto-fixing for certain rules.

@jpolo
Copy link
Author

jpolo commented Jan 29, 2024

Sorry my explanation was confusing. I meant by "reformatting the code other people wrote", that you are in VSCode, you save => it is possible that a code that you don't see (maybe scrolling down) is fixed by the rule.

Like you, I am absolutely convinced that this rule is very important :), that is why I am giving the feedback : I wish I could turn it on again.

I know it would be a chore without auto-fixing, but I found that this rule can break so many things, that it is still a chore to validate the modifications made...
Reformulated : either you have few changes and it is not so hard to type refactor the changes -OR- there are many changes and the potential breakage are so important that it takes a lot of time.
In the mean time, the "refactor variable" feature in VSCode is much safer for renaming purpose.

@fregante
Copy link
Collaborator

I'm convinced autofix-on-save is not a safe behavior, so I'm 👎 on this basis.

While formatting on save is fine, "fixing bad code" isn't, because my code is at various stages of bad until I commit it. It doesn't make sense to attempt to fix incomplete code because lint rules (and their autofixes) assume they're not working on WIP code.

I get red underlines while I write code, but the editor should not automatically alter my code while I'm still working on it (unless again it's just formatting)

@jpolo
Copy link
Author

jpolo commented Feb 8, 2024

You might be right on the "fix on-save" behavior and I am opened to change my dev practices.

Nevertheless, even if you just launch manually the eslint --fix command, the risk of breakage is the same.
The rule seems to work well on a file basis but not on a project basis, which makes the fixing of this rule not safe. (that could be delegated to IDE refactoring features)

Maybe a better way of fixing would be supporting the ESLint suggestion API : avajs/eslint-plugin-ava#281 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants