Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Fix #11483 — Incorrect type inference from context manager `__exit__` return type with Generic
## Status: ✅ Complete

Status: ✅ Complete (pending pipeline push/PR)

Issue: https://github.com/microsoft/pyright/issues/11483

Comment thread
rchiodo marked this conversation as resolved.
Outdated
## Problem

When a generic context manager annotates its `__exit__` (or `__aexit__`) return
type with a `TypeVar` (e.g. `class ContextManager(Generic[T])` whose `__exit__`
returns `T` with `T` bound to `bool | None`), pyright determines exception
suppression from the **unspecialized declared** return type instead of the
solved type argument. As a result `ContextManager[bool]` was treated as
non-suppressing, so code-flow narrowing after the `with` block was wrong
(`reveal_type(foo)` was `Literal[1]` instead of `Literal["str", 1]`).

## Root cause (debugger-verified)

`isExceptionContextManager` in `packages/pyright-internal/src/analyzer/codeFlowEngine.ts`
read `exitType.shared.declaredReturnType`, which is the raw declared return type.

Debugger evidence at the `bool` case breakpoint:
- `exitType.shared.declaredReturnType.shared.name` = `"T"` (TypeVar — the bug)
- `exitType.priv.specializedTypes.returnType.shared.name` = `"bool"` (solved)

Because the declared type was the TypeVar `T`, the `ClassType.isBuiltIn(returnType, 'bool')`
check never matched and `cmSwallowsExceptions` stayed `false`.

## Fix

Use `FunctionType.getEffectiveReturnType(exitType, /* includeInferred */ false)`,
which prefers `priv.specializedTypes.returnType` over `shared.declaredReturnType`.
This returns the specialized `bool` / `None` and covers both `__exit__` and
`__aexit__` (the async Coroutine-unwrapping logic is preserved).

## Tests

- Sample: `packages/pyright-internal/src/tests/samples/with7.py` — generic sync
and async context managers specialized to `None` and `bool`, asserting the
narrowed type after the block via `assert_type`.
- Test case: `With7` in `packages/pyright-internal/src/tests/checker.test.ts`.

## Validation

- Red → green: With7 failed before the fix (4 assert_type mismatches), passes after.
- Debugger confirmed declared=`T` vs specialized=`bool`/`None`.
- `checker.test` (72) and `typeEvaluator1-8` (1163) suites pass.
- Prettier, ESLint (legacy config), and `tsc --noEmit` clean.
43 changes: 24 additions & 19 deletions packages/pyright-internal/src/analyzer/codeFlowEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1962,27 +1962,32 @@ export function getCodeFlowEngine(
const exitMethodName = isAsync ? '__aexit__' : '__exit__';
const exitType = evaluator.getBoundMagicMethod(cmType, exitMethodName);

if (exitType && isFunction(exitType) && exitType.shared.declaredReturnType) {
let returnType = exitType.shared.declaredReturnType;

// If it's an __aexit__ method, its return type will typically be wrapped
// in a Coroutine, so we need to extract the return type from the third
// type argument.
if (isAsync) {
if (
isClassInstance(returnType) &&
ClassType.isBuiltIn(returnType, ['Coroutine', 'CoroutineType']) &&
returnType.priv.typeArgs &&
returnType.priv.typeArgs.length >= 3
) {
returnType = returnType.priv.typeArgs[2];
if (exitType && isFunction(exitType)) {
// Use the effective (specialized) return type rather than the declared
// return type so that generic context managers whose __exit__ return
// type is a TypeVar are evaluated using the solved type argument.
let returnType = FunctionType.getEffectiveReturnType(exitType, /* includeInferred */ false);

if (returnType) {
// If it's an __aexit__ method, its return type will typically be wrapped
// in a Coroutine, so we need to extract the return type from the third
// type argument.
if (isAsync) {
if (
isClassInstance(returnType) &&
ClassType.isBuiltIn(returnType, ['Coroutine', 'CoroutineType']) &&
returnType.priv.typeArgs &&
returnType.priv.typeArgs.length >= 3
) {
returnType = returnType.priv.typeArgs[2];
}
}
}

cmSwallowsExceptions = false;
if (isClassInstance(returnType) && ClassType.isBuiltIn(returnType, 'bool')) {
if (returnType.priv.literalValue === undefined || returnType.priv.literalValue === true) {
cmSwallowsExceptions = true;
cmSwallowsExceptions = false;
if (isClassInstance(returnType) && ClassType.isBuiltIn(returnType, 'bool')) {
if (returnType.priv.literalValue === undefined || returnType.priv.literalValue === true) {
cmSwallowsExceptions = true;
}
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions packages/pyright-internal/src/tests/checker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ test('With6', () => {
TestUtils.validateResults(analysisResults, 0);
});

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

TestUtils.validateResults(analysisResults, 0);
});

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

Expand Down
46 changes: 46 additions & 0 deletions packages/pyright-internal/src/tests/samples/with7.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# This sample tests that the exception-swallowing behavior of a context
# manager is determined from the specialized __exit__ / __aexit__ return
# type when the context manager is generic and the return type is annotated
# with a TypeVar.

from typing import Generic, Literal, Self, TypeVar, assert_type

T = TypeVar("T", bound="bool | None")


class ContextManager(Generic[T]):
def __enter__(self) -> Self: ...
def __exit__(self, exc_type, exc_val, exc_tb) -> T: ...


# The __exit__ return type specializes to None, so the exception is never
# suppressed and the entire body must have run.
foo = "str"
with ContextManager[None]() as ctx:
foo = 1
assert_type(foo, Literal[1])


# The __exit__ return type specializes to bool, so the exception may have
# been suppressed and the body may not have run.
bar = "str"
with ContextManager[bool]() as ctx:
bar = 1
assert_type(bar, Literal["str", 1])


class AsyncContextManager(Generic[T]):
async def __aenter__(self) -> Self: ...
async def __aexit__(self, exc_type, exc_val, exc_tb) -> T: ...


async def func1() -> None:
baz = "str"
async with AsyncContextManager[None]() as ctx:
baz = 1
assert_type(baz, Literal[1])

qux = "str"
async with AsyncContextManager[bool]() as ctx:
qux = 1
assert_type(qux, Literal["str", 1])
Comment thread
rchiodo marked this conversation as resolved.
Outdated
Loading