From 869d728757d82102ffc09cb4e43853a117ee34ca Mon Sep 17 00:00:00 2001 From: Erik Soma Date: Fri, 9 Oct 2020 19:54:32 -0400 Subject: [PATCH 1/6] Allow multiple keyword vararg unpacking. --- mypy/argmap.py | 30 +++++++++++++++++----------- mypy/checkexpr.py | 12 ++++++++--- test-data/unit/check-kwargs.test | 31 +++++++++++++++++++++++++++++ test-data/unit/check-typeddict.test | 18 +++++++++++++++++ 4 files changed, 76 insertions(+), 15 deletions(-) diff --git a/mypy/argmap.py b/mypy/argmap.py index 324ccaf833d5..ec5fe0db551b 100644 --- a/mypy/argmap.py +++ b/mypy/argmap.py @@ -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: @@ -76,18 +77,23 @@ 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: + 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 diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 9c0e2f4048b3..9921213dacaf 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -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)): @@ -4112,7 +4112,9 @@ 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: +def is_duplicate_mapping(mapping: List[int], + actual_types: List[Type], + 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 @@ -4120,7 +4122,11 @@ def is_duplicate_mapping(mapping: List[int], actual_kinds: List[int]) -> bool: 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) + actual_kinds[mapping[1]] == nodes.ARG_STAR2) and not all( + actual_kinds[m] == nodes.ARG_STAR2 and + not isinstance(actual_types[m], TypedDictType) + for m in mapping + ) def replace_callable_return_type(c: CallableType, new_ret_type: Type) -> CallableType: diff --git a/test-data/unit/check-kwargs.test b/test-data/unit/check-kwargs.test index a587be3e06f8..b7f7e9e996aa 100644 --- a/test-data/unit/check-kwargs.test +++ b/test-data/unit/check-kwargs.test @@ -377,6 +377,37 @@ 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 testPassingKeywordVarArgsToPositionalOnlyFunction] +from typing import Any, Dict +def f(a: 'A', /) -> None: pass +d = None # type: Dict[Any, Any] +f(**d) # E: Too many arguments for "f" # E: Too few arguments for "f" +class A: pass +[builtins fixtures/dict.pyi] + [case testKeywordArgumentAndCommentSignature] import typing def f(x): # type: (int) -> str # N: "f" defined here diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index 4dc80b1ecd44..2c474f389ad4 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -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 From c3c20a899a4b74668ecaf7cfeeaf8ba0ce25821e Mon Sep 17 00:00:00 2001 From: Erik Soma Date: Sat, 10 Oct 2020 08:42:09 -0400 Subject: [PATCH 2/6] Move kwarg unpacking into positional function test to 3.8 tests. --- test-data/unit/check-kwargs.test | 8 -------- test-data/unit/check-python38.test | 5 +++++ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/test-data/unit/check-kwargs.test b/test-data/unit/check-kwargs.test index b7f7e9e996aa..96669e7eea36 100644 --- a/test-data/unit/check-kwargs.test +++ b/test-data/unit/check-kwargs.test @@ -400,14 +400,6 @@ f(**d) # E: Too many arguments for "f" class A: pass [builtins fixtures/dict.pyi] -[case testPassingKeywordVarArgsToPositionalOnlyFunction] -from typing import Any, Dict -def f(a: 'A', /) -> None: pass -d = None # type: Dict[Any, Any] -f(**d) # E: Too many arguments for "f" # E: Too few arguments for "f" -class A: pass -[builtins fixtures/dict.pyi] - [case testKeywordArgumentAndCommentSignature] import typing def f(x): # type: (int) -> str # N: "f" defined here diff --git a/test-data/unit/check-python38.test b/test-data/unit/check-python38.test index 8e013751835f..a115c05bb23e 100644 --- a/test-data/unit/check-python38.test +++ b/test-data/unit/check-python38.test @@ -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: From aed8c3d07012c34f537522942d3e8ee8450e96c2 Mon Sep 17 00:00:00 2001 From: Erik Soma Date: Sat, 10 Oct 2020 09:19:06 -0400 Subject: [PATCH 3/6] Formatting and comments. --- mypy/argmap.py | 5 +++-- mypy/checkexpr.py | 28 ++++++++++++++++------------ 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/mypy/argmap.py b/mypy/argmap.py index ec5fe0db551b..49dd923f5976 100644 --- a/mypy/argmap.py +++ b/mypy/argmap.py @@ -80,8 +80,9 @@ def map_actuals_to_formals(actual_kinds: List[int], # 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. unmatched_formals = [ fi for fi in range(nformals) if (formal_names[fi] @@ -93,7 +94,7 @@ def map_actuals_to_formals(actual_kinds: List[int], for ai in ambiguous_actual_kwargs: for fi in unmatched_formals: formal_to_actual[fi].append(ai) - + return formal_to_actual diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 9921213dacaf..3e10d6f97147 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -4115,18 +4115,22 @@ def is_non_empty_tuple(t: Type) -> bool: def is_duplicate_mapping(mapping: List[int], actual_types: List[Type], 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) and not all( - actual_kinds[m] == nodes.ARG_STAR2 and - not isinstance(actual_types[m], TypedDictType) - for m in mapping - ) + 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(actual_types[m], TypedDictType) + for m in mapping) + ) def replace_callable_return_type(c: CallableType, new_ret_type: Type) -> CallableType: From 947a29f2e1f88722e6dbb485e67d912e98f6c0e7 Mon Sep 17 00:00:00 2001 From: Erik Soma Date: Sat, 10 Oct 2020 10:09:16 -0400 Subject: [PATCH 4/6] Add TODO back in. --- mypy/argmap.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mypy/argmap.py b/mypy/argmap.py index 49dd923f5976..bc47662d680f 100644 --- a/mypy/argmap.py +++ b/mypy/argmap.py @@ -83,6 +83,9 @@ def map_actuals_to_formals(actual_kinds: List[int], 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] From c6aedea93ba569cffa4425858e28d122c0f3e789 Mon Sep 17 00:00:00 2001 From: Erik Soma Date: Sat, 10 Oct 2020 11:18:05 -0400 Subject: [PATCH 5/6] Fix hanging indent lint error. --- mypy/argmap.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/mypy/argmap.py b/mypy/argmap.py index bc47662d680f..ff7e94e93cbe 100644 --- a/mypy/argmap.py +++ b/mypy/argmap.py @@ -86,14 +86,12 @@ def map_actuals_to_formals(actual_kinds: List[int], # # 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 - ] + 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) From f49163e221938d6d0369ea2c6e87879460b9a729 Mon Sep 17 00:00:00 2001 From: Erik Soma Date: Sat, 10 Oct 2020 22:20:03 -0400 Subject: [PATCH 6/6] Fix isinstance usage on unexpanded type. --- mypy/checkexpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 3e10d6f97147..fa33161fe4ad 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -4128,7 +4128,7 @@ def is_duplicate_mapping(mapping: List[int], # **kwargs which cannot be mapped with certainty (non-TypedDict # **kwargs). and not all(actual_kinds[m] == nodes.ARG_STAR2 and - not isinstance(actual_types[m], TypedDictType) + not isinstance(get_proper_type(actual_types[m]), TypedDictType) for m in mapping) )