Skip to content

✨ Added new Error to TypedDict #14225

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 20 commits into from
Jan 27, 2023
Merged

✨ Added new Error to TypedDict #14225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c7824be
✨ Added new Error to TypedDict
JoaquimEsteves Nov 30, 2022
f19a7f7
🐛 Fixed 'missing keys' not being reported
JoaquimEsteves Nov 30, 2022
2a4220c
🐛 Early return in case there's missing or extra keys
JoaquimEsteves Nov 30, 2022
fcf4909
🧑‍🔬 Fixed simple error-code tests
JoaquimEsteves Nov 30, 2022
deb6543
Update test-data/unit/check-typeddict.test
JoaquimEsteves Dec 2, 2022
222869d
Update test-data/unit/check-errorcodes.test
JoaquimEsteves Dec 2, 2022
70c0a18
✨ We now typecheck despite having an extra-key
JoaquimEsteves Dec 5, 2022
eda0c8b
🧹 Setting an extra value on a TypedDict now has the correct error code
JoaquimEsteves Dec 5, 2022
25f3dbc
✨ We can now set an item and get the correct error code
JoaquimEsteves Dec 5, 2022
c7c687f
🧑‍🔬 Redid the `IgnotErrorCodeTypedDictNoteIgnore` test
JoaquimEsteves Dec 5, 2022
a275c86
🐈‍⬛ Black
JoaquimEsteves Dec 8, 2022
f7a7f4f
🐛 Added Review Suggestions
JoaquimEsteves Dec 8, 2022
06a3866
📝 Fixed docstring not being in the correct format.
JoaquimEsteves Dec 8, 2022
780e10d
🐛 Fixed ignoring unknown-key not silencing the 'Did you mean' message
JoaquimEsteves Dec 8, 2022
fb98524
Review Feedback - Better Docstrings
JoaquimEsteves Jan 23, 2023
708d322
📝 Added `unknown-key` to the `error_code_list`
JoaquimEsteves Jan 23, 2023
e4b82b6
🐛 Black and sphinx bugs
JoaquimEsteves Jan 23, 2023
1c9a623
Merge branch 'master' into master
JoaquimEsteves Jan 23, 2023
f4f0cd0
Merge branch 'master' into master
JoaquimEsteves Jan 23, 2023
957ba17
Apply suggestions from code review (docs)
ilevkivskyi Jan 25, 2023
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
50 changes: 50 additions & 0 deletions docs/source/error_code_list.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,56 @@ Example:
# Error: Incompatible types (expression has type "float",
# TypedDict item "x" has type "int") [typeddict-item]
p: Point = {'x': 1.2, 'y': 4}

Check TypedDict Keys [typeddict-unknown-key]
--------------------------------------------

When constructing a ``TypedDict`` object, mypy checks whether the definition
contains unknown keys. For convenience's sake, mypy will not generate an error
when a ``TypedDict`` has extra keys if it's passed to a function as an argument.
However, it will generate an error when these are created. Example:

.. code-block:: python

from typing_extensions import TypedDict

class Point(TypedDict):
x: int
y: int

class Point3D(Point):
z: int

def add_x_coordinates(a: Point, b: Point) -> int:
return a["x"] + b["x"]

a: Point = {"x": 1, "y": 4}
b: Point3D = {"x": 2, "y": 5, "z": 6}

# OK
add_x_coordinates(a, b)
# Error: Extra key "z" for TypedDict "Point" [typeddict-unknown-key]
add_x_coordinates(a, {"x": 1, "y": 4, "z": 5})


Setting an unknown value on a ``TypedDict`` will also generate this error:

.. code-block:: python

a: Point = {"x": 1, "y": 2}
# Error: Extra key "z" for TypedDict "Point" [typeddict-unknown-key]
a["z"] = 3


Whereas reading an unknown value will generate the more generic/serious
``typeddict-item``:

.. code-block:: python

a: Point = {"x": 1, "y": 2}
# Error: TypedDict "Point" has no key "z" [typeddict-item]
_ = a["z"]


Check that type of target is known [has-type]
---------------------------------------------
Expand Down
18 changes: 12 additions & 6 deletions mypy/checkexpr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,17 +790,21 @@ 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()
if not (callee.required_keys <= actual_keys <= 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 actual_keys
]
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 > actual_keys:
Copy link
Member

Choose a reason for hiding this comment

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

This condition is unreachable, there is callee.required_keys <= actual_keys in the enclosing if statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello!

