From 64bd8fcb72fa28e05103092c386e9679842822a8 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 24 Apr 2019 18:49:25 +0100 Subject: [PATCH 1/6] Naive fix for generic overrides in multiple inheritance --- mypy/checker.py | 41 ++++++++++++++----- .../unit/check-multiple-inheritance.test | 16 ++++++++ 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 85a2759185813..87f283a048174 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1387,16 +1387,12 @@ def check_method_override_for_base_with_name( if isinstance(original_type, AnyType) or isinstance(typ, AnyType): pass elif isinstance(original_type, FunctionLike) and isinstance(typ, FunctionLike): - if (isinstance(base_attr.node, (FuncDef, OverloadedFuncDef, Decorator)) - and not is_static(base_attr.node)): - bound = bind_self(original_type, self.scope.active_self_type()) - else: - bound = original_type - original = map_type_from_supertype(bound, defn.info, base) + original = self.bind_and_map_method(base_attr, original_type, + defn.info, base) # Check that the types are compatible. # TODO overloaded signatures self.check_override(typ, - cast(FunctionLike, original), + original, defn.name(), name, base.name(), @@ -1415,6 +1411,23 @@ def check_method_override_for_base_with_name( defn.name(), name, base.name(), context) return False + def bind_and_map_method(self, sym: SymbolTableNode, typ: FunctionLike, + sub_info: TypeInfo, super_info: TypeInfo) -> FunctionLike: + """Bind self-type and map type variables for a method. + + Arguments: + sym: a symbol that points to method definition + typ: method type on the definition + sub_info: class where the method is mapped to + super_info: class where the method was defined + """ + if (isinstance(sym.node, (FuncDef, OverloadedFuncDef, Decorator)) + and not is_static(sym.node)): + bound = bind_self(typ, self.scope.active_self_type()) + else: + bound = typ + return cast(FunctionLike, map_type_from_supertype(bound, sub_info, super_info)) + def get_op_other_domain(self, tp: FunctionLike) -> Optional[Type]: if isinstance(tp, CallableType): if tp.arg_kinds and tp.arg_kinds[0] == ARG_POS: @@ -1643,7 +1656,6 @@ def check_compatibility(self, name: str, base1: TypeInfo, first_type = self.determine_type_of_class_member(first) second_type = self.determine_type_of_class_member(second) - # TODO: What if some classes are generic? if (isinstance(first_type, FunctionLike) and isinstance(second_type, FunctionLike)): if first_type.is_type_obj() and second_type.is_type_obj(): @@ -1652,8 +1664,17 @@ def check_compatibility(self, name: str, base1: TypeInfo, ok = is_subtype(left=fill_typevars_with_any(first_type.type_object()), right=fill_typevars_with_any(second_type.type_object())) else: - first_sig = bind_self(first_type) - second_sig = bind_self(second_type) + # First bind/map method types when necessary. + if isinstance(first.node, (FuncDef, OverloadedFuncDef, Decorator)): + first_sig = self.bind_and_map_method(first, first_type, + base1, first.node.info) + else: + first_sig = first_type + if isinstance(first.node, (FuncDef, OverloadedFuncDef, Decorator)): + second_sig = self.bind_and_map_method(second, second_type, + base2, second.node.info) + else: + second_sig = second_type ok = is_subtype(first_sig, second_sig, ignore_pos_arg_names=True) elif first_type and second_type: ok = is_equivalent(first_type, second_type) diff --git a/test-data/unit/check-multiple-inheritance.test b/test-data/unit/check-multiple-inheritance.test index d87b4a5c48193..310a01488a0cf 100644 --- a/test-data/unit/check-multiple-inheritance.test +++ b/test-data/unit/check-multiple-inheritance.test @@ -624,3 +624,19 @@ class Mixin2: class A(Mixin1, Mixin2): pass [out] + +[case testGenericMultipleOverride] +from typing import TypeVar, Generic, Tuple + +K = TypeVar('K') +V = TypeVar('V') +T = TypeVar('T') + +class ItemsView(Generic[K, V]): + def __iter__(self) -> Tuple[K, V]: ... + +class Sequence(Generic[T]): + def __iter__(self) -> T: ... + +class OrderedItemsView(ItemsView[K, V], Sequence[Tuple[K, V]]): + def __iter__(self) -> Tuple[K, V]: ... From a6f0427985b9c57e63f8c35456ca056e1dab0034 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 24 Apr 2019 23:01:20 +0100 Subject: [PATCH 2/6] Map to the class where the method is used --- mypy/checker.py | 8 ++++---- test-data/unit/pythoneval.test | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 87f283a048174..877cde76c3ad0 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1418,7 +1418,7 @@ def bind_and_map_method(self, sym: SymbolTableNode, typ: FunctionLike, Arguments: sym: a symbol that points to method definition typ: method type on the definition - sub_info: class where the method is mapped to + sub_info: class where the method is used super_info: class where the method was defined """ if (isinstance(sym.node, (FuncDef, OverloadedFuncDef, Decorator)) @@ -1641,7 +1641,7 @@ def determine_type_of_class_member(self, sym: SymbolTableNode) -> Optional[Type] return None def check_compatibility(self, name: str, base1: TypeInfo, - base2: TypeInfo, ctx: Context) -> None: + base2: TypeInfo, ctx: TypeInfo) -> None: """Check if attribute name in base1 is compatible with base2 in multiple inheritance. Assume base1 comes before base2 in the MRO, and that base1 and base2 don't have @@ -1667,12 +1667,12 @@ def check_compatibility(self, name: str, base1: TypeInfo, # First bind/map method types when necessary. if isinstance(first.node, (FuncDef, OverloadedFuncDef, Decorator)): first_sig = self.bind_and_map_method(first, first_type, - base1, first.node.info) + ctx, first.node.info) else: first_sig = first_type if isinstance(first.node, (FuncDef, OverloadedFuncDef, Decorator)): second_sig = self.bind_and_map_method(second, second_type, - base2, second.node.info) + ctx, second.node.info) else: second_sig = second_type ok = is_subtype(first_sig, second_sig, ignore_pos_arg_names=True) diff --git a/test-data/unit/pythoneval.test b/test-data/unit/pythoneval.test index 3c7df911ab4e9..a5c6dc4d1d98e 100644 --- a/test-data/unit/pythoneval.test +++ b/test-data/unit/pythoneval.test @@ -1366,3 +1366,18 @@ x: Dict[str, List[int]] reveal_type(x['test'][0]) [out] _testNewAnalyzerBasicTypeshed_newsemanal.py:4: error: Revealed type is 'builtins.int*' + +[case testGenericMultipleOverrideDict] +from typing import Any + +class JSONDict(dict): + pass + +class WithChallenge(object): + def __getitem__(self, key: str) -> Any: ... + +class WithKeyHandle(object): + def __getitem__(self, key: str) -> Any: ... + +class RegisteredKey(JSONDict, WithChallenge, WithKeyHandle): + pass From b90c35f978a9dbbf86086f24dbff0120a1a0667d Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 24 Apr 2019 23:15:15 +0100 Subject: [PATCH 3/6] Move a test and make it more generic --- .../unit/check-multiple-inheritance.test | 27 ++++++++++++++++++- test-data/unit/pythoneval.test | 15 ----------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/test-data/unit/check-multiple-inheritance.test b/test-data/unit/check-multiple-inheritance.test index 310a01488a0cf..dc69a4187cead 100644 --- a/test-data/unit/check-multiple-inheritance.test +++ b/test-data/unit/check-multiple-inheritance.test @@ -625,7 +625,7 @@ class A(Mixin1, Mixin2): pass [out] -[case testGenericMultipleOverride] +[case testGenericMultipleOverrideRemap] from typing import TypeVar, Generic, Tuple K = TypeVar('K') @@ -638,5 +638,30 @@ class ItemsView(Generic[K, V]): class Sequence(Generic[T]): def __iter__(self) -> T: ... +# Override compatible between bases. class OrderedItemsView(ItemsView[K, V], Sequence[Tuple[K, V]]): def __iter__(self) -> Tuple[K, V]: ... + +class OrderedItemsViewDirect(ItemsView[K, V], Sequence[Tuple[K, V]]): + pass + +[case testGenericMultipleOverrideReplace] +from typing import TypeVar, Generic, Union + +T = TypeVar('T') + +class A(Generic[T]): + def foo(self, x: T) -> None: ... + +class B(A[T]): ... + +class C1: + def foo(self, x: str) -> None: ... + +class C2: + def foo(self, x: Union[str, int]) -> None: ... + +class D1(B[str], C1): ... +class D2(B[Union[int, str]], C2): ... +class D3(C2, B[str]): ... +class D4(B[str], C2): ... # E: Definition of "foo" in base class "A" is incompatible with definition in base class "C2" diff --git a/test-data/unit/pythoneval.test b/test-data/unit/pythoneval.test index a5c6dc4d1d98e..3c7df911ab4e9 100644 --- a/test-data/unit/pythoneval.test +++ b/test-data/unit/pythoneval.test @@ -1366,18 +1366,3 @@ x: Dict[str, List[int]] reveal_type(x['test'][0]) [out] _testNewAnalyzerBasicTypeshed_newsemanal.py:4: error: Revealed type is 'builtins.int*' - -[case testGenericMultipleOverrideDict] -from typing import Any - -class JSONDict(dict): - pass - -class WithChallenge(object): - def __getitem__(self, key: str) -> Any: ... - -class WithKeyHandle(object): - def __getitem__(self, key: str) -> Any: ... - -class RegisteredKey(JSONDict, WithChallenge, WithKeyHandle): - pass From fa7085899670afe62f9352070a7dc81bb648d3b1 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 24 Apr 2019 23:20:43 +0100 Subject: [PATCH 4/6] Simplify code --- mypy/checker.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 877cde76c3ad0..2e4fa7b30a8cb 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1651,8 +1651,8 @@ def check_compatibility(self, name: str, base1: TypeInfo, if name in ('__init__', '__new__', '__init_subclass__'): # __init__ and friends can be incompatible -- it's a special case. return - first = base1[name] - second = base2[name] + first = base1.names[name] + second = base2.names[name] first_type = self.determine_type_of_class_member(first) second_type = self.determine_type_of_class_member(second) @@ -1665,16 +1665,8 @@ def check_compatibility(self, name: str, base1: TypeInfo, right=fill_typevars_with_any(second_type.type_object())) else: # First bind/map method types when necessary. - if isinstance(first.node, (FuncDef, OverloadedFuncDef, Decorator)): - first_sig = self.bind_and_map_method(first, first_type, - ctx, first.node.info) - else: - first_sig = first_type - if isinstance(first.node, (FuncDef, OverloadedFuncDef, Decorator)): - second_sig = self.bind_and_map_method(second, second_type, - ctx, second.node.info) - else: - second_sig = second_type + first_sig = self.bind_and_map_method(first, first_type, ctx, base1) + second_sig = self.bind_and_map_method(second, second_type, ctx, base2) ok = is_subtype(first_sig, second_sig, ignore_pos_arg_names=True) elif first_type and second_type: ok = is_equivalent(first_type, second_type) From fe2b1d8cba5a0e3e591230d95c61722802e0edcf Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 24 Apr 2019 23:32:52 +0100 Subject: [PATCH 5/6] Add a docstring --- mypy/checker.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/mypy/checker.py b/mypy/checker.py index 2e4fa7b30a8cb..51ab8733b4557 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1647,6 +1647,20 @@ def check_compatibility(self, name: str, base1: TypeInfo, Assume base1 comes before base2 in the MRO, and that base1 and base2 don't have a direct subclass relationship (i.e., the compatibility requirement only derives from multiple inheritance). + + This checks verifies that a definition taken from base1 (and mapped to the current + class ctx), is type compatible with the definition taken from base2 (also mapped), so + that unsafe subclassing like this can be detected: + class A(Generic[T]): + def foo(self, x: T) -> None: ... + + class B: + def foo(self, x: str) -> None: ... + + class C(B, A[int]): ... # this is unsafe because... + + x: A[int] = C() + x.foo # ...runtime type is (str) -> None, while static type is (int) -> None """ if name in ('__init__', '__new__', '__init_subclass__'): # __init__ and friends can be incompatible -- it's a special case. From 14620f5bde44cb6cd6a309569a6f5e5992ce05f2 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 25 Apr 2019 11:54:26 -0700 Subject: [PATCH 6/6] Fix typo --- mypy/checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index 51ab8733b4557..51137b7ad5fb6 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1648,7 +1648,7 @@ def check_compatibility(self, name: str, base1: TypeInfo, a direct subclass relationship (i.e., the compatibility requirement only derives from multiple inheritance). - This checks verifies that a definition taken from base1 (and mapped to the current + This check verifies that a definition taken from base1 (and mapped to the current class ctx), is type compatible with the definition taken from base2 (also mapped), so that unsafe subclassing like this can be detected: class A(Generic[T]):