Restore FuncBase.__instancecheck__ and fix decorators another way

@ilevkivskyi, I see your point about how overriding `__instancecheck__`
would create problems with debugging and `mypyc`, so I have implemented
an alternate solution. I have created a `get_callable` method to check
whether a node is callable and, in the case of a decorator, return an
instance of a separate `CallableDecorator` class that inherits from
`FuncItem`. I had originally thought to just have an `is_callable` method
that would test whether a `Node` was either an instance of `FuncBase`
or a callable `Decorator`, but this created problems with type resolution.
This solution is not exactly ideal either, but I think will create fewer
problems. I agree that the best solution would be to remove the `Decorator`
class altogether, but this would be a fairly major refactor. The problem
is that we do need a `Decorator` class in early passes before we have
type information, and we use the `Node.accept` method to add type information
to an instance, we don't have a simple way to replace a node with another
node of a different type. Let me know what you think of this approach.

One other stray observation:
* This fix requires that the decorator is explicitly annotated to return
  a `Callable`. It seems like this should be something we could determine
  through type inference, but I'm not sure how easy that would be to do.

Author: root

mypy/checker.py

@@ -21,8 +21,8 @@
     Context, Decorator, PrintStmt, BreakStmt, PassStmt, ContinueStmt,
     ComparisonExpr, StarExpr, EllipsisExpr, RefExpr, PromoteExpr,
     Import, ImportFrom, ImportAll, ImportBase, TypeAlias,
-    ARG_POS, ARG_STAR, LITERAL_TYPE, MDEF, GDEF,
-    CONTRAVARIANT, COVARIANT, INVARIANT,
+    ARG_POS, ARG_STAR, LITERAL_TYPE, MDEF, GDEF, CallableDecorator,
+    CONTRAVARIANT, COVARIANT, INVARIANT, get_callable
 )
 from mypy import nodes
 from mypy.literals import literal, literal_hash
@@ -1619,11 +1619,15 @@ def check_compatibility(self, name: str, base1: TypeInfo,
         first = base1[name]
         second = base2[name]
         first_type = first.type
-        if first_type is None and isinstance(first.node, FuncBase):
-            first_type = self.function_type(first.node)
+        if first_type is None:
+            method = get_callable(first.node)
+            if method:
+                first_type = self.function_type(method)
         second_type = second.type
-        if second_type is None and isinstance(second.node, FuncBase):
-            second_type = self.function_type(second.node)
+        if second_type is None:
+            method = get_callable(second.node)
+            if method:
+                second_type = self.function_type(method)
         # TODO: What if some classes are generic?
         if (isinstance(first_type, FunctionLike) and
                 isinstance(second_type, FunctionLike)):
@@ -3026,9 +3030,10 @@ def visit_decorator(self, e: Decorator) -> None:
         if isinstance(sig, CallableType):
             if e.func.is_property:
                 assert isinstance(sig, CallableType)
-                e.is_callable = isinstance(sig.ret_type, CallableType)
+                if isinstance(sig.ret_type, CallableType):
+                    e.callable_decorator = CallableDecorator(e)
             else:
-                e.is_callable = True
+                e.callable_decorator = CallableDecorator(e)
         if e.func.info and not e.func.is_dynamic():
             self.check_method_override(e)
 

mypy/nodes.py

@@ -383,18 +383,7 @@ def __str__(self) -> str:
 ]  # type: Final
 
 
