Skip to content

Detect metaclass conflicts, refs #13563 #13565

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

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions docs/source/metaclasses.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,16 @@ so it's better not to combine metaclasses and class hierarchies:
class A1(metaclass=M1): pass
class A2(metaclass=M2): pass

class B1(A1, metaclass=M2): pass # Mypy Error: Inconsistent metaclass structure for "B1"
class B1(A1, metaclass=M2): pass # Mypy Error: metaclass conflict "B1"
# At runtime the above definition raises an exception
# TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

# Same runtime error as in B1, but mypy does not catch it yet
class B12(A1, A2): pass
class B12(A1, A2): pass # Mypy Error: metaclass conflict "B12"

# This can be solved via a common metaclass subtype:
class CorrectMeta(M1, M2): pass
class B2(A1, A2, metaclass=CorrectMeta): pass # OK, runtime is also OK


* Mypy does not understand dynamically-computed metaclasses,
such as ``class A(metaclass=f()): ...``
Expand Down
42 changes: 34 additions & 8 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1203,7 +1203,9 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) ->
return_type = get_proper_type(return_type)

if self.options.warn_no_return:
if not isinstance(return_type, (NoneType, AnyType)):
if not self.current_node_deferred and not isinstance(
return_type, (NoneType, AnyType)
):
# Control flow fell off the end of a function that was
# declared to return a non-None type and is not
# entirely pass/Ellipsis/raise NotImplementedError.
Expand Down Expand Up @@ -2042,6 +2044,7 @@ def visit_class_def(self, defn: ClassDef) -> None:
if not defn.has_incompatible_baseclass:
# Otherwise we've already found errors; more errors are not useful
self.check_multiple_inheritance(typ)
self.check_metaclass_compatibility(typ)
self.check_final_deletable(typ)

if defn.decorators:
Expand Down Expand Up @@ -2381,6 +2384,35 @@ class C(B, A[int]): ... # this is unsafe because...
if not ok:
self.msg.base_class_definitions_incompatible(name, base1, base2, ctx)

def check_metaclass_compatibility(self, typ: TypeInfo) -> None:
"""Ensures that metaclasses of all parent types are compatible."""
if (
typ.is_metaclass()
or typ.is_protocol
or typ.is_named_tuple
or typ.is_enum
or typ.typeddict_type is not None
):
return # Reasonable exceptions from this check

metaclasses = [
entry.metaclass_type
for entry in typ.mro[1:-1]
if entry.metaclass_type
and not is_named_instance(entry.metaclass_type, "builtins.type")
]
if not metaclasses:
return
if typ.metaclass_type is not None and all(
is_subtype(typ.metaclass_type, meta) for meta in metaclasses
):
return
self.fail(
"Metaclass conflict: the metaclass of a derived class must be "
"a (non-strict) subclass of the metaclasses of all its bases",
typ,
)

def visit_import_from(self, node: ImportFrom) -> None:
self.check_import(node)

Expand Down Expand Up @@ -2431,6 +2463,7 @@ def should_report_unreachable_issues(self) -> bool:
return (
self.in_checked_function()
and self.options.warn_unreachable
and not self.current_node_deferred
and not self.binder.is_unreachable_warning_suppressed()
)

