--report-unused-disable-directives should be autofixable

[dmnd]

The version of ESLint you are using.
v5.16.0

The problem you want to solve.
I want to enable --report-unused-disable-directives on a large existing codebase, but it surfaces too many problems to be practical to fix manually. So I'd need to write a script. At that point, why not make the directive autofixable?

Your take on the correct solution to problem.
Run eslint --fix --report-unused-disable-directives

Are you willing to submit a pull request to implement this change?
Yes

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

[platinumazure]

Apologies for letting this slip through the cracks.

Personally, I'm not sure this should be fixable for the same reason that rules like no-unused-vars should not be fixable: We can't know if a user had typed a correct eslint-disable comment, and now it's no longer needed due to a change in the code; vs if a user had typed an incorrect eslint-disable comment (e.g., mistyped the rule name to be disabled) and it never worked, and so now we're just removing the comment and the user might not even be aware they had mistyped something.

That's just my opinion, though-- if other team members disagree and think this is worth implementing, I will defer to consensus.

[dmnd]

@platinumazure can you kindly help me understand how a user could be unaware they mistyped something?

If the user mistyped a disable comment and it never worked, I think removing it is desirable, because either:

1. The build is already broken, and the user will be prompted to add the disable comment again when CI fails
2. The build isn't broken, and the disable comment is pointless and should be removed for readability

I suppose I'm assuming eslint users have builds fail when errors are reported. Is it common to use eslint differently?

[platinumazure]

@dmnd Thanks for your reply. To be honest, I hadn't thought into it very deeply and your reasoning makes sense. I think I could imagine myself being annoyed if I had my disable comment get nuked and I had to type it out again, just because of a typo, but I agree the impact is no worse than that.

[dmnd]

@platinumazure also thanks to you for engaging in this issue ❤️

do you think eslint would be receptive to this idea? should the issue be opened again?

[platinumazure]

Looks like @mysticatea had left a 👍 on the issue. @mysticatea, would you like to reopen and champion this issue?

[mysticatea]

Thank you for the ping.

Yes, I will champion.

In most cases, people write disable comments to disable existing errors, different from no-unused-vars, so I think that autofix will not be annoying them.

[kaicataldo]

@mysticatea Should we make an RFC for this?

[JoshuaKGoldberg]

I'm interested tackling this issue & writing that RFC but I'm still a little shaky on the governance process here. Is this ready for me to send a PR to https://github.com/eslint/rfcs?

[aladdin-add]

seems a reasonable enhancement, I'll support it!

not sure an RFC is required - it's just a non-breaking enhancement. // cc @mdjermanovic

[nzakas]

Is anyone still interested in this? It’s been open for over a year without any movement. Unless someone feels very strongly, we should close this as it was never accepted by the team.

[JoshuaKGoldberg]

FWIW I'd be very interested in tackling it soon, if the team wills it.

[aladdin-add]

TSC Summary: This proposal proposes to make the --report-unused-disable-directives warnings fixable(runing eslint ---fix).

TSC Question: Shall we accept this proposal? Is a RFC required?

[btmills]

The team agreed that we'd like a way to autofix unused disable directives! Now we need an RFC to figure out how it should be implemented. So far we only autofix rules, so this will be a completely new type of autofixing. We're not sure that including this autofix in --fix is necessarily the right solution. Maybe we need a new CLI option, or maybe we can use --fix but with a new --fix-type value, but those are just ideas to get the brainstorm started.

@JoshuaKGoldberg thanks for your continued interest in working on this feature! As you're starting on the RFC, feel free to discuss anything or ask for early feedback on ideas in this issue, then we can move discussion to the RFC PR once you've opened one with a concrete proposal.

[JoshuaKGoldberg]

eslint/rfcs#78 ✨