Skip to content

Fix TypedDict.get("missing_key") with string literal #9906

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 21, 2021
Merged

Fix TypedDict.get("missing_key") with string literal #9906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2efb27c
Fix TypedDict.get("missing_key") with string literal
freundTech Jan 12, 2021
415d338
Fix another failing test
freundTech Jan 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions mypy/plugins/default.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,7 @@ def typed_dict_get_callback(ctx: MethodContext) -> Type:
for key in keys:
value_type = get_proper_type(ctx.type.items.get(key))
if value_type is None:
ctx.api.msg.typeddict_key_not_found(ctx.type, key, ctx.context)
return AnyType(TypeOfAny.from_error)
return ctx.default_return_type

if len(ctx.arg_types) == 1:
output_types.append(value_type)
Expand Down
14 changes: 6 additions & 8 deletions test-data/unit/check-literal.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2227,7 +2227,7 @@ d[c_key] # E: TypedDict "Outer" has no key 'c'

reveal_type(d.get(a_key, u)) # N: Revealed type is 'Union[builtins.int, __main__.Unrelated]'
reveal_type(d.get(b_key, u)) # N: Revealed type is 'Union[builtins.str, __main__.Unrelated]'
d.get(c_key, u) # E: TypedDict "Outer" has no key 'c'
reveal_type(d.get(c_key, u)) # N: Revealed type is 'builtins.object'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this infer Unrelated, or at least a Union of something with Unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like in the other discussion, the type here would be Union[builtins.object, Unrelated], which mypy simplifies to builtins.object


reveal_type(d.pop(a_key)) # E: Key 'a' of TypedDict "Outer" cannot be deleted \
# N: Revealed type is 'builtins.int'
Expand Down Expand Up @@ -2271,11 +2271,11 @@ reveal_type(a[int_key_good]) # N: Revealed type is 'builtins.int'
reveal_type(b[int_key_good]) # N: Revealed type is 'builtins.int'
reveal_type(c[str_key_good]) # N: Revealed type is 'builtins.int'
reveal_type(c.get(str_key_good, u)) # N: Revealed type is 'Union[builtins.int, __main__.Unrelated]'
reveal_type(c.get(str_key_bad, u)) # N: Revealed type is 'builtins.object'

a[int_key_bad] # E: Tuple index out of range
b[int_key_bad] # E: Tuple index out of range
c[str_key_bad] # E: TypedDict "MyDict" has no key 'missing'
c.get(str_key_bad, u) # E: TypedDict "MyDict" has no key 'missing'
[builtins fixtures/dict.pyi]
[typing fixtures/typing-typeddict.pyi]
[out]
Expand Down Expand Up @@ -2342,12 +2342,12 @@ reveal_type(test.get(good_keys, 3)) # N: Revealed type is 'Union[_
reveal_type(test.pop(optional_keys)) # N: Revealed type is 'Union[__main__.D, __main__.E]'
reveal_type(test.pop(optional_keys, 3)) # N: Revealed type is 'Union[__main__.D, __main__.E, Literal[3]?]'
reveal_type(test.setdefault(good_keys, AAndB())) # N: Revealed type is 'Union[__main__.A, __main__.B]'
reveal_type(test.get(bad_keys)) # N: Revealed type is 'builtins.object*'
reveal_type(test.get(bad_keys, 3)) # N: Revealed type is 'builtins.object'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if a maintainer could have a look at this.
I don't know the difference between builtins.object and builtins.object* and couldn't find any reference to the asterisk in the documentation.

Therefor I just placed the needed asterisks by trial and error. This might or might not be wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The asterisk means that it's an inferred type; I think it can be safely ignored.

I'm not sure why mypy infers object here though, and it doesn't seem very useful.

Copy link
Contributor Author

@freundTech freundTech Jan 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to PEP 589 "The static type of d.get(e) should be object if the string value of e cannot be determined statically".

As far as I can tell PEP 589 doesn't say anything about d.get(e, default).

Also something else not related to my code seems to convert Union[..., builtins.object, ...] to just builtins.object, which can bee seen when making defauly.py line 234 retun Any instead. In that case mypy infers Union[__main__.A, Literal[3]?, Any].

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default doesn't change anything, since we can't infer the type of the value from the default (the value type could be an arbitrary union type, for example).

