Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 60 additions & 64 deletions packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ import {

interface SwitchMetadata {
readonly symbolName: string | undefined;
readonly missingBranchTypes: ts.Type[];
readonly defaultCase: TSESTree.SwitchCase | undefined;
readonly isUnion: boolean;
readonly missingLiteralBranchTypes: ts.Type[];
readonly containsNonLiteralType: boolean;
}

Expand Down Expand Up @@ -109,16 +108,6 @@ export default createRule<Options, MessageIds>({
const containsNonLiteralType =
doesTypeContainNonLiteralType(discriminantType);

if (!discriminantType.isUnion()) {
return {
symbolName,
missingBranchTypes: [],
defaultCase,
isUnion: false,
containsNonLiteralType,
};
}

const caseTypes = new Set<ts.Type>();
for (const switchCase of node.cases) {
// If the `test` property of the switch case is `null`, then we are on a
Expand All @@ -134,54 +123,47 @@ export default createRule<Options, MessageIds>({
caseTypes.add(caseType);
}

const unionTypes = tsutils.unionTypeParts(discriminantType);
const missingBranchTypes = unionTypes.filter(
unionType => !caseTypes.has(unionType),
);
const missingLiteralBranchTypes: ts.Type[] = [];

for (const unionPart of tsutils.unionTypeParts(discriminantType)) {
for (const intersectionPart of tsutils.intersectionTypeParts(
unionPart,
)) {
Comment on lines +128 to +131
Copy link
Member

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 to ts-api-utils and then follow up in this repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (
caseTypes.has(intersectionPart) ||
!isTypeLiteralLikeType(intersectionPart)
) {
continue;
}

missingLiteralBranchTypes.push(intersectionPart);
}
}

return {
symbolName,
missingBranchTypes,
missingLiteralBranchTypes,
defaultCase,
isUnion: true,
containsNonLiteralType,
};
}

/**
* For example:
*
* - `"foo" | "bar"` is a type with all literal types.
* - `"foo" | number` is a type that contains non-literal types.
*
* Default cases are never superfluous in switches with non-literal types.
*/
function doesTypeContainNonLiteralType(type: ts.Type): boolean {
const types = tsutils.unionTypeParts(type);
return types.some(
type =>
!isFlagSet(
type.getFlags(),
ts.TypeFlags.Literal | ts.TypeFlags.Undefined | ts.TypeFlags.Null,
),
);
}

function checkSwitchExhaustive(
node: TSESTree.SwitchStatement,
switchMetadata: SwitchMetadata,
): void {
const { missingBranchTypes, symbolName, defaultCase } = switchMetadata;
const { missingLiteralBranchTypes, symbolName, defaultCase } =
switchMetadata;

// We only trigger the rule if a `default` case does not exist, since that
// would disqualify the switch statement from having cases that exactly
// match the members of a union.
if (missingBranchTypes.length > 0 && defaultCase === undefined) {
if (missingLiteralBranchTypes.length > 0 && defaultCase === undefined) {
context.report({
node: node.discriminant,
messageId: 'switchIsNotExhaustive',
data: {
missingBranches: missingBranchTypes
missingBranches: missingLiteralBranchTypes
.map(missingType =>
tsutils.isTypeFlagSet(missingType, ts.TypeFlags.ESSymbolLike)
? `typeof ${missingType.getSymbol()?.escapedName as string}`
Expand All @@ -196,7 +178,7 @@ export default createRule<Options, MessageIds>({
return fixSwitch(
fixer,
node,
missingBranchTypes,
missingLiteralBranchTypes,
symbolName?.toString(),
);
},
Expand Down Expand Up @@ -227,24 +209,13 @@ export default createRule<Options, MessageIds>({
continue;
}

// While running this rule on the "checker.ts" file of TypeScript, the
// the fix introduced a compiler error due to:
//
// ```ts
// type __String = (string & {
// __escapedIdentifier: void;
// }) | (void & {
// __escapedIdentifier: void;
// }) | InternalSymbolName;
// ```
//
// The following check fixes it.
if (missingBranchType.isIntersection()) {
continue;
}

const missingBranchName = missingBranchType.getSymbol()?.escapedName;
let caseTest = checker.typeToString(missingBranchType);
let caseTest = tsutils.isTypeFlagSet(
missingBranchType,
ts.TypeFlags.ESSymbolLike,
)
? missingBranchName!
: checker.typeToString(missingBranchType);

if (
symbolName &&
Expand Down Expand Up @@ -298,11 +269,11 @@ export default createRule<Options, MessageIds>({
return;
}

const { missingBranchTypes, defaultCase, containsNonLiteralType } =
const { missingLiteralBranchTypes, defaultCase, containsNonLiteralType } =
switchMetadata;

if (
missingBranchTypes.length === 0 &&
missingLiteralBranchTypes.length === 0 &&
defaultCase !== undefined &&
!containsNonLiteralType
) {
Expand All @@ -321,9 +292,9 @@ export default createRule<Options, MessageIds>({
return;
}

const { isUnion, defaultCase } = switchMetadata;
const { defaultCase, containsNonLiteralType } = switchMetadata;

if (!isUnion && defaultCase === undefined) {
if (containsNonLiteralType && defaultCase === undefined) {
context.report({
node: node.discriminant,
messageId: 'switchIsNotExhaustive',
Expand Down Expand Up @@ -354,6 +325,31 @@ export default createRule<Options, MessageIds>({
},
});

function isFlagSet(flags: number, flag: number): boolean {
return (flags & flag) !== 0;
function isTypeLiteralLikeType(type: ts.Type): boolean {
return tsutils.isTypeFlagSet(
type,
ts.TypeFlags.Literal |
ts.TypeFlags.Undefined |
ts.TypeFlags.Null |
ts.TypeFlags.UniqueESSymbol,
);
}

/**
* For example:
*
* - `"foo" | "bar"` is a type with all literal types.
* - `"foo" | number` is a type that contains non-literal types.
* - `"foo" & { bar: 1 }` is a type that contains non-literal types.
*
* Default cases are never superfluous in switches with non-literal types.
*/
function doesTypeContainNonLiteralType(type: ts.Type): boolean {
return tsutils
.unionTypeParts(type)
.some(type =>
tsutils
.intersectionTypeParts(type)
.every(subType => !isTypeLiteralLikeType(subType)),
);
}
Loading