fix(eslint): replace untyped default-case and consistent-return with typed switch-exhaustiveness-check#109743
Merged
Conversation
JoshuaKGoldberg
commented
Mar 2, 2026
| case AST_NODE_TYPES.JSXNamespacedName: | ||
| return `${node.namespace.name}:${node.name.name}`; | ||
| default: | ||
| return node.name; |
Member
Author
There was a problem hiding this comment.
This is from the original conversation in #109725 (comment). I'd used default: instead of the more specific AST_NODE_TYPES.JSXIdentifier to appease the two lint rules.
3511c2a to
29fc1de
Compare
a8f8a52 to
9711d83
Compare
JoshuaKGoldberg
commented
Mar 26, 2026
| '@typescript-eslint/no-unnecessary-type-assertion': 'error', | ||
| '@typescript-eslint/switch-exhaustiveness-check': [ | ||
| 'error', | ||
| {considerDefaultExhaustiveForUnions: true}, |
Member
Author
There was a problem hiding this comment.
considerDefaultExhaustiveForUnions is the juice that makes the lint rule behave like I think Sentry wants. tl;dr: it tells the rule that a default: case should make a switch statement be considered an exhaustive switch over a union. Otherwise, we'd have to have a case for every constituent (possible type).
natemoo-re
reviewed
Mar 26, 2026
natemoo-re
left a comment
Member
There was a problem hiding this comment.
Diff looks good!
One thing to note is that enableTypeAwareLinting is only set in CI because we've had major performance problems which makes it unsuitable for running in precommit. As long as you're not concerned that this rule moves over, I'm not!
…typed switch-exhaustiveness-check Made-with: Cursor
9711d83 to
9d4b27c
Compare
Member
Author
|
Fixes ENG-7200. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following up on #109725 (comment): our lint rules that attempt to enforce exhaustive logical handling aren't set up right. We're using ESLint's core rules that are not type-aware and so are actually buggy / problematic:
consistent-return: Requiresreturnstatements to either always or never specify values. This can be thought of as a safety point (never accidentally implicitly returningundefined) or a stylistic point (clearly indicating returned values). TypeScript handles the safety point.default-case: Requiresdefaultcases inswitchstatements. This doesn't factor in type information for union types, resulting in a need to add adefaulteven if theswitchis already exhaustive or (e.g. for finite unions) or cannot be (e.g. for primitive types).This PR switches from them to the type-aware
@typescript-eslint/switch-exhaustiveness-check. That rule knows to only enforce asking for handling all cases when theswitchis over a union type.In other words, for a
switch (value), if avalueis...string: nothing will require adefault:"a" | "b": then if there isn't bothcase "a":andcase "b":, there will need to bedefault: