Repository: python/mypy
Pull request: Loosen base class attribute compatibility checks when attribute is redefined
This eliminates the error `Definition of "Nested" in base class "Base1" is incompatible with definition in base class "Base2"` in the following code, where `Nested` is redefined in `A`:
```
from typing import TypeVar, Generic
T = TypeVar('T', covariant=True)
class GenericBase(Generic[T]):
    pass
class Base1:
    Nested: GenericBase['Base1']
class Base2:
    Nested: GenericBase['Base2']
class A(Base1, Base2):
    Nested: GenericBase['A']
```

- In the case of multiple inheritance, don't give errors about definitions of an
  attribute in base classes being incompatible when the attribute is redefined.
  The redefinition must itself be compatible with all (non-type-ignored)
  definitions of the attribute in all base classes.
  This is achieved by making the following change to checking of incompatible
  types in assignments.
- Don't stop checking after the first base where the attribute is defined
  when checking for incompatible types in assignments.
  There is still a maximum of one "Incompatible type in assignment" error
  per assignment.

Resolves #2619

Problem statement:
Base class function signature compatibility checks overly strict
Actual example of this code can be found in Python 3.6's enum with `Flag` and `IntFlag`.

**Minimal repro:**:
```py3
from typing import TypeVar

_T = TypeVar('_T')


class Base:
    def __or__(self: _T, other: _T) -> _T: ...
    def __and__(self: _T, other: _T) -> _T: ...
    def __xor__(self: _T, other: _T) -> _T: ...


class Other(int, Base):  # <-- errors here
    def __or__(self: _T, other: Union[int, _T]) -> _T: ...  # type: ignore
    def __and__(self: _T, other: Union[int, _T]) -> _T: ...  # type: ignore
    def __xor__(self: _T, other: Union[int, _T]) -> _T: ...  # type: ignore
```

**Expected behavior**: no errors.
**Actual behavior**:
```
f:11: error: Definition of "__and__" in base class "int" is incompatible with definition in base class "Base"
f:11: error: Definition of "__or__" in base class "int" is incompatible with definition in base class "Base"
f:11: error: Definition of "__xor__" in base class "int" is incompatible with definition in base class "Base"
```

The errors would be fine if `Other` didn't redefine the methods itself. In this case the incompatibility in base classes is irrelevant.

Hints from issue discussion:
Keywords to help searching: self type, union type
Just ran into this at Dropbox.

Slightly more distilled repro:
```python
class A(object):
    def x(self):
        # type: () -> A
        return self

class B(object):
    def x(self):
        # type: () -> B
        return self

class C(A, B):
    def x(self):
        # type: () -> C
        return self

test.py:11: error: Definition of "x" in base class "A" is incompatible with definition in base class "B"
```

And equivalent compilable Java code:
```java
interface A {
    A x();
}
interface B {
    B x();
}
class C implements A, B {
    public C x() {
        return this;
    }
}
```

I presume mypy already has checks that C.x is compatible with A.x and B.x. Is it possible to resolve this issue by doing something along the lines of removing the check that A.x and B.x are compatible in the case where x is overridden? (Note, I don't really know what I'm talking about.)

Commit message: Loosen base class attribute compatibility checks when attribute is redefined

- In the case of multiple inheritance, don't give errors about definitions of an
  attribute in base classes being incompatible when the attribute is redefined.
  The redefinition must itself be compatible with all (non-type-ignored)
  definitions of the attribute in all base classes.
  This is achieved by making the following change to checking of incompatible
  types in assignments.
- Don't stop checking after the first base where the attribute is defined
  when checking for incompatible types in assignments.
  There is still a maximum of one "Incompatible type in assignment" error
  per assignment.

Resolves #2619

Patch to review:
diff --git a/mypy/checker.py b/mypy/checker.py
index b721c8690fc7..94379901d051 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -1602,7 +1602,10 @@ def check_multiple_inheritance(self, typ: TypeInfo) -> None:
         # Verify that inherited attributes are compatible.
         mro = typ.mro[1:]
         for i, base in enumerate(mro):
-            for name in base.names:
+            # Attributes defined in both the type and base are skipped.
+            # Normal checks for attribute compatibility should catch any problems elsewhere.
+            non_overridden_attrs = base.names.keys() - typ.names.keys()
+            for name in non_overridden_attrs:
                 for base2 in mro[i + 1:]:
                     # We only need to check compatibility of attributes from classes not
                     # in a subclass relationship. For subclasses, normal (single inheritance)
