diff --git a/crates/ty_python_semantic/resources/mdtest/assignment/annotations.md b/crates/ty_python_semantic/resources/mdtest/assignment/annotations.md index 99cacd81933bd..9471f4bc82fc4 100644 --- a/crates/ty_python_semantic/resources/mdtest/assignment/annotations.md +++ b/crates/ty_python_semantic/resources/mdtest/assignment/annotations.md @@ -482,17 +482,14 @@ class TD2(TypedDict): x: str def f(self, dt: dict[str, Any], key: str): - # TODO: This should not error once typed dict assignability is implemented. - # error: [invalid-assignment] x1: TD = dt.get(key, {}) - reveal_type(x1) # revealed: TD + reveal_type(x1) # revealed: Any x2: TD = dt.get(key, {"x": 0}) reveal_type(x2) # revealed: Any x3: TD | None = dt.get(key, {}) - # TODO: This should reveal `Any` once typed dict assignability is implemented. - reveal_type(x3) # revealed: Any | None + reveal_type(x3) # revealed: Any x4: TD | None = dt.get(key, {"x": 0}) reveal_type(x4) # revealed: Any diff --git a/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md b/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md index c0578469d6441..7711a7c3db936 100644 --- a/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md +++ b/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md @@ -265,7 +265,7 @@ import dict_a import dict_b def _(b_person: dict_b.Person): - # TODO should be error: [invalid-assignment] "Object of type `dict_b.Person` is not assignable to `dict_a.Person`" + # error: [invalid-assignment] "Object of type `dict_b.Person` is not assignable to `dict_a.Person`" person_var: dict_a.Person = b_person ``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_all_\342\200\246_(1c685d9d10678263).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_all_\342\200\246_(8a0f0e8ceccc51b2).snap" similarity index 67% rename from "crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_all_\342\200\246_(1c685d9d10678263).snap" rename to "crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_all_\342\200\246_(8a0f0e8ceccc51b2).snap" index 6c9e4de308bdb..17a005b9c222f 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_all_\342\200\246_(1c685d9d10678263).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_all_\342\200\246_(8a0f0e8ceccc51b2).snap" @@ -3,7 +3,7 @@ source: crates/ty_test/src/lib.rs expression: snapshot --- --- -mdtest name: assignment_diagnostics.md - Subscript assignment diagnostics - Unknown key for all elemens of a union +mdtest name: assignment_diagnostics.md - Subscript assignment diagnostics - Unknown key for all elements of a union mdtest path: crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md --- @@ -16,26 +16,27 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/subscript/assignment_dia 2 | 3 | class Person(TypedDict): 4 | name: str - 5 | - 6 | class Animal(TypedDict): - 7 | name: str - 8 | legs: int - 9 | -10 | def _(being: Person | Animal) -> None: -11 | # error: [invalid-key] + 5 | phone_number: str + 6 | + 7 | class Animal(TypedDict): + 8 | name: str + 9 | legs: int +10 | +11 | def _(being: Person | Animal) -> None: 12 | # error: [invalid-key] -13 | being["surname"] = "unknown" +13 | # error: [invalid-key] +14 | being["surname"] = "unknown" ``` # Diagnostics ``` error[invalid-key]: Unknown key "surname" for TypedDict `Person` - --> src/mdtest_snippet.py:13:5 + --> src/mdtest_snippet.py:14:5 | -11 | # error: [invalid-key] 12 | # error: [invalid-key] -13 | being["surname"] = "unknown" +13 | # error: [invalid-key] +14 | being["surname"] = "unknown" | ----- ^^^^^^^^^ Did you mean "name"? | | | TypedDict `Person` in union type `Person | Animal` @@ -46,11 +47,11 @@ info: rule `invalid-key` is enabled by default ``` error[invalid-key]: Unknown key "surname" for TypedDict `Animal` - --> src/mdtest_snippet.py:13:5 + --> src/mdtest_snippet.py:14:5 | -11 | # error: [invalid-key] 12 | # error: [invalid-key] -13 | being["surname"] = "unknown" +13 | # error: [invalid-key] +14 | being["surname"] = "unknown" | ----- ^^^^^^^^^ Did you mean "name"? | | | TypedDict `Animal` in union type `Person | Animal` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_one_\342\200\246_(b515711c0a451a86).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_one_\342\200\246_(b515711c0a451a86).snap" index 4ca40015cd224..f8d48f3721058 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_one_\342\200\246_(b515711c0a451a86).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_Unknown_key_for_one_\342\200\246_(b515711c0a451a86).snap" @@ -16,23 +16,24 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/subscript/assignment_dia 2 | 3 | class Person(TypedDict): 4 | name: str - 5 | - 6 | class Animal(TypedDict): - 7 | name: str - 8 | legs: int - 9 | -10 | def _(being: Person | Animal) -> None: -11 | being["legs"] = 4 # error: [invalid-key] + 5 | phone_number: str + 6 | + 7 | class Animal(TypedDict): + 8 | name: str + 9 | legs: int +10 | +11 | def _(being: Person | Animal) -> None: +12 | being["legs"] = 4 # error: [invalid-key] ``` # Diagnostics ``` error[invalid-key]: Unknown key "legs" for TypedDict `Person` - --> src/mdtest_snippet.py:11:5 + --> src/mdtest_snippet.py:12:5 | -10 | def _(being: Person | Animal) -> None: -11 | being["legs"] = 4 # error: [invalid-key] +11 | def _(being: Person | Animal) -> None: +12 | being["legs"] = 4 # error: [invalid-key] | ----- ^^^^^^ Unknown key "legs" | | | TypedDict `Person` in union type `Person | Animal` diff --git a/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md b/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md index f606d80e1f9d7..ac76a95211c9b 100644 --- a/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md +++ b/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md @@ -95,6 +95,7 @@ from typing import TypedDict class Person(TypedDict): name: str + phone_number: str class Animal(TypedDict): name: str @@ -104,13 +105,14 @@ def _(being: Person | Animal) -> None: being["legs"] = 4 # error: [invalid-key] ``` -## Unknown key for all elemens of a union +## Unknown key for all elements of a union ```py from typing import TypedDict class Person(TypedDict): name: str + phone_number: str class Animal(TypedDict): name: str diff --git a/crates/ty_python_semantic/resources/mdtest/typed_dict.md b/crates/ty_python_semantic/resources/mdtest/typed_dict.md index ad24b03f7cd43..701dc7848aa34 100644 --- a/crates/ty_python_semantic/resources/mdtest/typed_dict.md +++ b/crates/ty_python_semantic/resources/mdtest/typed_dict.md @@ -460,6 +460,8 @@ and their types, rather than the class hierarchy: ```py from typing import TypedDict +from typing_extensions import ReadOnly +from ty_extensions import static_assert, is_assignable_to, is_subtype_of class Person(TypedDict): name: str @@ -468,10 +470,201 @@ class Employee(TypedDict): name: str employee_id: int -p1: Person = Employee(name="Alice", employee_id=1) +class Robot(TypedDict): + name: int -# TODO: this should be an error -e1: Employee = Person(name="Eve") +static_assert(is_assignable_to(Employee, Person)) + +static_assert(not is_assignable_to(Person, Employee)) +static_assert(not is_assignable_to(Robot, Person)) +static_assert(not is_assignable_to(Person, Robot)) +``` + +In order for one `TypedDict` `B` to be assignable to another `TypedDict` `A`, all required keys in +`A`'s schema must be required in `B`'s schema. If a key is not-required and also mutable in `A`, +then it must be not-required in `B` (because `A` allows the caller to `del` that key). These rules +cover keys that are explicitly marked `NotRequired`, and also all the keys in a `TypedDict` with +`total=False`. + +```py +from typing_extensions import NotRequired + +class Spy1(TypedDict): + name: NotRequired[str] + +class Spy2(TypedDict, total=False): + name: str + +# invalid because `Spy1` and `Spy2` might be missing `name` +static_assert(not is_assignable_to(Spy1, Person)) +static_assert(not is_assignable_to(Spy2, Person)) + +# invalid because `Spy1` and `Spy2` are allowed to delete `name`, while `Person` is not +static_assert(not is_assignable_to(Person, Spy1)) +static_assert(not is_assignable_to(Person, Spy2)) + +class Amnesiac1(TypedDict): + name: NotRequired[ReadOnly[str]] + +class Amnesiac2(TypedDict, total=False): + name: ReadOnly[str] + +# invalid because `Amnesiac1` and `Amnesiac2` might be missing `name` +static_assert(not is_assignable_to(Amnesiac1, Person)) +static_assert(not is_assignable_to(Amnesiac2, Person)) + +# Allowed. Neither `Amnesiac1` nor `Amnesiac2` can delete `name`, because it's read-only. +static_assert(is_assignable_to(Person, Amnesiac1)) +static_assert(is_assignable_to(Person, Amnesiac2)) +``` + +If an item in `A` (the destination `TypedDict` type) is read-only, then the corresponding item in +`B` can have any assignable type. But if the item in `A` is mutable, the item type in `B` must be +"consistent", i.e. both assignable-to and assignable-from. (For fully-static types, consistent is +the same as equivalent.) The required and not-required cases are different codepaths, so we need +test all the permutations: + +```py +from typing import Any +from typing_extensions import ReadOnly + +class RequiredMutableInt(TypedDict): + x: int + +class RequiredReadOnlyInt(TypedDict): + x: ReadOnly[int] + +class NotRequiredMutableInt(TypedDict): + x: NotRequired[int] + +class NotRequiredReadOnlyInt(TypedDict): + x: NotRequired[ReadOnly[int]] + +class RequiredMutableBool(TypedDict): + x: bool + +class RequiredReadOnlyBool(TypedDict): + x: ReadOnly[bool] + +class NotRequiredMutableBool(TypedDict): + x: NotRequired[bool] + +class NotRequiredReadOnlyBool(TypedDict): + x: NotRequired[ReadOnly[bool]] + +class RequiredMutableAny(TypedDict): + x: Any + +class RequiredReadOnlyAny(TypedDict): + x: ReadOnly[Any] + +class NotRequiredMutableAny(TypedDict): + x: NotRequired[Any] + +class NotRequiredReadOnlyAny(TypedDict): + x: NotRequired[ReadOnly[Any]] + +# fmt: off +static_assert( is_assignable_to( RequiredMutableInt, RequiredMutableInt)) +static_assert( is_subtype_of( RequiredMutableInt, RequiredMutableInt)) +static_assert(not is_assignable_to( RequiredReadOnlyInt, RequiredMutableInt)) +static_assert(not is_subtype_of( RequiredReadOnlyInt, RequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredMutableInt, RequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredMutableInt, RequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyInt, RequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyInt, RequiredMutableInt)) +static_assert(not is_assignable_to( RequiredMutableBool, RequiredMutableInt)) +static_assert(not is_subtype_of( RequiredMutableBool, RequiredMutableInt)) +static_assert(not is_assignable_to( RequiredReadOnlyBool, RequiredMutableInt)) +static_assert(not is_subtype_of( RequiredReadOnlyBool, RequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredMutableBool, RequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredMutableBool, RequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyBool, RequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyBool, RequiredMutableInt)) +static_assert( is_assignable_to( RequiredMutableAny, RequiredMutableInt)) +static_assert(not is_subtype_of( RequiredMutableAny, RequiredMutableInt)) +static_assert(not is_assignable_to( RequiredReadOnlyAny, RequiredMutableInt)) +static_assert(not is_subtype_of( RequiredReadOnlyAny, RequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredMutableAny, RequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredMutableAny, RequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyAny, RequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyAny, RequiredMutableInt)) + +static_assert( is_assignable_to( RequiredMutableInt, RequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredMutableInt, RequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredReadOnlyInt, RequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredReadOnlyInt, RequiredReadOnlyInt)) +static_assert(not is_assignable_to( NotRequiredMutableInt, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredMutableInt, RequiredReadOnlyInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyInt, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyInt, RequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredMutableBool, RequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredMutableBool, RequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredReadOnlyBool, RequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredReadOnlyBool, RequiredReadOnlyInt)) +static_assert(not is_assignable_to( NotRequiredMutableBool, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredMutableBool, RequiredReadOnlyInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyBool, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyBool, RequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredMutableAny, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( RequiredMutableAny, RequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredReadOnlyAny, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( RequiredReadOnlyAny, RequiredReadOnlyInt)) +static_assert(not is_assignable_to( NotRequiredMutableAny, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredMutableAny, RequiredReadOnlyInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyAny, RequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyAny, RequiredReadOnlyInt)) + +static_assert(not is_assignable_to( RequiredMutableInt, NotRequiredMutableInt)) +static_assert(not is_subtype_of( RequiredMutableInt, NotRequiredMutableInt)) +static_assert(not is_assignable_to( RequiredReadOnlyInt, NotRequiredMutableInt)) +static_assert(not is_subtype_of( RequiredReadOnlyInt, NotRequiredMutableInt)) +static_assert( is_assignable_to( NotRequiredMutableInt, NotRequiredMutableInt)) +static_assert( is_subtype_of( NotRequiredMutableInt, NotRequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyInt, NotRequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyInt, NotRequiredMutableInt)) +static_assert(not is_assignable_to( RequiredMutableBool, NotRequiredMutableInt)) +static_assert(not is_subtype_of( RequiredMutableBool, NotRequiredMutableInt)) +static_assert(not is_assignable_to( RequiredReadOnlyBool, NotRequiredMutableInt)) +static_assert(not is_subtype_of( RequiredReadOnlyBool, NotRequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredMutableBool, NotRequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredMutableBool, NotRequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyBool, NotRequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyBool, NotRequiredMutableInt)) +static_assert(not is_assignable_to( RequiredMutableAny, NotRequiredMutableInt)) +static_assert(not is_subtype_of( RequiredMutableAny, NotRequiredMutableInt)) +static_assert(not is_assignable_to( RequiredReadOnlyAny, NotRequiredMutableInt)) +static_assert(not is_subtype_of( RequiredReadOnlyAny, NotRequiredMutableInt)) +static_assert( is_assignable_to( NotRequiredMutableAny, NotRequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredMutableAny, NotRequiredMutableInt)) +static_assert(not is_assignable_to( NotRequiredReadOnlyAny, NotRequiredMutableInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyAny, NotRequiredMutableInt)) + +static_assert( is_assignable_to( RequiredMutableInt, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredMutableInt, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredReadOnlyInt, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredReadOnlyInt, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( NotRequiredMutableInt, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( NotRequiredMutableInt, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( NotRequiredReadOnlyInt, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( NotRequiredReadOnlyInt, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredMutableBool, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredMutableBool, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredReadOnlyBool, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( RequiredReadOnlyBool, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( NotRequiredMutableBool, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( NotRequiredMutableBool, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( NotRequiredReadOnlyBool, NotRequiredReadOnlyInt)) +static_assert( is_subtype_of( NotRequiredReadOnlyBool, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredMutableAny, NotRequiredReadOnlyInt)) +static_assert(not is_subtype_of( RequiredMutableAny, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( RequiredReadOnlyAny, NotRequiredReadOnlyInt)) +static_assert(not is_subtype_of( RequiredReadOnlyAny, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( NotRequiredMutableAny, NotRequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredMutableAny, NotRequiredReadOnlyInt)) +static_assert( is_assignable_to( NotRequiredReadOnlyAny, NotRequiredReadOnlyInt)) +static_assert(not is_subtype_of( NotRequiredReadOnlyAny, NotRequiredReadOnlyInt)) +# fmt: on ``` All typed dictionaries can be assigned to `Mapping[str, object]`: @@ -483,10 +676,23 @@ class Person(TypedDict): name: str age: int | None -m: Mapping[str, object] = Person(name="Alice", age=30) +alice = Person(name="Alice", age=30) +# Always assignable. +_: Mapping[str, object] = alice +# Follows from above. +_: Mapping[str, Any] = alice +# Also follows from above, because `update` accepts the `SupportsKeysAndGetItem` protocol. +{}.update(alice) +# Not assignable. +# error: [invalid-assignment] "Object of type `Person` is not assignable to `Mapping[str, int]`" +_: Mapping[str, int] = alice +# `Person` does not have `closed=True` or `extra_items`, so it may have additional keys with values +# of unknown type, therefore it can't be assigned to a `Mapping` with value type smaller than `object`. +# error: [invalid-assignment] +_: Mapping[str, str | int | None] = alice ``` -They can *not* be assigned to `dict[str, object]`, as that would allow them to be mutated in unsafe +They *cannot* be assigned to `dict[str, object]`, as that would allow them to be mutated in unsafe ways: ```py @@ -500,7 +706,7 @@ class Person(TypedDict): alice: Person = {"name": "Alice"} -# TODO: this should be an invalid-assignment error +# error: [invalid-argument-type] "Argument to function `dangerous` is incorrect: Expected `dict[str, object]`, found `Person`" dangerous(alice) reveal_type(alice["name"]) # revealed: str @@ -515,6 +721,153 @@ alice: dict[str, str] = {"name": "Alice"} alice: Person = alice ``` +## A subtle interaction between two structural assignability rules prevents unsoundness + +> For the purposes of these conditions, an open `TypedDict` is treated as if it had **read-only** +> extra items of type `object`. + +That language is at the top of [subtyping section of the `TypedDict` spec][subtyping section]. It +sounds like an obscure technicality, especially since `extra_items` is still TODO, but it has an +important interaction with another rule: + +> For each item in [the destination type]...If it is non-required...If it is mutable...If \[the +> source type does not have an item with the same key and also\] has extra items, the extra items +> type **must not be read-only**... + +In other words, by default (`closed=False`) a `TypedDict` cannot be assigned to a different +`TypedDict` that has an additional, optional, mutable item. That implicit rule turns out to be the +only thing standing in the way of this unsound example: + +```py +from typing_extensions import TypedDict, NotRequired + +class C(TypedDict): + x: int + y: str + +class B(TypedDict): + x: int + +class A(TypedDict): + x: int + y: NotRequired[object] # incompatible with both C and (surprisingly!) B + +def b_from_c(c: C) -> B: + return c # allowed + +def a_from_b(b: B) -> A: + # error: [invalid-return-type] "Return type does not match returned value: expected `A`, found `B`" + return b + +# The [invalid-return-type] error above is the only thing that keeps us from corrupting the type of c['y']. +c: C = {"x": 1, "y": "hello"} +a: A = a_from_b(b_from_c(c)) +a["y"] = 42 +``` + +If the additional, optional item in the target is read-only, the requirements are *somewhat* +relaxed. In this case, because the source might contain have undeclared extra items of any type, the +target item must be assignable from `object`: + +```py +from typing_extensions import ReadOnly + +class A2(TypedDict): + x: int + y: NotRequired[ReadOnly[object]] + +def a2_from_b(b: B) -> A2: + return b # allowed + +class A3(TypedDict): + x: int + y: NotRequired[ReadOnly[int]] # not assignable from `object` + +def a3_from_b(b: B) -> A3: + return b # error: [invalid-return-type] +``` + +## Structural assignability supports `TypedDict`s that contain other `TypedDict`s + +```py +from typing_extensions import TypedDict, ReadOnly, NotRequired +from ty_extensions import static_assert, is_assignable_to, is_subtype_of + +class Inner1(TypedDict): + name: str + +class Inner2(TypedDict): + name: str + +class Outer1(TypedDict): + a: Inner1 + b: ReadOnly[Inner1] + c: NotRequired[Inner1] + d: ReadOnly[NotRequired[Inner1]] + +class Outer2(TypedDict): + a: Inner2 + b: ReadOnly[Inner2] + c: NotRequired[Inner2] + d: ReadOnly[NotRequired[Inner2]] + +def _(o1: Outer1, o2: Outer2): + static_assert(is_assignable_to(Outer1, Outer2)) + static_assert(is_subtype_of(Outer1, Outer2)) + static_assert(is_assignable_to(Outer2, Outer1)) + static_assert(is_subtype_of(Outer2, Outer1)) +``` + +This also extends to gradual types: + +```py +from typing import Any + +class Inner3(TypedDict): + name: Any + +class Outer3(TypedDict): + a: Inner3 + b: ReadOnly[Inner3] + c: NotRequired[Inner3] + d: ReadOnly[NotRequired[Inner3]] + +class Outer4(TypedDict): + a: Any + b: ReadOnly[Any] + c: NotRequired[Any] + d: ReadOnly[NotRequired[Any]] + +def _(o1: Outer1, o2: Outer2, o3: Outer3, o4: Outer4): + static_assert(is_assignable_to(Outer3, Outer1)) + static_assert(not is_subtype_of(Outer3, Outer1)) + static_assert(is_assignable_to(Outer4, Outer1)) + static_assert(not is_subtype_of(Outer4, Outer1)) + + static_assert(is_assignable_to(Outer3, Outer2)) + static_assert(not is_subtype_of(Outer3, Outer2)) + static_assert(is_assignable_to(Outer4, Outer2)) + static_assert(not is_subtype_of(Outer4, Outer2)) + + static_assert(is_assignable_to(Outer1, Outer3)) + static_assert(not is_subtype_of(Outer1, Outer3)) + static_assert(is_assignable_to(Outer2, Outer3)) + static_assert(not is_subtype_of(Outer2, Outer3)) + static_assert(is_assignable_to(Outer3, Outer3)) + static_assert(is_subtype_of(Outer3, Outer3)) + static_assert(is_assignable_to(Outer4, Outer3)) + static_assert(not is_subtype_of(Outer4, Outer3)) + + static_assert(is_assignable_to(Outer1, Outer4)) + static_assert(not is_subtype_of(Outer1, Outer4)) + static_assert(is_assignable_to(Outer2, Outer4)) + static_assert(not is_subtype_of(Outer2, Outer4)) + static_assert(is_assignable_to(Outer3, Outer4)) + static_assert(not is_subtype_of(Outer3, Outer4)) + static_assert(is_assignable_to(Outer4, Outer4)) + static_assert(is_subtype_of(Outer4, Outer4)) +``` + ## Key-based access ### Reading @@ -835,7 +1188,7 @@ def combine(p: Person, e: Employee): reveal_type(p | p) # revealed: Person reveal_type(e | e) # revealed: Employee - # TODO: Should be `Person` once we support subtyping for TypedDicts + # TODO: Should be `Person`; simplifying TypedDicts in Unions is pending better cycle handling reveal_type(p | e) # revealed: Person | Employee ``` @@ -915,7 +1268,8 @@ emp_invalid2 = Employee(id=3) ### Legacy generics ```py -from typing import Generic, TypeVar, TypedDict +from typing import Generic, TypeVar, TypedDict, Any +from ty_extensions import static_assert, is_assignable_to, is_subtype_of T = TypeVar("T") @@ -940,6 +1294,14 @@ items2: Items[str] = {"items": ["a", "b", "c"]} items3: Items[int] = {"items": homogeneous_list(1, 2, 3)} items4: Items[str] = {"items": homogeneous_list("a", "b", "c")} items5: Items[int | str] = {"items": homogeneous_list(1, 2, 3)} + +# structural assignability +static_assert(is_assignable_to(Items[int], Items[int])) +static_assert(is_subtype_of(Items[int], Items[int])) +static_assert(not is_assignable_to(Items[str], Items[int])) +static_assert(not is_subtype_of(Items[str], Items[int])) +static_assert(is_assignable_to(Items[Any], Items[int])) +static_assert(not is_subtype_of(Items[Any], Items[int])) ``` ### PEP-695 generics @@ -950,7 +1312,8 @@ python-version = "3.12" ``` ```py -from typing import TypedDict +from typing import TypedDict, Any +from ty_extensions import static_assert, is_assignable_to, is_subtype_of class TaggedData[T](TypedDict): data: T @@ -973,6 +1336,14 @@ items2: Items[str] = {"items": ["a", "b", "c"]} items3: Items[int] = {"items": homogeneous_list(1, 2, 3)} items4: Items[str] = {"items": homogeneous_list("a", "b", "c")} items5: Items[int | str] = {"items": homogeneous_list(1, 2, 3)} + +# structural assignability +static_assert(is_assignable_to(Items[int], Items[int])) +static_assert(is_subtype_of(Items[int], Items[int])) +static_assert(not is_assignable_to(Items[str], Items[int])) +static_assert(not is_subtype_of(Items[str], Items[int])) +static_assert(is_assignable_to(Items[Any], Items[int])) +static_assert(not is_subtype_of(Items[Any], Items[int])) ``` ## Recursive `TypedDict` @@ -997,6 +1368,20 @@ nested: Node = {"name": "n1", "parent": {"name": "n2", "parent": {"name": "n3", nested_invalid: Node = {"name": "n1", "parent": {"name": "n2", "parent": {"name": 3, "parent": None}}} ``` +Structural assignment works for recursive `TypedDict`s too: + +```py +class Person(TypedDict): + name: str + parent: Person | None + +def _(node: Node, person: Person): + _: Person = node + _: Node = person + +_: Node = Person(name="Alice", parent=Node(name="Bob", parent=Person(name="Charlie", parent=None))) +``` + ## Function/assignment syntax This is not yet supported. Make sure that we do not emit false positives for this syntax: @@ -1165,4 +1550,5 @@ reveal_type(actual_td) # revealed: ActualTypedDict reveal_type(actual_td["name"]) # revealed: str ``` +[subtyping section]: https://typing.python.org/en/latest/spec/typeddict.html#subtyping-between-typeddict-types [`typeddict`]: https://typing.python.org/en/latest/spec/typeddict.html diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 7090f16df020d..dde55be126b9a 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -2093,14 +2093,6 @@ impl<'db> Type<'db> { ConstraintSet::from(false) } - (Type::TypedDict(_), _) => { - // TODO: Implement assignability and subtyping for TypedDict - ConstraintSet::from(relation.is_assignability()) - } - - // A non-`TypedDict` cannot subtype a `TypedDict` - (_, Type::TypedDict(_)) => ConstraintSet::from(false), - // Note that the definition of `Type::AlwaysFalsy` depends on the return value of `__bool__`. // If `__bool__` always returns True or False, it can be treated as a subtype of `AlwaysTruthy` or `AlwaysFalsy`, respectively. (left, Type::AlwaysFalsy) => ConstraintSet::from(left.bool(db).is_always_false()), @@ -2207,6 +2199,38 @@ impl<'db> Type<'db> { // A protocol instance can never be a subtype of a nominal type, with the *sole* exception of `object`. (Type::ProtocolInstance(_), _) => ConstraintSet::from(false), + (Type::TypedDict(self_typeddict), Type::TypedDict(other_typeddict)) => relation_visitor + .visit((self, target, relation), || { + self_typeddict.has_relation_to_impl( + db, + other_typeddict, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + }), + + // TODO: When we support `closed` and/or `extra_items`, we could allow assignments to other + // compatible `Mapping`s. `extra_items` could also allow for some assignments to `dict`, as + // long as `total=False`. (But then again, does anyone want a non-total `TypedDict` where all + // key types are a supertype of the extra items type?) + (Type::TypedDict(_), _) => relation_visitor.visit((self, target, relation), || { + KnownClass::Mapping + .to_specialized_instance(db, [KnownClass::Str.to_instance(db), Type::object()]) + .has_relation_to_impl( + db, + target, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + }), + + // A non-`TypedDict` cannot subtype a `TypedDict` + (_, Type::TypedDict(_)) => ConstraintSet::from(false), + // All `StringLiteral` types are a subtype of `LiteralString`. (Type::StringLiteral(_), Type::LiteralString) => ConstraintSet::from(true), diff --git a/crates/ty_python_semantic/src/types/builder.rs b/crates/ty_python_semantic/src/types/builder.rs index 11017d157141f..ac6603f8f5bbf 100644 --- a/crates/ty_python_semantic/src/types/builder.rs +++ b/crates/ty_python_semantic/src/types/builder.rs @@ -502,6 +502,17 @@ impl<'db> UnionBuilder<'db> { return; } + // Comparing `TypedDict`s for redundancy requires iterating over their fields, which is + // problematic if some of those fields point to recursive `Union`s. To avoid cycles, + // compare `TypedDict`s by name/identity instead of using the `has_relation_to` + // machinery. + if let (Type::TypedDict(element_td), Type::TypedDict(ty_td)) = (element_type, ty) { + if element_td == ty_td { + return; + } + continue; + } + if should_simplify_full && !matches!(element_type, Type::TypeAlias(_)) { if ty.is_redundant_with(self.db, element_type) { return; diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index b57e813e5213b..fc3b9d4227aad 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -126,6 +126,16 @@ fn try_metaclass_cycle_initial<'db>( }) } +fn fields_cycle_initial<'db>( + _db: &'db dyn Db, + _id: salsa::Id, + _self: ClassLiteral<'db>, + _specialization: Option>, + _field_policy: CodeGeneratorKind<'db>, +) -> FxIndexMap> { + FxIndexMap::default() +} + /// A category of classes with code generation capabilities (with synthesized methods). #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, salsa::Update, get_size2::GetSize)] pub(crate) enum CodeGeneratorKind<'db> { @@ -555,8 +565,10 @@ impl<'db> ClassType<'db> { TypeRelation::Assignability => ConstraintSet::from(!other.is_final(db)), }, - // Protocol and Generic are not represented by a ClassType. - ClassBase::Protocol | ClassBase::Generic => ConstraintSet::from(false), + // Protocol, Generic, and TypedDict are not represented by a ClassType. + ClassBase::Protocol | ClassBase::Generic | ClassBase::TypedDict => { + ConstraintSet::from(false) + } ClassBase::Class(base) => match (base, other) { (ClassType::NonGeneric(base), ClassType::NonGeneric(other)) => { @@ -579,11 +591,6 @@ impl<'db> ClassType<'db> { ConstraintSet::from(false) } }, - - ClassBase::TypedDict => { - // TODO: Implement subclassing and assignability for TypedDicts. - ConstraintSet::from(true) - } } }) } @@ -2824,7 +2831,10 @@ impl<'db> ClassLiteral<'db> { /// Returns a list of all annotated attributes defined in this class, or any of its superclasses. /// /// See [`ClassLiteral::own_fields`] for more details. - #[salsa::tracked(returns(ref), heap_size=get_size2::GetSize::get_heap_size)] + #[salsa::tracked( + returns(ref), + cycle_initial=fields_cycle_initial, + heap_size=get_size2::GetSize::get_heap_size)] pub(crate) fn fields( self, db: &'db dyn Db, @@ -3933,6 +3943,7 @@ pub enum KnownClass { SupportsIndex, Iterable, Iterator, + Mapping, // typing_extensions ExtensionsTypeVar, // must be distinct from typing.TypeVar, backports new features // Collections @@ -4047,6 +4058,7 @@ impl KnownClass { | Self::ABCMeta | Self::Iterable | Self::Iterator + | Self::Mapping // Evaluating `NotImplementedType` in a boolean context was deprecated in Python 3.9 // and raises a `TypeError` in Python >=3.14 // (see https://docs.python.org/3/library/constants.html#NotImplemented) @@ -4133,6 +4145,7 @@ impl KnownClass { | KnownClass::SupportsIndex | KnownClass::Iterable | KnownClass::Iterator + | KnownClass::Mapping | KnownClass::ChainMap | KnownClass::Counter | KnownClass::DefaultDict @@ -4218,6 +4231,7 @@ impl KnownClass { | KnownClass::SupportsIndex | KnownClass::Iterable | KnownClass::Iterator + | KnownClass::Mapping | KnownClass::ChainMap | KnownClass::Counter | KnownClass::DefaultDict @@ -4303,6 +4317,7 @@ impl KnownClass { | KnownClass::SupportsIndex | KnownClass::Iterable | KnownClass::Iterator + | KnownClass::Mapping | KnownClass::ChainMap | KnownClass::Counter | KnownClass::DefaultDict @@ -4419,7 +4434,8 @@ impl KnownClass { | Self::BuiltinFunctionType | Self::ProtocolMeta | Self::Template - | KnownClass::Path => false, + | Self::Path + | Self::Mapping => false, } } @@ -4492,6 +4508,7 @@ impl KnownClass { | KnownClass::SupportsIndex | KnownClass::Iterable | KnownClass::Iterator + | KnownClass::Mapping | KnownClass::ChainMap | KnownClass::Counter | KnownClass::DefaultDict @@ -4582,6 +4599,7 @@ impl KnownClass { Self::Super => "super", Self::Iterable => "Iterable", Self::Iterator => "Iterator", + Self::Mapping => "Mapping", // For example, `typing.List` is defined as `List = _Alias()` in typeshed Self::StdlibAlias => "_Alias", // This is the name the type of `sys.version_info` has in typeshed, @@ -4880,6 +4898,7 @@ impl KnownClass { | Self::StdlibAlias | Self::Iterable | Self::Iterator + | Self::Mapping | Self::ProtocolMeta | Self::SupportsIndex => KnownModule::Typing, Self::TypeAliasType @@ -5010,6 +5029,7 @@ impl KnownClass { | Self::InitVar | Self::Iterable | Self::Iterator + | Self::Mapping | Self::NamedTupleFallback | Self::NamedTupleLike | Self::ConstraintSet @@ -5100,6 +5120,7 @@ impl KnownClass { | Self::InitVar | Self::Iterable | Self::Iterator + | Self::Mapping | Self::NamedTupleFallback | Self::NamedTupleLike | Self::ConstraintSet @@ -5163,6 +5184,7 @@ impl KnownClass { "TypeVar" => &[Self::TypeVar, Self::ExtensionsTypeVar], "Iterable" => &[Self::Iterable], "Iterator" => &[Self::Iterator], + "Mapping" => &[Self::Mapping], "ParamSpec" => &[Self::ParamSpec], "ParamSpecArgs" => &[Self::ParamSpecArgs], "ParamSpecKwargs" => &[Self::ParamSpecKwargs], @@ -5304,6 +5326,7 @@ impl KnownClass { | Self::TypeVarTuple | Self::Iterable | Self::Iterator + | Self::Mapping | Self::ProtocolMeta | Self::NewType => matches!(module, KnownModule::Typing | KnownModule::TypingExtensions), Self::Deprecated => matches!(module, KnownModule::Warnings | KnownModule::TypingExtensions), diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index bccd10f83f248..f913047494726 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -8043,6 +8043,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // are handled by the default constructor-call logic (we synthesize a `__new__` method for them // in `ClassType::own_class_member()`). class.is_known(self.db(), KnownClass::Tuple) && !class.is_generic() + ) || CodeGeneratorKind::TypedDict.matches( + self.db(), + class.class_literal(self.db()).0, + class.class_literal(self.db()).1, ); // temporary special-casing for all subclasses of `enum.Enum` diff --git a/crates/ty_python_semantic/src/types/typed_dict.rs b/crates/ty_python_semantic/src/types/typed_dict.rs index d8f8c5c54b944..7a22f625077fe 100644 --- a/crates/ty_python_semantic/src/types/typed_dict.rs +++ b/crates/ty_python_semantic/src/types/typed_dict.rs @@ -12,7 +12,9 @@ use super::diagnostic::{ report_missing_typed_dict_key, }; use super::{ApplyTypeMappingVisitor, Type, TypeMapping, visitor}; -use crate::types::TypeContext; +use crate::types::constraints::ConstraintSet; +use crate::types::generics::InferableTypeVars; +use crate::types::{HasRelationToVisitor, IsDisjointVisitor, TypeContext, TypeRelation}; use crate::{Db, FxIndexMap}; use ordermap::OrderSet; @@ -76,6 +78,174 @@ impl<'db> TypedDictType<'db> { ), } } + + // Subtyping between `TypedDict`s follows the algorithm described at: + // https://typing.python.org/en/latest/spec/typeddict.html#subtyping-between-typeddict-types + pub(super) fn has_relation_to_impl( + self, + db: &'db dyn Db, + target: TypedDictType<'db>, + inferable: InferableTypeVars<'_, 'db>, + relation: TypeRelation<'db>, + relation_visitor: &HasRelationToVisitor<'db>, + disjointness_visitor: &IsDisjointVisitor<'db>, + ) -> ConstraintSet<'db> { + // First do a quick nominal check that (if it succeeds) means that we can avoid + // materializing the full `TypedDict` schema for either `self` or `target`. + // This should be cheaper in many cases, and also helps us avoid some cycles. + if self + .defining_class + .is_subclass_of(db, target.defining_class) + { + return ConstraintSet::from(true); + } + + let self_items = self.items(db); + let target_items = target.items(db); + // Many rules violations short-circuit with "never", but asking whether one field is + // [relation] to/of another can produce more complicated constraints, and we collect those. + let mut constraints = ConstraintSet::from(true); + for (target_item_name, target_item_field) in target_items { + let field_constraints = if target_item_field.is_required() { + // required target fields + let Some(self_item_field) = self_items.get(target_item_name) else { + // Self is missing a required field. + return ConstraintSet::from(false); + }; + if !self_item_field.is_required() { + // A required field is not required in self. + return ConstraintSet::from(false); + } + if target_item_field.is_read_only() { + // For `ReadOnly[]` fields in the target, the corresponding fields in + // self need to have the same assignability/subtyping/etc relation + // individually that we're looking for overall between the + // `TypedDict`s. + self_item_field.declared_ty.has_relation_to_impl( + db, + target_item_field.declared_ty, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + } else { + if self_item_field.is_read_only() { + // A read-only field can't be assigned to a mutable target. + return ConstraintSet::from(false); + } + // For mutable fields in the target, the relation needs to apply both + // ways, or else mutating the target could violate the structural + // invariants of self. For fully-static types, this is "equivalence". + // For gradual types, it depends on the relation, but mutual + // assignability is "consistency". + self_item_field + .declared_ty + .has_relation_to_impl( + db, + target_item_field.declared_ty, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + .and(db, || { + target_item_field.declared_ty.has_relation_to_impl( + db, + self_item_field.declared_ty, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + }) + } + } else { + // `NotRequired[]` target fields + if target_item_field.is_read_only() { + // As above, for `NotRequired[]` + `ReadOnly[]` fields in the target. It's + // tempting to refactor things and unify some of these calls to + // `has_relation_to_impl`, but this branch will get more complicated when we + // add support for `closed` and `extra_items` (which is why the rules in the + // spec are structured like they are), and following the structure of the spec + // makes it easier to check the logic here. + if let Some(self_item_field) = self_items.get(target_item_name) { + self_item_field.declared_ty.has_relation_to_impl( + db, + target_item_field.declared_ty, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + } else { + // Self is missing this not-required, read-only item. However, since all + // `TypedDict`s by default are allowed to have "extra items" of any type + // (until we support `closed` and explicit `extra_items`), this key could + // actually turn out to have a value. To make sure this is type-safe, the + // not-required field in the target needs to be assignable from `object`. + // TODO: `closed` and `extra_items` support will go here. + Type::object().when_assignable_to( + db, + target_item_field.declared_ty, + inferable, + ) + } + } else { + // As above, for `NotRequired[]` mutable fields in the target. Again the logic + // is largely the same for now, but it will get more complicated with `closed` + // and `extra_items`. + if let Some(self_item_field) = self_items.get(target_item_name) { + if self_item_field.is_read_only() { + // A read-only field can't be assigned to a mutable target. + return ConstraintSet::from(false); + } + if self_item_field.is_required() { + // A required field can't be assigned to a not-required, mutable field + // in the target, because `del` is allowed on the target field. + return ConstraintSet::from(false); + } + + // As above, for mutable fields in the target, the relation needs + // to apply both ways. + self_item_field + .declared_ty + .has_relation_to_impl( + db, + target_item_field.declared_ty, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + .and(db, || { + target_item_field.declared_ty.has_relation_to_impl( + db, + self_item_field.declared_ty, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + }) + } else { + // Self is missing this not-required, mutable field. This isn't ok if self + // has read-only extra items, which all `TypedDict`s effectively do until + // we support `closed` and explicit `extra_items`. See "A subtle + // interaction between two structural assignability rules prevents + // unsoundness" in `typed_dict.md`. + // TODO: `closed` and `extra_items` support will go here. + ConstraintSet::from(false) + } + } + }; + constraints.intersect(db, field_constraints); + if constraints.is_never_satisfied(db) { + return constraints; + } + } + constraints + } } pub(crate) fn walk_typed_dict_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>(