basedpyright icon indicating copy to clipboard operation
basedpyright copied to clipboard

Type narrowing and `TypedDict`

Open decorator-factory opened this issue 1 year ago • 10 comments

from typing import TypedDict

class Foo(TypedDict):
    x: int

class Foo2(Foo):
    y: int

class Bar(TypedDict):
    y: str

def f(foobar: Foo | Bar):
    if 'y' in foobar:
        reveal_type(foobar)  # [based]pyright thinks it's Bar
        print(foobar['y'].lower())

x: Foo2 = {'x': 1, 'y': 2}
f(x)  # blows up at runtime

This type narrowing is fallacious: Foo | Bar having a y key doesn't mean that it's a Bar, because TypedDicts are open (can have arbitrary extra keys).

This was fixed upstream in https://github.com/microsoft/pyright/issues/1899 a long time ago with a good solution: if you want this check to work, use @final on a TypedDict, and disallow providing a non-final TypedDict where a final one is expected. But for it got reverted along the way (https://github.com/python/mypy/issues/15695#issuecomment-1638955963)

(the code where this check should keep working:

from typing import final TypedDict

@final
class Foo(TypedDict):
    x: int

class Foo2(Foo):  # nuh uh, can't subclass a final class
    y: int

class Bar(TypedDict):
    y: str

def f(foobar: Foo | Bar):
    if 'y' in foobar:
        reveal_type(foobar)  # now it's definitely a Bar
        print(foobar['y'].lower())

###

class Evil(TypedDict):
    x: int

class Evil2(Evil):
    y: int

def g(evil: Evil):
    x: Foo = evil  # not allowed: an `Evil` may have extra keys

)

decorator-factory avatar Feb 12 '25 08:02 decorator-factory

I'm guessing this would be not "rejected upstream" anymore if pep 728 is accepted.

beauxq avatar Feb 12 '25 10:02 beauxq

i guess i'll remove this label because this hasn't been re-raised since that pep was made

DetachHead avatar Feb 12 '25 11:02 DetachHead

Yeah, if PEP 728 is accepted, closed=True would be the way to mark a TypedDict as closed instead of @final (in new Python versions)

typing-extensions seems to already support closed=True in version 4.10.0

decorator-factory avatar Feb 12 '25 11:02 decorator-factory

Seems like this bug has been fixed upstream. https://github.com/microsoft/pyright/releases/tag/1.1.401

Bug Fixes:

  • Fixed a bug in the type narrowing logic for the S in D type guard pattern (where S is a string literal and D is a TypedDict). If the TypedDict is not closed, the absence of the key within the TypedDict definition cannot eliminate the type during narrowing.

decorator-factory avatar May 21 '25 05:05 decorator-factory

From @decorator-factory on discord:

Sounds like https://github.com/DetachHead/basedpyright/issues/1063 got un-fixed (https://github.com/microsoft/pyright/issues/10517). Would it be possible to just not include the upstream change for this?

DetachHead avatar Jun 13 '25 10:06 DetachHead

Please support @final to close TypedDicts in the short-term if you don't merge back the "un-fix" -- otherwise there is no way to idiomatically express the very common pattern of union discrimination at all. Upstream pyright did not support @final this way, hence why the un-fix needed to happen, so you'd need to reimplement the older solution.

mypy supports @final TypedDicts for this use-case, and likely will do until a PEP introducing an "official" way to prevent extra keys being added is allowed, and pyright implicitly "supports" it too, so you should match both.

(To throw an additional cat among the pigeons, PEP 705 makes it much less clear what @final means for a TypedDict. Does it just mean no extra keys are present? Does it additionally mean nobody has a reference to the dictionary with an annotation where an item is not read-only, i.e. the item must be effectively final? I would argue it needs to mean the former, not the latter, or it will be very hard to make use of in a world with ReadOnly items.)

alicederyn avatar Jun 18 '25 10:06 alicederyn

@alicederyn It's possible to do this with PEP 728 if you're willing to use typing-extensions

from typing_extensions import TypedDict

class Foo(TypedDict, closed=True):
    x: int
    y: str

class Bar(TypedDict, closed=True):
    z: list[str]

def f(arg: Foo | Bar) -> None:
    if "z" in arg:
        reveal_type(arg)  # Bar
    else:
        reveal_type(arg)  # Foo

I think allowing @final (with the exact same meaning as closed=True) as a stop-gap solution is good, since not everyone is fine with requiring typing-extensions as a runtime dependency. (and there's a small chance that PEP728 never gets accepted) It used to work in Pyright but then was removed (https://github.com/microsoft/pyright/commit/6a25a7bf0b5cb3721a06d0e0d6245b2ebfbf053b)

decorator-factory avatar Jun 18 '25 11:06 decorator-factory

Does mypy allow closed? If not, I'd argue it's not really an option, since I need to write libraries that work with multiple type checkers :(

Definitely happier with the status-quo stop gap.

alicederyn avatar Jun 18 '25 13:06 alicederyn

i agree that the behavior should not have been reverted because it makes the narrowing less type-safe, however i think we should probably just hold off until PEP728 is either accepted or rejected before doing anything

DetachHead avatar Jun 19 '25 05:06 DetachHead

PEP728 is now accepted

JamesYFC avatar Oct 16 '25 03:10 JamesYFC