fix(eslint-plugin): [switch-exhaustive-check] handle infinite types #6922
Conversation
|
Thanks for the PR, @cparros! 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 configuration. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6922 +/- ##
==========================================
- Coverage 87.43% 87.41% -0.02%
==========================================
Files 386 386
Lines 13192 13200 +8
Branches 3872 3874 +2
==========================================
+ Hits 11534 11539 +5
- Misses 1292 1294 +2
- Partials 366 367 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
heads up @cparros - no need to label the PR title with |
packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Outdated
Show resolved
Hide resolved
|
Sorry for the long time between commits here. Update but in the tests a intersecting type is still failing I'm definitely missing something whenI look at this, I feel dumb ha |
| ), | ||
| ); | ||
|
|
||
| if (!isFiniteType && !isSymbolType && !isStringLiteralOrBoolean) { |
There was a problem hiding this comment.
[Performance] This probably will never be a real issue, but - you can short circuit each of these cases to not have to go through later checks after an earlier one is true.
if (unionTypes.every((isLiteralType)) {
return;
}
if (unionTypes.some( ... )) {
return;
}We've been bit in the past by doing unnecessary work that was fine when written but ended up being a lot of extra cycles.
| const isStringLiteralOrBoolean = unionTypes.some(type => | ||
| isTypeFlagSet( | ||
| type, | ||
| ts.TypeFlags.BooleanLiteral || ts.TypeFlags.StringLiteral, |
There was a problem hiding this comment.
[Bug] || is the logical or operator. I'm guessing you meant | as the bitwise or.
But, this feels weird to me. It's generally not ideal for us to be hardcoding to specific primitives. What about bigint literals, number literals, etc.? This code should handle them too. ts.TypeFlags.Literal should be what you want to use.
Could you also add some test cases with bigint literals (1n) and template literal strings (a
| case 'bar': { | ||
| result = 42; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Just posting here so it doesn't get lost - tests should end up including bigints, booleans, symbols, unique symbols, and other fun wacky combinations/things just to be safe.
|
👋 Hey @cparros! 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 weeks without activity. Feel free to reopen @cparros 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 co-author attribution at the end of your PR description. Thanks! 😊 |
PR Checklist
Overview
I have updated this rule to handle infinite types. It will not report an error.