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

Do not ignore generic type args when checking multiple inheritance compatibility #18270

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
165 changes: 117 additions & 48 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,9 @@ def check_method_override_for_base_with_name(
original_class_or_static = False # a variable can't be class or static

if isinstance(original_type, FunctionLike):
original_type = self.bind_and_map_method(base_attr, original_type, defn.info, base)
original_type = self.bind_and_map_method(
base_attr.node, original_type, defn.info, base
)
if original_node and is_property(original_node):
original_type = get_property_type(original_type)

Expand Down Expand Up @@ -2200,7 +2202,7 @@ def check_method_override_for_base_with_name(
return False

def bind_and_map_method(
self, sym: SymbolTableNode, typ: FunctionLike, sub_info: TypeInfo, super_info: TypeInfo
self, node: Node | None, typ: FunctionLike, sub_info: TypeInfo, super_info: TypeInfo
) -> FunctionLike:
"""Bind self-type and map type variables for a method.

Expand All @@ -2210,13 +2212,11 @@ def bind_and_map_method(
sub_info: class where the method is used
super_info: class where the method was defined
"""
if isinstance(sym.node, (FuncDef, OverloadedFuncDef, Decorator)) and not is_static(
sym.node
):
if isinstance(sym.node, Decorator):
is_class_method = sym.node.func.is_class
if isinstance(node, (FuncDef, OverloadedFuncDef, Decorator)) and not is_static(node):
if isinstance(node, Decorator):
is_class_method = node.func.is_class
else:
is_class_method = sym.node.is_class
is_class_method = node.is_class

mapped_typ = cast(FunctionLike, map_type_from_supertype(typ, sub_info, super_info))
active_self_type = self.scope.active_self_type()
Expand Down Expand Up @@ -2745,46 +2745,46 @@ def check_multiple_inheritance(self, typ: TypeInfo) -> None:
# No multiple inheritance.
return
# Verify that inherited attributes are compatible.
mro = typ.mro[1:]
for i, base in enumerate(mro):
bases = typ.bases
all_names = [{n for p in b.type.mro for n in p.names} for b in bases]
sterliakov marked this conversation as resolved.
Show resolved Hide resolved
for i, base in enumerate(bases):
# 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()
# Sort for consistent messages order.
non_overridden_attrs = sorted(all_names[i] - typ.names.keys())
for name in non_overridden_attrs:
if is_private(name):
continue
for base2 in mro[i + 1 :]:
for j, base2 in enumerate(bases[i + 1 :], i + 1):
# We only need to check compatibility of attributes from classes not
# in a subclass relationship. For subclasses, normal (single inheritance)
# checks suffice (these are implemented elsewhere).
if name in base2.names and base2 not in base.mro:
if name in all_names[j] and base.type != base2.type:
self.check_compatibility(name, base, base2, typ)

def determine_type_of_member(self, sym: SymbolTableNode) -> Type | None:
if sym.type is not None:
return sym.type
if isinstance(sym.node, FuncBase):
return self.function_type(sym.node)
if isinstance(sym.node, TypeInfo):
if sym.node.typeddict_type:
def determine_type_of_member(self, node: SymbolNode) -> Type | None:
if isinstance(node, FuncBase):
return self.function_type(node)
if isinstance(node, TypeInfo):
if node.typeddict_type:
# We special-case TypedDict, because they don't define any constructor.
return self.expr_checker.typeddict_callable(sym.node)
return self.expr_checker.typeddict_callable(node)
else:
return type_object_type(sym.node, self.named_type)
if isinstance(sym.node, TypeVarExpr):
return type_object_type(node, self.named_type)
if isinstance(node, TypeVarExpr):
# Use of TypeVars is rejected in an expression/runtime context, so
# we don't need to check supertype compatibility for them.
return AnyType(TypeOfAny.special_form)
if isinstance(sym.node, TypeAlias):
if isinstance(node, TypeAlias):
with self.msg.filter_errors():
# Suppress any errors, they will be given when analyzing the corresponding node.
# Here we may have incorrect options and location context.
return self.expr_checker.alias_type_in_runtime_context(sym.node, ctx=sym.node)
return self.expr_checker.alias_type_in_runtime_context(node, ctx=node)
# TODO: handle more node kinds here.
return None

def check_compatibility(
self, name: str, base1: TypeInfo, base2: TypeInfo, ctx: TypeInfo
self, name: str, base1: Instance, base2: Instance, ctx: TypeInfo
) -> None:
"""Check if attribute name in base1 is compatible with base2 in multiple inheritance.

Expand All @@ -2809,10 +2809,51 @@ class C(B, A[int]): ... # this is unsafe because...
if name in ("__init__", "__new__", "__init_subclass__"):
# __init__ and friends can be incompatible -- it's a special case.
return
first = base1.names[name]
second = base2.names[name]
first_type = get_proper_type(self.determine_type_of_member(first))
second_type = get_proper_type(self.determine_type_of_member(second))
first_type = first_node = None
second_type = second_node = None
orig_var = ctx.get(name)
if orig_var is not None and orig_var.node is not None:
if (b1type := base1.type.get_containing_type_info(name)) is not None:
base1 = map_instance_to_supertype(base1, b1type)
first_type, first_node = self.attribute_type_from_base(
orig_var.node, base1.type, base1
)

if (b2type := base2.type.get_containing_type_info(name)) is not None:
base2 = map_instance_to_supertype(base2, b2type)
second_type, second_node = self.attribute_type_from_base(
orig_var.node, base2.type, base2
)

# Fix the order. We iterate over the explicit bases, which means we may
# end up with the following structure:
# class A:
# def fn(self, x: int) -> None: ...
# class B(A): ...
# class C(A):
# def fn(self, x: int|str) -> None: ...
# class D(B, C): ...
# Here D.fn will actually be dispatched to C.fn which is assignable to A.fn,
# but without this fixup we'd check A.fn against C.fn instead.
# See testMultipleInheritanceTransitive in check-multiple-inheritance.test
if (
b1type is not None
and b2type is not None
and ctx.mro.index(b1type) > ctx.mro.index(b2type)
):
b1type, b2type = b2type, b1type
base1, base2 = base2, base1
first_type, second_type = second_type, first_type
first_node, second_node = second_node, first_node

if (
base1 is not None
and base2 is not None
and is_subtype(base1, base2, ignore_promotions=True)
):
# If base1 already inherits from base2 with correct type args,
# we have reported errors if any. Avoid reporting them again.
return

# TODO: use more principled logic to decide is_subtype() vs is_equivalent().
# We should rely on mutability of superclass node, not on types being Callable.
Expand All @@ -2822,7 +2863,7 @@ class C(B, A[int]): ... # this is unsafe because...
if isinstance(first_type, Instance):
call = find_member("__call__", first_type, first_type, is_operator=True)
if call and isinstance(second_type, FunctionLike):
second_sig = self.bind_and_map_method(second, second_type, ctx, base2)
second_sig = self.bind_and_map_method(second_node, second_type, ctx, base2.type)
ok = is_subtype(call, second_sig, ignore_pos_arg_names=True)
elif isinstance(first_type, FunctionLike) and isinstance(second_type, FunctionLike):
if first_type.is_type_obj() and second_type.is_type_obj():
Expand All @@ -2834,42 +2875,70 @@ class C(B, A[int]): ... # this is unsafe because...
)
else:
# First bind/map method types when necessary.
first_sig = self.bind_and_map_method(first, first_type, ctx, base1)
second_sig = self.bind_and_map_method(second, second_type, ctx, base2)
first_sig = self.bind_and_map_method(first_node, first_type, ctx, base1.type)
second_sig = self.bind_and_map_method(second_node, second_type, ctx, base2.type)
ok = is_subtype(first_sig, second_sig, ignore_pos_arg_names=True)
elif first_type and second_type:
if isinstance(first.node, Var):
first_type = expand_self_type(first.node, first_type, fill_typevars(ctx))
if isinstance(second.node, Var):
second_type = expand_self_type(second.node, second_type, fill_typevars(ctx))
if isinstance(first_node, Var):
first_type = expand_self_type(first_node, first_type, fill_typevars(ctx))
if isinstance(second_node, Var):
second_type = expand_self_type(second_node, second_type, fill_typevars(ctx))
ok = is_equivalent(first_type, second_type)
if not ok:
second_node = base2[name].node
second_var = base2.type.get(name)
if (
isinstance(second_type, FunctionLike)
and second_node is not None
and is_property(second_node)
and second_var is not None
and second_var.node is not None
and is_property(second_var.node)
):
second_type = get_property_type(second_type)
ok = is_subtype(first_type, second_type)
else:
if first_type is None:
self.msg.cannot_determine_type_in_base(name, base1.name, ctx)
self.msg.cannot_determine_type_in_base(name, base1.type.name, ctx)
if second_type is None:
self.msg.cannot_determine_type_in_base(name, base2.name, ctx)
self.msg.cannot_determine_type_in_base(name, base2.type.name, ctx)
ok = True
# Final attributes can never be overridden, but can override
# non-final read-only attributes.
if is_final_node(second.node) and not is_private(name):
self.msg.cant_override_final(name, base2.name, ctx)
if is_final_node(first.node):
self.check_if_final_var_override_writable(name, second.node, ctx)
if is_final_node(second_node) and not is_private(name):
self.msg.cant_override_final(name, base2.type.name, ctx)
if is_final_node(first_node):
self.check_if_final_var_override_writable(name, second_node, ctx)
# Some attributes like __slots__ and __deletable__ are special, and the type can
# vary across class hierarchy.
if isinstance(second.node, Var) and second.node.allow_incompatible_override:
if isinstance(second_node, Var) and second_node.allow_incompatible_override:
ok = True
if not ok:
self.msg.base_class_definitions_incompatible(name, base1, base2, ctx)
self.msg.base_class_definitions_incompatible(name, base1.type, base2.type, ctx)

def attribute_type_from_base(
self, expr_node: SymbolNode, base: TypeInfo, self_type: Instance
) -> tuple[ProperType | None, Node | None]:
"""For a NameExpr that is part of a class, walk all base classes and try
to find the first class that defines a Type for the same name."""
expr_name = expr_node.name
base_var = base.names.get(expr_name)

if base_var:
base_node = base_var.node
base_type = base_var.type

if base_type:
if not has_no_typevars(base_type):
itype = map_instance_to_supertype(self_type, base)
base_type = expand_type_by_instance(base_type, itype)

return get_proper_type(base_type), base_node

if (
base_node is not None
and (base_type := self.determine_type_of_member(base_node)) is not None
):
return get_proper_type(base_type), base_node

return None, None

def check_metaclass_compatibility(self, typ: TypeInfo) -> None:
"""Ensures that metaclasses of all parent types are compatible."""
Expand Down
7 changes: 6 additions & 1 deletion mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,12 @@ def module_type(self, node: MypyFile) -> Instance:
continue
if isinstance(n.node, Var) and n.node.is_final:
immutable.add(name)
typ = self.chk.determine_type_of_member(n)

typ = None
if n.type is not None:
typ = n.type
elif n.node is not None:
typ = self.chk.determine_type_of_member(n.node)
if typ:
module_attrs[name] = typ
else:
Expand Down
2 changes: 1 addition & 1 deletion mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -4202,7 +4202,7 @@ def is_class_var(expr: NameExpr) -> bool:
return False


def is_final_node(node: SymbolNode | None) -> bool:
def is_final_node(node: Node | None) -> bool:
"""Check whether `node` corresponds to a final attribute."""
return isinstance(node, (Var, FuncDef, OverloadedFuncDef, Decorator)) and node.is_final

Expand Down
46 changes: 41 additions & 5 deletions test-data/unit/check-generic-subtyping.test
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ x1: X1[str, int]
reveal_type(list(x1)) # N: Revealed type is "builtins.list[builtins.int]"
reveal_type([*x1]) # N: Revealed type is "builtins.list[builtins.int]"

class X2(Generic[T, U], Iterator[U], Mapping[T, U]):
class X2(Generic[T, U], Iterator[U], Mapping[T, U]): # E: Definition of "__iter__" in base class "Iterable" is incompatible with definition in base class "Iterable"
pass

x2: X2[str, int]
Expand Down Expand Up @@ -1017,10 +1017,7 @@ x1: X1[str, int]
reveal_type(iter(x1)) # N: Revealed type is "typing.Iterator[builtins.int]"
reveal_type({**x1}) # N: Revealed type is "builtins.dict[builtins.int, builtins.str]"

# Some people would expect this to raise an error, but this currently does not:
# `Mapping` has `Iterable[U]` base class, `X2` has direct `Iterable[T]` base class.
# It would be impossible to define correct `__iter__` method for incompatible `T` and `U`.
class X2(Generic[T, U], Mapping[U, T], Iterable[T]):
class X2(Generic[T, U], Mapping[U, T], Iterable[T]): # E: Definition of "__iter__" in base class "Iterable" is incompatible with definition in base class "Iterable"
pass

x2: X2[str, int]
Expand Down Expand Up @@ -1065,3 +1062,42 @@ class F(E[T_co], Generic[T_co]): ... # E: Variance of TypeVar "T_co" incompatib

class G(Generic[T]): ...
class H(G[T_contra], Generic[T_contra]): ... # E: Variance of TypeVar "T_contra" incompatible with variance in parent type

[case testMultipleInheritanceCompatibleTypeVar]
from typing import Generic, TypeVar

T = TypeVar("T")
U = TypeVar("U")

class A(Generic[T]):
x: T
def fn(self, t: T) -> None: ...

class A2(A[T]):
y: str
z: str

class B(Generic[T]):
x: T
def fn(self, t: T) -> None: ...

class C1(A2[str], B[str]): pass
class C2(A2[str], B[int]): pass # E: Definition of "x" in base class "A" is incompatible with definition in base class "B" \
# E: Definition of "fn" in base class "A" is incompatible with definition in base class "B"
class C3(A2[T], B[T]): pass
class C4(A2[U], B[U]): pass
class C5(A2[U], B[T]): pass # E: Definition of "x" in base class "A" is incompatible with definition in base class "B" \
# E: Definition of "fn" in base class "A" is incompatible with definition in base class "B"

[builtins fixtures/tuple.pyi]

[case testMultipleInheritanceCompatErrorPropagation]
class A:
foo: bytes
class B(A):
foo: str # type: ignore[assignment]

class Ok(B, A): pass

class C(A): pass
class Ok2(B, C): pass
9 changes: 8 additions & 1 deletion test-data/unit/check-multiple-inheritance.test
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,6 @@ 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"


[case testMultipleInheritanceOverridingOfFunctionsWithCallableInstances]
from typing import Any, Callable

Expand Down Expand Up @@ -706,3 +705,11 @@ class C34(B3, B4): ...
class C41(B4, B1): ...
class C42(B4, B2): ...
class C43(B4, B3): ...

[case testMultipleInheritanceTransitive]
class A:
def fn(self, x: int) -> None: ...
class B(A): ...
class C(A):
def fn(self, x: "int | str") -> None: ...
class D(B, C): ...
Loading