Skip to content
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

Make overload impl checks correctly handle TypeVars and untyped impls #5236

Merged
merged 2 commits into from Jun 25, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+150 −65
Diff settings

Always

Just for now

Copy path View file
@@ -42,6 +42,7 @@
restrict_subtype_away, is_subtype_ignoring_tvars, is_callable_compatible,
unify_generic_callable, find_member
)
from mypy.constraints import SUPERTYPE_OF
from mypy.maptype import map_instance_to_supertype
from mypy.typevars import fill_typevars, has_no_typevars
from mypy.semanal import set_callable_name, refers_to_fullname, calculate_mro
@@ -414,6 +415,23 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
def check_overlapping_overloads(self, defn: OverloadedFuncDef) -> None:
# At this point we should have set the impl already, and all remaining
# items are decorators

# Compute some info about the implementation (if it exists) for use below
impl_type = None # type: Optional[CallableType]
if defn.impl:
if isinstance(defn.impl, FuncDef):
inner_type = defn.impl.type
elif isinstance(defn.impl, Decorator):
inner_type = defn.impl.var.type
else:
assert False, "Impl isn't the right type"

# This can happen if we've got an overload with a different
# decorator or if the implementation is untyped -- we gave up on the types.
if inner_type is not None and not isinstance(inner_type, AnyType):
assert isinstance(inner_type, CallableType)
impl_type = inner_type

is_descriptor_get = defn.info is not None and defn.name() == "__get__"
for i, item in enumerate(defn.items):
# TODO overloads involving decorators
@@ -451,43 +469,36 @@ def check_overlapping_overloads(self, defn: OverloadedFuncDef) -> None:
self.msg.overloaded_signatures_overlap(
i + 1, i + j + 2, item.func)

if defn.impl:
if isinstance(defn.impl, FuncDef):
impl_type = defn.impl.type
elif isinstance(defn.impl, Decorator):
impl_type = defn.impl.var.type
else:
assert False, "Impl isn't the right type"
if impl_type is not None:
assert defn.impl is not None

# This can happen if we've got an overload with a different
# decorator too -- we gave up on the types.
if impl_type is None or isinstance(impl_type, AnyType):
return
assert isinstance(impl_type, CallableType)
# We perform a unification step that's very similar to what
# 'is_callable_compatible' would have done if we had set
# 'unify_generics' to True -- the only difference is that
# we check and see if the impl_type's return value is a
# *supertype* of the overload alternative, not a *subtype*.
#
# This is to match the direction the implementation's return
# needs to be compatible in.
if impl_type.variables:
impl = unify_generic_callable(impl_type, sig1,
ignore_return=False,
return_constraint_direction=SUPERTYPE_OF)
if impl is None:
self.msg.overloaded_signatures_typevar_specific(i + 1, defn.impl)
continue
else:
impl = impl_type

# Is the overload alternative's arguments subtypes of the implementation's?
if not is_callable_compatible(impl_type, sig1,
if not is_callable_compatible(impl, sig1,
is_compat=is_subtype,
ignore_return=True):
ignore_return=True,
unify_generics=False):

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Jun 25, 2018

Collaborator

This function has already too many arguments. Is there a way to avoid unify_generics? Like if you already unified above, the unification in is_callable_compatible will be a no-op?

This comment has been minimized.

Copy link
@Michael0x2a

Michael0x2a Jun 25, 2018

Author Collaborator

Huh, that's weird. I could have sworn we needed this parameter, but I tried removing it and everything still worked. Maybe it was a holdover from one of my previous attempts at tackling the bug?

Anyways, I updated the PR to remove this param. Regarding the number of arguments: I'm planning on refactoring and splitting up is_callable_compatible in the upcoming PR for improving error messages -- that should help us cut down on the number of params/ad-hoc special casing.

self.msg.overloaded_signatures_arg_specific(i + 1, defn.impl)

# Repeat the same unification process 'is_callable_compatible'
# internally performs so we can examine the return type separately.
if impl_type.variables:
# Note: we set 'ignore_return=True' because 'unify_generic_callable'
# normally checks the arguments and return types with differing variance.
#
# This is normally what we want, but for checking the validity of overload
# implementations, we actually want to use the same variance for both.
#
# TODO: Patch 'is_callable_compatible' and 'unify_generic_callable'?
# somehow so we can customize the variance in all different sorts
# of ways? This would let us infer more constraints, letting us
# infer more precise types.
impl_type = unify_generic_callable(impl_type, sig1, ignore_return=True)