Expand Down Expand Up @@ -4179,14 +4212,7 @@ def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None:
# To support local variables, we make this a definition line,
# causing assignment to set the variable's type.
var.is_inferred_def = True
# We also temporarily set current_node_deferred to False to
# make sure the inference happens.
# TODO: Use a better solution, e.g. a
# separate Var for each except block.
am_deferring = self.current_node_deferred
self.current_node_deferred = False
self.check_assignment(var, self.temp_node(t, var))
self.current_node_deferred = am_deferring
self.accept(s.handlers[i])
var = s.vars[i]
if var:
Expand Down
85 changes: 48 additions & 37 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -606,14 +606,18 @@ def add_implicit_module_attrs(self, file_node: MypyFile) -> None:
if not sym:
continue
node = sym.node
assert isinstance(node, TypeInfo)
if not isinstance(node, TypeInfo):
self.defer(node)
return
typ = Instance(node, [self.str_type()])
elif name == "__annotations__":
sym = self.lookup_qualified("__builtins__.dict", Context(), suppress_errors=True)
if not sym:
continue
node = sym.node
assert isinstance(node, TypeInfo)
if not isinstance(node, TypeInfo):
self.defer(node)
return
typ = Instance(node, [self.str_type(), AnyType(TypeOfAny.special_form)])
else:
assert t is not None, f"type should be specified for {name}"
Expand Down Expand Up @@ -1374,7 +1378,7 @@ def analyze_class(self, defn: ClassDef) -> None:
defn.base_type_exprs.extend(defn.removed_base_type_exprs)
defn.removed_base_type_exprs.clear()

self.update_metaclass(defn)
self.infer_metaclass_and_bases_from_compat_helpers(defn)