@@ -1890,7 +1893,6 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, lvalue_type: Optional[
                         # Only show one error per variable; even if other
                         # base classes are also incompatible
                         return True
-                    break
         return False
 
     def check_compatibility_super(self, lvalue: RefExpr, lvalue_type: Optional[Type],
diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test
index ccc683088cde..0620004c9a56 100644
--- a/test-data/unit/check-classes.test
+++ b/test-data/unit/check-classes.test
@@ -3725,6 +3725,7 @@ class C(B):
     a = "a"
 [out]
 main:4: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
+main:6: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
 
 [case testVariableSubclassTypeOverwriteImplicit]
 class A:
@@ -4009,7 +4010,7 @@ class B(A):
 main:4: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
 main:5: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
 
-[case testClassIgnoreType]
+[case testClassIgnoreType_RedefinedAttributeAndGrandparentAttributeTypesNotIgnored]
 class A:
     x = 0
 class B(A):
@@ -4017,6 +4018,17 @@ class B(A):
 class C(B):
     x = ''
 [out]
+main:6: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
+
+
+[case testClassIgnoreType_RedefinedAttributeTypeIgnoredInChildren]
+class A:
+    x = 0
+class B(A):
+    x = ''  # type: ignore
+class C(B):
+    x = ''  # type: ignore
+[out]
 
 [case testInvalidMetaclassStructure]
 class X(type): pass
diff --git a/test-data/unit/check-multiple-inheritance.test b/test-data/unit/check-multiple-inheritance.test
index 1bdc70c19fa1..a0b367918dc1 100644
--- a/test-data/unit/check-multiple-inheritance.test
+++ b/test-data/unit/check-multiple-inheritance.test
@@ -228,7 +228,7 @@ class B(A, int): pass
 from typing import Callable, TypeVar
 T = TypeVar('T')
 class A(B, C):
-    def f(self): pass
+    pass
 class B:
     @dec
     def f(self): pass
@@ -461,6 +461,128 @@ class Combo(Base2, Base1): ...
 [out]
 main:10: error: Definition of "NestedVar" in base class "Base2" is incompatible with definition in base class "Base1"
 
+[case testMultipleInheritance_NestedVariableOverriddenWithCompatibleType]
+
+from typing import TypeVar, Generic
+T = TypeVar('T', covariant=True)
+class GenericBase(Generic[T]):
+    pass
+class Base1:
+    Nested: GenericBase['Base1']
+class Base2:
+    Nested: GenericBase['Base2']
+class A(Base1, Base2):
+    Nested: GenericBase['A']
+[out]
+
+[case testMultipleInheritance_NestedVariableOverriddenWithIncompatibleType1]
+
+from typing import TypeVar, Generic
+T = TypeVar('T', covariant=True)
+class GenericBase(Generic[T]):
+    pass
+class Base1:
+    Nested: GenericBase['Base1']
+class Base2:
+    Nested: GenericBase['Base2']
+class A(Base1, Base2):
+    Nested: GenericBase['Base1']
+[out]
+main:11: error: Incompatible types in assignment (expression has type "GenericBase[Base1]", base class "Base2" defined the type as "GenericBase[Base2]")
+
+[case testMultipleInheritance_NestedVariableOverriddenWithIncompatibleType2]
+
+from typing import TypeVar, Generic
+T = TypeVar('T', covariant=True)
+class GenericBase(Generic[T]):
+    pass
+class Base1:
+    Nested: GenericBase['Base1']
+class Base2:
+    Nested: GenericBase['Base2']
+class A(Base1, Base2):
+    Nested: GenericBase['Base2']
+[out]
+main:11: error: Incompatible types in assignment (expression has type "GenericBase[Base2]", base class "Base1" defined the type as "GenericBase[Base1]")
+
+
+[case testMultipleInheritance_NestedVariableOverriddenWithCompatibleType]
+
+from typing import TypeVar, Generic
+T = TypeVar('T', covariant=True)
+class GenericBase(Generic[T]):
+    pass
+class Base1:
+    Nested: GenericBase['Base1']
+class Base2:
+    Nested: GenericBase['Base1']
+class A(Base1, Base2):
+    Nested: GenericBase['Base1']
+[out]
+
+
+
+[case testMultipleInheritance_MethodDefinitionsCompatibleWithOverride]
+from typing import TypeVar, Union
+_T = TypeVar('_T')
+
+class Flag:
+    def __or__(self: _T, other: _T) -> _T: ...
+
+# int defines __or__ as:
+#     def __or__(self, n: int) -> int: ...
+class IntFlag(int, Flag):
+    def __or__(self: _T, other: Union[int, _T]) -> _T: ...
+[out]
+
+[case testMultipleInheritance_MethodDefinitionsIncompatibleOverride]
+from typing import TypeVar, Union
+_T = TypeVar('_T')
+
+class Flag:
+    def __or__(self: _T, other: _T) -> _T: ...
+
+class IntFlag(int, Flag):
+    def __or__(self: _T, other: str) -> _T: ...
+[out]
+main:8: error: Argument 1 of "__or__" incompatible with supertype "Flag"
+
+[case testMultipleInheritance_MethodDefinitionsCompatibleNoOverride]
+from typing import TypeVar, Union
+_T = TypeVar('_T')
+
+class Flag:
+    def __or__(self: _T, other: _T) -> _T: ...
+
+class IntFlag(int, Flag):
+    pass
+[out]
+
+[case testMultipleInheritance_MethodsReturningSelfCompatible]
+class A(object):
+    def x(self) -> 'A':
+        return self
+
+class B(object):
+    def x(self) -> 'B':
+        return self
+
+class C(A, B):
+    def x(self) -> 'C':
+        return self
+
+[case testMultipleInheritance_MethodsReturningSelfIncompatible]
+class A(object):
+    def x(self) -> 'A':
+        return self
+
+class B(object):
+    def x(self) -> 'B':
+        return self
+
+class C(A, B): # E: Definition of "x" in base class "A" is incompatible with definition in base class "B"
+    pass
+
 [case testNestedVariableRefersToSubclassOfAnotherNestedClass]
 class Mixin1:
     class Meta:


Code review comments:
diff_hunk: @@ -3725,6 +3725,7 @@ class C(B):
     a = "a"
 [out]
 main:4: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
+main:6: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
path: test-data/unit/check-classes.test
text: @user1:
This looks like a false positive. It can cause errors in code that already passes mypy.

I assume this is related to the `break` statement you removed.

@author:
Yes, it's related to the `break` statement being removed.

I wouldn't say that it is a false positive. `C.a` is incompatible with `A.a`, just as `B.a` is incompatible with `A.a`.

@author:
Mypy simply would not show you the error before. If you fixed the compatibility problem in `B.a`, then you would get an error about `C.a`. Isn't it unexpected to start getting new errors when mypy could have reported it the first time?

@user1:
A user might want to ignore the existing error for some reasons (legacy interfaces, code owned by other team, etc.) Breaking their build will be not helpful, it will not discover any _new_ type errors and will be just annoying.

diff_hunk: @@ -4009,14 +4010,25 @@ class B(A):
 main:4: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
 main:5: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
 
-[case testClassIgnoreType]
+[case testClassIgnoreType_RedefinedAttributeAndGrandparentAttributeTypesNotIgnored]
 class A:
     x = 0
 class B(A):
     x = ''  # type: ignore
 class C(B):
     x = ''
 [out]
+main:6: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int")
path: test-data/unit/check-classes.test
text: @user1:
Same as above. A user might want to ignore the error _once_ in the base class, instead of ignoring it in _every_ subclass.

@author:
This is slightly different than the above situation, as here, there was no error before.

I think it depends on how you want to interpret the `# type: ignore`.
In the current behavior, because `B.x` has an ignore, you are no longer warned when you redefine `x` in `C.x` that that redefinition is actually incompatible with ancestor's `A.x`.

A library author may have defined `B.x` with the ignore comment, and let us say that you are writing class `C` yourself. I would find it unexpected that because the library author chose to add an ignore comment on `B.x` that that would suppress a certain type of error when defining my own class.
I may not know in detail how `B` is defined/type-annotated, so I may reasonably expect that the type checker would perform its normal checks for my definition of class `C` (in this case, that it is compatible with `A`).

Are there other places in mypy where `# type: ignore` comments "spread" in that sense?


@author:
Oops:
~(in this case, that it is compatible with `A`)~
should be:
(in this case, that `C.x` is actually compatible with `A.x`)

@user1:
> Are there other places in mypy where `# type: ignore` comments "spread" in that sense?

Practically everywhere, an invalid type, a failed attribute access, an operator, or sometimes even failed function call, produce an `Any` type thus preventing subsequent errors.