# Is the overload alternative's return type a subtype of the implementation's?
if impl_type is not None and not is_subtype(sig1.ret_type, impl_type.ret_type):
if not is_subtype(sig1.ret_type, impl.ret_type):
self.msg.overloaded_signatures_ret_specific(i + 1, defn.impl)

# Here's the scoop about generators and coroutines.
Copy path View file
@@ -961,13 +961,17 @@ def overloaded_signature_will_never_match(self, index1: int, index2: int,
index2=index2),
context)

def overloaded_signatures_arg_specific(self, index1: int, context: Context) -> None:
def overloaded_signatures_typevar_specific(self, index: int, context: Context) -> None:
self.fail('Overloaded function implementation cannot satisfy signature {} '.format(index) +
'due to inconsistencies in how they use type variables', context)

def overloaded_signatures_arg_specific(self, index: int, context: Context) -> None:
self.fail('Overloaded function implementation does not accept all possible arguments '
'of signature {}'.format(index1), context)
'of signature {}'.format(index), context)

def overloaded_signatures_ret_specific(self, index1: int, context: Context) -> None:
def overloaded_signatures_ret_specific(self, index: int, context: Context) -> None:
self.fail('Overloaded function implementation cannot produce return type '
'of signature {}'.format(index1), context)
'of signature {}'.format(index), context)

def operator_method_signatures_overlap(
self, reverse_class: TypeInfo, reverse_method: str, forward_class: Type,
Copy path View file
@@ -198,6 +198,8 @@ def visit_type_var(self, left: TypeVarType) -> bool:
right = self.right
if isinstance(right, TypeVarType) and left.id == right.id:
return True
if left.values and is_subtype(UnionType.make_simplified_union(left.values), right):
return True
return is_subtype(left.upper_bound, self.right)

def visit_callable_type(self, left: CallableType) -> bool:
@@ -578,7 +580,8 @@ def is_callable_compatible(left: CallableType, right: CallableType,
ignore_return: bool = False,
ignore_pos_arg_names: bool = False,
check_args_covariantly: bool = False,
allow_partial_overlap: bool = False) -> bool:
allow_partial_overlap: bool = False,
unify_generics: bool = True) -> bool:
"""Is the left compatible with the right, using the provided compatibility check?
is_compat:
@@ -666,6 +669,11 @@ def g(x: int) -> int: ...
If the 'some_check' function is also symmetric, the two calls would be equivalent
whether or not we check the args covariantly.
unify_generics:
If this parameter is set to False, we do not attempt to unify away TypeVars before
checking the callable. This option should be set to False only if you want to externally
handle generics in some custom way.
"""
if is_compat_return is None:
is_compat_return = is_compat
@@ -678,34 +686,35 @@ def g(x: int) -> int: ...
if right.is_type_obj() and not left.is_type_obj():
return False

# A callable L is a subtype of a generic callable R if L is a
# subtype of every type obtained from R by substituting types for
# the variables of R. We can check this by simply leaving the
# generic variables of R as type variables, effectively varying
# over all possible values.

# It's okay even if these variables share ids with generic
# type variables of L, because generating and solving
# constraints for the variables of L to make L a subtype of R
# (below) treats type variables on the two sides as independent.
if left.variables:
# Apply generic type variables away in left via type inference.
unified = unify_generic_callable(left, right, ignore_return=ignore_return)
if unified is None:
return False
else:
left = unified
if unify_generics:
# A callable L is a subtype of a generic callable R if L is a
# subtype of every type obtained from R by substituting types for
# the variables of R. We can check this by simply leaving the
# generic variables of R as type variables, effectively varying
# over all possible values.

# It's okay even if these variables share ids with generic
# type variables of L, because generating and solving
# constraints for the variables of L to make L a subtype of R
# (below) treats type variables on the two sides as independent.
if left.variables:
# Apply generic type variables away in left via type inference.
unified = unify_generic_callable(left, right, ignore_return=ignore_return)
if unified is None:
return False
else:
left = unified

# If we allow partial overlaps, we don't need to leave R generic:
# if we can find even just a single typevar assignment which
# would make these callables compatible, we should return True.
# If we allow partial overlaps, we don't need to leave R generic:
# if we can find even just a single typevar assignment which
# would make these callables compatible, we should return True.

