-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Make Type[X] compatible with metaclass of X #3346
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
Changes from all commits
907f07b
75d8073
2209639
18edb2d
ae0c77b
34b5817
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 |
---|---|---|
|
@@ -3067,11 +3067,11 @@ class A(metaclass=M): pass | |
|
||
reveal_type(A[M]) # E: Revealed type is 'builtins.int' | ||
|
||
[case testMetaclassSelftype] | ||
[case testMetaclassSelfType] | ||
from typing import TypeVar, Type | ||
|
||
class M(type): pass | ||
T = TypeVar('T', bound='A') | ||
T = TypeVar('T') | ||
|
||
class M1(M): | ||
def foo(cls: Type[T]) -> T: ... | ||
|
@@ -3137,6 +3137,80 @@ class M(type): | |
class A(metaclass=M): pass | ||
reveal_type(type(A).x) # E: Revealed type is 'builtins.int' | ||
|
||
[case testMetaclassStrictSupertypeOfTypeWithClassmethods] | ||
from typing import Type, TypeVar | ||
TA = TypeVar('TA', bound='A') | ||
TTA = TypeVar('TTA', bound='Type[A]') | ||
TM = TypeVar('TM', bound='M') | ||
|
||
class M(type): | ||
def g1(cls: 'Type[A]') -> A: pass # E: The erased type of self 'Type[__main__.A]' is not a supertype of its class '__main__.M' | ||
def g2(cls: Type[TA]) -> TA: pass # E: The erased type of self 'Type[__main__.A]' is not a supertype of its class '__main__.M' | ||
def g3(cls: TTA) -> TTA: pass # E: The erased type of self 'Type[__main__.A]' is not a supertype of its class '__main__.M' | ||
def g4(cls: TM) -> TM: pass | ||
m: M | ||
|
||
class A(metaclass=M): | ||
def foo(self): pass | ||
|
||
reveal_type(A.g1) # E: Revealed type is 'def () -> __main__.A' | ||
reveal_type(A.g2) # E: Revealed type is 'def () -> __main__.A*' | ||
reveal_type(A.g3) # E: Revealed type is 'def () -> def () -> __main__.A' | ||
reveal_type(A.g4) # E: Revealed type is 'def () -> def () -> __main__.A' | ||
|
||
class B(metaclass=M): | ||
def foo(self): pass | ||
|
||
B.g1 # Should be error: Argument 0 to "g1" of "M" has incompatible type "B"; expected Type[A] | ||
B.g2 # Should be error: Argument 0 to "g2" of "M" has incompatible type "B"; expected Type[TA] | ||
B.g3 # Should be error: Argument 0 to "g3" of "M" has incompatible type "B"; expected "TTA" | ||
reveal_type(B.g4) # E: Revealed type is 'def () -> def () -> __main__.B' | ||
|
||
# 4 examples of unsoundness - instantiation, classmethod, staticmethod and ClassVar: | ||
|
||
ta: Type[A] = m # E: Incompatible types in assignment (expression has type "M", variable has type Type[A]) | ||
a: A = ta() | ||
reveal_type(ta.g1) # E: Revealed type is 'def () -> __main__.A' | ||
reveal_type(ta.g2) # E: Revealed type is 'def () -> __main__.A*' | ||
reveal_type(ta.g3) # E: Revealed type is 'def () -> Type[__main__.A]' | ||
reveal_type(ta.g4) # E: Revealed type is 'def () -> Type[__main__.A]' | ||
|
||
x: M = ta | ||
x.g1 # should be error: Argument 0 to "g1" of "M" has incompatible type "M"; expected Type[A] | ||
x.g2 # should be error: Argument 0 to "g2" of "M" has incompatible type "M"; expected Type[TA] | ||
x.g3 # should be error: Argument 0 to "g3" of "M" has incompatible type "M"; expected "TTA" | ||
reveal_type(x.g4) # E: Revealed type is 'def () -> __main__.M*' | ||
|
||
def r(ta: Type[TA], tta: TTA) -> None: | ||
x: M = ta | ||
y: M = tta | ||
|
||
class Class(metaclass=M): | ||
@classmethod | ||
def f1(cls: Type[Class]) -> None: pass | ||
@classmethod | ||
def f2(cls: M) -> None: pass | ||
cl: Type[Class] = m # E: Incompatible types in assignment (expression has type "M", variable has type Type[Class]) | ||
reveal_type(cl.f1) # E: Revealed type is 'def ()' | ||
reveal_type(cl.f2) # E: Revealed type is 'def ()' | ||
x1: M = cl | ||
|
||
class Static(metaclass=M): | ||
@staticmethod | ||
def f() -> None: pass | ||
s: Type[Static] = m # E: Incompatible types in assignment (expression has type "M", variable has type Type[Static]) | ||
reveal_type(s.f) # E: Revealed type is 'def ()' | ||
x2: M = s | ||
|
||
from typing import ClassVar | ||
class Cvar(metaclass=M): | ||
x = 1 # type: ClassVar[int] | ||
cv: Type[Cvar] = m # E: Incompatible types in assignment (expression has type "M", variable has type Type[Cvar]) | ||
cv.x | ||
x3: M = cv | ||
|
||
[builtins fixtures/classmethod.pyi] | ||
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 would add few 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 tried, although I'm not sure what information is gained. 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. Actually I wanted something like 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.
@elazarg Have you added these tests? Otherwise I think this PR looks good. 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. Hmm I'm pretty sure I did, but probably forgot to push... :\ I will check. 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. Ok, I don't have time at this moment, so I tried pushing kinda blindly. I might have mixed up something. This supposed to be the tests only, not the check-on-access you've asked for. I hope I did not mess up too bad; if I did, I can probably try to fix it later this week. In such case, it can still be useful to check earlier commit against the internal repo. Sorry about the mess. 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. @ilevkivskyi I can try this with Dropbox internal code tomorrow (need to go now). 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. No worries, I have tried this with our internal code, there are no new errors this found (either in our codebase or in the PR). I haven't reviewed the PR though so I can't "Approve" it. But Ivan can. |
||
|
||
-- Synthetic types crashes | ||
-- ----------------------- | ||
|
||
|
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 this comment should stay here, since the
__init__
signature should be checked for all items in an overload.