-class FuncBaseMeta(type):
-    def __instancecheck__(self, instance: object) -> bool:
-        if isinstance(instance, Decorator):
-            if instance.is_overload:
-                return issubclass(OverloadedFuncDef, self) and instance.is_callable
-            else:
-                return issubclass(FuncBase, self) and instance.is_callable
-        else:
-            return super().__instancecheck__(instance)
-
-
-class FuncBase(Node, metaclass=FuncBaseMeta):
+class FuncBase(Node):
     """Abstract base class for function-like nodes"""
 
     __slots__ = ('type',
@@ -668,7 +657,7 @@ class Decorator(SymbolNode, Statement):
     # TODO: This is mostly used for the type; consider replacing with a 'type' attribute
     var = None  # type: Var                     # Represents the decorated function obj
     is_overload = False
-    is_callable = False
+    callable_decorator = None  # type: Optional[CallableDecorator]
 
     def __init__(self, func: FuncDef, decorators: List[Expression],
                  var: 'Var') -> None:
@@ -684,23 +673,6 @@ def name(self) -> str:
     def fullname(self) -> Bogus[str]:
         return self.func.fullname()
 
-    @property
-    def items(self) -> List[OverloadPart]:
-        assert isinstance(self.func, OverloadedFuncDef)
-        return self.func.items
-
-    @property
-    def is_property(self) -> bool:
-        return self.func.is_property
-
-    @property
-    def is_static(self) -> bool:
-        return self.func.is_static
-
-    @property
-    def is_class(self) -> bool:
-        return self.func.is_class
-
     @property
     def is_final(self) -> bool:
         return self.func.is_final
@@ -733,6 +705,28 @@ def deserialize(cls, data: JsonDict) -> 'Decorator':
         return dec
 
 
+class CallableDecorator(FuncItem):
+    """A wrapper around a Decorator that allows it to be treated as a callable function"""
+    def __init__(self, decorator: Decorator) -> None:
+        super().__init__(decorator.func.arguments, decorator.func.body,
+                         cast('mypy.types.CallableType', decorator.type))
+        self.is_final = decorator.is_final
+        self.is_class = decorator.func.is_class
+        self.is_property = decorator.func.is_property
+        self.is_static = decorator.func.is_static
+        self.is_overload = decorator.func.is_overload
+        self.is_generator = decorator.func.is_generator
+        self.is_async_generator = decorator.func.is_async_generator
+        self.is_awaitable_coroutine = decorator.func.is_awaitable_coroutine
+        self.expanded = decorator.func.expanded
+        self.info = decorator.info
+        self._name = decorator.func.name()
+        self._fullname = decorator.func._fullname
+
+    def name(self) -> str:
+        return self._name
+
+
 VAR_FLAGS = [
     'is_self', 'is_initialized_in_class', 'is_staticmethod',
     'is_classmethod', 'is_property', 'is_settable_property', 'is_suppressed_import',
@@ -2337,11 +2331,7 @@ def has_readable_member(self, name: str) -> bool:
     def get_method(self, name: str) -> Optional[FuncBase]:
         for cls in self.mro:
             if name in cls.names:
-                node = cls.names[name].node
-                if isinstance(node, FuncBase):
-                    return node
-                else:
-                    return None
+                return get_callable(cls.names[name].node)
         return None
 
     def calculate_metaclass_type(self) -> 'Optional[mypy.types.Instance]':
@@ -2964,3 +2954,13 @@ def is_class_var(expr: NameExpr) -> bool:
     if isinstance(expr.node, Var):
         return expr.node.is_classvar
     return False
+
+
+def get_callable(node: Optional[Node]) -> Optional[FuncBase]:
+    """Check if the passed node represents a callable function or funcion-like object"""
+    if isinstance(node, FuncBase):
+        return node
+    elif isinstance(node, Decorator):
+        return node.callable_decorator
+    else:
+        return None

mypy/plugins/attrs.py

@@ -7,9 +7,9 @@
 from mypy.fixup import lookup_qualified_stnode
 from mypy.nodes import (
     Context, Argument, Var, ARG_OPT, ARG_POS, TypeInfo, AssignmentStmt,
-    TupleExpr, ListExpr, NameExpr, CallExpr, RefExpr, FuncBase,
-    is_class_var, TempNode, Decorator, MemberExpr, Expression, FuncDef, Block,
-    PassStmt, SymbolTableNode, MDEF, JsonDict, OverloadedFuncDef
+    TupleExpr, ListExpr, NameExpr, CallExpr, RefExpr, is_class_var,
+    TempNode, Decorator, MemberExpr, Expression, FuncDef, Block,
+    PassStmt, SymbolTableNode, MDEF, JsonDict, OverloadedFuncDef, get_callable
 )
 from mypy.plugins.common import (
     _get_argument, _get_bool_argument, _get_decorator_bool_argument
@@ -405,9 +405,8 @@ def _parse_converter(ctx: 'mypy.plugin.ClassDefContext',
     # TODO: Support complex converters, e.g. lambdas, calls, etc.
     if converter:
         if isinstance(converter, RefExpr) and converter.node:
-            if (isinstance(converter.node, FuncBase)
-                    and converter.node.type
-                    and isinstance(converter.node.type, FunctionLike)):
+            method = get_callable(converter.node)
+            if method and method.type and isinstance(method.type, FunctionLike):
                 return Converter(converter.node.fullname())
             elif isinstance(converter.node, TypeInfo):
                 return Converter(converter.node.fullname())

mypy/plugins/common.py

@@ -1,8 +1,8 @@
 from typing import List, Optional, Any
 
 from mypy.nodes import (
-    ARG_POS, MDEF, Argument, Block, CallExpr, Expression, FuncBase,
-    FuncDef, PassStmt, RefExpr, SymbolTableNode, Var
+    ARG_POS, MDEF, Argument, Block, CallExpr, Expression, FuncDef,
+    PassStmt, RefExpr, SymbolTableNode, Var, get_callable
 )
 from mypy.plugin import ClassDefContext
 from mypy.semanal import set_callable_name
@@ -53,8 +53,9 @@ def _get_argument(call: CallExpr, name: str) -> Optional[Expression]:
     callee_type = None
     # mypyc hack to workaround mypy misunderstanding multiple inheritance (#3603)
     callee_node = call.callee.node  # type: Any
-    if (isinstance(callee_node, (Var, FuncBase))
-            and callee_node.type):
+    if not isinstance(callee_node, Var):
+        callee_node = get_callable(callee_node)
+    if callee_node and callee_node.type:
         callee_node_type = callee_node.type
         if isinstance(callee_node_type, Overloaded):
             # We take the last overload.

mypy/semanal.py

@@ -2518,8 +2518,6 @@ def visit_decorator(self, dec: Decorator) -> None:
                 self.check_decorated_function_is_method('classmethod', dec)
             elif (refers_to_fullname(d, 'builtins.property') or
                   refers_to_fullname(d, 'abc.abstractproperty')):
-                # TODO: Stop treating properties as special cases and treat as generalized
-                # TODO: descriptors
                 removed.append(i)
                 dec.func.is_property = True
                 dec.var.is_property = True

mypy/server/deps.py

@@ -91,9 +91,9 @@ class 'mod.Cls'. This can also refer to an attribute inherited from a
     ImportFrom, CallExpr, CastExpr, TypeVarExpr, TypeApplication, IndexExpr, UnaryExpr, OpExpr,
     ComparisonExpr, GeneratorExpr, DictionaryComprehension, StarExpr, PrintStmt, ForStmt, WithStmt,
     TupleExpr, OperatorAssignmentStmt, DelStmt, YieldFromExpr, Decorator, Block,
-    TypeInfo, FuncBase, OverloadedFuncDef, RefExpr, SuperExpr, Var, NamedTupleExpr, TypedDictExpr,
+    TypeInfo, OverloadedFuncDef, RefExpr, SuperExpr, Var, NamedTupleExpr, TypedDictExpr,
     LDEF, MDEF, GDEF, TypeAliasExpr, NewTypeExpr, ImportAll, EnumCallExpr, AwaitExpr,
-    op_methods, reverse_op_methods, ops_with_inplace_method, unary_op_methods
+    op_methods, reverse_op_methods, ops_with_inplace_method, unary_op_methods, get_callable
 )
 from mypy.traverser import TraverserVisitor
 from mypy.types import (
@@ -127,17 +127,18 @@ def get_dependencies_of_target(module_id: str,
     # TODO: Add tests for this function.
     visitor = DependencyVisitor(type_map, python_version, module_tree.alias_deps)
     visitor.scope.enter_file(module_id)
+    method = get_callable(target)
     if isinstance(target, MypyFile):
         # Only get dependencies of the top-level of the module. Don't recurse into
         # functions.
         for defn in target.defs:
             # TODO: Recurse into top-level statements and class bodies but skip functions.
             if not isinstance(defn, (ClassDef, Decorator, FuncDef, OverloadedFuncDef)):
                 defn.accept(visitor)
-    elif isinstance(target, FuncBase) and target.info:
+    elif method and method.info:
         # It's a method.
         # TODO: Methods in nested classes.
-        visitor.scope.enter_class(target.info)
+        visitor.scope.enter_class(method.info)
         target.accept(visitor)
         visitor.scope.leave()
     else:
@@ -425,8 +426,10 @@ def visit_assignment_stmt(self, o: AssignmentStmt) -> None:
                 if isinstance(rvalue.callee.node, TypeInfo):
                     # use actual __init__ as a dependency source
                     init = rvalue.callee.node.get('__init__')
-                    if init and isinstance(init.node, FuncBase):
-                        fname = init.node.fullname()
+                    if init:
+                        method = get_callable(init.node)
+                        if method:
+                            fname = method.fullname()
                 else:
                     fname = rvalue.callee.fullname
                 if fname is None: