Allow instantiation of Type[A], if A is abstract (#2853)

ilevkivskyi · gvanrossum · commit 72967fdafb1f · 2017-03-27T12:18:39.000-07:00
Fixes #1843 (It was also necessary to fix few minor things to make this work correctly) The rules are simple, assuming we have: ```python class A: @AbstractMethod def m(self) -> None: pass class C(A): def m(self) -> None: ... ``` then ```python def fun(cls: Type[A]): cls() # OK fun(A) # Error fun(C) # OK ``` The same applies to variables: ```python var: Type[A] var() # OK var = A # Error var = C # OK ``` Also there is an option for people who want to pass abstract classes around: type aliases, they work as before. For non-abstract ``A``, ``Type[A]`` also works as before. My intuition why you opened #1843 is when someone writes annotation ``Type[A]`` with an abstract ``A``, then most probably one wants a class object that _implements_ a certain protocol, not just inherits from ``A``. NOTE: As discussed in python/peps#224 this behaviour is good for both protocols and usual ABCs.
diff --git a/mypy/checker.py b/mypy/checker.py
@@ -656,6 +656,8 @@ def is_implicit_any(t: Type) -> bool:
                             self.fail(msg, defn)
                             if note:
                                 self.note(note, defn)
+                        if defn.is_class and isinstance(arg_type, CallableType):
+                            arg_type.is_classmethod_class = True
                     elif isinstance(arg_type, TypeVarType):
                         # Refuse covariant parameter type variables
                         # TODO: check recursively for inner type variables
@@ -1208,6 +1210,16 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type
                 else:
                     rvalue_type = self.check_simple_assignment(lvalue_type, rvalue, lvalue)
 
+                # Special case: only non-abstract classes can be assigned to variables
+                # with explicit type Type[A].
+                if (isinstance(rvalue_type, CallableType) and rvalue_type.is_type_obj() and
+                        rvalue_type.type_object().is_abstract and
+                        isinstance(lvalue_type, TypeType) and
+                        isinstance(lvalue_type.item, Instance) and
+                        lvalue_type.item.type.is_abstract):
+                    self.fail("Can only assign non-abstract classes"
+                              " to a variable of type '{}'".format(lvalue_type), rvalue)
+                    return
                 if rvalue_type and infer_lvalue_type:
                     self.binder.assign_type(lvalue,
                                             rvalue_type,
diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py
@@ -354,7 +354,9 @@ def check_call(self, callee: Type, args: List[Expression],
         """
         arg_messages = arg_messages or self.msg
         if isinstance(callee, CallableType):
-            if callee.is_concrete_type_obj() and callee.type_object().is_abstract:
+            if (callee.is_type_obj() and callee.type_object().is_abstract
+                    # Exceptions for Type[...] and classmethod first argument
+                    and not callee.from_type_type and not callee.is_classmethod_class):
                 type = callee.type_object()
                 self.msg.cannot_instantiate_abstract_class(
                     callee.type_object().name(), type.abstract_attributes,
@@ -440,7 +442,10 @@ def analyze_type_type_callee(self, item: Type, context: Context) -> Type:
         if isinstance(item, AnyType):
             return AnyType()
         if isinstance(item, Instance):
-            return type_object_type(item.type, self.named_type)
+            res = type_object_type(item.type, self.named_type)
+            if isinstance(res, CallableType):
+                res = res.copy_modified(from_type_type=True)
+            return res
         if isinstance(item, UnionType):
             return UnionType([self.analyze_type_type_callee(item, context)
                               for item in item.items], item.line)
@@ -838,6 +843,14 @@ def check_arg(self, caller_type: Type, original_caller_type: Type,
         """Check the type of a single argument in a call."""
         if isinstance(caller_type, DeletedType):
             messages.deleted_as_rvalue(caller_type, context)
+        # Only non-abstract class can be given where Type[...] is expected...
+        elif (isinstance(caller_type, CallableType) and isinstance(callee_type, TypeType) and
+              caller_type.is_type_obj() and caller_type.type_object().is_abstract and
+              isinstance(callee_type.item, Instance) and callee_type.item.type.is_abstract and
+              # ...except for classmethod first argument
+              not caller_type.is_classmethod_class):
+            messages.fail("Only non-abstract class can be given where '{}' is expected"
+                          .format(callee_type), context)
         elif not is_subtype(caller_type, callee_type):
             if self.chk.should_suppress_optional_error([caller_type, callee_type]):
                 return
diff --git a/mypy/checkmember.py b/mypy/checkmember.py
@@ -535,7 +535,6 @@ def class_callable(init_type: CallableType, info: TypeInfo, type_type: Instance,
         ret_type=fill_typevars(info), fallback=type_type, name=None, variables=variables,
         special_sig=special_sig)
     c = callable_type.with_name('"{}"'.format(info.name()))
-    c.is_classmethod_class = True
     return c
 
 
diff --git a/mypy/meet.py b/mypy/meet.py
@@ -104,11 +104,12 @@ class C(A, B): ...
     elif isinstance(t, TypeType) or isinstance(s, TypeType):
         # If exactly only one of t or s is a TypeType, check if one of them
         # is an `object` or a `type` and otherwise assume no overlap.
+        one = t if isinstance(t, TypeType) else s
         other = s if isinstance(t, TypeType) else t
         if isinstance(other, Instance):
             return other.type.fullname() in {'builtins.object', 'builtins.type'}
         else:
-            return False
+            return isinstance(other, CallableType) and is_subtype(other, one)
     if experiments.STRICT_OPTIONAL:
         if isinstance(t, NoneTyp) != isinstance(s, NoneTyp):
             # NoneTyp does not overlap with other non-Union types under strict Optional checking
diff --git a/mypy/types.py b/mypy/types.py
@@ -537,6 +537,8 @@ class CallableType(FunctionLike):
     # Defined for signatures that require special handling (currently only value is 'dict'
     # for a signature similar to 'dict')
     special_sig = None  # type: Optional[str]
+    # Was this callable generated by analyzing Type[...] instantiation?
+    from_type_type = False  # type: bool
 
     def __init__(self,
                  arg_types: List[Type],
@@ -553,6 +555,7 @@ def __init__(self,
                  implicit: bool = False,
                  is_classmethod_class: bool = False,
                  special_sig: Optional[str] = None,
+                 from_type_type: bool = False,
                  ) -> None:
         if variables is None:
             variables = []
@@ -571,7 +574,9 @@ def __init__(self,
         self.variables = variables
         self.is_ellipsis_args = is_ellipsis_args
         self.implicit = implicit
+        self.is_classmethod_class = is_classmethod_class
         self.special_sig = special_sig
+        self.from_type_type = from_type_type
         super().__init__(line, column)
 
     def copy_modified(self,
@@ -586,7 +591,8 @@ def copy_modified(self,
                       line: int = _dummy,
                       column: int = _dummy,
                       is_ellipsis_args: bool = _dummy,
-                      special_sig: Optional[str] = _dummy) -> 'CallableType':
+                      special_sig: Optional[str] = _dummy,
+                      from_type_type: bool = _dummy) -> 'CallableType':
         return CallableType(
             arg_types=arg_types if arg_types is not _dummy else self.arg_types,
             arg_kinds=arg_kinds if arg_kinds is not _dummy else self.arg_kinds,
@@ -603,6 +609,7 @@ def copy_modified(self,
             implicit=self.implicit,
             is_classmethod_class=self.is_classmethod_class,
             special_sig=special_sig if special_sig is not _dummy else self.special_sig,
+            from_type_type=from_type_type if from_type_type is not _dummy else self.from_type_type,
         )
 
     def is_type_obj(self) -> bool:
@@ -617,7 +624,10 @@ def type_object(self) -> mypy.nodes.TypeInfo:
         ret = self.ret_type
         if isinstance(ret, TupleType):
             ret = ret.fallback
-        return cast(Instance, ret).type
+        if isinstance(ret, TypeVarType):
+            ret = ret.upper_bound
+        assert isinstance(ret, Instance)
+        return ret.type
 
     def accept(self, visitor: 'TypeVisitor[T]') -> T:
         return visitor.visit_callable_type(self)
diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test
@@ -157,6 +157,97 @@ class B(A): pass
 B()# E: Cannot instantiate abstract class 'B' with abstract attributes 'f' and 'g'
 [out]
 
+[case testInstantiationAbstractsInTypeForFunctions]
+from typing import Type
+from abc import abstractmethod
+
+class A:
+    @abstractmethod
+    def m(self) -> None: pass
+class B(A): pass
+class C(B):
+    def m(self) -> None:
+        pass
+
+def f(cls: Type[A]) -> A:
+    return cls()  # OK
+def g() -> A:
+    return A()  # E: Cannot instantiate abstract class 'A' with abstract attribute 'm'
+
+f(A)  # E: Only non-abstract class can be given where 'Type[__main__.A]' is expected
+f(B)  # E: Only non-abstract class can be given where 'Type[__main__.A]' is expected
+f(C)  # OK
+x: Type[B]
+f(x)  # OK
+[out]
+
+[case testInstantiationAbstractsInTypeForAliases]
+from typing import Type
+from abc import abstractmethod
+
+class A:
+    @abstractmethod
+    def m(self) -> None: pass
+class B(A): pass
+class C(B):
+    def m(self) -> None:
+        pass
+
+def f(cls: Type[A]) -> A:
+    return cls()  # OK
+
+Alias = A
+GoodAlias = C
+Alias()  # E: Cannot instantiate abstract class 'A' with abstract attribute 'm'
+GoodAlias()
+f(Alias)  # E: Only non-abstract class can be given where 'Type[__main__.A]' is expected
+f(GoodAlias)
+[out]
+
+[case testInstantiationAbstractsInTypeForVariables]
+from typing import Type
+from abc import abstractmethod
+
+class A:
+    @abstractmethod
+    def m(self) -> None: pass
+class B(A): pass
+class C(B):
+    def m(self) -> None:
+        pass
+
+var: Type[A]
+var()
+var = A # E: Can only assign non-abstract classes to a variable of type 'Type[__main__.A]'
+var = B # E: Can only assign non-abstract classes to a variable of type 'Type[__main__.A]'
+var = C # OK
+
+var_old = None # type: Type[A] # Old syntax for variable annotations
+var_old()
+var_old = A # E: Can only assign non-abstract classes to a variable of type 'Type[__main__.A]'
+var_old = B # E: Can only assign non-abstract classes to a variable of type 'Type[__main__.A]'
+var_old = C # OK
+[out]
+
+[case testInstantiationAbstractsInTypeForClassMethods]
+from typing import Type
+from abc import abstractmethod
+
+class Logger:
+    @staticmethod
+    def log(a: Type[C]):
+        pass
+class C:
+    @classmethod
+    def action(cls) -> None:
+        cls() #OK for classmethods
+        Logger.log(cls)  #OK for classmethods
+    @abstractmethod
+    def m(self) -> None:
+        pass
+[builtins fixtures/classmethod.pyi]
+[out]
+
 [case testInstantiatingClassWithInheritedAbstractMethodAndSuppression]
 from abc import abstractmethod, ABCMeta
 import typing
