-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
✨ Added new Error to TypedDict #14225
Changes from 10 commits
c7824be
f19a7f7
2a4220c
fcf4909
deb6543
222869d
70c0a18
eda0c8b
25f3dbc
c7c687f
a275c86
f7a7f4f
06a3866
780e10d
fb98524
708d322
e4b82b6
1c9a623
f4f0cd0
957ba17
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 |
---|---|---|
|
@@ -789,17 +789,22 @@ def check_typeddict_call_with_kwargs( | |
context: Context, | ||
orig_callee: Type | None, | ||
) -> Type: | ||
if not (callee.required_keys <= set(kwargs.keys()) <= set(callee.items.keys())): | ||
actual_keys = kwargs.keys() | ||
found_set = set(actual_keys) | ||
if not (callee.required_keys <= found_set <= set(callee.items.keys())): | ||
expected_keys = [ | ||
key | ||
for key in callee.items.keys() | ||
if key in callee.required_keys or key in kwargs.keys() | ||
if key in callee.required_keys or key in found_set | ||
] | ||
actual_keys = kwargs.keys() | ||
self.msg.unexpected_typeddict_keys( | ||
callee, expected_keys=expected_keys, actual_keys=list(actual_keys), context=context | ||
) | ||
return AnyType(TypeOfAny.from_error) | ||
if callee.required_keys > found_set: | ||
# found_set is not a sub-set of the required_keys | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment contradicts the condition, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
# This means we're dealing with something weird we can't | ||
# properly type | ||
return AnyType(TypeOfAny.from_error) | ||
|
||
orig_callee = get_proper_type(orig_callee) | ||
if isinstance(orig_callee, CallableType): | ||
|
@@ -3735,7 +3740,9 @@ def nonliteral_tuple_index_helper(self, left_type: TupleType, index: Expression) | |
return self.chk.named_generic_type("builtins.tuple", [union]) | ||
return union | ||
|
||
def visit_typeddict_index_expr(self, td_type: TypedDictType, index: Expression) -> Type: | ||
def visit_typeddict_index_expr( | ||
self, td_type: TypedDictType, index: Expression, setitem: bool = False | ||
) -> Type: | ||
if isinstance(index, StrExpr): | ||
key_names = [index.value] | ||
else: | ||
|
@@ -3764,7 +3771,7 @@ def visit_typeddict_index_expr(self, td_type: TypedDictType, index: Expression) | |
for key_name in key_names: | ||
value_type = td_type.items.get(key_name) | ||
if value_type is None: | ||
self.msg.typeddict_key_not_found(td_type, key_name, index) | ||
self.msg.typeddict_key_not_found(td_type, key_name, index, setitem) | ||
return AnyType(TypeOfAny.from_error) | ||
else: | ||
value_types.append(value_type) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,6 +67,9 @@ def __str__(self) -> str: | |
TYPEDDICT_ITEM: Final = ErrorCode( | ||
"typeddict-item", "Check items when constructing TypedDict", "General" | ||
) | ||
TYPPEDICT_UNKNOWN_KEY: Final = ErrorCode( | ||
"typeddict-unknown-key", "Check unknown keys when constructing TypedDict", "General" | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned on the issue, for consistency this error code should be also used for errors in case like: d["unknown"]
d["unknown"] = 42 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, that makes sense, but I'm afraid I don't like it. I'm worried about what would happen when the user would ignore this error. For example Running A = T.TypedDict("A", {"x": int})
def f(x: A) -> None:
X["obvious error"]
f({"x": 1, "y": "foo"}) # I want this to be OK Would result in an obvious error being completely ignored by mypy. So to my view we have to options: 1 - rename the unknown-key to something that better reflects the code ( Open to suggestions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, reading an unknown key may be too much, but setting it (i.e. d: TD = {"known": 1, "unknown": 2} and d: TD = {"known": 1}
d["unknown"] = 2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's very reasonable. Will ping you when I have that sorted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! Not sure if the solution is pretty. Added a test for it on the typeddict file. |
||
HAS_TYPE: Final = ErrorCode( | ||
"has-type", "Check that type of reference can be determined", "General" | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1612,30 +1612,28 @@ def unexpected_typeddict_keys( | |
expected_set = set(expected_keys) | ||
if not typ.is_anonymous(): | ||
# Generate simpler messages for some common special cases. | ||
if actual_set < expected_set: | ||
# Use list comprehension instead of set operations to preserve order. | ||
missing = [key for key in expected_keys if key not in actual_set] | ||
# Use list comprehension instead of set operations to preserve order. | ||
missing = [key for key in expected_keys if key not in actual_set] | ||
if missing: | ||
self.fail( | ||
"Missing {} for TypedDict {}".format( | ||
format_key_list(missing, short=True), format_type(typ) | ||
), | ||
context, | ||
code=codes.TYPEDDICT_ITEM, | ||
) | ||
extra = [key for key in actual_keys if key not in expected_set] | ||
if extra: | ||
self.fail( | ||
"Extra {} for TypedDict {}".format( | ||
format_key_list(extra, short=True), format_type(typ) | ||
), | ||
context, | ||
code=codes.TYPPEDICT_UNKNOWN_KEY, | ||
) | ||
if missing or extra: | ||
# No need to check for further errors | ||
return | ||
else: | ||
extra = [key for key in actual_keys if key not in expected_set] | ||
if extra: | ||
# If there are both extra and missing keys, only report extra ones for | ||
# simplicity. | ||
self.fail( | ||
"Extra {} for TypedDict {}".format( | ||
format_key_list(extra, short=True), format_type(typ) | ||
), | ||
context, | ||
code=codes.TYPEDDICT_ITEM, | ||
) | ||
return | ||
found = format_key_list(actual_keys, short=True) | ||
if not expected_keys: | ||
self.fail(f"Unexpected TypedDict {found}", context) | ||
|
@@ -1655,8 +1653,16 @@ def typeddict_key_must_be_string_literal(self, typ: TypedDictType, context: Cont | |
) | ||
|
||
def typeddict_key_not_found( | ||
self, typ: TypedDictType, item_name: str, context: Context | ||
self, typ: TypedDictType, item_name: str, context: Context, setitem: bool = False | ||
) -> None: | ||
""" | ||
Handles error messages. | ||
|
||
Note, that we differentiate in between reading a value and setting | ||
a value. | ||
Setting a value on a TypedDict is an 'unknown-key' error, | ||
whereas reading it is the more serious/general 'item' error. | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use common convention for docstrings. It should be like this. def func() -> None:
"""Short description in form of "do something".
After empty line, long description indented with function body.
The closing quotes n a separate line.
""" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in: 06a3866 |
||
if typ.is_anonymous(): | ||
self.fail( | ||
'"{}" is not a valid TypedDict key; expected one of {}'.format( | ||
|
@@ -1668,7 +1674,7 @@ def typeddict_key_not_found( | |
self.fail( | ||
f'TypedDict {format_type(typ)} has no key "{item_name}"', | ||
context, | ||
code=codes.TYPEDDICT_ITEM, | ||
code=codes.TYPPEDICT_UNKNOWN_KEY if setitem else codes.TYPEDDICT_ITEM, | ||
) | ||
matches = best_matches(item_name, typ.items.keys()) | ||
if matches: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -455,11 +455,15 @@ class E(TypedDict): | |
y: int | ||
|
||
a: D = {'x': ''} # E: Incompatible types (expression has type "str", TypedDict item "x" has type "int") [typeddict-item] | ||
b: D = {'y': ''} # E: Extra key "y" for TypedDict "D" [typeddict-item] | ||
b: D = {'y': ''} # E: Missing key "x" for TypedDict "D" [typeddict-item] \ | ||
# E: Extra key "y" for TypedDict "D" [typeddict-unknown-key] | ||
c = D(x=0) if int() else E(x=0, y=0) | ||
c = {} # E: Expected TypedDict key "x" but found no keys [typeddict-item] | ||
d: D = {'x': '', 'y': 1} # E: Extra key "y" for TypedDict "D" [typeddict-unknown-key] \ | ||
# E: Incompatible types (expression has type "str", TypedDict item "x" has type "int") [typeddict-item] | ||
|
||
a['y'] = 1 # E: TypedDict "D" has no key "y" [typeddict-item] | ||
|
||
a['y'] = 1 # E: TypedDict "D" has no key "y" [typeddict-unknown-key] | ||
a['x'] = 'x' # E: Value of "x" has incompatible type "str"; expected "int" [typeddict-item] | ||
a['y'] # E: TypedDict "D" has no key "y" [typeddict-item] | ||
[builtins fixtures/dict.pyi] | ||
|
@@ -472,7 +476,10 @@ class A(TypedDict): | |
two_commonparts: int | ||
|
||
a: A = {'one_commonpart': 1, 'two_commonparts': 2} | ||
a['other_commonpart'] = 3 # type: ignore[typeddict-item] | ||
a['other_commonpart'] = 3 # type: ignore[typeddict-unknown-key] \ | ||
# N: Did you mean "one_commonpart" or "two_commonparts"? \ | ||
# N: Error code "typeddict-item" not covered by "type: ignore" comment | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks wrong. The whole idea is that if an error is suppressed, the related notes should be suppressed as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've implemented your suggestion in commit 780e10d Originally, I did it on purpose as I liked the 'Did you mean' feature. It would be helpful for situations like so: # Assume we're running mypy while ignoring the typpedict-unknown-key
F = TypedDict('F', { 'foo':int })
foo: F = { 'foo': 1 }
foo['bar'] = 2
# OOPS! I made a typo
foo['fooo'] = 2 My rationale being that it would be quite rare for a user to both want to add extra keys AND have those keys be very similar looking. |
||
not_exist = a['not_exist'] # type: ignore[typeddict-item] | ||
[builtins fixtures/dict.pyi] | ||
[typing fixtures/typing-typeddict.pyi] | ||
|
||
|
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.
Do you really need to convert these to sets? IIRC dictionary
.keys()
already behave like sets.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.
You're quite right. I had no idea.
Fixed in f7a7f4f