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(eslint-plugin): added new rule use-default-type-parameter #562
feat(eslint-plugin): added new rule use-default-type-parameter #562
Conversation
Adds the equivalent of TSLint's `use-default-type-parameter` rule.
Codecov Report
@@ Coverage Diff @@
## master #562 +/- ##
==========================================
- Coverage 94.34% 94.31% -0.03%
==========================================
Files 111 112 +1
Lines 4735 4787 +52
Branches 1315 1331 +16
==========================================
+ Hits 4467 4515 +48
- Misses 156 157 +1
- Partials 112 115 +3
|
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
const arg = typeArguments[i]; | ||
const param = typeParameters[i]; | ||
|
||
// TODO: would like checker.areTypesEquivalent. https://github.com/Microsoft/TypeScript/issues/13502 |
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.
could we just introduce https://github.com/runem/ts-simple-type#readme ?
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.
In theory, sure. Would you like me to do this here or in a separate PR?
…//github.com/JoshuaKGoldberg/typescript-eslint into typescript-eslint-use-default-type-parameter
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.
Thanks a lot for doing this, the implementation LGTM.
However, I can't help feeling this rule should be called no-unnecessary-type-arguments
. I think that is subjectively more descriptive of what it enforces, and fits better with the existing conventions for these kinds of rules, both in our plugin and the ESLint ecosystem as a whole.
WDYT?
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.
Thanks @JoshuaKGoldberg! Sorry about the conflict, hopefully it's easy enough to reconcile I've attempted to clean up the conflict via the GitHub UI
Adds the equivalent of TSLint's
use-default-type-parameter
rule.I'm not sure what to do about the lack of test coverage on the files, since most of them are for edge cases in the TypeScript API not finding information. Would you prefer nodes of unknown types be added as test cases? Will wait for further instructions...