From d98d0ebe0dba174f307d71e84d281e3db4833b04 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 5 Sep 2017 10:27:34 +0100 Subject: [PATCH 1/3] Infer better type for call to overload with Any actual arguments If the overload return type is ambiguous and there is an `Any` actual argument that seems to cause the ambiguity, infer `Any` as the type of the call expression instead of some precise type. Previously we sometimes inferred a precise type even though the type was ambiguous, resulting in false positives. This is not quite the most correct implementation since we sometimes incorrectly assume ambiguity caused by `Any` even though none actually exists. This is acceptable since this only affects calls with `Any` arguments that are imprecise anyway. Also, the correct logic would be quite complex, hard to test and probably not worth it, at least right now. Fixes #3662. --- mypy/checkexpr.py | 76 ++++++++++++++++- test-data/unit/check-overloading.test | 113 ++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 3 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 41523a95b967b..623b093da4f2c 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -1,7 +1,7 @@ """Expression type checker. This file is conceptually part of TypeChecker.""" from collections import OrderedDict -from typing import cast, Dict, Set, List, Tuple, Callable, Union, Optional +from typing import cast, Dict, Set, List, Tuple, Callable, Union, Optional, Iterable from mypy.errors import report_internal_error from mypy.typeanal import has_any_from_unimported_type, check_for_explicit_any, set_any_tvars @@ -1076,8 +1076,10 @@ def overload_call_target(self, arg_types: List[Type], arg_kinds: List[int], if similarity > 0 and similarity >= best_match: if (match and not is_same_type(match[-1].ret_type, typ.ret_type) and - not mypy.checker.is_more_precise_signature( - match[-1], typ)): + (not mypy.checker.is_more_precise_signature(match[-1], typ) + or (any(isinstance(arg, AnyType) for arg in arg_types) + and any_arg_causes_overload_ambiguity( + match + [typ], arg_types, arg_kinds, arg_names)))): # Ambiguous return type. Either the function overload is # overlapping (which we don't handle very well here) or the # caller has provided some Any argument types; in either @@ -2764,3 +2766,71 @@ def overload_arg_similarity(actual: Type, formal: Type) -> int: return 2 # Fall back to a conservative equality check for the remaining kinds of type. return 2 if is_same_type(erasetype.erase_type(actual), erasetype.erase_type(formal)) else 0 + + +def any_arg_causes_overload_ambiguity(items: List[CallableType], + arg_types: List[Type], + arg_kinds: List[int], + arg_names: List[Optional[str]]) -> bool: + """May an Any actual argument cause ambiguous result type on call to overloaded function? + + Note that this sometimes returns True even if there is no ambiguity, since a correct + implementation would be complex (and the call would be imprecisely typed due to Any + types anyway). + + Args: + items: Overload items matching the actual arguments + arg_types: Actual argument types + arg_kinds: Actual argument kinds + arg_names: Actual argument names + """ + actual_to_formal = [ + map_formals_to_actuals( + arg_kinds, arg_names, item.arg_kinds, item.arg_names, lambda i: arg_types[i]) + for item in items + ] + + for i, arg_type in enumerate(arg_types): + if isinstance(arg_type, AnyType): + matching_formals_unfiltered = [(j, lookup[i]) + for j, lookup in enumerate(actual_to_formal) + if lookup[i]] + matching_formals = [] + for j, formals in matching_formals_unfiltered: + if len(formals) > 1: + # An actual maps to multiple formals -- give up as too + # complex, just assume it overlaps. + return True + matching_formals.append((j, items[j].arg_types[formals[0]])) + if (not all_same_types(t for _, t in matching_formals) and + not all_same_types(items[j].ret_type for j, _ in matching_formals)): + # Any maps to multiple different types, and the return types of these items differ. + return True + return False + + +def all_same_types(types: Iterable[Type]) -> bool: + types = list(types) + if len(types) == 0: + return True + return all(is_same_type(t, types[0]) for t in types[1:]) + + +def map_formals_to_actuals(caller_kinds: List[int], + caller_names: List[Optional[str]], + callee_kinds: List[int], + callee_names: List[Optional[str]], + caller_arg_type: Callable[[int], + Type]) -> List[List[int]]: + """Calculate the reverse mapping of map_actuals_to_formals.""" + formal_to_actual = map_actuals_to_formals(caller_kinds, + caller_names, + callee_kinds, + callee_names, + caller_arg_type) + # Now reverse the mapping. + actual_to_formal = [[] for _ in caller_kinds] # type: List[List[int]] + for formal, actuals in enumerate(formal_to_actual): + for actual in actuals: + actual_to_formal[actual].append(formal) + return actual_to_formal diff --git a/test-data/unit/check-overloading.test b/test-data/unit/check-overloading.test index 0f5060fe3a12a..a3b9c2e103b3a 100644 --- a/test-data/unit/check-overloading.test +++ b/test-data/unit/check-overloading.test @@ -1157,3 +1157,116 @@ class Test(object): t = Test() reveal_type(t.do_chain) # E: Revealed type is '__main__.Chain' + +[case testOverloadWithOverlappingItemsAndAnyArgument1] +from typing import overload, Any + +@overload +def f(x: int) -> int: ... +@overload +def f(x: object) -> object: ... +def f(x): pass + +a: Any +reveal_type(f(a)) # E: Revealed type is 'Any' + +[case testOverloadWithOverlappingItemsAndAnyArgument2] +from typing import overload, Any + +@overload +def f(x: int) -> int: ... +@overload +def f(x: float) -> float: ... +def f(x): pass + +a: Any +reveal_type(f(a)) # E: Revealed type is 'Any' + +[case testOverloadWithOverlappingItemsAndAnyArgument3] +from typing import overload, Any + +@overload +def f(x: int) -> int: ... +@overload +def f(x: str) -> str: ... +def f(x): pass + +a: Any +reveal_type(f(a)) # E: Revealed type is 'Any' + +[case testOverloadWithOverlappingItemsAndAnyArgument4] +from typing import overload, Any + +@overload +def f(x: int, y: int, z: str) -> int: ... +@overload +def f(x: object, y: int, z: str) -> object: ... +def f(x): pass + +a: Any +# Any causes ambiguity +reveal_type(f(a, 1, '')) # E: Revealed type is 'Any' +# Any causes no ambiguity +reveal_type(f(1, a, a)) # E: Revealed type is 'builtins.int' +reveal_type(f('', a, a)) # E: Revealed type is 'builtins.object' +# Like above, but use keyword arguments. +reveal_type(f(y=1, z='', x=a)) # E: Revealed type is 'Any' +reveal_type(f(y=a, z='', x=1)) # E: Revealed type is 'builtins.int' + +[case testOverloadWithOverlappingItemsAndAnyArgument5] +from typing import overload, Any, Union + +@overload +def f(x: int) -> int: ... +@overload +def f(x: Union[int, float]) -> float: ... +def f(x): pass + +a: Any +reveal_type(f(a)) # E: Revealed type is 'Any' + +[case testOverloadWithOverlappingItemsAndAnyArgument6] +from typing import overload, Any + +@overload +def f(x: int, y: int) -> int: ... +@overload +def f(x: float, y: int, z: str) -> float: ... +@overload +def f(x: object, y: int, z: str, a: None) -> object: ... +def f(x): pass + +a: Any +# Any causes ambiguity +reveal_type(f(*a)) # E: Revealed type is 'Any' +reveal_type(f(a, *a)) # E: Revealed type is 'Any' +reveal_type(f(1, *a)) # E: Revealed type is 'Any' +reveal_type(f(1.1, *a)) # E: Revealed type is 'Any' +reveal_type(f('', *a)) # E: Revealed type is 'builtins.object' + +[case testOverloadWithOverlappingItemsAndAnyArgument7] +from typing import overload, Any + +@overload +def f(x: int, y: int, z: int) -> int: ... +@overload +def f(x: object, y: int, z: int) -> object: ... +def f(x): pass + +a: Any +# TODO: We could infer 'int' here +reveal_type(f(1, *a)) # E: Revealed type is 'Any' + +[case testOverloadWithOverlappingItemsAndAnyArgument8] +from typing import overload, Any + +@overload +def f(x: int, y: int, z: int) -> str: ... +@overload +def f(x: object, y: int, z: int) -> str: ... +def f(x): pass + +a: Any +# The return type is not ambiguous so Any arguments cause no ambiguity. +reveal_type(f(a, 1, 1)) # E: Revealed type is 'builtins.str' +reveal_type(f(1, *a)) # E: Revealed type is 'builtins.str' From bc751c26d409b3e8bcef2595c9a10bcd3411ef70 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 5 Sep 2017 18:03:47 +0100 Subject: [PATCH 2/3] Rename `i` and `j` to something more meaningful --- mypy/checkexpr.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 623b093da4f2c..0598ca5918147 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -2790,20 +2790,21 @@ def any_arg_causes_overload_ambiguity(items: List[CallableType], for item in items ] - for i, arg_type in enumerate(arg_types): + for arg_idx, arg_type in enumerate(arg_types): if isinstance(arg_type, AnyType): - matching_formals_unfiltered = [(j, lookup[i]) - for j, lookup in enumerate(actual_to_formal) - if lookup[i]] + matching_formals_unfiltered = [(item_idx, lookup[arg_idx]) + for item_idx, lookup in enumerate(actual_to_formal) + if lookup[arg_idx]] matching_formals = [] - for j, formals in matching_formals_unfiltered: + for item_idx, formals in matching_formals_unfiltered: if len(formals) > 1: # An actual maps to multiple formals -- give up as too # complex, just assume it overlaps. return True - matching_formals.append((j, items[j].arg_types[formals[0]])) + matching_formals.append((item_idx, items[item_idx].arg_types[formals[0]])) if (not all_same_types(t for _, t in matching_formals) and - not all_same_types(items[j].ret_type for j, _ in matching_formals)): + not all_same_types(items[idx].ret_type + for idx, _ in matching_formals)): # Any maps to multiple different types, and the return types of these items differ. return True return False From aec6151a84961e71e2f179c433fe340126121c20 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 5 Sep 2017 18:06:29 +0100 Subject: [PATCH 3/3] Add more argument shuffling --- test-data/unit/check-overloading.test | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test-data/unit/check-overloading.test b/test-data/unit/check-overloading.test index 77b9fe459dee5..74b39fdd9bc38 100644 --- a/test-data/unit/check-overloading.test +++ b/test-data/unit/check-overloading.test @@ -1212,6 +1212,8 @@ reveal_type(f('', a, a)) # E: Revealed type is 'builtins.object' # Like above, but use keyword arguments. reveal_type(f(y=1, z='', x=a)) # E: Revealed type is 'Any' reveal_type(f(y=a, z='', x=1)) # E: Revealed type is 'builtins.int' +reveal_type(f(z='', x=1, y=a)) # E: Revealed type is 'builtins.int' +reveal_type(f(z='', x=a, y=1)) # E: Revealed type is 'Any' [case testOverloadWithOverlappingItemsAndAnyArgument5] from typing import overload, Any, Union