-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix TypeGuard and TypeIs narrowing for functions with union return types #11608
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,7 @@ import { | |
| isTypeSame, | ||
| isTypeVar, | ||
| isUnpackedTypeVarTuple, | ||
| isUnion, | ||
| maxTypeRecursionCount, | ||
| OverloadedType, | ||
| TupleTypeArg, | ||
|
|
@@ -709,11 +710,23 @@ export function getTypeNarrowingCallback( | |
| let isPossiblyTypeGuard = false; | ||
|
|
||
| const isFunctionReturnTypeGuard = (type: FunctionType) => { | ||
| return ( | ||
| type.shared.declaredReturnType && | ||
| isClassInstance(type.shared.declaredReturnType) && | ||
| ClassType.isBuiltIn(type.shared.declaredReturnType, ['TypeGuard', 'TypeIs']) | ||
| ); | ||
| const returnType = type.shared.declaredReturnType; | ||
| if (!returnType) { | ||
| return false; | ||
| } | ||
| if (isClassInstance(returnType)) { | ||
| return ClassType.isBuiltIn(returnType, ['TypeGuard', 'TypeIs']); | ||
| } | ||
| if (isUnion(returnType)) { | ||
| let isAllGuards = true; | ||
| doForEachSubtype(returnType, (subtype) => { | ||
| if (!isClassInstance(subtype) || !ClassType.isBuiltIn(subtype, ['TypeGuard', 'TypeIs'])) { | ||
| isAllGuards = false; | ||
| } | ||
| }); | ||
| return isAllGuards; | ||
| } | ||
| return false; | ||
| }; | ||
|
|
||
| const callTypeResult = evaluator.getTypeOfExpression( | ||
|
|
@@ -738,22 +751,52 @@ export function getTypeNarrowingCallback( | |
| const functionReturnTypeResult = evaluator.getTypeOfExpression(testExpression); | ||
| const functionReturnType = functionReturnTypeResult.type; | ||
|
|
||
| let typeGuardType: Type | undefined; | ||
| let isStrictTypeGuard = false; | ||
|
|
||
| if ( | ||
| isClassInstance(functionReturnType) && | ||
| ClassType.isBuiltIn(functionReturnType, ['TypeGuard', 'TypeIs']) && | ||
| functionReturnType.priv.typeArgs && | ||
| functionReturnType.priv.typeArgs.length > 0 | ||
| ) { | ||
| const isStrictTypeGuard = ClassType.isBuiltIn(functionReturnType, 'TypeIs'); | ||
| const typeGuardType = functionReturnType.priv.typeArgs[0]; | ||
| isStrictTypeGuard = ClassType.isBuiltIn(functionReturnType, 'TypeIs'); | ||
| typeGuardType = functionReturnType.priv.typeArgs[0]; | ||
| } else if (isUnion(functionReturnType)) { | ||
| const typeGuardSubtypes: ClassType[] = []; | ||
| let isAllGuards = true; | ||
|
|
||
| doForEachSubtype(functionReturnType, (subtype) => { | ||
| if ( | ||
| isClassInstance(subtype) && | ||
| ClassType.isBuiltIn(subtype, ['TypeGuard', 'TypeIs']) && | ||
| subtype.priv.typeArgs && | ||
| subtype.priv.typeArgs.length > 0 | ||
| ) { | ||
| typeGuardSubtypes.push(subtype); | ||
| } else { | ||
| isAllGuards = false; | ||
| } | ||
| }); | ||
|
|
||
| if (isAllGuards && typeGuardSubtypes.length > 0) { | ||
| // A union of type guards cannot be strict in the negative case because at runtime | ||
| // only one arm of the overload/union is selected. Treating it as strict would | ||
| // unsoundly eliminate types in the negative branch. | ||
| isStrictTypeGuard = false; | ||
| typeGuardType = combineTypes(typeGuardSubtypes.map((subtype) => subtype.priv.typeArgs![0])); | ||
| } | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This now narrows directly declared
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This newly enables narrowing for explicitly declared |
||
|
|
||
| if (typeGuardType) { | ||
| const isIncomplete = !!callTypeResult.isIncomplete || !!functionReturnTypeResult.isIncomplete; | ||
|
|
||
| return (type: Type) => { | ||
| return { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Combining |
||
| type: narrowTypeForUserDefinedTypeGuard( | ||
| evaluator, | ||
| type, | ||
| typeGuardType, | ||
| typeGuardType!, | ||
| isPositiveTest, | ||
| isStrictTypeGuard, | ||
| testExpression | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # This sample tests user-defined TypeIs and TypeGuard functions whose return type | ||
| # is a union of TypeIs or TypeGuard instances. | ||
|
|
||
| from typing import TypeGuard, TypeIs, assert_type, overload | ||
|
|
||
|
|
||
| def check_single(val: object) -> TypeIs[int] | TypeIs[str]: | ||
| return isinstance(val, (int, str)) | ||
|
|
||
|
|
||
| # This should generate an error because "int" is not a subtype of "str". | ||
| def invalid_typeis_union(val: str) -> TypeIs[int] | TypeIs[str]: # pyright: ignore[reportGeneralTypeIssues] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This suppresses the diagnostic the [verified] |
||
| return False | ||
|
|
||
|
|
||
| @overload | ||
| def check_overload(val: object, target: type[int]) -> TypeIs[int]: ... | ||
| @overload | ||
| def check_overload(val: object, target: type[str]) -> TypeIs[str]: ... | ||
|
|
||
|
|
||
| def check_overload(val: object, target: type) -> bool: | ||
| return isinstance(val, target) | ||
|
|
||
|
|
||
| def check_mixed(val: object) -> TypeIs[int] | TypeGuard[str]: | ||
| return isinstance(val, (int, str)) | ||
|
|
||
|
|
||
| def check_nonguard(val: object) -> TypeIs[int] | None: | ||
| return isinstance(val, int) if val else None | ||
|
|
||
|
|
||
| def test_single(x: object): | ||
| if check_single(x): | ||
| assert_type(x, int | str) | ||
|
|
||
|
|
||
| def test_overload_positive(x: object, target: type[int] | type[str]): | ||
| if check_overload(x, target): | ||
| assert_type(x, int | str) | ||
|
|
||
|
|
||
| def test_overload_negative(x: int | str | bytes, target: type[int] | type[str]): | ||
| if check_overload(x, target): | ||
| assert_type(x, int | str) | ||
| else: | ||
| # A union of type guards is non-strict in the negative case to prevent | ||
| # unsound type elimination when only one overload/arm applies at runtime. | ||
| assert_type(x, int | str | bytes) | ||
|
|
||
|
|
||
| def test_mixed(x: object): | ||
| if check_mixed(x): | ||
| assert_type(x, int | str) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Add coverage for mixed |
||
|
|
||
|
|
||
| def test_nonguard(x: object): | ||
| if check_nonguard(x): | ||
| # Non-guard members in the return type union cause the type guard to be rejected. | ||
| assert_type(x, object) | ||
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.
The gate accepts a union if any member is a TypeGuard or TypeIs, while the later collector silently discards non-guard members. A return such as
TypeIs[int] | Nonecan therefore apply TypeIs narrowing even when the non-guard arm produced the result. Require every union subtype to be a supported guard before applying this narrowing.