-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
gh-20512: Fix specialization leak in generic TypedDict.update() #20517
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
Conversation
This comment has been minimized.
This comment has been minimized.
hauntsaninja
left a comment
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.
Nice solve!
It looks like there isn't actually a use for as_anonymous... Should we change it to get_anonymous_fallback? Could help other code paths avoid a similar mistake
|
Yeah! I can make that change. |
|
@hauntsaninja let me know if that's what you had in mind. I tried a few different ways of cleaning this up, but mostly failed the test suite, so landed on the above changes. |
|
Ah sorry, I should have explained myself better. I just pushed a commit with the refactor I meant... Trying to prevent similar bugs in the future |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Ah, I’m tracking now! Thanks for the refactor. |
|
Thanks again! |
Fixes gh-20512
This PR fixes a bug where calling .update() on a specialized generic TypedDict like Group[int] would fail type checking because the plugin reverted to the unspecialized definition (Group[ValT]).
After a lot of dead ends this one wasn't as straightforward as I thought initially. After spending a lot of time in
checkmember.pyandcheckexpr.pyI finally found the issue inmypy/plugins/default.py. The call got the anonymous version of the TypedDict from the TypeInfo, which pointed back to the original generic declaration. This caused specialized types to be replaced by their original TypeVar placeholders during the signature construction for update(). Whew!The fix basically uses
ctx.type(the specialized type of TypedDict being updated) as the source for the items, then applies the anonymous fallback and setsrequired_keysto an empty set to keep the correct behavior of the update method.Added a test in
check-typeddict.testwhich seemed like the right place. All tests pass, as well as the original repro script from the linked issue.