Skip to content

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

Merged
merged 6 commits into from
May 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 20 additions & 20 deletions mypy/subtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ def is_subtype(left: Type, right: Type,
elif is_subtype_of_item:
return True
# otherwise, fall through
# Treat builtins.type the same as Type[Any]
elif is_named_instance(left, 'builtins.type'):
return is_subtype(TypeType(AnyType()), right)
elif is_named_instance(right, 'builtins.type'):
return is_subtype(left, TypeType(AnyType()))
return left.accept(SubtypeVisitor(right, type_parameter_checker,
ignore_pos_arg_names=ignore_pos_arg_names))

Expand Down Expand Up @@ -158,16 +153,18 @@ def visit_instance(self, left: Instance) -> bool:
item = right.item
if isinstance(item, TupleType):
item = item.fallback
if isinstance(item, Instance):
return is_subtype(left, item.type.metaclass_type)
elif isinstance(item, AnyType):
# Special case: all metaclasses are subtypes of Type[Any]
mro = left.type.mro or []
return any(base.fullname() == 'builtins.type' for base in mro)
else:
return False
else:
return False
if is_named_instance(left, 'builtins.type'):
return is_subtype(TypeType(AnyType()), right)
if left.type.is_metaclass():
if isinstance(item, AnyType):
return True
if isinstance(item, Instance):
# Special-case enum since we don't have better way of expressing it
if (is_named_instance(left, 'enum.EnumMeta')
and is_named_instance(item, 'enum.Enum')):
return True
return is_named_instance(item, 'builtins.object')
return False

def visit_type_var(self, left: TypeVarType) -> bool:
right = self.right
Expand Down Expand Up @@ -263,8 +260,8 @@ def visit_overloaded(self, left: Overloaded) -> bool:
elif isinstance(right, TypeType):
# All the items must have the same type object status, so
# it's sufficient to query only (any) one of them.
# This is unsound, we don't check the __init__ signature.
Copy link
Member

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.

return left.is_type_obj() and is_subtype(left.items()[0].ret_type, right.item)
# This is unsound, we don't check all the __init__ signatures.
return left.is_type_obj() and is_subtype(left.items()[0], right)
else:
return False

Expand All @@ -284,11 +281,14 @@ def visit_type_type(self, left: TypeType) -> bool:
# This is unsound, we don't check the __init__ signature.
return is_subtype(left.item, right.ret_type)
if isinstance(right, Instance):
if right.type.fullname() == 'builtins.object':
# treat builtins.object the same as Any.
if right.type.fullname() in ['builtins.object', 'builtins.type']:
return True
item = left.item
return isinstance(item, Instance) and is_subtype(item, right.type.metaclass_type)
if isinstance(item, TypeVarType):
item = item.upper_bound
if isinstance(item, Instance):
metaclass = item.type.metaclass_type
return metaclass is not None and is_subtype(metaclass, right)
return False


Expand Down
78 changes: 76 additions & 2 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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: ...
Expand Down Expand Up @@ -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]
Copy link
Member

Choose a reason for hiding this comment

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

I would add few reveal_types for methods in these tests, just in case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried, although I'm not sure what information is gained.

Copy link
Member

Choose a reason for hiding this comment

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

Actually I wanted something like reveal_type(A.g1) and reveal_type(A.g1()), in particular, where return types of g2 etc. depend on type variables (like List[TA]). Plus also the situations where name of staticmethod/classmethod in class coincides with a name of a method in metaclass.

Copy link
Member

Choose a reason for hiding this comment

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

Actually I wanted something like reveal_type(A.g1) and reveal_type(A.g1()), in particular, where return types of g2 etc. depend on type variables (like List[TA]). Plus also the situations where name of staticmethod/classmethod in class coincides with a name of a method in metaclass.

@elazarg Have you added these tests? Otherwise I think this PR looks good.
@JukkaL could you please try this PR with the Dropbox internal codebase before it is merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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).

Copy link
Member

Choose a reason for hiding this comment

The 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
-- -----------------------

Expand Down