Support six.add_metaclass #3842
Conversation
test-data/unit/check-classes.test
Outdated
| @six.add_metaclass(M) | ||
| class A(object): pass | ||
| reveal_type(type(A).x) # E: Revealed type is 'builtins.int' | ||
|
|
There was a problem hiding this comment.
I would add more tests. Some ideas:
- An invalid metaclass (not a subtype of
type) - Something involving
Any(for example a metaclass form silent import) __getattr__and/or other special attributes like__iter__- Multiple decorators on a class
- An incompatible metaclass
There was a problem hiding this comment.
But these will test other (later) mechanisms, so aren't they redundant (white-box-wise)?
There was a problem hiding this comment.
Look at the number of tests for with_metaclass, they are there for a reason, tests are not for the present (implementation), they are for the future (when everything may be refactored in unpredictable ways).
|
OK, this one can be now updated after #3848 is merged (plus the suggested tests). |
Conflicts: mypy/fastparse.py mypy/semanal.py test-data/unit/semanal-abstractclasses.test test-data/unit/semanal-classes.test
7187423 to
85fd3f6
Compare
85fd3f6 to
4ae1dea
Compare
| @@ -3539,25 +3569,50 @@ class ArcMeta(GenericMeta, DestroyableMeta): | |||
| pass | |||
| class Arc(six.with_metaclass(ArcMeta, Generic[T_co], Destroyable)): | |||
| pass | |||
| @six.add_metaclass(ArcMeta) | |||
| class Arc1(Generic[T_co], Destroyable): | |||
There was a problem hiding this comment.
This actually fails at runtime because of a metaclass conflict. Is it easy to detect this? If this is non-trivial, then I think we should not include it in this PR.
There was a problem hiding this comment.
I believe this is not caught because the base class Generic is removed earlier in the analysis. It doesn't seem obvious to me how this should be fixed.
There was a problem hiding this comment.
OK, them it is not easy to support this.
test-data/unit/check-classes.test
Outdated
| @six.add_metaclass(M) # E: Multiple metaclass definitions | ||
| class CD(six.with_metaclass(M)): pass | ||
|
|
||
| def q(t): |
There was a problem hiding this comment.
This is not a good way to test Any, since the function is not checked at all. I would prefer something like import a metaclass and/or base class from a non-existing module. This will generate an error and the imported names will have types Any. (Or maybe use --follow-imports=silent to silence the error.)
| @@ -3539,25 +3569,50 @@ class ArcMeta(GenericMeta, DestroyableMeta): | |||
| pass | |||
| class Arc(six.with_metaclass(ArcMeta, Generic[T_co], Destroyable)): | |||
| pass | |||
| @six.add_metaclass(ArcMeta) | |||
| class Arc1(Generic[T_co], Destroyable): | |||
There was a problem hiding this comment.
OK, them it is not easy to support this.
Fix #3365
There are no special checks for improper use (similar to
with_metaclass).Consistency checks are handled like in any other metaclass.