-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
More precise return types for TypedDict.get
#19897
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
Changes from 12 commits
27e50e2
e708813
63224f2
808f593
de8effe
4ebe192
a9d2388
16c97ba
628a0be
443f6c7
6bf9401
cbbbd4d
63f4975
f3e8556
a0fbcee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| from mypy.meet import is_overlapping_types | ||
| from mypy.messages import MessageBuilder | ||
| from mypy.nodes import ( | ||
| ARG_OPT, | ||
| ARG_POS, | ||
| ARG_STAR, | ||
| ARG_STAR2, | ||
|
|
@@ -68,6 +69,7 @@ | |
| TypedDictType, | ||
| TypeOfAny, | ||
| TypeType, | ||
| TypeVarId, | ||
| TypeVarLikeType, | ||
| TypeVarTupleType, | ||
| TypeVarType, | ||
|
|
@@ -1404,6 +1406,76 @@ def analyze_typeddict_access( | |
| fallback=mx.chk.named_type("builtins.function"), | ||
| name=name, | ||
| ) | ||
| elif name == "get": | ||
| # synthesize TypedDict.get() overloads | ||
| str_type = mx.chk.named_type("builtins.str") | ||
| fn_type = mx.chk.named_type("builtins.function") | ||
| object_type = mx.chk.named_type("builtins.object") | ||
| type_info = typ.fallback.type | ||
| # type variable for default value | ||
| tvar = TypeVarType( | ||
|
||
| "T", | ||
| f"{type_info.fullname}.get.T", | ||
| id=TypeVarId(-1, namespace=f"{type_info.fullname}.get"), | ||
| values=[], | ||
| upper_bound=object_type, | ||
| default=AnyType(TypeOfAny.from_omitted_generics), | ||
| ) | ||
| # generate the overloads | ||
| overloads: list[CallableType] = [] | ||
| for key, value_type in typ.items.items(): | ||
| key_type = LiteralType(key, fallback=str_type) | ||
|
|
||
| if key in typ.required_keys: | ||
| # If the key is required, we know it must be present in the TypedDict. | ||
| # def (K, object=...) -> V | ||
| overload = CallableType( | ||
| arg_types=[key_type, object_type], | ||
| arg_kinds=[ARG_POS, ARG_OPT], | ||
| arg_names=[None, None], | ||
| ret_type=value_type, | ||
| fallback=fn_type, | ||
| name=name, | ||
| ) | ||
| overloads.append(overload) | ||
| else: | ||
| # The key is not required, but if it is present, we know its type. | ||
| # def (K) -> V | None (implicit default) | ||
| overload = CallableType( | ||
| arg_types=[key_type], | ||
| arg_kinds=[ARG_POS], | ||
| arg_names=[None], | ||
| ret_type=UnionType.make_union([value_type, NoneType()]), | ||
| fallback=fn_type, | ||
| name=name, | ||
| ) | ||
| overloads.append(overload) | ||
|
|
||
| # def [T](K, T) -> V | T (explicit default) | ||
| overload = CallableType( | ||
| variables=[tvar], | ||
| arg_types=[key_type, tvar], | ||
| arg_kinds=[ARG_POS, ARG_POS], | ||
| arg_names=[None, None], | ||
| ret_type=UnionType.make_union([value_type, tvar]), | ||
| fallback=fn_type, | ||
| name=name, | ||
| ) | ||
| overloads.append(overload) | ||
|
|
||
| # finally, add fallback overload when a key is used that is not in the TypedDict | ||
| # TODO: add support for extra items (PEP 728) | ||
| # def (str, object=...) -> object | ||
| fallback_overload = CallableType( | ||
| arg_types=[str_type, object_type], | ||
| arg_kinds=[ARG_POS, ARG_OPT], | ||
| arg_names=[None, None], | ||
| ret_type=object_type, | ||
randolf-scholz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| fallback=fn_type, | ||
| name=name, | ||
| ) | ||
| overloads.append(fallback_overload) | ||
| return Overloaded(overloads) | ||
| return _analyze_member_access(name, typ.fallback, mx, override_info) | ||
|
|
||
|
|
||
|
|
||
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 am actually worried about the performance implications of this part of the PR. Some people have TypedDicts with few hundreds keys (not an exaggeration). This will then create an enormous overload. And overloads are slow to type-check (for various reasons). What exactly do we need this part for? Is it just for the better
reveal_type(), or also for something else?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.
Yes, I would say that is mostly for
reveal_type. It will also change the error message if a user passes illegal arguments, but I don't think that is as important.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.
Maybe it would be possible to:
Alternative, I could roll back this part of the PR and only keep the updated logic in the default plugin.
Uh oh!
There was an error while loading. Please reload this page.
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.
Calling existing plugin hook early would be a breaking change in the plugin system (IIRC we pass "default return type" as part of the plugin context), and adding a new hook is too big change for something like this. FWIW I don't think
reveal_type()is really worth the potential problems, so I think it would be best to roll back this part.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.
There does seem to be one test where it makes a difference: (#19902)
(on master this just gives
object)I think this should be fixable by doing a
flatten_nested_unioninsidecheck_union_call_expr.