Skip to content
Open
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
69 changes: 39 additions & 30 deletions packages/pyright-internal/src/analyzer/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4669,14 +4669,19 @@ export class Checker extends ParseTreeWalker {
return;
}

if (!isClassInstance(returnType) || !returnType.priv.typeArgs || returnType.priv.typeArgs.length < 1) {
return;
}

const isTypeGuard = ClassType.isBuiltIn(returnType, 'TypeGuard');
const isTypeIs = ClassType.isBuiltIn(returnType, 'TypeIs');
const guardSubtypes: ClassType[] = [];
doForEachSubtype(returnType, (subtype) => {
if (
isClassInstance(subtype) &&
(ClassType.isBuiltIn(subtype, 'TypeGuard') || ClassType.isBuiltIn(subtype, 'TypeIs')) &&
subtype.priv.typeArgs &&
subtype.priv.typeArgs.length >= 1
) {
guardSubtypes.push(subtype);
}
});

if (!isTypeGuard && !isTypeIs) {
if (guardSubtypes.length === 0) {
return;
}

Expand All @@ -4700,32 +4705,36 @@ export class Checker extends ParseTreeWalker {
);
}

if (isTypeIs) {
const scopeIds = getTypeVarScopeIds(functionType);
const narrowedType = returnType.priv.typeArgs[0];
let typeGuardType = makeTypeVarsBound(narrowedType, scopeIds);
typeGuardType = TypeBase.cloneWithTypeForm(typeGuardType, typeGuardType);
const scopeIds = getTypeVarScopeIds(functionType);

// Determine the type of the first parameter.
const paramIndex = isMethod && !FunctionType.isStaticMethod(functionType) ? 1 : 0;
if (paramIndex >= functionType.shared.parameters.length) {
return;
}
// Determine the type of the first parameter.
const paramIndex = isMethod && !FunctionType.isStaticMethod(functionType) ? 1 : 0;
if (paramIndex >= functionType.shared.parameters.length) {
return;
}

const paramType = makeTypeVarsBound(FunctionType.getParamType(functionType, paramIndex), scopeIds);
const paramType = makeTypeVarsBound(FunctionType.getParamType(functionType, paramIndex), scopeIds);

// Verify that the typeGuardType is a narrower type than the paramType.
if (!this._evaluator.assignType(paramType, typeGuardType)) {
const returnAnnotation = node.d.returnAnnotation || node.d.funcAnnotationComment?.d.returnAnnotation;
if (returnAnnotation) {
this._evaluator.addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeIsReturnType().format({
type: this._evaluator.printType(paramType),
returnType: this._evaluator.printType(narrowedType),
}),
returnAnnotation
);
for (const guardSubtype of guardSubtypes) {
if (ClassType.isBuiltIn(guardSubtype, 'TypeIs')) {
const narrowedType = guardSubtype.priv.typeArgs![0];
let typeGuardType = makeTypeVarsBound(narrowedType, scopeIds);
typeGuardType = TypeBase.cloneWithTypeForm(typeGuardType, typeGuardType);

// Verify that the typeGuardType is a narrower type than the paramType.
if (!this._evaluator.assignType(paramType, typeGuardType)) {
const returnAnnotation =
node.d.returnAnnotation || node.d.funcAnnotationComment?.d.returnAnnotation;
if (returnAnnotation) {
this._evaluator.addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeIsReturnType().format({
type: this._evaluator.printType(paramType),
returnType: this._evaluator.printType(narrowedType),
}),
returnAnnotation
);
}
}
}
}
Expand Down
59 changes: 51 additions & 8 deletions packages/pyright-internal/src/analyzer/typeGuards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
isTypeSame,
isTypeVar,
isUnpackedTypeVarTuple,
isUnion,
maxTypeRecursionCount,
OverloadedType,
TupleTypeArg,
Expand Down Expand Up @@ -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;
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

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] | None can 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.


const callTypeResult = evaluator.getTypeOfExpression(
Expand All @@ -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]));
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issue · Please address or respond

This now narrows directly declared TypeIs[int] | TypeIs[str] return annotations, but _validateTypeGuardFunction only validates class-instance return types and therefore skips the TypeIs assignability check for these unions. Please either validate every union member in the checker or restrict this handling to overload-produced unions; otherwise an invalid direct TypeIs union can influence narrowing without the usual soundness validation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issue · Please address or respond

This newly enables narrowing for explicitly declared TypeIs[A] | TypeIs[B] returns, but _validateTypeGuardFunction still skips TypeIs validation when the declared return type is a union. As a result, an invalid declaration such as def guard(x: str) -> TypeIs[int] | TypeIs[str] bypasses the required TypeIs assignability check and is nevertheless treated as a guard here. Please either validate every union arm in the checker or reject/scope out directly declared guard unions; overload-produced unions remain validated per overload.


if (typeGuardType) {
const isIncomplete = !!callTypeResult.isIncomplete || !!functionReturnTypeResult.isIncomplete;

return (type: Type) => {
return {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issue · Please address or respond

Combining TypeIs[int] and TypeIs[str] from overload resolution and treating the result as strict is unsound in the negative branch. Only one overload is selected at runtime: with x: str and target: type[int], check_overload(x, target) is false, but this removes str and narrows x to bytes. Treat overload-joined guard unions as non-strict, or otherwise retain the original type in the negative branch.

type: narrowTypeForUserDefinedTypeGuard(
evaluator,
type,
typeGuardType,
typeGuardType!,
isPositiveTest,
isStrictTypeGuard,
testExpression
Expand Down
61 changes: 61 additions & 0 deletions packages/pyright-internal/src/tests/samples/typeIs5.py
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]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

This suppresses the diagnostic the checker.ts change is intended to produce, while the test expects zero diagnostics. Reverting that validation would therefore leave this test green. Remove the ignore and update TypeIs5 to expect the resulting diagnostic so this behavior is actually covered.

[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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

Add coverage for mixed TypeIs/TypeGuard unions, unions containing a non-guard member, and the relevant negative branches. These cases exercise the new strictness and subtype-collection behavior, including the distinction between TypeIs negative narrowing and TypeGuard positive-only narrowing.



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)
5 changes: 5 additions & 0 deletions packages/pyright-internal/src/tests/typeEvaluator6.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ test('TypeIs4', () => {
TestUtils.validateResults(analysisResults, 0);
});

test('TypeIs5', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeIs5.py']);
TestUtils.validateResults(analysisResults, 0);
});

test('Never1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['never1.py']);

Expand Down
Loading