feat(eslint-plugin): [consistent-type-assertions] add arrayLiteralTypeAssertions option
#6749
Conversation
|
Thanks for the PR, @danvk! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
arrayLiteralTypeAssertions option to consistent-type-assertions rule
arrayLiteralTypeAssertions option to consistent-type-assertions rulearrayLiteralTypeAssertions option
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6749 +/- ##
==========================================
+ Coverage 87.24% 87.25% +0.01%
==========================================
Files 383 383
Lines 13080 13091 +11
Branches 3845 3849 +4
==========================================
+ Hits 11411 11422 +11
Misses 1302 1302
Partials 367 367
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Sorry this took so long to get to @danvk - the implementation looks very clean and the tests are thorough. Thanks for sending it in!
Just requesting changes on the missing "allow-as-parameter" equivalent.
| type OptUnion = | ||
| | { | ||
| assertionStyle: 'as' | 'angle-bracket'; | ||
| objectLiteralTypeAssertions?: 'allow' | 'allow-as-parameter' | 'never'; | ||
| arrayLiteralTypeAssertions?: 'allow' | 'never'; |
There was a problem hiding this comment.
I didn't include an equivalent of the
"allow-as-parameter"option but I can do so if there's a need.
Yeah it feels weird to have one without the other... Could you please?
You most certainly may 😄 I agree with your points. It does seem odd to be so specific, especially since it means the exception doesn't cover |
|
👋 Hey @danvk! Just checking in, is this still something you have time for? No worries if not - I just don't want to leave it hanging. |
|
Closing this PR as it's been stale for a few months without activity. Feel free to reopen @danvk if you have time - but no worries if not! If anybody wants to drive it forward, please do post your own PR - and if you use this as a start, consider adding |
PR Checklist
Overview
This PR adds an
arrayLiteralTypeAssertionsoption to theconsistent-type-assertionsrule as described in the linked issue. This uses the same auto-fixer as theobjectLiteralTypeAssertionsoption.I didn't include an equivalent of the
"allow-as-parameter"option but I can do so if there's a need.If I may step on my soapbox for a moment:
objectLiteralTypeAssertionsoption (and this new one) are an awkward fit for theconsistent-type-assertions, which you would expect to just enforce consistency betweenx as Tand<T>x. Both options would make more sense in a distinct rule that bans type assertions (rather than enforcing a consistent style).objectLiteralTypeAssertionsbut notarrayLiteralTypeAssertions, or vice versa. It also feels odd that we're singling out these two types but allowing constructs with other types such asconst x = 'foo' as string;