Repository: python/mypy
Pull request: Support decorated constructors
Fixes https://github.com/python/mypy/issues/1706
Fixes https://github.com/python/mypy/issues/5398

Previously, decorated constructors either resulted in silent `Any` types for class objects (in case of a decorated `__init__()`) or incorrect signatures (in case of a decorated `__new__()`).

This PR adds support for decorated constructors, implementation is straightforward.

This PR supersedes https://github.com/python/mypy/pull/5798.



Problem statement:
Adding @abstractmethod __init__ to ABC breaks staticmethods
I was trying to create an ABC that also worked as a factory.

```
from abc import (
    ABCMeta,
    abstractmethod,
)

class WidgetWithInit(object):
    __metaclass__ = ABCMeta

    @abstractmethod
    def __init__(self, foo, bar):
        # type: (int, int) -> None
        pass

    @staticmethod
    def add(foo, bar):
        # type: (int, int) -> int
        return foo + bar

    @classmethod
    def add_class(cls, foo, bar):
        # type: (int, int) -> int
        return foo + bar

WidgetWithInit.add(1, '2')
WidgetWithInit.add_class(1, '2')

class WidgetNoInit(object):
    __metaclass__ = ABCMeta

    @staticmethod
    def add(foo, bar):
        # type: (int, int) -> int
        return foo + bar

    @classmethod
    def add_class(cls, foo, bar):
        # type: (int, int) -> int
        return foo + bar

WidgetNoInit.add(1, '2')
WidgetNoInit.add_class(1, '2')
```

MyPy will respond:

```
rt6.py:40: error: Argument 2 to "add" of "WidgetNoInit" has incompatible type "str"; expected "int"
rt6.py:41: error: Argument 2 to "add_class" of "WidgetNoInit" has incompatible type "str"; expected "int"
```

So adding the `def __init__` seems to make the staticmethod and classmethod have a type of Any.

Fortunately I can work around it by not defining **init** in the ABC, and having my `def create` enforce the signature.


Decorator on __init__ causes loss of type info
Here's a simple example:

```
from __future__ import annotations
from typing import Callable

def decorator(method: Callable[[Foo], None]) -> Callable[[Foo], None]:
    return method

class Foo():
    @decorator
    def __init__(self):
        pass

reveal_type(Foo.__init__)
```

The output of mypy is:

```
% mypy --version
mypy 0.620
% mypy foo.py
foo.py:13: error: Revealed type is 'Any'
```

This seems to be specific to methods as similar type inferences work fine for functions outside of the class scope.

Hints from issue discussion:
In fact mypy thinks the expression `WidgetNoInit` already has type `Any` with the definition

``` py
class WidgetWithInit(object):
    @abstractmethod
    def __init__(self, foo, bar):
        # type: (int, int) -> None
        pass
```

In fact `__init__` having any decorator will result in the class being treated as `Any`, since `type_object_type` starts like this:

``` py
    init_method = info.get_method('__init__')
    if not init_method:
        # Must be an invalid class definition.
        return AnyType()
    else: ...
```

and `get_method` doesn't return decorated things.

Here's a possible test case

``` py
[case testAbstractInit]
from abc import abstractmethod, ABCMeta
class A(metaclass=ABCMeta):
    @abstractmethod
    def __init__(self, a: int) -> None:
        pass
class B(A):
    pass
class C(B):
    def __init__(self, a: int) -> None:
        self.c = a
a = A(1) # E: Cannot instantiate abstract class 'A' with abstract attribute '__init__'
A.c # E: "A" has no attribute "c"
b = B(2) # E: Cannot instantiate abstract class 'B' with abstract attribute '__init__'
B.c # E: "B" has no attribute "c"
c = C(3)
c.c
C.c
```

But I gave up on trying to fix this because the logic around recognizing decorators is too hairy.

