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
64 changes: 57 additions & 7 deletions packages/pyright-internal/src/analyzer/typeGuards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
isParamSpec,
isTypeSame,
isTypeVar,
isUnion,
isUnpackedTypeVarTuple,
maxTypeRecursionCount,
OverloadedType,
Expand Down Expand Up @@ -2199,23 +2200,72 @@ function narrowTypeForContainerType(
});
}

export function getElementTypeForContainerNarrowing(containerType: Type) {
// We support contains narrowing only for certain built-in types that have been specialized.
const supportedContainers = ['list', 'set', 'frozenset', 'deque', 'tuple', 'dict', 'defaultdict', 'OrderedDict'];
if (!isClassInstance(containerType) || !ClassType.isBuiltIn(containerType, supportedContainers)) {
export function getElementTypeForContainerNarrowing(containerType: Type): Type | undefined {
if (isUnion(containerType)) {
const elementTypes: Type[] = [];
for (const subtype of containerType.priv.subtypes) {
const elemType = getElementTypeForContainerNarrowing(subtype);
if (!elemType) {
return undefined;
}
elementTypes.push(elemType);
}
return combineTypes(elementTypes);
}

if (!isClassInstance(containerType)) {
return undefined;
}

// We support contains narrowing only for certain built-in and stdlib collection types that have been specialized.
const supportedContainers = [
'list',
'set',
'frozenset',
'deque',
'tuple',
'dict',
'defaultdict',
'OrderedDict',
'Sequence',
'MutableSequence',
'Set',
'AbstractSet',
'MutableSet',
'Collection',
'Container',
'Mapping',
'MutableMapping',
'KeysView',
'ValuesView',
'dict_keys',
'dict_values',
];

const isSupported =
ClassType.isBuiltIn(containerType, supportedContainers) ||
((containerType.shared.fullName.startsWith('_collections_abc.') ||
containerType.shared.fullName.startsWith('collections.abc.') ||
containerType.shared.fullName.startsWith('typing.')) &&
supportedContainers.includes(containerType.shared.name));

if (!isSupported) {
return undefined;
}

if (!containerType.priv.typeArgs || containerType.priv.typeArgs.length < 1) {
return undefined;
}

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

Container[str] (and similarly the newly supported abstract collection interfaces) only promises __contains__(self, x: object) -> bool; a conforming implementation can return True for an int. This path would then narrow that int to str after if x in container. Please restrict narrowing to containers whose membership semantics establish the element-type relationship, and add a custom Container[str] regression case.


let elementType = containerType.priv.typeArgs[0];
if (containerType.shared.name === 'dict_values' || containerType.shared.name === 'ValuesView') {

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.

Info · Optional note

📍 packages/pyright-internal/src/analyzer/typeGuards.ts:2258
This logic couples narrowing to specific typeshed names and generic argument layouts, particularly the one-versus-two-argument ValuesView handling. Add focused tests for both layouts so future typeshed alias or signature changes cannot silently select the wrong element type.

[verified]

return containerType.priv.typeArgs.length > 1 ? containerType.priv.typeArgs[1] : containerType.priv.typeArgs[0];
}

if (isTupleClass(containerType) && containerType.priv.tupleTypeArgs) {
elementType = combineTypes(containerType.priv.tupleTypeArgs.map((t) => t.type));
return combineTypes(containerType.priv.tupleTypeArgs.map((t) => t.type));
}

return elementType;
return containerType.priv.typeArgs[0];
}

export function narrowTypeForContainerElementType(evaluator: TypeEvaluator, referenceType: Type, elementType: Type) {
Expand Down
29 changes: 29 additions & 0 deletions packages/pyright-internal/src/tests/samples/typeNarrowingIn1.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,32 @@ def func23[T: LiteralString](x: str, y: tuple[T, ...]) -> T:
if x in y:
return x
raise ValueError(f"Invalid value {x!r}")


def func24(val: str | None, container: list[str] | tuple[str, ...]):
if val in container:
reveal_type(val, expected_text="str")
else:
reveal_type(val, expected_text="str | None")


def func25(val: str | int | None, container: list[str] | set[int]):
if val in container:
reveal_type(val, expected_text="str | int")
else:
reveal_type(val, expected_text="str | int | None")


def func26(k: str | int, d: dict[str, float]):
if k in d.keys():
reveal_type(k, expected_text="str")
else:
reveal_type(k, expected_text="str | int")


def func27(v: float | str, d: dict[str, float]):
if v in d.values():
reveal_type(v, expected_text="float")
else:
reveal_type(v, expected_text="float | 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

📍 packages/pyright-internal/src/tests/samples/typeNarrowingIn1.py:227
The tests cover concrete unions and dictionary views but not the newly supported Sequence, Set, Mapping, KeysView, ValuesView, or Container branches. Add focused positive cases plus a custom ABC implementation with permissive __contains__ to establish both intended behavior and the conservative boundary.

[verified]

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

Please add representative regression coverage for the newly supported typing and collections.abc collection types, such as Sequence, Set, Mapping, Container, KeysView, and ValuesView. The current additions cover unions and concrete dictionary views only, leaving the new module-prefix and generic-layout paths untested.