# So, we repeat the above checks in the opposite direction. This also
# lets us preserve the 'symmetry' property of allow_partial_overlap.
if allow_partial_overlap and right.variables:
unified = unify_generic_callable(right, left, ignore_return=ignore_return)
if unified is not None:
right = unified
# So, we repeat the above checks in the opposite direction. This also
# lets us preserve the 'symmetry' property of allow_partial_overlap.
if allow_partial_overlap and right.variables:
unified = unify_generic_callable(right, left, ignore_return=ignore_return)
if unified is not None:
right = unified

# Check return types.
if not ignore_return and not is_compat_return(left.ret_type, right.ret_type):
@@ -901,7 +910,9 @@ def new_is_compat(left: Type, right: Type) -> bool:


def unify_generic_callable(type: CallableType, target: CallableType,
ignore_return: bool) -> Optional[CallableType]:
ignore_return: bool,
return_constraint_direction: int = mypy.constraints.SUBTYPE_OF,
) -> Optional[CallableType]:
"""Try to unify a generic callable type with another callable type.
Return unified CallableType if successful; otherwise, return None.
@@ -914,7 +925,7 @@ def unify_generic_callable(type: CallableType, target: CallableType,
constraints.extend(c)
if not ignore_return:
c = mypy.constraints.infer_constraints(
type.ret_type, target.ret_type, mypy.constraints.SUBTYPE_OF)
type.ret_type, target.ret_type, return_constraint_direction)
constraints.extend(c)
type_var_ids = [tvar.id for tvar in type.variables]
inferred_vars = mypy.solve.solve_constraints(type_var_ids, constraints)
@@ -1036,7 +1047,8 @@ def check_argument(leftarg: Type, rightarg: Type, variance: int) -> bool:
def visit_type_var(self, left: TypeVarType) -> bool:
if isinstance(self.right, TypeVarType) and left.id == self.right.id:
return True
# TODO: Value restrictions
if left.values and is_subtype(UnionType.make_simplified_union(left.values), self.right):
return True
return is_proper_subtype(left.upper_bound, self.right)

def visit_callable_type(self, left: CallableType) -> bool:
@@ -204,6 +204,17 @@ class A: pass
class B: pass
[builtins fixtures/isinstance.pyi]

[case testTypeCheckOverloadWithUntypedImplAndMultipleVariants]
from typing import overload

@overload
def f(x: int) -> str: ...
@overload
def f(x: str) -> int: ... # E: Overloaded function signatures 2 and 3 overlap with incompatible return types
@overload
def f(x: object) -> str: ...
def f(x): ...

[case testTypeCheckOverloadWithImplTooSpecificArg]
from typing import overload, Any

@@ -284,14 +295,61 @@ def f(x: 'A') -> 'A': ...
@overload
def f(x: 'B') -> 'B': ...

def f(x: Union[T, B]) -> T: # E: Overloaded function implementation cannot produce return type of signature 2
def f(x: Union[T, B]) -> T: # E: Overloaded function implementation cannot satisfy signature 2 due to inconsistencies in how they use type variables
...

reveal_type(f(A())) # E: Revealed type is '__main__.A'
reveal_type(f(B())) # E: Revealed type is '__main__.B'

[builtins fixtures/isinstance.pyi]

[case testTypeCheckOverloadImplementationTypeVarWithValueRestriction]
from typing import overload, TypeVar, Union

class A: pass
class B: pass
class C: pass

T = TypeVar('T', A, B)

@overload
def foo(x: T) -> T: ...
@overload
def foo(x: C) -> int: ...
def foo(x: Union[A, B, C]) -> Union[A, B, int]:
if isinstance(x, C):
return 3
else:
return x

@overload
def bar(x: T) -> T: ...
@overload
def bar(x: C) -> int: ...
def bar(x: Union[T, C]) -> Union[T, int]:
if isinstance(x, C):
return 3
else:
return x

[builtins fixtures/isinstancelist.pyi]

[case testTypeCheckOverloadImplementationTypeVarDifferingUsage]
from typing import overload, Union, List, TypeVar

T = TypeVar('T')

@overload
def foo(t: List[T]) -> T: ...
@overload
def foo(t: T) -> T: ...
def foo(t: Union[List[T], T]) -> T:
if isinstance(t, list):
return t[0]
else:
return t
[builtins fixtures/isinstancelist.pyi]

[case testTypeCheckOverloadedFunctionBody]
from foo import *
[file foo.pyi]
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.