I believe this is correct as the callee.required_keys <= actual_keys has a not preceeding it.

Indeed, removing this return wil fail the test [case TestCannotCreateTypedDictInstanceWithMissingItems]

Let's imagine the following:

assert callee.items.keys == {1,2,3,4}
assert callee.required_keys == {1,2,3}
actual_keys = {1,2}

if not (required <= actual_keys <= callee.items.keys ):
    # call unexpected_typeddict_keys on the actual vs expected

    if required > actual:
        return AnyType

Will return AnyType and print out a single error message: "missing required key 3"

Whereas with actual_keys = {1,2,3, 'unrelated'} will print out "extra key 'unrelated' but not return AnyType; ie it will continue to check the TypedDict for any other errors (For example, the actual keys might all be accounted for but have the wrong type).

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, right, I have missed the not.

# found_set is a sub-set of the required_keys
# This means we're missing some keys and as such, we can't
# properly type the object
return AnyType(TypeOfAny.from_error)

orig_callee = get_proper_type(orig_callee)
if isinstance(orig_callee, CallableType):
Expand Down Expand Up @@ -3762,7 +3766,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:
Expand Down Expand Up @@ -3791,7 +3797,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)
Expand Down
4 changes: 3 additions & 1 deletion mypy/checkmember.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,9 @@ def analyze_typeddict_access(
if isinstance(mx.context, IndexExpr):
# Since we can get this during `a['key'] = ...`
# it is safe to assume that the context is `IndexExpr`.
item_type = mx.chk.expr_checker.visit_typeddict_index_expr(typ, mx.context.index)
item_type = mx.chk.expr_checker.visit_typeddict_index_expr(
typ, mx.context.index, setitem=True
)
else:
# It can also be `a.__setitem__(...)` direct call.
# In this case `item_type` can be `Any`,
Expand Down
3 changes: 3 additions & 0 deletions mypy/errorcodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

@JoaquimEsteves JoaquimEsteves Dec 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The 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 mypy --disable-error-code=typeddict-unknown-key on the snippet:

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 (unknown-anon-key for example)
2 - Create another error code for this usecase.

Open to suggestions.

Copy link
Member

Choose a reason for hiding this comment

The 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["unknown"] = 42) should definitely use the same error code. I don't see a difference between:

d: TD = {"known": 1, "unknown": 2}

and

d: TD = {"known": 1}
d["unknown"] = 2

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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"
)
Expand Down
48 changes: 25 additions & 23 deletions mypy/messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1637,30 +1637,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)
Expand All @@ -1680,8 +1678,15 @@ 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:
"""Handle error messages for TypedDicts that have unknown keys.

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.
"""
Copy link
Member

Choose a reason for hiding this comment

The 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.
    """

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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(
Expand All @@ -1690,17 +1695,14 @@ def typeddict_key_not_found(
context,
)
else:
err_code = codes.TYPPEDICT_UNKNOWN_KEY if setitem else codes.TYPEDDICT_ITEM
self.fail(
f'TypedDict {format_type(typ)} has no key "{item_name}"',
context,
code=codes.TYPEDDICT_ITEM,
f'TypedDict {format_type(typ)} has no key "{item_name}"', context, code=err_code
)
matches = best_matches(item_name, typ.items.keys(), n=3)
if matches:
self.note(
"Did you mean {}?".format(pretty_seq(matches, "or")),
context,
code=codes.TYPEDDICT_ITEM,
"Did you mean {}?".format(pretty_seq(matches, "or")), context, code=err_code
)

def typeddict_context_ambiguous(self, types: list[TypedDictType], context: Context) -> None:
Expand Down
11 changes: 8 additions & 3 deletions test-data/unit/check-errorcodes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -472,7 +476,8 @@ 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]
not_exist = a['not_exist'] # type: ignore[typeddict-item]
[builtins fixtures/dict.pyi]
[typing fixtures/typing-typeddict.pyi]

Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/check-typeddict.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2030,7 +2030,8 @@ v = {union: 2} # E: Expected TypedDict key to be string literal
num2: Literal['num']
v = {num2: 2}
bad2: Literal['bad']
v = {bad2: 2} # E: Extra key "bad" for TypedDict "Value"
v = {bad2: 2} # E: Missing key "num" for TypedDict "Value" \
# E: Extra key "bad" for TypedDict "Value"

[builtins fixtures/dict.pyi]
[typing fixtures/typing-typeddict.pyi]
Expand Down