-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Possibility of writing "no-useless-ts-ignore-comments" rule #236
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
Comments
it might be worth looking at https://github.com/mysticatea/eslint-plugin-eslint-comments Glancing at that code makes me suspect this is something we can't handle purely within the plugin? |
Just spitballing: It seems like it could be possible if TSC includes all errors/warnings, including the ignored warnings under a different object property. Then it would be easy to check those warnings' locations against the TS ignore comments and see if any of the ignore comments have no corresponding warnings. But not sure if that's the intended/ideal direction, or if TSC should be handling this. |
The problem there is that we don't attach comments to nodes, and eslint deprecated the functionality to do that. #158 (comment) So whilst we know about the comments, we don't know what node they're attached to (unless tsc exposes that information). This means we don't know which node to check for errors (even if tsc did surface the ignored errors for us). For that reason alone, I would say that this rule would be non-trivial to implement. |
@bradzacher You wouldn't need to use comment attachment- you would just need to compare range or location information. That said, I agree that this would, in general, not be trivial to implement. |
// @ts-ignore
let a: string = 1; let b: number = 'str' I agree with @platinumazure that TSC should provide information about ignored diagnostics so we can cross check them with the comments. Made an issue for that in TypeScript: |
Relevant: microsoft/TypeScript#36014 will add a Edit: I just noticed |
It's already a rule (as of a week or so)! Just missed this issue. |
This is different than what I suggest. I wanted to make sure we don’t have dead This is useful when you have an issue that you can’t resolve and you use ts ignore to surpass it but then later via some mechanism like dependency update the type error is no longer there but the ts-ignore comment is there and it doesn’t allow ts to check the next line. |
Relevant ts issues: |
This can be closed now, as microsoft/TypeScript#36014 has been merged, and will be released as part of the next TS version. You'll have to migrate your codebases to use |
I'm exploring if it's possible to write a rule that warns against
// @ts-ignore
comments that have become stale as result of changes in other places of even on the line that tsc is being ignored. My approach is to get all diagnostics and cross reference them with// @ts-ignore
comments and see which comment does not actually ignoring an error. But if the// @ts-ignore
comment is still there TypeScript will not report any diagnostics for the next line.Is there any way of getting diagnostics if we remove the
// @ts-ignore
comment?One solution is to remove all
// @ts-ignore
comments from source files of existing program, construct a new program with those source files and get diagnostics. This seems very very expensiveAny thoughts on this?
Versions
@typescript-eslint/eslint-plugin
1.3.0
TypeScript
3.0
The text was updated successfully, but these errors were encountered: