Skip to content

Commit

Permalink
Allow instantiation of Type[A], if A is abstract (#2853)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ilevkivskyi authored and gvanrossum committed Mar 27, 2017
1 parent e674e25 commit 72967fd
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 6 deletions.
12 changes: 12 additions & 0 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 15 additions & 2 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
3 changes: 2 additions & 1 deletion mypy/meet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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 = []
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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:
Expand All @@ -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)
Expand Down
91 changes: 91 additions & 0 deletions test-data/unit/check-abstract.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 72967fd

Please sign in to comment.