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

Similar rule to eslint-comments/no-use but only preventing non-restricted version #5

Closed mansona closed 6 years ago

mansona commented 6 years ago

Howdy 👋

We have just come across the need to prevent developers from willy-nilly turning off all eslint-rules, but want to allow them to specify the rules they want to turn off. Our specific use case would be to disable using eslint-disable-next-line without the rule qualifiers but I would imagine that it might be a more generic implementation that might apply across all the comment types.

Is this something that would make sense to add to your repo? If so I wouldn't mind contributing with a bit of guidance.

mysticatea commented 6 years ago

Thank you for this issue!

Sounds good to me. Maybe, includes and excludes would be useful.

{
    "eslint-comments/no-use": ["error", {"allow": [
        { "type": "eslint-disable", "includes": ["a-rule", "another-rule"] },
        { "type": "globals", "includes": ["a-var", "another-var"] },
    ]}]
}
{
    "eslint-comments/no-use": ["error", {"allow": [
        { "type": "eslint-disable", "excludes": ["no-undef", "no-unused-vars"] },
    ]}]
}
mansona commented 6 years ago

So what you're proposing is essentially a whitelist (or blacklist) of eslint-disable comments with the whitelist for the rules that you are allowed to disable.

I'm trying to think if this will fit my usecase 🤔 essentially we want to prevent all cases of // eslint-disable-next-line max-statements which we could achieve with the following rule:

{
    "eslint-comments/no-use": ["error", {"disallow": [
        { "type": "eslint-disable", "includes": ["max-statements"] },
    ]}]
}

maybe in this case deny would be better instead of disallow but you get my point. Would this sort of syntax also dissalow "bare" // eslint-disable-next-line statements? i.e. ones without a specifier?

I hope my question makes sense 😖 maybe you could give an example config that would meet my use case and we could then move onto a PR with failing tests and implement this as TDD

futpib commented 6 years ago

I assume we are only talking about having a list of enforced rules that users should not be able to disable via a comment.

If that's the case, @mysticatea, I don't think adding options to eslint-comments/no-use will do. If you want to enforce a rule you probably don't want it do be disableable in any imaginable way (be it eslint-disable, eslint-disable-line or eslint-disable-next-line — those should all be banned), but in case of eslint-comments/no-use you would have to duplicate your list of enforced rules for each kind of comment that can disable it.

This sound more like a separate rule like eslint-comments/enforced-rules or maybe eslint-comments/protected-rules or something, accepting options like this:

{
  "blacklist": [
    "the-rule-you-cant-disable-with-a-comment"
  ]
}

or:

{
  "whitelist": [
    "the-rule-you-can-disable-with-a-comment"
  ]
}

This new rule would also imply eslint-comments/no-unlimited-disable, otherwise it's really easy to get around.

futpib commented 6 years ago

@mansona also note that if this gets implemented (and if I understand your intentions correctly), your nasty developers will still be able to suppress whatever rules they want by first disabling enforced-rules.

mysticatea commented 6 years ago

I apology for my lacking communication. This thread was buried into busy life.

After I rethought, I realized that I ran too far. I added a simple rule eslint-comments/no-restricted-disable which disallows disabling of rules with glob patterns. Is it enough?

mysticatea commented 6 years ago

Please reopen or open new issue if it's not enough. Thank you.

mansona commented 6 years ago

@futpib thanks for weighing in, I think your insight is completely what I was thinking but put in a different way. Yes, you are right that the "nasty developer" would be able to just change these rules but it is easier for us to be more vigilant this way.

@mysticatea I think you may have perfectly implemented exactly what I wanted 👍 🎉 thanks a million