Skip to content

Improve ambiguous **kwarg checking #9573

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

Merged
merged 6 commits into from
Oct 18, 2020
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
32 changes: 20 additions & 12 deletions mypy/argmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def map_actuals_to_formals(actual_kinds: List[int],
"""
nformals = len(formal_kinds)
formal_to_actual = [[] for i in range(nformals)] # type: List[List[int]]
ambiguous_actual_kwargs = [] # type: List[int]
fi = 0
for ai, actual_kind in enumerate(actual_kinds):
if actual_kind == nodes.ARG_POS:
Expand Down Expand Up @@ -76,18 +77,25 @@ def map_actuals_to_formals(actual_kinds: List[int],
formal_to_actual[formal_kinds.index(nodes.ARG_STAR2)].append(ai)
else:
# We don't exactly know which **kwargs are provided by the
# caller. Assume that they will fill the remaining arguments.
for fi in range(nformals):
# TODO: If there are also tuple varargs, we might be missing some potential
# matches if the tuple was short enough to not match everything.
no_certain_match = (
not formal_to_actual[fi]
or actual_kinds[formal_to_actual[fi][0]] == nodes.ARG_STAR)
if ((formal_names[fi]
and no_certain_match
and formal_kinds[fi] != nodes.ARG_STAR) or
formal_kinds[fi] == nodes.ARG_STAR2):
formal_to_actual[fi].append(ai)
# caller, so we'll defer until all the other unambiguous
# actuals have been processed
ambiguous_actual_kwargs.append(ai)

if ambiguous_actual_kwargs:
# Assume the ambiguous kwargs will fill the remaining arguments.
#
# TODO: If there are also tuple varargs, we might be missing some potential
# matches if the tuple was short enough to not match everything.
unmatched_formals = [fi for fi in range(nformals)
if (formal_names[fi]
and (not formal_to_actual[fi]
or actual_kinds[formal_to_actual[fi][0]] == nodes.ARG_STAR)
and formal_kinds[fi] != nodes.ARG_STAR)
or formal_kinds[fi] == nodes.ARG_STAR2]
for ai in ambiguous_actual_kwargs:
for fi in unmatched_formals:
formal_to_actual[fi].append(ai)

return formal_to_actual


Expand Down
30 changes: 20 additions & 10 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ def check_argument_count(self,
ok = False
elif kind in [nodes.ARG_POS, nodes.ARG_OPT,
nodes.ARG_NAMED, nodes.ARG_NAMED_OPT] and is_duplicate_mapping(
formal_to_actual[i], actual_kinds):
formal_to_actual[i], actual_types, actual_kinds):
if (self.chk.in_checked_function() or
isinstance(get_proper_type(actual_types[formal_to_actual[i][0]]),
TupleType)):
Expand Down Expand Up @@ -4112,15 +4112,25 @@ def is_non_empty_tuple(t: Type) -> bool:
return isinstance(t, TupleType) and bool(t.items)


def is_duplicate_mapping(mapping: List[int], actual_kinds: List[int]) -> bool:
# Multiple actuals can map to the same formal only if they both come from
# varargs (*args and **kwargs); in this case at runtime it is possible that
# there are no duplicates. We need to allow this, as the convention
# f(..., *args, **kwargs) is common enough.
return len(mapping) > 1 and not (
len(mapping) == 2 and
actual_kinds[mapping[0]] == nodes.ARG_STAR and
actual_kinds[mapping[1]] == nodes.ARG_STAR2)
def is_duplicate_mapping(mapping: List[int],
actual_types: List[Type],
actual_kinds: List[int]) -> bool:
return (
len(mapping) > 1
# Multiple actuals can map to the same formal if they both come from
# varargs (*args and **kwargs); in this case at runtime it is possible
# that here are no duplicates. We need to allow this, as the convention
# f(..., *args, **kwargs) is common enough.
and not (len(mapping) == 2
and actual_kinds[mapping[0]] == nodes.ARG_STAR
and actual_kinds[mapping[1]] == nodes.ARG_STAR2)
# Multiple actuals can map to the same formal if there are multiple
# **kwargs which cannot be mapped with certainty (non-TypedDict
# **kwargs).
and not all(actual_kinds[m] == nodes.ARG_STAR2 and
not isinstance(get_proper_type(actual_types[m]), TypedDictType)
for m in mapping)
)


def replace_callable_return_type(c: CallableType, new_ret_type: Type) -> CallableType:
Expand Down
23 changes: 23 additions & 0 deletions test-data/unit/check-kwargs.test
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,29 @@ f(*l, **d)
class A: pass
[builtins fixtures/dict.pyi]

[case testPassingMultipleKeywordVarArgs]
from typing import Any, Dict
def f1(a: 'A', b: 'A') -> None: pass
def f2(a: 'A') -> None: pass
def f3(a: 'A', **kwargs: 'A') -> None: pass
def f4(**kwargs: 'A') -> None: pass
d = None # type: Dict[Any, Any]
d2 = None # type: Dict[Any, Any]
f1(**d, **d2)
f2(**d, **d2)
f3(**d, **d2)
f4(**d, **d2)
class A: pass
[builtins fixtures/dict.pyi]

[case testPassingKeywordVarArgsToVarArgsOnlyFunction]
from typing import Any, Dict
def f(*args: 'A') -> None: pass
d = None # type: Dict[Any, Any]
f(**d) # E: Too many arguments for "f"
class A: pass
[builtins fixtures/dict.pyi]

[case testKeywordArgumentAndCommentSignature]
import typing
def f(x): # type: (int) -> str # N: "f" defined here
Expand Down
5 changes: 5 additions & 0 deletions test-data/unit/check-python38.test
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,16 @@ f(arg=1) # E: Unexpected keyword argument "arg" for "f"
f(arg="ERROR") # E: Unexpected keyword argument "arg" for "f"

[case testPEP570Calls]
from typing import Any, Dict
def f(p, /, p_or_kw, *, kw) -> None: ... # N: "f" defined here
d = None # type: Dict[Any, Any]
f(0, 0, 0) # E: Too many positional arguments for "f"
f(0, 0, kw=0)
f(0, p_or_kw=0, kw=0)
f(p=0, p_or_kw=0, kw=0) # E: Unexpected keyword argument "p" for "f"
f(0, **d)
f(**d) # E: Too few arguments for "f"
[builtins fixtures/dict.pyi]

[case testPEP570Signatures1]
def f(p1: bytes, p2: float, /, p_or_kw: int, *, kw: str) -> None:
Expand Down
18 changes: 18 additions & 0 deletions test-data/unit/check-typeddict.test
Original file line number Diff line number Diff line change
Expand Up @@ -1570,6 +1570,24 @@ f1(**c, **a) # E: "f1" gets multiple values for keyword argument "x" \
# E: Argument "x" to "f1" has incompatible type "str"; expected "int"
[builtins fixtures/tuple.pyi]

[case testTypedDictAsStarStarAndDictAsStarStar]
from mypy_extensions import TypedDict
from typing import Any, Dict

TD = TypedDict('TD', {'x': int, 'y': str})

def f1(x: int, y: str, z: bytes) -> None: ...
def f2(x: int, y: str) -> None: ...

td: TD
d = None # type: Dict[Any, Any]

f1(**td, **d)
f1(**d, **td)
f2(**td, **d) # E: Too many arguments for "f2"
f2(**d, **td) # E: Too many arguments for "f2"
[builtins fixtures/dict.pyi]

[case testTypedDictNonMappingMethods]
from typing import List
from mypy_extensions import TypedDict
Expand Down