I think this bug is deep and important. I've encountered something similar in #3227. The logic that handles decorators seems to be flawed in a way which is difficult to fix.
True, decorators are one of mypy's weak spots. We're slowly chipping away at it (e.g. #3918) but it's not easy because of special-casing in various places (both in Python and in mypy).
Try again after adding `-> None` to the `__init__` signature.
Adding annotations to `__init__` does not change the result; here's the updated code:

```
from __future__ import annotations

from typing import Callable

def decorator(method: Callable[[Foo], None]) -> Callable[[Foo], None]:
    return method

class Foo():
    @decorator
    def __init__(self: Foo) -> None:
        pass

reveal_type(Foo.__init__)
```

I get the same result (the revealed type is `Any`). If I change the name of the method, the type inference works correctly, so this issue seems to be specific to `__init__`.
`__init__` is special in a number of ways, but this still seems wrong.
I just stumbled at this again. Even precisely typed decorator on `__init__` not just erases types, it completely erases the signature of `__init__`. For example:
```python
from typing import Any, Callable, TypeVar

_C = TypeVar("_C", bound=Callable[..., None])

def decorator(f: _C) -> _C:
    return f
    
class C:
    @decorator
    def __init__(self, x: int) -> None:
        ...
        
C("whatever", "works", "now") + 42
```
Increased priority to "high" since this behavior is very unexpected and can hide bugs.
It looks like #1706 is a special case of this. FYI, I am working on a PR to fix this. It seems like the problem is that quite a few places check whether a node represents a callable function by checking if it is an instance of `FuncBase`, which `Decorator` is not. We don't want to make `Decorator` inherit from `FuncBase`, because whether it is callable or not depends on the type returned by the decorator, and we don't want to assume that it is, so my thought was to override `__instancecheck__` in `FuncBase` to specifically check and return true if the decorated function is callable. There I still some issues I need to work out with my implementation—my current version has some false positives that I am trying to track down, but if anyone would like to offer input on what I'm doing, my code is at https://github.com/TV4Fun/mypy/tree/fix_decorators

Commit message: Flag bad decorators for constructors

Patch to review:
diff --git a/mypy/checker.py b/mypy/checker.py
index 83dc2ead9035..83362dbcb8d1 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -3088,6 +3088,10 @@ def visit_decorator(self, e: Decorator) -> None:
         if e.func.info and not e.func.is_dynamic():
             self.check_method_override(e)
 
+        if e.func.info and e.func.name() in ('__init__', '__new__'):
+            if e.type and not isinstance(e.type, (FunctionLike, AnyType)):
+                self.fail(message_registry.BAD_CONSTRUCTOR_TYPE, e)
+
     def check_for_untyped_decorator(self,
                                     func: FuncDef,
                                     dec_type: Type,
diff --git a/mypy/checkmember.py b/mypy/checkmember.py
index 2117194e7fba..0b8dc03644fa 100644
--- a/mypy/checkmember.py
+++ b/mypy/checkmember.py
@@ -1,6 +1,6 @@
 """Type checking of attribute access"""
 
-from typing import cast, Callable, List, Optional, TypeVar
+from typing import cast, Callable, List, Optional, TypeVar, Union
 
 from mypy.types import (
     Type, Instance, AnyType, TupleType, TypedDictType, CallableType, FunctionLike, TypeVarDef,
@@ -763,25 +763,32 @@ def type_object_type(info: TypeInfo, builtin_type: Callable[[str], Instance]) ->
 
     # We take the type from whichever of __init__ and __new__ is first
     # in the MRO, preferring __init__ if there is a tie.
-    init_method = info.get_method('__init__')
-    new_method = info.get_method('__new__')
-    if not init_method:
+    init_method = info.get('__init__')
+    new_method = info.get('__new__')
+    if not init_method or not is_valid_constructor(init_method.node):
         # Must be an invalid class definition.
         return AnyType(TypeOfAny.from_error)
     # There *should* always be a __new__ method except the test stubs
     # lack it, so just copy init_method in that situation
     new_method = new_method or init_method
+    if not is_valid_constructor(new_method.node):
+        # Must be an invalid class definition.
+        return AnyType(TypeOfAny.from_error)
+
+    # The two is_valid_constructor() checks ensure this.
+    assert isinstance(new_method.node, (FuncBase, Decorator))
+    assert isinstance(init_method.node, (FuncBase, Decorator))
 
-    init_index = info.mro.index(init_method.info)
-    new_index = info.mro.index(new_method.info)
+    init_index = info.mro.index(init_method.node.info)
+    new_index = info.mro.index(new_method.node.info)
 
     fallback = info.metaclass_type or builtin_type('builtins.type')
     if init_index < new_index:
-        method = init_method
+        method = init_method.node  # type: Union[FuncBase, Decorator]
     elif init_index > new_index:
-        method = new_method
+        method = new_method.node
     else:
-        if init_method.info.fullname() == 'builtins.object':
+        if init_method.node.info.fullname() == 'builtins.object':
             # Both are defined by object.  But if we've got a bogus
             # base class, we can't know for sure, so check for that.
             if info.fallback_to_any:
@@ -797,17 +804,35 @@ def type_object_type(info: TypeInfo, builtin_type: Callable[[str], Instance]) ->
         # Otherwise prefer __init__ in a tie. It isn't clear that this
         # is the right thing, but __new__ caused problems with
         # typeshed (#5647).
-        method = init_method
+        method = init_method.node
     # Construct callable type based on signature of __init__. Adjust
     # return type and insert type arguments.
-    return type_object_type_from_function(method, info, fallback)
+    if isinstance(method, FuncBase):
+        t = function_type(method, fallback)
+    else:
+        assert isinstance(method.type, FunctionLike)  # is_valid_constructor() ensures this
+        t = method.type
+    signature = bind_self(t)
+    return type_object_type_from_function(signature, info, method.info, fallback)
+
 
+def is_valid_constructor(n: Optional[SymbolNode]) -> bool:
+    """Does this node represents a valid constructor method?
 
-def type_object_type_from_function(init_or_new: FuncBase,
+    This includes nomral functions, overloaded functions, and decorators
+    that return a callable type.
+    """
+    if isinstance(n, FuncBase):
+        return True
+    if isinstance(n, Decorator):
+        return isinstance(n.type, FunctionLike)
+    return False
+
+
+def type_object_type_from_function(signature: FunctionLike,
                                    info: TypeInfo,
+                                   def_info: TypeInfo,
                                    fallback: Instance) -> FunctionLike:
-    signature = bind_self(function_type(init_or_new, fallback))
-
     # The __init__ method might come from a generic superclass
     # (init_or_new.info) with type variables that do not map
     # identically to the type variables of the class being constructed
@@ -818,9 +843,9 @@ def type_object_type_from_function(init_or_new: FuncBase,
     #
     # We need to first map B's __init__ to the type (List[T]) -> None.
     signature = cast(FunctionLike,
-                     map_type_from_supertype(signature, info, init_or_new.info))
+                     map_type_from_supertype(signature, info, def_info))
     special_sig = None  # type: Optional[str]
-    if init_or_new.info.fullname() == 'builtins.dict':
+    if def_info.fullname() == 'builtins.dict':
         # Special signature!
         special_sig = 'dict'
 
diff --git a/mypy/message_registry.py b/mypy/message_registry.py
index 7279ea20dca5..020cea73f1c1 100644
--- a/mypy/message_registry.py
+++ b/mypy/message_registry.py
@@ -52,6 +52,7 @@
 INVALID_SLICE_INDEX = 'Slice index must be an integer or None'  # type: Final
 CANNOT_INFER_LAMBDA_TYPE = 'Cannot infer type of lambda'  # type: Final
 CANNOT_ACCESS_INIT = 'Cannot access "__init__" directly'  # type: Final
+BAD_CONSTRUCTOR_TYPE = 'Unsupported decorated constructor type'  # type: Final
 CANNOT_ASSIGN_TO_METHOD = 'Cannot assign to a method'  # type: Final
 CANNOT_ASSIGN_TO_TYPE = 'Cannot assign to a type'  # type: Final
 INCONSISTENT_ABSTRACT_OVERLOAD = \
diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test
index 8710a7ee6b3c..2f782adf18ee 100644
--- a/test-data/unit/check-classes.test
+++ b/test-data/unit/check-classes.test
@@ -5812,3 +5812,53 @@ def test() -> None:
         return
     reveal_type(x)  # E: Revealed type is 'Union[Type[__main__.One], Type[__main__.Other]]'
 [builtins fixtures/isinstancelist.pyi]
+
+[case testAbstractInit]
+from abc import abstractmethod, ABCMeta
+class A(metaclass=ABCMeta):
+    @abstractmethod
+    def __init__(self, a: int) -> None:
+        pass
+class B(A):
+    pass
+class C(B):
+    def __init__(self, a: int) -> None:
+        self.c = a
+a = A(1) # E: Cannot instantiate abstract class 'A' with abstract attribute '__init__'
+A.c # E: "Type[A]" has no attribute "c"
+b = B(2) # E: Cannot instantiate abstract class 'B' with abstract attribute '__init__'
+B.c # E: "Type[B]" has no attribute "c"
+c = C(3)
+c.c
+C.c
+
+[case testDecoratedConstructors]
+from typing import TypeVar, Callable, Any
+
+F = TypeVar('F', bound=Callable[..., Any])
+
+def dec(f: F) -> F: ...
+
+class A:
+    @dec
+    def __init__(self, x: int) -> None: ...
+
+class B:
+    @dec
+    def __new__(cls, x: int) -> B: ...
+
+reveal_type(A)  # E: Revealed type is 'def (x: builtins.int) -> __main__.A'
+reveal_type(B)  # E: Revealed type is 'def (x: builtins.int) -> __main__.B'
+
+[case testDecoratedConstructorsBad]
+from typing import Callable, Any
+
+def dec(f: Callable[[Any, int], Any]) -> int: ...
+
+class A:
+    @dec  # E: Unsupported decorated constructor type
+    def __init__(self, x: int) -> None: ...
+
+class B:
+    @dec  # E: Unsupported decorated constructor type
+    def __new__(cls, x: int) -> B: ...


Code review comments:
diff_hunk: @@ -797,17 +804,35 @@ def type_object_type(info: TypeInfo, builtin_type: Callable[[str], Instance]) ->
         # Otherwise prefer __init__ in a tie. It isn't clear that this
         # is the right thing, but __new__ caused problems with
         # typeshed (#5647).
-        method = init_method
+        method = init_method.node
     # Construct callable type based on signature of __init__. Adjust
     # return type and insert type arguments.
-    return type_object_type_from_function(method, info, fallback)
+    if isinstance(method, FuncBase):
+        t = function_type(method, fallback)
+    else:
+        assert isinstance(method.type, FunctionLike)  # is_valid_constructor() ensures this
+        t = method.type
+    signature = bind_self(t)
path: mypy/checkmember.py
text: @user1:
Any particular reason for moving this `bind_self` out of `type_object_type_from_function`? I don't have any objection, I'm just wondering if there is a technical reason I'm missing or you just like it more.

@author:
No particular reason. I think I started refactoring by pulling `function_type()` out of there, and pulled whole line out of there. But looking again at it I think it makes sense to keep bind and map together, so I will put it back.

diff_hunk: @@ -797,17 +804,35 @@ def type_object_type(info: TypeInfo, builtin_type: Callable[[str], Instance]) ->
         # Otherwise prefer __init__ in a tie. It isn't clear that this
         # is the right thing, but __new__ caused problems with
         # typeshed (#5647).
-        method = init_method
+        method = init_method.node
     # Construct callable type based on signature of __init__. Adjust
     # return type and insert type arguments.
-    return type_object_type_from_function(method, info, fallback)
+    if isinstance(method, FuncBase):
+        t = function_type(method, fallback)
+    else:
+        assert isinstance(method.type, FunctionLike)  # is_valid_constructor() ensures this
+        t = method.type
+    signature = bind_self(t)
+    return type_object_type_from_function(signature, info, method.info, fallback)
+
 
+def is_valid_constructor(n: Optional[SymbolNode]) -> bool:
+    """Does this node represents a valid constructor method?
 
-def type_object_type_from_function(init_or_new: FuncBase,
+    This includes nomral functions, overloaded functions, and decorators
path: mypy/checkmember.py
text: @user1:
typo - "nomral"