-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
eslint-plugin-eslint-comments will be broken in 4.6.0 #9193
Comments
Thank you for this issue. It was a bad thing that I'm using the internal API. But it's helpful to developers if they can know unused directive comments because the remaining directive comments can hide reports unintentionally in future. I'm glad if I can maintain |
I agree. For now, I think maybe it would be best to revert d672aef until we have a better way to lint for unused directive comments. I tried to update In the long term, I think the best solution would be to check for unused directive comments in core, using something similar to what was discussed in #6190. |
I'm happy if we revert d672aef to keep detectable for unused directive comments.
Yes, I think. However, in my experience, a rule to warn unused disable comments was good. It tells me the unused disable comments in real-time on editors. Also, it can stop CI builds. It's configurable with If..., I understand that it's opposite of ESLint philosophy, but if a public API to catch reports from other rules, it's useful. For example: exports.create = (context) => {
return {
onReport(report) {
console.log(report.ruleId, report.message, report.node, report.line, report.column)
// Cancel the report to return false.
return false
}
}
} This is a generalized system of The |
For module.exports = {
create(context) {
const eslint = require("eslint");
const linter = new eslint.Linter();
linter.verify(context.getSourceCode(), { rules: { "no-invalid-this": "error" } })
.filter(problem => !isInEventHandler(problem))
.forEach(problem => {
context.report({ message: problem.message, loc: { line: problem.line, column: problem.column - 1 });
});
});
}
}; Then users could easily use To avoid needing to do another traversal, maybe it would be possible to extract the listeners manually and call them as part of the |
eslint-plugin-eslint-comments
is a plugin created by @mysticatea to solve the issue in #6190 of linting for unused/* eslint-disable */
comments. Since ESLint rules aren't supposed to be aware of other rules, a plugin like this technically isn't supposed to be possible. However it was successfully implemented by monkeypatching a private APIcontext.eslint.report
), and has been useful to a lot of people for avoiding unused disable comments.I just realized that
eslint-plugin-eslint-comments
will be broken in v4.6.0 as a result of d672aef, which refactors ESLint's internal reporting logic and removes the private API that was being monkeypatched. With the current version of ESLint onmaster
, it is no longer possible to implement something likeeslint-plugin-eslint-comments
, becausecontext
objects in rules don't have a reference to the underlyingLinter
instance anymore. We explicitly don't make any guarantees about the stability of undocumented APIs, but it would still be nice to avoid breaking peoples' linting builds if possible.So we have a few things to consider:
cc @ilyavolodin @mysticatea
The text was updated successfully, but these errors were encountered: