-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Support metaclasses #2475
Support metaclasses #2475
Changes from 15 commits
b20cfa4
7eee764
6ab7368
0d0e9ff
41ecc84
6dc66b2
314a300
3a05361
c42f777
5de0e80
cf02b0d
f90fc80
ac49100
4c70367
7876d3f
c256da5
d65c3b4
b083a97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1946,6 +1946,27 @@ def __init__(self, names: 'SymbolTable', defn: ClassDef, module_name: str) -> No | |
for vd in defn.type_vars: | ||
self.type_vars.append(vd.name) | ||
|
||
declared_metaclass = None # type: Optional[mypy.types.Instance] | ||
metaclass_type = None # type: Optional[mypy.types.Instance] | ||
|
||
def calculate_metaclass_type(self) -> 'Optional[mypy.types.Instance]': | ||
if self.mro is None: | ||
# XXX why does this happen? | ||
return None | ||
declared = self.declared_metaclass | ||
if declared is not None and not declared.type.has_base('builtins.type'): | ||
return declared | ||
if self._fullname == 'builtins.type': | ||
return mypy.types.Instance(self, []) | ||
candidates = {s.declared_metaclass for s in self.mro} - {None} | ||
for c in candidates: | ||
if all(other.type in c.type.mro for other in candidates): | ||
return c | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How sure are you that the answer does not depend on the order of candidates? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not at all. Perhaps redundant There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have replaced the set with a simple list candidates = [s.declared_metaclass for s in self.mro if s.declared_metaclass is not None] |
||
return None | ||
|
||
def is_metaclass(self) -> bool: | ||
return self.has_base('builtins.type') | ||
|
||
def name(self) -> str: | ||
"""Short name.""" | ||
return self.defn.name | ||
|
@@ -2060,6 +2081,8 @@ def serialize(self) -> JsonDict: | |
'type_vars': self.type_vars, | ||
'bases': [b.serialize() for b in self.bases], | ||
'_promote': None if self._promote is None else self._promote.serialize(), | ||
'declared_metaclass': (None if self.declared_metaclass is None | ||
else self.declared_metaclass.serialize()), | ||
'tuple_type': None if self.tuple_type is None else self.tuple_type.serialize(), | ||
'typeddict_type': | ||
None if self.typeddict_type is None else self.typeddict_type.serialize(), | ||
|
@@ -2081,6 +2104,9 @@ def deserialize(cls, data: JsonDict) -> 'TypeInfo': | |
ti.bases = [mypy.types.Instance.deserialize(b) for b in data['bases']] | ||
ti._promote = (None if data['_promote'] is None | ||
else mypy.types.Type.deserialize(data['_promote'])) | ||
ti.declared_metaclass = (None if data['declared_metaclass'] is None | ||
else mypy.types.Instance.deserialize(data['declared_metaclass'])) | ||
ti.metaclass_type = ti.calculate_metaclass_type() | ||
ti.tuple_type = (None if data['tuple_type'] is None | ||
else mypy.types.TupleType.deserialize(data['tuple_type'])) | ||
ti.typeddict_type = (None if data['typeddict_type'] is None | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -905,8 +905,20 @@ def analyze_metaclass(self, defn: ClassDef) -> None: | |
self.fail("Dynamic metaclass not supported for '%s'" % defn.name, defn) | ||
return | ||
sym = self.lookup_qualified(defn.metaclass, defn) | ||
if sym is not None and not isinstance(sym.node, TypeInfo): | ||
if sym is None: | ||
# Probably a name error - it is already handled elsewhere | ||
return | ||
if not isinstance(sym.node, TypeInfo): | ||
self.fail("Invalid metaclass '%s'" % defn.metaclass, defn) | ||
return | ||
inst = fill_typevars(sym.node) | ||
assert isinstance(inst, Instance) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can trigger this assert as follows:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be an error then. Do you agree? I don't want to go silently to a fallback, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you can add the check to line 911. |
||
defn.info.declared_metaclass = inst | ||
defn.info.metaclass_type = defn.info.calculate_metaclass_type() | ||
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 | ||
self.fail("Inconsistent metaclass structure for '%s'" % defn.name, defn) | ||
|
||
def object_type(self) -> Instance: | ||
return self.named_type('__builtins__.object') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2759,3 +2759,50 @@ class B(A): | |
class C(B): | ||
x = '' | ||
[out] | ||
|
||
[case testInvalidMetaclassStructure] | ||
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' | ||
|
||
[case testMetaclassNoTypeReveal] | ||
class M: | ||
x = 0 # type: int | ||
|
||
class A(metaclass=M): pass | ||
|
||
reveal_type(A.x) # E: Revealed type is 'builtins.int' | ||
|
||
[case testMetaclassTypeReveal] | ||
from typing import Type | ||
class M(type): | ||
x = 0 # type: int | ||
|
||
class A(metaclass=M): pass | ||
|
||
reveal_type(A.x) # E: Revealed type is 'builtins.int' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line seems redundant, it's the same as in the previous test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have removed it in the last commit but it still appears here. |
||
|
||
def f(TA: Type[A]): | ||
reveal_type(TA) # E: Revealed type is 'Type[__main__.A]' | ||
reveal_type(TA.x) # E: Revealed type is 'builtins.int' | ||
|
||
[case testMetaclassIterable] | ||
from typing import Iterable, Iterator | ||
|
||
class BadMeta(type): | ||
def __iter__(self) -> Iterator[int]: yield 1 | ||
|
||
class Bad(metaclass=BadMeta): pass | ||
|
||
for _ in Bad: pass # E: Iterable expected | ||
|
||
class GoodMeta(type, Iterable[int]): | ||
def __iter__(self) -> Iterator[int]: yield 1 | ||
|
||
class Good(metaclass=GoodMeta): pass | ||
for _ in Good: pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this works, shouldn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just assumed it will work. Turns out it didn't, but after handling the fallback in the solver it does. |
||
reveal_type(list(Good)) # E: Revealed type is 'builtins.list[builtins.int*]' | ||
|
||
[builtins fixtures/list.pyi] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
class ABCMeta: pass | ||
class ABCMeta(type): pass | ||
abstractmethod = object() | ||
abstractproperty = object() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a sample that triggers this condition?
Presumably the clue is the comment in
__init__
on line 1940 above. Maybe it has something to do with the MRO being calculated twice -- the first time (in semanal.py on line 850) it may fail if there's an import cycle causing a base class being a forward reference; the second time on line 3128 in ThirdPass, called from build.py line 1440. (There's also a call from fixup.py, but I think that's not what you're seeing.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out it happens during deserialization in incremental mode: the type of a direct base class may be None. (I should have looked into it long time ago. Sorry)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will remove the call from deserialize, since it shouldn't be done yet - it should be done from fixup.py. Does it sound reasonable?