bases = defn.base_type_exprs
bases, tvar_defs, is_protocol = self.clean_up_bases_and_infer_type_variables(
Expand All @@ -1390,20 +1394,25 @@ def analyze_class(self, defn: ClassDef) -> None:
self.defer()

self.analyze_class_keywords(defn)
result = self.analyze_base_classes(bases)

if result is None or self.found_incomplete_ref(tag):
bases_result = self.analyze_base_classes(bases)
if bases_result is None or self.found_incomplete_ref(tag):
# Something was incomplete. Defer current target.
self.mark_incomplete(defn.name, defn)
return

base_types, base_error = result
base_types, base_error = bases_result
if any(isinstance(base, PlaceholderType) for base, _ in base_types):
# We need to know the TypeInfo of each base to construct the MRO. Placeholder types
# are okay in nested positions, since they can't affect the MRO.
self.mark_incomplete(defn.name, defn)
return

declared_metaclass, should_defer = self.get_declared_metaclass(defn.name, defn.metaclass)
if should_defer or self.found_incomplete_ref(tag):
# Metaclass was not ready. Defer current target.
self.mark_incomplete(defn.name, defn)
return

if self.analyze_typeddict_classdef(defn):
if defn.info:
self.setup_type_vars(defn, tvar_defs)
Expand All @@ -1422,7 +1431,7 @@ def analyze_class(self, defn: ClassDef) -> None:
with self.scope.class_scope(defn.info):
self.configure_base_classes(defn, base_types)
defn.info.is_protocol = is_protocol
self.analyze_metaclass(defn)
self.recalculate_metaclass(defn, declared_metaclass)
defn.info.runtime_protocol = False
for decorator in defn.decorators:
self.analyze_class_decorator(defn, decorator)
Expand Down Expand Up @@ -1968,7 +1977,7 @@ def calculate_class_mro(
if hook:
hook(ClassDefContext(defn, FakeExpression(), self))

def update_metaclass(self, defn: ClassDef) -> None:
def infer_metaclass_and_bases_from_compat_helpers(self, defn: ClassDef) -> None:
"""Lookup for special metaclass declarations, and update defn fields accordingly.

* six.with_metaclass(M, B1, B2, ...)
Expand Down Expand Up @@ -2046,30 +2055,33 @@ def is_base_class(self, t: TypeInfo, s: TypeInfo) -> bool:
visited.add(base.type)
return False

def analyze_metaclass(self, defn: ClassDef) -> None:
if defn.metaclass:
def get_declared_metaclass(
self, name: str, metaclass_expr: Expression | None
) -> tuple[Instance | None, bool]:
"""Returns either metaclass instance or boolean whether we should defer."""
declared_metaclass = None
if metaclass_expr:
metaclass_name = None
if isinstance(defn.metaclass, NameExpr):
metaclass_name = defn.metaclass.name
elif isinstance(defn.metaclass, MemberExpr):
metaclass_name = get_member_expr_fullname(defn.metaclass)
if isinstance(metaclass_expr, NameExpr):
metaclass_name = metaclass_expr.name
elif isinstance(metaclass_expr, MemberExpr):
metaclass_name = get_member_expr_fullname(metaclass_expr)
if metaclass_name is None:
self.fail(f'Dynamic metaclass not supported for "{defn.name}"', defn.metaclass)
return
sym = self.lookup_qualified(metaclass_name, defn.metaclass)
self.fail(f'Dynamic metaclass not supported for "{name}"', metaclass_expr)
return None, False
sym = self.lookup_qualified(metaclass_name, metaclass_expr)
if sym is None:
# Probably a name error - it is already handled elsewhere
return
return None, False
if isinstance(sym.node, Var) and isinstance(get_proper_type(sym.node.type), AnyType):
# 'Any' metaclass -- just ignore it.
#
# TODO: A better approach would be to record this information
# and assume that the type object supports arbitrary
# attributes, similar to an 'Any' base class.
return
return None, False
if isinstance(sym.node, PlaceholderNode):
self.defer(defn)
return
return None, True # defer later in the caller

# Support type aliases, like `_Meta: TypeAlias = type`
if (
Expand All @@ -2083,16 +2095,21 @@ def analyze_metaclass(self, defn: ClassDef) -> None:
metaclass_info = sym.node

if not isinstance(metaclass_info, TypeInfo) or metaclass_info.tuple_type is not None:
self.fail(f'Invalid metaclass "{metaclass_name}"', defn.metaclass)
return
self.fail(f'Invalid metaclass "{metaclass_name}"', metaclass_expr)
return None, False
if not metaclass_info.is_metaclass():
self.fail(
'Metaclasses not inheriting from "type" are not supported', defn.metaclass
'Metaclasses not inheriting from "type" are not supported', metaclass_expr
)
return
return None, False
inst = fill_typevars(metaclass_info)
assert isinstance(inst, Instance)
defn.info.declared_metaclass = inst
declared_metaclass = inst
# We check metaclass conflicts in `checker.py`
return declared_metaclass, False

def recalculate_metaclass(self, defn: ClassDef, declared_metaclass: Instance | None) -> None:
defn.info.declared_metaclass = declared_metaclass
defn.info.metaclass_type = defn.info.calculate_metaclass_type()
if any(info.is_protocol for info in defn.info.mro):
if (
Expand All @@ -2104,16 +2121,10 @@ def analyze_metaclass(self, defn: ClassDef) -> None:
abc_meta = self.named_type_or_none("abc.ABCMeta", [])
if abc_meta is not None: # May be None in tests with incomplete lib-stub.
defn.info.metaclass_type = abc_meta
if defn.info.metaclass_type is None:
# Inconsistency may happen due to multiple baseclasses even in classes that
# do not declare explicit metaclass, but it's harder to catch at this stage
if defn.metaclass is not None:
self.fail(f'Inconsistent metaclass structure for "{defn.name}"', defn)
else:
if defn.info.metaclass_type.type.has_base("enum.EnumMeta"):
defn.info.is_enum = True
if defn.type_vars:
self.fail("Enum class cannot be generic", defn)
if defn.info.metaclass_type and defn.info.metaclass_type.type.has_base("enum.EnumMeta"):
defn.info.is_enum = True
if defn.type_vars:
self.fail("Enum class cannot be generic", defn)

#
# Imports
Expand Down
2 changes: 1 addition & 1 deletion mypy/semanal_classprop.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def calculate_class_abstract_status(typ: TypeInfo, is_stub_file: bool, errors: E
# implement some methods.
typ.abstract_attributes = sorted(abstract)
if is_stub_file:
if typ.declared_metaclass and typ.declared_metaclass.type.fullname == "abc.ABCMeta":
if typ.declared_metaclass and typ.declared_metaclass.type.has_base("abc.ABCMeta"):
return
if typ.is_protocol:
return
Expand Down
Loading