[go: up one dir, main page]
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.

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

Jump to bottom

Fix metaclass resolution algorithm #17713

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix metaclass resolution algorithm #17713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Fix metaclass resolution algorithm
  • Loading branch information
@robsdedude
robsdedude committed Aug 27, 2024
commit b8d656f2786912da530a2dbf5a81ccd8d598e796
23 changes: 7 additions & 16 deletions mypy/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2819,23 +2819,14 @@ def check_metaclass_compatibility(self, typ: TypeInfo) -> 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
if typ.metaclass_type is None and any(
base.type.metaclass_type is not None for base in typ.bases
):
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,
)
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
28 changes: 19 additions & 9 deletions mypy/nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3246,15 +3246,25 @@ def calculate_metaclass_type(self) -> mypy.types.Instance | None:
return declared
if self._fullname == "builtins.type":
return mypy.types.Instance(self, [])
candidates = [
s.declared_metaclass
for s in self.mro
if s.declared_metaclass is not None and s.declared_metaclass.type is not None
]
for c in candidates:
if all(other.type in c.type.mro for other in candidates):
return c
return None

winner = declared
for super_class in self.mro[1:]:
super_meta = super_class.declared_metaclass
Comment on lines +3251 to +3252
Copy link
Author

Choose a reason for hiding this comment

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

For context: I was originally going for just checking metaclass_type for each of self.bases, as that's even more in line with what cpython actually does to determine a classes metaclass.

However, that had some implications I didn't want to deal with:

  • The metaclass of everything would become builtin.type (which technically is correct, I believe)
  • Classes like builtin.str would report abc.ABCMeta as their metaclass. That seems to stem from typeshed pretending str to inherit from Sequence[str]. Not sure how it's not showing up in the declared_metaclass in the MRO, but it has been working like this, so I didn't want to change that.

if super_meta is None or super_meta.type is None:
continue
if winner is None:
winner = super_meta
continue
if winner.type.has_base(super_meta.type.fullname):
continue
if super_meta.type.has_base(winner.type.fullname):
winner = super_meta
continue
# metaclass conflict
winner = None
break

return winner

def is_metaclass(self) -> bool:
return (
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-abstract.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ from abc import abstractmethod, ABCMeta
import typing

class A(metaclass=ABCMeta): pass
class B(object, A): pass \
class B(object, A, metaclass=ABCMeta): pass \
# E: Cannot determine consistent method resolution order (MRO) for "B"

[case testOverloadedAbstractMethod]
Expand Down
33 changes: 32 additions & 1 deletion test-data/unit/check-classes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7104,7 +7104,7 @@ class Conflict1(A1, B, E): ... # E: Metaclass conflict: the metaclass of a deri
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 ChildOfConflict1(Conflict3): ...
class ChildOfConflict2(Conflict3, metaclass=CorrectMeta): ...

class ConflictingMeta(MyMeta1, MyMeta3): ...
Expand All @@ -7113,6 +7113,37 @@ class Conflict4(A1, B, E, metaclass=ConflictingMeta): ... # E: Metaclass confli
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 testMetaClassConflictIssue14033]
class M1(type): pass
class M2(type): pass
class Mx(M1, M2): pass

class A1(metaclass=M1): pass
class A2(A1): pass

class B1(metaclass=M2): pass

class C1(metaclass=Mx): pass

class TestABC(A2, B1, C1): pass # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
class TestBAC(B1, A2, C1): pass # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

# should not warn again for children
class ChildOfTestABC(TestABC): pass

# no metaclass is assumed if super class has a metaclass conflict
class ChildOfTestABCMetaMx(TestABC, metaclass=Mx): pass
class ChildOfTestABCMetaM1(TestABC, metaclass=M1): pass

class TestABCMx(A2, B1, C1, metaclass=Mx): pass
class TestBACMx(B1, A2, C1, metaclass=Mx): pass

class TestACB(A2, C1, B1): pass
class TestBCA(B1, C1, A2): pass

class TestCAB(C1, A2, B1): pass
class TestCBA(C1, B1, A2): pass

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

Expand Down
Loading