Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix(eslint-plugin): [switch-exhaustiveness-check] better support for intersections, infinite types, non-union values#8250
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for the PR,@auvred! 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 onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedJan 14, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
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.
What a lovely, thoroughly tested PR! Thanks for sending this in! 👏
| for (const unionPart of tsutils.unionTypeParts(discriminantType)) { | ||
| for (const intersectionPart of tsutils.intersectionTypeParts( | ||
| unionPart, | ||
| )) { |
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.
[Non-Actionable] This feels like something we'd want a shared utility for... and, ha, I filed an issue for this back in August!#7466 (comment) ->JoshuaKGoldberg/ts-api-utils#258
Not requesting changes here. I'll go ahead and send that PR tots-api-utils and then follow up in this repo.
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.
…intersections, infinite types, non-union values (typescript-eslint#8250)* feat(eslint-plugin): [switch-exhaustiveness-check] better support for intersections, infinite types, non-union values* chore: try to fix weird diff with main* refactor: no need to collect missing branches in function* fix: provide valid fixes for unique symbols* fix: valid fixes for unique symbols + few test cases for enums
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview
I decided to fix both issues in one pr, because it was really hard to fix the first one without fixing the second.
This PR introduces better support for all combinations of unions, intersections, infinite types. It also fix suggestions more accurate.
In addition, this PR also includes several fixes for bugs that were present on
mainbut wasn't mentioned in issues:1. Provide correct fixes for missing
unique symbolsBefore (playground):
const a = Symbol('a')const b = Symbol('b')declare const value: typeof a | typeof b switch (value) { case a: break;+ case unique symbol: { throw new Error('Not implemented yet: unique symbol case') }}After:
const a = Symbol('a')const b = Symbol('b')declare const value: typeof a | typeof b switch (value) { case a: break;+ case b: { throw new Error('Not implemented yet: b case') }}2. Support intersections that includes literals
Before (playground):
After (no false positive):
3. Consistently add a default branch both for bare infinite types and unions of infinite types
Before (playground):
declare const value: numberswitch (value) { case 1: break;+ default: { throw new Error('default case') }}declare const value: number | stringswitch (value) { case 1: break;+ case string: { throw new Error('Not implemented yet: string case') }+ case number: { throw new Error('Not implemented yet: number case') }}After:
declare const value: numberswitch (value) { case 1: break;+ default: { throw new Error('default case') }}declare const value: number | stringswitch (value) { case 1: break;+ default: { throw new Error('default case') }}I've removed
isUnionfromSwitchMetadatato avoid confusion: we shouldn't treat unions and non-unions differently. We should only care whether a type contains non-literal types or not