-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Add support for functional TypedDict
syntax
#20732
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
base: main
Are you sure you want to change the base?
Conversation
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-10-08 00:11:18.065496583 +0000
+++ new-output.txt 2025-10-08 00:11:21.376504168 +0000
@@ -1,6 +1,7 @@
WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_type_statement.py`: `PEP695TypeAliasType < 'db >::value_type_(Id(cc17)): execute: too many cycle iterations`
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(16432)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/typeddicts_required.py`: `infer_definition_types(Id(13458)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(16c32)): execute: too many cycle iterations`
_directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
_directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
_directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__`
@@ -839,9 +840,11 @@
tuples_type_form.py:15:1: error[invalid-assignment] Object of type `tuple[Literal[1], Literal[""]]` is not assignable to `tuple[int, int]`
tuples_type_form.py:25:1: error[invalid-assignment] Object of type `tuple[Literal[1]]` is not assignable to `tuple[()]`
tuples_type_form.py:36:1: error[invalid-assignment] Object of type `tuple[Literal[1], Literal[2], Literal[3], Literal[""]]` is not assignable to `tuple[int, ...]`
+typeddicts_alt_syntax.py:23:44: error[invalid-argument-type] Argument is incorrect: Expected `_TypedDictSchema`, found `dict[Unknown | str, Unknown | <class 'str'>]`
typeddicts_operations.py:37:20: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
typeddicts_operations.py:62:1: error[unresolved-attribute] Type `MovieOptional` has no attribute `clear`
typeddicts_readonly.py:24:4: error[invalid-assignment] Cannot assign to key "members" on TypedDict `Band`: key is marked read-only
+typeddicts_readonly.py:36:4: error[invalid-assignment] Cannot assign to key "members" on TypedDict `Band2`: key is marked read-only
typeddicts_readonly.py:50:4: error[invalid-assignment] Cannot assign to key "title" on TypedDict `Movie1`: key is marked read-only
typeddicts_readonly.py:51:4: error[invalid-assignment] Cannot assign to key "year" on TypedDict `Movie1`: key is marked read-only
typeddicts_readonly.py:60:4: error[invalid-assignment] Cannot assign to key "title" on TypedDict `Movie2`: key is marked read-only
@@ -849,12 +852,11 @@
typeddicts_readonly_inheritance.py:36:4: error[invalid-assignment] Cannot assign to key "name" on TypedDict `Album2`: key is marked read-only
typeddicts_readonly_inheritance.py:65:19: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `RequiredName` constructor
typeddicts_type_consistency.py:69:21: error[invalid-key] Invalid key access on TypedDict `A3`: Unknown key "y"
-typeddicts_type_consistency.py:101:1: error[invalid-assignment] Object of type `Unknown | None` is not assignable to `str`
typeddicts_type_consistency.py:126:56: error[invalid-argument-type] Invalid argument to key "inner_key" with declared type `str` on TypedDict `Inner1`: value of type `Literal[1]`
typeddicts_usage.py:23:7: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "director"
typeddicts_usage.py:24:17: error[invalid-assignment] Invalid assignment to key "year" with declared type `int` on TypedDict `Movie`: value of type `Literal["1982"]`
typeddicts_usage.py:28:17: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
typeddicts_usage.py:28:18: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "title"
typeddicts_usage.py:40:24: error[invalid-type-form] The special form `typing.TypedDict` is not allowed in type expressions. Did you mean to use a concrete TypedDict or `collections.abc.Mapping[str, object]` instead?
-Found 857 diagnostics
+Found 859 diagnostics
WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details. |
else { | ||
// Emit a diagnostic here? We seem to support non-string literals. | ||
unimplemented!() | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be erroring here? pyright doesn't seem to support string constants as TypedDict
keys, but we currently ignore non-literal keys when type-checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From https://typing.python.org/en/latest/spec/typeddict.html#use-of-final-values-and-literal-types:
Type checkers are only expected to support actual string literals, not final names or literal types, for specifying keys in a TypedDict type definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Only expected to support" is not the same as "must emit an error for anything else", though...
In general, we defer a lot of operations to type-check time that other type checkers attempt to do during (their equivalent of) semantic indexing. That puts limitations on other type checkers that we don't necessarily have for things like this, which is why language like this appears in the spec about what type checkers are "expected" to support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the code to ignore these for now. I think eventually it makes sense to at least warn if we don't support them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fantastic — thank you very much.
crates/ty_python_semantic/resources/mdtest/annotations/unsupported_special_types.md
Show resolved
Hide resolved
// understand a more specific meta type in order to correctly handle `__getitem__`. | ||
match self { | ||
TypedDictType::FromClass(class) => SubclassOfType::from(db, class), | ||
TypedDictType::Synthesized(_) => KnownClass::TypedDictFallback.to_class_literal(db), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I'm wondering why this doesn't cause problems for dunder-calls on synthesized TypedDict
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KnownInstanceType::TypedDictType
is special-cased in Type::bindings
, but I think it would otherwise go through instance_fallback
to an instance of KnownClass::TypedDictFallback
, not meta_type
to its class literal.
else { | ||
// Emit a diagnostic here? We seem to support non-string literals. | ||
unimplemented!() | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From https://typing.python.org/en/latest/spec/typeddict.html#use-of-final-values-and-literal-types:
Type checkers are only expected to support actual string literals, not final names or literal types, for specifying keys in a TypedDict type definition
646c631
to
d2cb5b8
Compare
d2cb5b8
to
2959ff1
Compare
|
/// A single instance of `typing.TypedDict`. | ||
TypedDictType(TypedDictType<'db>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I haven't read through this whole PR yet so I'm not sure what the correct doc-comment here should be, but I don't think this can be an accurate description of what TypedDictType
represents... TypedDict
is a function at runtime, so it is impossible to create an "instance of TypedDict
". Calling TypedDict
"as a function" at runtime creates a new class in exactly the same way as inheriting from TypedDict
at runtime:
>>> import typing
>>> type(typing.TypedDict)
<class 'function'>
>>> X = typing.TypedDict("X", {})
>>> X
<class '__main__.X'>
>>> type(X)
<class 'typing._TypedDictMeta'>
>>> class Y(typing.TypedDict): ...
...
>>> Y
<class '__main__.Y'>
>>> type(Y)
<class 'typing._TypedDictMeta'>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems incorrect to me that we infer different types for X
and Y
(and different types for X.__class__
and Y.__class__
) here on your PR branch. They're both class objects in exactly the same way at runtime:
from typing import TypedDict, reveal_type
X = TypedDict("X", {})
reveal_type(X) # revealed: typing.TypedDict
reveal_type(X.__class__) # revealed: TypedDictFallback
class Y(TypedDict): ...
reveal_type(Y) # revealed: <class 'Y'>
reveal_type(Y.__class__) # revealed: type
We previously experimented with making the Type::ClassLiteral()
variant internally wrap an enum, so that we could express the fact that not all class-literal objects at runtime are created via class statements. #19998 turned out not to be the correct approach for NewType
s (calls to NewType
don't actually create classes!), but I think it could be the right approach here, and for the functional enum.Enum
syntax, and for collections.namedtuple()
calls, and for the functional typing.NamedTuple
syntax, and for three-argument calls to type()
, since those all do actually create classes at runtime.
Having said all that, this would all complicate your approach (possibly by quite a lot), so I'm okay if you don't want to try making Type::ClassLiteral
an enum. I only mention it because I think we'll have the same issue of "class objects created via function calls" for all these other things we'll need to support in the long term, too. So it may be worth digging in and trying to pull off that refactor at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I see. Maybe I misread this comment, which I thought meant that we don't need to support definitionless classes for functional TypedDict
s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I think that was just my oversight in that comment, not thinking about the fact that functional TypedDict does actually create a class at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't speak for Carl, but I think possibly what he was saying there was that it turned out that NewType
calls don't actually create new class objects afterall, so actually NewType
is different from functional NamedTuple
/enum
/TypedDict
/classes created using three-argument type()
. Meaning that in fact, #20126 doesn't help us here, because unlike #19998, it doesn't propose turning ClassLiteral
into an enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, I posted my reply before I saw Carl's!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that makes a lot of sense. I would prefer to get this PR merged even if it is slightly incorrect (assuming it doesn't generate a lot of ecosystem false positives), but I'm happy to continue this work to support the ClassLiteral
refactor. It probably makes sense to do that with functional TypedDict
s first, rather than implement a new feature along with the refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That SGTM, breaking it up into chunks will make it easier to review too!
|
```py | ||
fields = {"name": str} | ||
|
||
# error: [invalid-argument-type] "Argument is incorrect: Expected `_TypedDictSchema`, found `dict[Unknown | str, Unknown | <class 'str'>]`" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how we should display TypedDictSchema
in diagnostics, given that it is an internal type. Ideally we would have a special diagnostic for this specific case, but I believe the TypedDictSchema
type might still show up in the IDE in other cases.
|
Lint rule | Added | Removed | Changed |
---|---|---|---|
invalid-argument-type |
6 | 0 | 3 |
invalid-key |
7 | 0 | 0 |
unused-ignore-comment |
0 | 4 | 0 |
invalid-assignment |
1 | 0 | 0 |
invalid-type-form |
1 | 0 | 0 |
unsupported-operator |
1 | 0 | 0 |
Total | 16 | 4 | 3 |
The main limitation of this PR based on the ecosystem report looks to be inheriting from a
The errors are a bit unfortunate, but I'm not sure there's an easy way to avoid these false positives before the |
Could you add a branch to ruff/crates/ty_python_semantic/src/types/class_base.rs Lines 260 to 264 in 7a347c4
|
36411dd
to
ac2ed3e
Compare
ac2ed3e
to
26c25be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking on board the feedback -- this looks great!
It would be nice if we could special-case this kind of diagnostic to say something like "Expected a dictionary literal with string-literal keys and types as values", rather than exposing the internal |
42e0455
to
b22cc1a
Compare
It looks like there's a panic with recursive RecursiveMovie = TypedDict(
"RecursiveMovie", {"title": Required[str], "predecessor": NotRequired["RecursiveMovie"]}
) |
Shoot, I should have thought of this in advance. I think it means that we will have to make inference of the actual dict spec lazy, similar to how it is for class typed-dicts by default (since all our understanding of class bodies is lazy). It may be that the best way to do this is similar to what I've done for TypeVar definitions in #20598, where we more fully special-case the entire assignment statement. This would probably eliminate the need for some of the machinery added in this PR around integrating with the full call-binding machinery. |
Summary
Adds support for the functional
TypedDict
syntax, e.g.Part of astral-sh/ty#154.