Skip to content

Detect metaclass conflicts, refs #13563 #13598

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 4, 2022
Merged

Detect metaclass conflicts, refs #13563 #13598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bebf848
Detect metaclass conflicts, refs #13563
sobolevn Sep 3, 2022
fa84a71
Fix ci
sobolevn Sep 3, 2022
848e5f7
Black
sobolevn Sep 3, 2022
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
9 changes: 6 additions & 3 deletions docs/source/metaclasses.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,15 @@ 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
# 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

# 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
30 changes: 30 additions & 0 deletions mypy/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2044,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 @@ -2383,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,
)
Comment on lines +2398 to +2414
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could do something like this, to improve the error message:

Suggested change
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,
)
subclass_metaclass = typ.metaclass_type
if subclass_metaclass is None:
return
for superclass in typ.mro[1:-1]:
superclass_metaclass = superclass.metaclass_type
if superclass_metaclass is None or is_named_instance(
superclass_metaclass, "builtins.type"
):
continue
if not is_subtype(subclass_metaclass, superclass_metaclass):
self.fail(
(
f"Metaclass conflict: metaclass {subclass_metaclass.fullname!r} "
f"is not a subclass of {superclass_metaclass.fullname!r}, "
f"the metaclass of base class {superclass.fullname!r}"
),
typ
)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried it and I didn't like it. Why?
Because it either reports several errors for several bases with incompatible metaclasses or reports just one (if break is added) which is potentially confusing or we need even more complex code to accumulate all problematic base types and then raise a single one.

Let's keep it as-is for now?

If someone complains, then we can easily make this more readable and more complex.

(This is why I love coping CPython's error messages 😆 )

Copy link
Member

@AlexWaygood AlexWaygood Sep 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it as-is for now?

Sure, the PR looks pretty good right now! It was just an idea :)


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

Expand Down
16 changes: 4 additions & 12 deletions mypy/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2120,18 +2120,10 @@ def recalculate_metaclass(self, defn: ClassDef, declared_metaclass: Instance | N
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 declared_metaclass is not None and 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 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)
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
41 changes: 37 additions & 4 deletions test-data/unit/check-classes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4351,7 +4351,7 @@ class C(B):
class X(type): pass
class Y(type): pass
class A(metaclass=X): pass
class B(A, metaclass=Y): pass # E: Inconsistent metaclass structure for "B"
class B(A, metaclass=Y): pass # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

[case testMetaclassNoTypeReveal]
class M:
Expand Down Expand Up @@ -5213,8 +5213,8 @@ class CD(six.with_metaclass(M)): pass # E: Multiple metaclass definitions
class M1(type): pass
class Q1(metaclass=M1): pass
@six.add_metaclass(M)
class CQA(Q1): pass # E: Inconsistent metaclass structure for "CQA"
class CQW(six.with_metaclass(M, Q1)): pass # E: Inconsistent metaclass structure for "CQW"
class CQA(Q1): pass # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
class CQW(six.with_metaclass(M, Q1)): pass # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
[builtins fixtures/tuple.pyi]

[case testSixMetaclassAny]
Expand Down Expand Up @@ -5319,7 +5319,7 @@ class C5(future.utils.with_metaclass(f())): pass # E: Dynamic metaclass not sup

class M1(type): pass
class Q1(metaclass=M1): pass
class CQW(future.utils.with_metaclass(M, Q1)): pass # E: Inconsistent metaclass structure for "CQW"
class CQW(future.utils.with_metaclass(M, Q1)): pass # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
[builtins fixtures/tuple.pyi]

[case testFutureMetaclassAny]
Expand Down Expand Up @@ -6718,6 +6718,39 @@ class Meta(A): pass
from m import Meta
class A(metaclass=Meta): pass

[case testMetaclassConflict]
class MyMeta1(type): ...
class MyMeta2(type): ...
class MyMeta3(type): ...
class A(metaclass=MyMeta1): ...
class B(metaclass=MyMeta2): ...
class C(metaclass=type): ...
class A1(A): ...
class E: ...

class CorrectMeta(MyMeta1, MyMeta2): ...
class CorrectSubclass1(A1, B, E, metaclass=CorrectMeta): ...
class CorrectSubclass2(A, B, E, metaclass=CorrectMeta): ...
class CorrectSubclass3(B, A, metaclass=CorrectMeta): ...

class ChildOfCorrectSubclass1(CorrectSubclass1): ...

class CorrectWithType1(C, A1): ...
class CorrectWithType2(B, C): ...

class Conflict1(A1, B, E): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
class Conflict2(A, B): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
class Conflict3(B, A): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

class ChildOfConflict1(Conflict3): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
class ChildOfConflict2(Conflict3, metaclass=CorrectMeta): ...

class ConflictingMeta(MyMeta1, MyMeta3): ...
class Conflict4(A1, B, E, metaclass=ConflictingMeta): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

class ChildOfCorrectButWrongMeta(CorrectSubclass1, metaclass=ConflictingMeta): # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
...

[case testGenericOverride]
from typing import Generic, TypeVar, Any

Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/fine-grained.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2968,7 +2968,7 @@ class M(type):
pass
[out]
==
a.py:3: error: Inconsistent metaclass structure for "D"
a.py:3: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

[case testFineMetaclassDeclaredUpdate]
import a
Expand All @@ -2984,7 +2984,7 @@ class M(type): pass
class M2(type): pass
[out]
==
a.py:3: error: Inconsistent metaclass structure for "D"
a.py:3: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

[case testFineMetaclassRemoveFromClass]
import a
Expand Down