Skip to content
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

Unions of TypedDict are incorrectly narrowed via "n in d" #9812

Closed
gh-andre opened this issue Feb 4, 2025 · 2 comments
Closed

Unions of TypedDict are incorrectly narrowed via "n in d" #9812

gh-andre opened this issue Feb 4, 2025 · 2 comments
Labels
as designed Not a bug, working as intended bug Something isn't working

Comments

@gh-andre
Copy link

gh-andre commented Feb 4, 2025

Describe the bug

When a field name is being used to narrow a union of TypedDict types, this narrowing does not seem to consider derived TypedDict classes, resulting in a union of types incorrectly narrowed to one of the types in that union.

Code or Screenshots

from typing import TypedDict

class X(TypedDict):
    a: int

class X1(X):
    b: str

class X2(X):
    c: str

def fn1(x: X1) -> None:
    pass
def fn2(x: X2) -> None:
    pass

def fn(x: X1|X2) -> None:
    if "b" in x:
        reveal_type(x)  # Type of "x" is "X1"
        fn1(x)          # should be an error because x is X1|X2
    elif "c" in x:
        reveal_type(x)  # Type of "x" is "X2"
        fn2(x)
    else:
        reveal_type(x)  # unreachable code
        raise ValueError()

The same code in MyPy fails when calling fn1 because x is still X1|X2 at that point (i.e. a class derived from X2 may have a b field). Adding @final to X1 and X2 in MyPy makes the error go away.

@gh-andre gh-andre added the bug Something isn't working label Feb 4, 2025
@erictraut
Copy link
Collaborator

Pyright is working as intended here.

A TypedDict defines a structural type, similar to a Protocol. It doesn't make sense to mark a structural type definition as @final because inheritance is not used when determining type compatibility with a structural type. The fact that mypy pays attention to @final in this case is incorrect from a typing standpoint.

The type narrowing pattern here allows for a discriminated union between two or more typed dictionaries. You're correct in pointing out that it does allow for a soundness hole, but this pattern is sufficiently common — and useful — that the tradeoff is worth it.

We may be able to close this hole if and when PEP 728 is approved and the usage of "closed" TypedDicts becomes more common. In the meantime, the behavior you're observing here is the best we can do.

@erictraut erictraut added the as designed Not a bug, working as intended label Feb 4, 2025
@gh-andre
Copy link
Author

gh-andre commented Feb 4, 2025

I'm not advocating for @final, just saying that today using @final is how MyPy knows that presence of b is sufficient to narrow the union to a single type. In fact, I cannot use @final for very reason you are describing in that such classes cannot be returned from @abstractmethod overrides with concrete return types.

It sounds like closed will replace @final in this in MyPy, so this is good. Just have to wait for a few years, I suppose. Thanks for the reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants