Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Identify lines where rules are unnecessarily disabled #1330

Closed
adidahiya opened this issue Jun 22, 2016 · 13 comments
Closed

Identify lines where rules are unnecessarily disabled #1330

adidahiya opened this issue Jun 22, 2016 · 13 comments

Comments

@adidahiya
Copy link
Contributor

adidahiya commented Jun 22, 2016

Idea: report errors where a tslint:disable comment has no effect (probably as a result of some changes in the linter which make it more lax, or some new configuration options)

@jkillian
Copy link
Contributor

Neat idea, seems useful! I think we'd want this to be an option that could be turned on or off

@adidahiya
Copy link
Contributor Author

Yep, I think this could be a boolean flag in tslint.json.

@adidahiya
Copy link
Contributor Author

from @andy-hanson: would be best to wait until #2369 is in first

@victornoel
Copy link

@adidahiya @jkillian @andy-hanson now that #2369 has been merged, is there some plan to support this? :)

@adidahiya
Copy link
Contributor Author

No active work is being done here right now, but it's up for grabs if anyone wants to write a brief proposal and send a PR.

@JoshuaKGoldberg
Copy link
Contributor

As mentioned by @reduckted in #4118:

ESLint has this option. Obviously it's a different tool with different infrastructure, but we might be able to get some inspiration from how it's implemented.

--report-unused-disable-directives

This option causes ESLint to report directive comments like // eslint-disable-line when no errors would have been reported on that line anyway. This can be useful to prevent future errors from unexpectedly being suppressed, by cleaning up old eslint-disable comments which are no longer applicable.

@lukelafountaine
Copy link

Is anyone currently working on this? Also, what does the proposal process entail?

@JoshuaKGoldberg
Copy link
Contributor

@lukelafountaine nobody's working on this right now; you're more then welcome to! IMO this would be a pretty useful feature.

The definition of a "proposal" is pretty vague, but in general we'd want an explanation of how the feature works from a user's perspective. What are some cases where it would complain? What are some cases where it wouldn't?

Going off of ESLint's --report-unused-disable-directives seems like a good start. How would that look for TSLint if so?

@adidahiya
Copy link
Contributor Author

can we make it a core lint rule with an auto fixer so that this change rolls out in tslint:latest without users having to change how they use the tslint API?

@JoshuaKGoldberg
Copy link
Contributor

From a technical perspective, can we? Is there a way for rules to have access to that info (after all other rules run)?

@ajafff
Copy link
Contributor

ajafff commented Jan 6, 2019

Some of you might already be familiar with my alternative linter project called Fimbullinter (@fimbul/wotan). The latest version of it can report unused and redundant enable and disable comments using wotan --report-useless-directives (see the documentation of the CLI for more details).

Here's the interesting part: there's a plugin @fimbul/valtyr that teaches the linter to use TSLint's configuration, rules, formatters and understand // tslint:disable comments.

That means you can use wotan -m @fimbul/valtyr --report-useless-directives as a drop-in replacement for tslint. In addition to the regular linting it will report useless enable and disable comments. If you combine it with --fix it will automatically remove the offending comments (or individual rules of a comment).

@JoshuaKGoldberg
Copy link
Contributor

💀 It's time! 💀

TSLint is being deprecated and no longer accepting pull requests for major new changes or features. See #4534. 😱

If you'd like to see this change implemented, you have two choices:

  • Recommended: Check if this is available in ESLint + typescript-eslint
  • Not Recommended: Fork TSLint locally 🤷‍♂️

👋 It was a pleasure open sourcing with you!

If you believe this message was posted here in error, please comment so we can re-open the issue!

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

@palantir palantir locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants