[ty] Faster subscript assignment checks for (unions of) TypedDicts#21378
Merged
[ty] Faster subscript assignment checks for (unions of) TypedDicts#21378
TypedDicts#21378Conversation
sharkdp
added
ty
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-11-12 15:50:26.056135771 +0000
+++ new-output.txt 2025-11-12 15:50:29.708160055 +0000
@@ -745,7 +745,7 @@
namedtuples_usage.py:34:7: error[index-out-of-bounds] Index 3 is out of bounds for tuple `Point` with length 3
namedtuples_usage.py:35:7: error[index-out-of-bounds] Index -4 is out of bounds for tuple `Point` with length 3
namedtuples_usage.py:40:1: error[invalid-assignment] Cannot assign to read-only property `x` on object of type `Point`
-namedtuples_usage.py:41:1: error[invalid-assignment] Cannot assign to object of type `Point` with no `__setitem__` method
+namedtuples_usage.py:41:1: error[invalid-assignment] Cannot assign to a subscript on an object of type `Point` with no `__setitem__` method
namedtuples_usage.py:52:1: error[invalid-assignment] Too many values to unpack: Expected 2
namedtuples_usage.py:53:1: error[invalid-assignment] Not enough values to unpack: Expected 4
narrowing_typeguard.py:17:9: error[type-assertion-failure] Argument does not have asserted type `tuple[str, str]`
@@ -971,7 +971,7 @@
typeddicts_extra_items.py:339:1: error[type-assertion-failure] Argument does not have asserted type `tuple[str, int]`
typeddicts_extra_items.py:339:13: error[unresolved-attribute] Object of type `IntDictWithNum` has no attribute `popitem`
typeddicts_extra_items.py:342:27: error[invalid-key] Cannot access `IntDictWithNum` with a key of type `str`. Only string literals are allowed as keys on TypedDicts.
-typeddicts_extra_items.py:343:31: error[invalid-key] Invalid key for TypedDict `IntDictWithNum` of type `str`
+typeddicts_extra_items.py:343:31: error[invalid-key] Invalid key of type `str` for TypedDict `IntDictWithNum`
typeddicts_extra_items.py:352:5: error[invalid-assignment] Object of type `dict[str, int]` is not assignable to `IntDict`
typeddicts_operations.py:22:17: error[invalid-assignment] Invalid assignment to key "name" with declared type `str` on TypedDict `Movie`: value of type `Literal[1982]`
typeddicts_operations.py:23:17: error[invalid-assignment] Invalid assignment to key "year" with declared type `int` on TypedDict `Movie`: value of type `Literal[""]`
|
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-key |
1,907 | 0 | 122 |
invalid-assignment |
504 | 49 | 183 |
possibly-missing-implicit-call |
0 | 272 | 0 |
unused-ignore-comment |
3 | 0 | 0 |
| Total | 2,414 | 321 | 305 |
sharkdp
force-pushed
the
david/faster-subscript-assignment-checks
branch
2 times, most recently
from
bd4e1b7 to
634597c
Compare
sharkdp
requested review from
AlexWaygood,
carljm and
dcreager
as code owners
Member
|
I wondered about how this might affect assignability and subtyping of from ty_extensions import static_assert, is_subtype_of, is_assignable_to
from typing import Protocol, Literal, TypedDict
class Foo(Protocol):
def __setitem__(self, key: Literal["x"], value: int, /) -> None: ...
class Bar(TypedDict):
x: int
static_assert(is_assignable_to(Bar, Foo))
static_assert(is_subtype_of(Bar, Foo))but I suppose it's impossible to test that right now, because |
Member
|
If need be, we could always add a special case for |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
sharkdp
force-pushed
the
david/faster-subscript-assignment-checks
branch
from
37b32b2 to
8653a6d
Compare
sharkdp
changed the title
TypedDictsTypedDicts
sharkdp
force-pushed
the
david/faster-subscript-assignment-checks
branch
from
8653a6d to
0c65dc6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
CodSpeed Performance ReportMerging #21378 will improve performances by 86.14%Comparing Summary
Benchmarks breakdown
Footnotes
|
sharkdp
force-pushed
the
david/faster-subscript-assignment-checks
branch
from
0c65dc6 to
c17b16e
Compare
sharkdp
force-pushed
the
david/faster-subscript-assignment-checks
branch
from
c17b16e to
5cce18c
Compare
sharkdp
force-pushed
the
david/faster-subscript-assignment-checks
branch
from
5cce18c to
306e619
Compare
sharkdp
commented
Nov 12, 2025
sharkdp
commented
Nov 12, 2025
sharkdp
force-pushed
the
david/faster-subscript-assignment-checks
branch
from
306e619 to
ccb7a0c
Compare
AlexWaygood
approved these changes
Nov 12, 2025
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Awesome work. None of my comments below are blocking, just thoughts that occurred to me as I read through the diff
...ssignment_diagnosti…_-_Subscript_assignment…_-_Possibly_missing_`__…_(efd3f0c02e9b89e9).snap
Outdated
Show resolved
Hide resolved
...ssignment_diagnosti…_-_Subscript_assignment…_-_Possibly_missing_`__…_(efd3f0c02e9b89e9).snap
Outdated
Show resolved
Hide resolved
Comment on lines
+33
to
+43
Member
There was a problem hiding this comment.
What about including the name of the unknown key in the summary line here and taking it out of the primary-annotation message?
error[invalid-key]: Invalid key "surname" for TypedDict `Person`
--> src/mdtest_snippet.py:13:5
|
11 | # error: [invalid-key]
12 | # error: [invalid-key]
13 | being["surname"] = "unknown"
| ----- ^^^^^^^^^ Did you mean "name"?
| |
| TypedDict `Person` in union type `Person | Animal`
|
info: rule `invalid-key` is enabled by default
We could also consider linking back to the definition of the Person typeddict in a subdiagnostic, though I guess that would be very verbose if you had a union of typeddicts.
Contributor
Author
There was a problem hiding this comment.
What about including the name of the unknown key in the summary line here and taking it out of the primary-annotation message?
I can try that, but it's always a bit tricky to get both the full diagnostic and the concise diagnostic to look good. I still wish we had an API to set a separate concise message.
...resources/mdtest/snapshots/typed_dict.md_-_`TypedDict`_-_Diagnostics_(e5289abf5c570c29).snap
Show resolved
Hide resolved
| @@ -143,30 +143,57 @@ impl TypedDictAssignmentKind { | |||
| pub(super) fn validate_typed_dict_key_assignment<'db, 'ast>( | |||
Member
There was a problem hiding this comment.
Calls to this function are becoming quite difficult to read because of how many arguments there are. Especially the third argument, where None is often passed, and the last argument which is a bool. We could consider refactoring this into a struct that holds all the options for better readability, e.g.
let validator = TypedDictValidator {
context: &self.context,
typed_dict,
full_object_ty: None,
key,
value_ty,
typed_dict_node,
key_node,
value_node,
assignment_kind,
emit_diagnostics: true
};
validator.validate();
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
We synthesize a (potentially large) set of
__setitem__overloads for every item in aTypedDict. Previously, validation of subscript assignments onTypedDicts relied on actually calling__setitem__with the provided key and value types, which implied that we needed to do the full overload call evaluation for this large set of overloads. This PR improves the performance of subscript assignment checks onTypedDicts by validating the assignment directly instead of calling__setitem__.This PR also adds better handling for assignments to subscripts on union and intersection types (but does not attempt to make it perfect). It achieves this by distributing the check over unions and intersections, instead of calling
__setitem__on the union/intersection directly. We already do something similar when validating attribute assignments.Ecosystem impact
Test Plan
New Markdown tests.