-
Notifications
You must be signed in to change notification settings - Fork 335
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
Handle unused directive problems #1588
Conversation
@@ -845,7 +845,7 @@ messageQueue.registerRequest(CodeActionRequest.type, async (params) => { | |||
}); | |||
} | |||
|
|||
if (settings.codeAction.disableRuleComment.enable) { | |||
if (settings.codeAction.disableRuleComment.enable && ruleId !== RuleMetaData.unusedDisableDirectiveId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since unused-directive problems are disabling comments themselves, we cannot disable them.
@MariaSolOs please see my comment above. |
@dbaeumer let me know if you have further comments :) |
@@ -167,6 +179,10 @@ export namespace RuleMetaData { | |||
export function hasRuleId(ruleId: string): boolean { | |||
return ruleId2Meta.has(ruleId); | |||
} | |||
|
|||
export function isUnusedDisableDirectiveProblem(problem: ESLintProblem): boolean { | |||
return problem.ruleId === null && problem.message.startsWith('Unused eslint-disable directive'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last question: I checked myself and I think the answer is no: but does ESLint support localization. If not right now then we are fine but this check might break in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. All I could find was this proposal but it looks stale, and for this specific diagnostic the message is hardcoded here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think we're good for now...
Thanks for the ping. See my last comment. I always wanted to ask that but I forgot about it. |
Thanks @MariaSolOs ! |
Fixes #1585.