From 5a2f5c64a7ca34686e6caebf5bea9f4a34ff393c Mon Sep 17 00:00:00 2001 From: Reid Barton Date: Fri, 13 May 2016 10:09:27 -0700 Subject: [PATCH] Allow any more general type for override of a method (#1524) This is a prerequisite for fixing #1261 because overrides with generic function type variables currently work by accident: the type variables have the same ids in the original method and the overriding method, so is_subtype on the argument type returns True. This also allows an overriding method to generalize a specific type in the original method to a type variable, as demonstrated in one of the tests (testOverrideGenericMethodInNonGenericClassGeneralize). is_subtype already determines correctly whether the override is valid, so all the hard work here is in providing a more specific error message when possible. --- mypy/checker.py | 82 ++++++++++++--------- mypy/erasetype.py | 15 +++- mypy/test/data/check-generic-subtyping.test | 38 +++++++++- 3 files changed, 96 insertions(+), 39 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 5d872ec5cf8b..782e058158a1 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -988,43 +988,55 @@ def check_override(self, override: FunctionLike, original: FunctionLike, only used for generating error messages. supertype: The name of the supertype. """ - if (isinstance(override, Overloaded) or - isinstance(original, Overloaded) or - len(cast(CallableType, override).arg_types) != - len(cast(CallableType, original).arg_types) or - cast(CallableType, override).min_args != - cast(CallableType, original).min_args): - # Use boolean variable to clarify code. - fail = False - if not is_subtype(override, original): - fail = True - elif (not isinstance(original, Overloaded) and - isinstance(override, Overloaded) and - name in nodes.reverse_op_methods.keys()): - # Operator method overrides cannot introduce overloading, as - # this could be unsafe with reverse operator methods. - fail = True - if fail: + # Use boolean variable to clarify code. + fail = False + if not is_subtype(override, original): + fail = True + elif (not isinstance(original, Overloaded) and + isinstance(override, Overloaded) and + name in nodes.reverse_op_methods.keys()): + # Operator method overrides cannot introduce overloading, as + # this could be unsafe with reverse operator methods. + fail = True + + if fail: + emitted_msg = False + if (isinstance(override, CallableType) and + isinstance(original, CallableType) and + len(override.arg_types) == len(original.arg_types) and + override.min_args == original.min_args): + # Give more detailed messages for the common case of both + # signatures having the same number of arguments and no + # overloads. + + # override might have its own generic function type + # variables. If an argument or return type of override + # does not have the correct subtyping relationship + # with the original type even after these variables + # are erased, then it is definitely an incompatiblity. + + override_ids = override.type_var_ids() + + def erase_override(t: Type) -> Type: + return erase_typevars(t, ids_to_erase=override_ids) + + for i in range(len(override.arg_types)): + if not is_subtype(original.arg_types[i], + erase_override(override.arg_types[i])): + self.msg.argument_incompatible_with_supertype( + i + 1, name, name_in_super, supertype, node) + emitted_msg = True + + if not is_subtype(erase_override(override.ret_type), + original.ret_type): + self.msg.return_type_incompatible_with_supertype( + name, name_in_super, supertype, node) + emitted_msg = True + + if not emitted_msg: + # Fall back to generic incompatibility message. self.msg.signature_incompatible_with_supertype( name, name_in_super, supertype, node) - return - else: - # Give more detailed messages for the common case of both - # signatures having the same number of arguments and no - # overloads. - - coverride = cast(CallableType, override) - coriginal = cast(CallableType, original) - - for i in range(len(coverride.arg_types)): - if not is_subtype(coriginal.arg_types[i], - coverride.arg_types[i]): - self.msg.argument_incompatible_with_supertype( - i + 1, name, name_in_super, supertype, node) - - if not is_subtype(coverride.ret_type, coriginal.ret_type): - self.msg.return_type_incompatible_with_supertype( - name, name_in_super, supertype, node) def visit_class_def(self, defn: ClassDef) -> Type: """Type check a class definition.""" diff --git a/mypy/erasetype.py b/mypy/erasetype.py index 15ccd4f9bb7e..be99acabc27b 100644 --- a/mypy/erasetype.py +++ b/mypy/erasetype.py @@ -1,3 +1,5 @@ +from typing import Optional, Container + from mypy.types import ( Type, TypeVisitor, UnboundType, ErrorType, AnyType, Void, NoneTyp, Instance, TypeVarType, CallableType, TupleType, UnionType, Overloaded, ErasedType, @@ -96,13 +98,20 @@ def visit_instance(self, t: Instance) -> Type: return Instance(t.type, [], t.line) -def erase_typevars(t: Type) -> Type: - """Replace all type variables in a type with any.""" - return t.accept(TypeVarEraser()) +def erase_typevars(t: Type, ids_to_erase: Optional[Container[int]] = None) -> Type: + """Replace all type variables in a type with any, + or just the ones in the provided collection. + """ + return t.accept(TypeVarEraser(ids_to_erase)) class TypeVarEraser(TypeTranslator): """Implementation of type erasure""" + def __init__(self, ids_to_erase: Optional[Container[int]]) -> None: + self.ids_to_erase = ids_to_erase + def visit_type_var(self, t: TypeVarType) -> Type: + if self.ids_to_erase is not None and t.id not in self.ids_to_erase: + return t return AnyType() diff --git a/mypy/test/data/check-generic-subtyping.test b/mypy/test/data/check-generic-subtyping.test index 8c37a92374f2..e5c4a86352ca 100644 --- a/mypy/test/data/check-generic-subtyping.test +++ b/mypy/test/data/check-generic-subtyping.test @@ -235,9 +235,45 @@ class A: class B(A): def f(self, x: S, y: T) -> None: pass class C(A): - def f(self, x: T, y: T) -> None: pass # E: Argument 2 of "f" incompatible with supertype "A" + # Okay, because T = object allows any type for the arguments. + def f(self, x: T, y: T) -> None: pass + +[case testOverrideGenericMethodInNonGenericClassLists] +from typing import TypeVar, List + +T = TypeVar('T') +S = TypeVar('S') + +class A: + def f(self, x: List[T], y: List[S]) -> None: pass +class B(A): + def f(self, x: List[S], y: List[T]) -> None: pass +class C(A): + def f(self, x: List[T], y: List[T]) -> None: pass # E: Signature of "f" incompatible with supertype "A" +[builtins fixtures/list.py] +[out] +main: note: In class "C": + +[case testOverrideGenericMethodInNonGenericClassGeneralize] +from typing import TypeVar + +T = TypeVar('T') +T1 = TypeVar('T1', bound=str) +S = TypeVar('S') + +class A: + def f(self, x: int, y: S) -> None: pass +class B(A): + def f(self, x: T, y: S) -> None: pass +class C(A): + def f(self, x: T, y: str) -> None: pass +class D(A): + def f(self, x: T1, y: S) -> None: pass # TODO: This error could be more specific. [out] main: note: In class "C": +main:12: error: Argument 2 of "f" incompatible with supertype "A" +main: note: In class "D": +main:14: error: Signature of "f" incompatible with supertype "A" -- Inheritance from generic types with implicit dynamic supertype