mysticatea / eslint-plugin-eslint-comments

Additional ESLint rules for directive comments of ESLint.
https://mysticatea.github.io/eslint-plugin-eslint-comments/
MIT License
354 stars 44 forks source link

`disable-enable-pair` option allowWholeFile should be the default #59

Open Zamiell opened 3 years ago

Zamiell commented 3 years ago

I propose that the allowWholeFile should be set to true by default if it is not specified by the end-user.

Setting up this plugin for the first time was frustrating, because I got a lot of false positives. I then assumed that this plugin was bugged, and eventually found out that this option existed.

I think that it is pretty standard in the JavaScript/TypeScript ecosystem to disable a while file like this, so I don't see any compelling reasons for the current behavior of allowWholeFile being set to false by default.

bschlenk commented 3 years ago

I came here to propose this feature, but glad to see it already exists! I agree it would be nice if this were the default...

bmish commented 3 years ago

As a counterpoint to changing the default, if you want to disable a rule for an entire file, it's actually better practice to use this comment style instead:

/* eslint no-console:"off" */

// rest of file

This actually disables the rule for the entire file, whereas using /* eslint-disable no-console */ only disables the rule starting after the comment, which means the rule can technically still flag a violation, see this ticket for why: https://github.com/eslint/eslint/issues/12245

bschlenk commented 3 years ago

Interesting! I didn't realize there was a "correct" way to do this.

In that case, what would be really cool would be if the disable-enable-pair rule came with a suggestion to covert it to the /* eslint <rule-name>:"off" */ format if it detected the eslint-disable comment at the top of the file without a corresponding eslint-enable.

Zamiell commented 3 years ago

Screw suggesting - I would go further and say that the linter should either a) not complain about it at all, or b) automatically change it to the "off" style with the --fix flag, so that the end-user doesn't have to do anything

(In this context, I am assuming that the end-user has their IDE configured to do eslint --fix on save, which seems to be fairly common with the advent of e.g. eslint-plugin-prettier.)

bschlenk commented 3 years ago

The issue with autofixing would be not knowing the intent of the developer. This plugin doesn't know if you forgot a corresponding eslint-enable comment, or if you really want the rule disabled for the whole file. A suggestion not only lets you quickly fix it, but also makes you aware of what the proper syntax is.

I guess that's why the allowWholeFile setting exists because you can set it up to "not complain about it at all". But given the above discussion, I'm going to start using the "correct" syntax and probably keep the allowWholeFile setting false.

Zamiell commented 3 years ago

This plugin doesn't know if you forgot a corresponding eslint-enable comment, or if you really want the rule disabled for the whole file.

It's somewhat safe (but not completely safe) to assume that if the eslint-disable statement is on the very first line of the file, then the developer intended for it to apply to the whole file. Seems like a reasonable middle ground though.

mansona commented 2 years ago

I think we might be past the point of making the /* eslint <rule-name>:"off" */ syntax required by default, every single team I have worked with and every single open source project I have worked with are using the /* eslint-disable <rule-name> */ format for file-based ignores.

Sure we can try to turn that ship but I strongly feel like that will be a failing battle. I understand that there may be some edge cases for rules that could report before the start of the comment but that must be a very very small portion of rules or teams that are affected by that 🤔

MichaelDeBoey commented 8 months ago

Hi @Zamiell!

Since this repo is unmaintained, you might want to re-open this issue in the @eslint-community fork https://github.com/eslint-community/eslint-plugin-eslint-comments

For more info about why we created this organization, you can read https://eslint.org/blog/2023/03/announcing-eslint-community-org