Since typed dicts are heterogeneous, we can't predict the type of arbitrary keys. In this case the only possible precise type is object. Any would also work, but the spirit of PEP 484 is that there shouldn't be Any types in fully and precisely annotated programs. We don't want to silently produce an Any type if a programmer mistypes a string literal.

A union that includes object as an item is equivalent to object (it can contain arbitrary objects). Mypy sometimes simplifies these unions and it's okay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the correction!

del test[optional_keys]


test[bad_keys] # E: TypedDict "Test" has no key 'bad'
test.get(bad_keys) # E: TypedDict "Test" has no key 'bad'
test.get(bad_keys, 3) # E: TypedDict "Test" has no key 'bad'
test.pop(good_keys) # E: Key 'a' of TypedDict "Test" cannot be deleted \
# E: Key 'b' of TypedDict "Test" cannot be deleted
test.pop(bad_keys) # E: Key 'a' of TypedDict "Test" cannot be deleted \
Expand Down Expand Up @@ -2436,14 +2436,12 @@ good_keys: Literal['b', 'c']

x[bad_keys] # E: TypedDict "D1" has no key 'd' \
# E: TypedDict "D2" has no key 'a'
x.get(bad_keys) # E: TypedDict "D1" has no key 'd' \
# E: TypedDict "D2" has no key 'a'
x.get(bad_keys, 3) # E: TypedDict "D1" has no key 'd' \
# E: TypedDict "D2" has no key 'a'

reveal_type(x[good_keys]) # N: Revealed type is 'Union[__main__.B, __main__.C]'
reveal_type(x.get(good_keys)) # N: Revealed type is 'Union[__main__.B, __main__.C]'
reveal_type(x.get(good_keys, 3)) # N: Revealed type is 'Union[__main__.B, Literal[3]?, __main__.C]'
reveal_type(x.get(bad_keys)) # N: Revealed type is 'builtins.object*'
reveal_type(x.get(bad_keys, 3)) # N: Revealed type is 'builtins.object'

[builtins fixtures/dict.pyi]
[typing fixtures/typing-typeddict.pyi]
Expand Down
7 changes: 3 additions & 4 deletions test-data/unit/check-typeddict.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,8 +969,8 @@ d.get('x', 1, 2) # E: No overload variant of "get" of "Mapping" matches argument
# N: Possible overload variants: \
# N: def get(self, k: str) -> object \
# N: def [V] get(self, k: str, default: Union[int, V]) -> object
x = d.get('z') # E: TypedDict "D" has no key 'z'
reveal_type(x) # N: Revealed type is 'Any'
x = d.get('z')
reveal_type(x) # N: Revealed type is 'builtins.object*'
s = ''
y = d.get(s)
reveal_type(y) # N: Revealed type is 'builtins.object*'
Expand Down Expand Up @@ -1689,8 +1689,7 @@ td: Union[TDA, TDB]

reveal_type(td.get('a')) # N: Revealed type is 'builtins.int'
reveal_type(td.get('b')) # N: Revealed type is 'Union[builtins.str, builtins.int]'
reveal_type(td.get('c')) # E: TypedDict "TDA" has no key 'c' \
# N: Revealed type is 'Union[Any, builtins.int]'
reveal_type(td.get('c')) # N: Revealed type is 'builtins.object*'

reveal_type(td['a']) # N: Revealed type is 'builtins.int'
reveal_type(td['b']) # N: Revealed type is 'Union[builtins.str, builtins.int]'
Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/pythoneval.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,14 +1074,14 @@ D = TypedDict('D', {'x': int, 'y': str})
d: D
reveal_type(d.get('x'))
reveal_type(d.get('y'))
d.get('z')
reveal_type(d.get('z'))
d.get()
s = ''
reveal_type(d.get(s))
[out]
_testTypedDictGet.py:7: note: Revealed type is 'builtins.int'
_testTypedDictGet.py:8: note: Revealed type is 'builtins.str'
_testTypedDictGet.py:9: error: TypedDict "D" has no key 'z'
_testTypedDictGet.py:9: note: Revealed type is 'builtins.object*'
_testTypedDictGet.py:10: error: All overload variants of "get" of "Mapping" require at least one argument
_testTypedDictGet.py:10: note: Possible overload variants:
_testTypedDictGet.py:10: note: def get(self, key: str) -> object
Expand Down