-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: support TypeScript syntax in no-useless-constructor #19535
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
feat: support TypeScript syntax in no-useless-constructor #19535
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
}); | ||
|
||
ruleTesterTypeScript.run("no-useless-constructor", rule, { | ||
valid: [ |
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.
Can we make sure to add all the test cases from 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.
I think I got all the ones that have TypeScript-specific keywords, and the existing ones cover the rest of the behavior. Is there anything missing between the two?
I am curious on this: should we include all tests in the TypeScript-specific ones? Just a bare minimum "smoke test" level? All the ones from the general JavaScript one as well?
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.
I meant all TS specific + some basic js because the parser is different. Also, can we add loc
information assertions on errors?
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.
LGTM. I'll leave it open for some time for others to review.
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.
LGTM. Thanks.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Continues #19173.
Updates
no-useless-constructor
to account for TypeScript syntax as handled in the@typescript-eslint/no-useless-constructor
extension rule:protected
/private
(i.e. marking a constructor as non-public)public
constructors when there is no superclassIs there anything you'd like reviewers to focus on?