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
For deprecated lifecycle methods, report identifier AST node instead of the class node #1854
For deprecated lifecycle methods, report identifier AST node instead of the class node #1854
Conversation
…he class node When the `no-deprecated` rule finds a deprecated lifecycle method, it reports the whole class declaration as the AST node for the error. Doesn't look good in editor and hides all other ESLint errors inside the class. This patch changes the reported AST node to the method name identifier. The errors are much better targeted at the infringing code and look much better when shown in editor UI. Added also test coverage to check for the reported AST node type.
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.
This seems great!
For the tests; could we add line and column info to all the error assertions, so we can prevent future issues?
@ljharb Thanks for reviewing this patch! I added a commit with line/column/endLine/endColumn checks. |
@jsnajdr looks like eslint 3 is failing on the |
Oops, ESLint 3 apparently didn't support the |
When the
no-deprecated
rule finds a deprecated lifecycle method, it reports the whole class declaration as the AST node for the error. Doesn't look good in editor: makes the whole class declaration red and obscures all other ESLint errors inside the class.This patch changes the reported AST node to the method name identifier. The errors are much better targeted at the infringing code and look much better when shown in editor UI.
Added also test coverage to check for the reported AST node type.