Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -6133,8 +6133,17 @@ def is_valid_var_arg(self, typ: Type) -> bool:

def is_valid_keyword_var_arg(self, typ: Type) -> bool:
"""Is a type valid as a **kwargs argument?"""
typ = get_proper_type(typ)
return (
is_subtype(
(
# This is a little ad hoc, ideally we would have a map_instance_to_supertype
Copy link
Contributor

Choose a reason for hiding this comment

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

Imo this whole approach is technically incorrect. SupportsKeysAndGetItem is invariant in key type, so prescribing str will cause errors if a dict with literal keys is given

# that worked for protocols
isinstance(typ, Instance)
and typ.type.fullname == "builtins.dict"
and is_subtype(typ.args[0], self.named_type("builtins.str"))
)
or isinstance(typ, ParamSpecType)
or is_subtype(
typ,
self.chk.named_generic_type(
"_typeshed.SupportsKeysAndGetItem",
Expand All @@ -6147,7 +6156,6 @@ def is_valid_keyword_var_arg(self, typ: Type) -> bool:
"_typeshed.SupportsKeysAndGetItem", [UninhabitedType(), UninhabitedType()]
),
)
or isinstance(typ, ParamSpecType)
)

def not_ready_callback(self, name: str, context: Context) -> None:
Expand Down
2 changes: 1 addition & 1 deletion mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -1401,7 +1401,7 @@ def invalid_var_arg(self, typ: Type, context: Context) -> None:
def invalid_keyword_var_arg(self, typ: Type, is_mapping: bool, context: Context) -> None:
typ = get_proper_type(typ)
if isinstance(typ, Instance) and is_mapping:
self.fail("Keywords must be strings", context)
self.fail("Argument after ** must have string keys", context, code=codes.ARG_TYPE)
else:
self.fail(
f"Argument after ** must be a mapping, not {format_type(typ, self.options)}",
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-generic-subtyping.test
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ func_with_kwargs(**x1)
[out]
main:12: note: Revealed type is "typing.Iterator[builtins.int]"
main:13: note: Revealed type is "builtins.dict[builtins.int, builtins.str]"
main:14: error: Keywords must be strings
main:14: error: Argument after ** must have string keys
main:14: error: Argument 1 to "func_with_kwargs" has incompatible type "**X1[str, int]"; expected "int"
[builtins fixtures/dict.pyi]
[typing fixtures/typing-medium.pyi]
Expand Down
16 changes: 13 additions & 3 deletions test-data/unit/check-kwargs.test
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def f(**kwargs: 'A') -> None: pass
b: Mapping
d: Mapping[A, A]
m: Mapping[str, A]
f(**d) # E: Keywords must be strings
f(**d) # E: Argument after ** must have string keys
f(**m)
f(**b)
class A: pass
Expand All @@ -354,7 +354,7 @@ from typing import Dict, Any, Optional
class A: pass
def f(**kwargs: 'A') -> None: pass
d = {} # type: Dict[A, A]
f(**d) # E: Keywords must be strings
f(**d) # E: Argument after ** must have string keys
f(**A()) # E: Argument after ** must be a mapping, not "A"
kwargs: Optional[Any]
f(**kwargs) # E: Argument after ** must be a mapping, not "Any | None"
Expand Down Expand Up @@ -449,7 +449,7 @@ f(b) # E: Argument 1 to "f" has incompatible type "dict[str, str]"; expected "in
f(**b) # E: Argument 1 to "f" has incompatible type "**dict[str, str]"; expected "int"

c = {0: 0}
f(**c) # E: Keywords must be strings
f(**c) # E: Argument after ** must have string keys
[builtins fixtures/dict.pyi]

[case testCallStar2WithStar]
Expand Down Expand Up @@ -567,3 +567,13 @@ main:38: error: Argument after ** must be a mapping, not "C[str, float]"
main:39: error: Argument after ** must be a mapping, not "D"
main:41: error: Argument 1 to "foo" has incompatible type "**dict[str, str]"; expected "float"
[builtins fixtures/dict.pyi]

[case testLiteralKwargs]
from typing import Any, Literal
kw: dict[Literal["a", "b"], Any]
def func(a, b): ...
func(**kw)

badkw: dict[Literal["one", 1], Any]
func(**badkw) # E: Argument after ** must have string keys
[builtins fixtures/dict.pyi]
Comment on lines +571 to +579
Copy link
Contributor

Choose a reason for hiding this comment

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

This modified test produces false-positive for good_kw:

# [case testLiteralKwargs]
from typing import Literal

def func(a: int, b: int) -> None: ...
class A: pass

good_kw: dict[Literal["a", "b"], int]
bad_kw: dict[Literal["one", 1], int]

func(**good_kw)
func(**bad_kw)  # E: Argument after ** must have string keys
# [builtins fixtures/dict.pyi]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm, this PR does handle correctly handle that test case. The patch is ad hoc though, so e.g. it would fail if you changed it to Mapping

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, I think tested with the wrong commit

Copy link
Contributor

Choose a reason for hiding this comment

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

I did make the proposed change into a helper function: #20419