Treat methods with empty bodies in Protocols as abstract

mypy/checker.py

@@ -115,7 +115,6 @@
     ReturnStmt,
     StarExpr,
     Statement,
-    StrExpr,
     SymbolTable,
     SymbolTableNode,
     TempNode,
@@ -134,7 +133,7 @@
 from mypy.plugin import CheckerPluginInterface, Plugin
 from mypy.sametypes import is_same_type
 from mypy.scope import Scope
-from mypy.semanal import refers_to_fullname, set_callable_name
+from mypy.semanal import is_trivial_body, refers_to_fullname, set_callable_name
 from mypy.semanal_enum import ENUM_BASES, ENUM_SPECIAL_PROPS
 from mypy.sharedparse import BINARY_MAGIC_METHODS
 from mypy.state import state
@@ -618,7 +617,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
         for fdef in defn.items:
             assert isinstance(fdef, Decorator)
             self.check_func_item(fdef.func, name=fdef.func.name)
-            if fdef.func.is_abstract:
+            if fdef.func.abstract_status:
                 num_abstract += 1
         if num_abstract not in (0, len(defn.items)):
             self.fail(message_registry.INCONSISTENT_ABSTRACT_OVERLOAD, defn)
@@ -1171,7 +1170,7 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str])
                     item.arguments[i].variable.type = arg_type
 
                 # Type check initialization expressions.
-                body_is_trivial = self.is_trivial_body(defn.body)
+                body_is_trivial = is_trivial_body(defn.body)
                 self.check_default_args(item, body_is_trivial)
 
             # Type check body in a new scope.
@@ -1339,49 +1338,6 @@ def check___new___signature(self, fdef: FuncDef, typ: CallableType) -> None:
                 "but must return a subtype of",
             )
 
-    def is_trivial_body(self, block: Block) -> bool:
-        """Returns 'true' if the given body is "trivial" -- if it contains just a "pass",
-        "..." (ellipsis), or "raise NotImplementedError()". A trivial body may also
-        start with a statement containing just a string (e.g. a docstring).
-
-        Note: functions that raise other kinds of exceptions do not count as
-        "trivial". We use this function to help us determine when it's ok to
-        relax certain checks on body, but functions that raise arbitrary exceptions
-        are more likely to do non-trivial work. For example:
-
-           def halt(self, reason: str = ...) -> NoReturn:
-               raise MyCustomError("Fatal error: " + reason, self.line, self.context)
-
-        A function that raises just NotImplementedError is much less likely to be
-        this complex.
-        """
-        body = block.body
-
-        # Skip a docstring
-        if body and isinstance(body[0], ExpressionStmt) and isinstance(body[0].expr, StrExpr):
-            body = block.body[1:]
-
-        if len(body) == 0:
-            # There's only a docstring (or no body at all).
-            return True
-        elif len(body) > 1:
-            return False
-
-        stmt = body[0]
-
-        if isinstance(stmt, RaiseStmt):
-            expr = stmt.expr
-            if expr is None:
-                return False
-            if isinstance(expr, CallExpr):
-                expr = expr.callee
-
-            return isinstance(expr, NameExpr) and expr.fullname == "builtins.NotImplementedError"
-
-        return isinstance(stmt, PassStmt) or (
-            isinstance(stmt, ExpressionStmt) and isinstance(stmt.expr, EllipsisExpr)
-        )
-
     def check_reverse_op_method(
         self, defn: FuncItem, reverse_type: CallableType, reverse_name: str, context: Context
     ) -> None:

mypy/checkexpr.py

@@ -27,6 +27,7 @@
     ARG_POS,
     ARG_STAR,
     ARG_STAR2,
+    IS_ABSTRACT,
     LITERAL_TYPE,
     REVEAL_TYPE,
     ArgKind,
@@ -96,6 +97,7 @@
 )
 from mypy.sametypes import is_same_type
 from mypy.semanal_enum import ENUM_BASES
+from mypy.state import state
 from mypy.subtypes import is_equivalent, is_subtype, non_method_protocol_members
 from mypy.traverser import has_await_expression
 from mypy.typeanal import (
@@ -1236,24 +1238,32 @@ def check_callable_call(
 
         if (
             callee.is_type_obj()
-            and callee.type_object().is_abstract
+            and callee.type_object().is_protocol
             # Exception for Type[...]
             and not callee.from_type_type
-            and not callee.type_object().fallback_to_any
         ):
-            type = callee.type_object()
-            self.msg.cannot_instantiate_abstract_class(
-                callee.type_object().name, type.abstract_attributes, context
+            self.chk.fail(
+                message_registry.CANNOT_INSTANTIATE_PROTOCOL.format(callee.type_object().name),
+                context,
             )
         elif (
             callee.is_type_obj()
-            and callee.type_object().is_protocol
+            and callee.type_object().is_abstract
             # Exception for Type[...]
             and not callee.from_type_type
+            and not callee.type_object().fallback_to_any
         ):
-            self.chk.fail(
-                message_registry.CANNOT_INSTANTIATE_PROTOCOL.format(callee.type_object().name),
-                context,
+            type = callee.type_object()
+            # Determine whether the abstract attributes are functions with
+            # None-compatible return types and whether they were defined in a protocol.
+            is_none_ret_from_prot: Dict[str, bool] = {}
+            for attr_name, abstract_status in type.abstract_attributes:
+                if abstract_status == IS_ABSTRACT:
+                    is_none_ret_from_prot[attr_name] = False
+                    continue
+                is_none_ret_from_prot[attr_name] = self._check_non_ret_from_prot(type, attr_name)
+            self.msg.cannot_instantiate_abstract_class(
+                callee.type_object().name, is_none_ret_from_prot, context
             )
 
         formal_to_actual = map_actuals_to_formals(
@@ -1335,6 +1345,46 @@ def check_callable_call(
             callee = callee.copy_modified(ret_type=new_ret_type)
         return callee.ret_type, callee
 
+    def _check_non_ret_from_prot(self, type: TypeInfo, attr_name: str) -> bool:
+        """Determine whether the attribute is a function with None-compatible return type.
+
+        Additionally, check whether the function was defined in a protocol.
+        """
+        if not state.strict_optional:
+            # If strict-optional is not set, is_subtype(NoneType(), T) is always True.
+            # So, we cannot do anything useful here in that case.
+            return False
+        for base in type.mro:
+            symnode = base.names.get(attr_name)
+            if symnode is None or not base.is_protocol:
+                continue
+            node = symnode.node
+            if isinstance(node, OverloadedFuncDef):
+                if node.impl is not None:
+                    typ = get_proper_type(node.impl.type)
+                    assert isinstance(typ, CallableType)
+                    return is_subtype(NoneType(), typ.ret_type)
+                for func in node.items:
+                    if isinstance(func, Decorator):
+                        func = func.func
+                    assert isinstance(func.type, CallableType)
+                    if is_subtype(NoneType(), func.type.ret_type):
+                        return True
+                    if func.is_property:
+                        return False  # Only check the first overload of properties.
+                else:
+                    return False
+            if isinstance(node, Decorator):
+                node = node.func
+            if isinstance(node, FuncDef):
+                if node.type is not None:
+                    assert isinstance(node.type, CallableType)
+                    return is_subtype(NoneType(), node.type.ret_type)
+                else:
+                    # No type annotations are available
+                    return True
+        return False
+
     def analyze_type_type_callee(self, item: ProperType, context: Context) -> Type:
         """Analyze the callee X in X(...) where X is Type[item].
 

mypy/messages.py

@@ -1321,7 +1321,7 @@ def incompatible_conditional_function_def(self, defn: FuncDef) -> None:
         self.fail("All conditional function variants must have identical " "signatures", defn)
 
     def cannot_instantiate_abstract_class(
-        self, class_name: str, abstract_attributes: List[str], context: Context
+        self, class_name: str, abstract_attributes: Dict[str, bool], context: Context
     ) -> None:
         attrs = format_string_list([f'"{a}"' for a in abstract_attributes])
         self.fail(
@@ -1330,6 +1330,15 @@ def cannot_instantiate_abstract_class(
             context,
             code=codes.ABSTRACT,
         )
+        for a, is_none_ret_and_prot in abstract_attributes.items():
+            if not is_none_ret_and_prot:
+                continue
+            self.note(
+                '"{}" was implicitly marked abstract because it has an empty function body. '
+                "If it is not meant to be abstract, explicitly return None.".format(a),
+                context,
+                code=codes.OVERRIDE,
+            )
 
     def base_class_definitions_incompatible(
         self, name: str, base1: TypeInfo, base2: TypeInfo, context: Context

mypy/nodes.py

@@ -758,7 +758,12 @@ def is_dynamic(self) -> bool:
         return self.type is None
 
 
-FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional", "is_abstract"]
+FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional"]
+
+# Abstract status of a function
+NOT_ABSTRACT: Final = 0
+IS_ABSTRACT: Final = 1
+IMPLICITLY_ABSTRACT: Final = 2
 
 
 class FuncDef(FuncItem, SymbolNode, Statement):
@@ -771,7 +776,7 @@ class FuncDef(FuncItem, SymbolNode, Statement):
         "_name",
         "is_decorated",
         "is_conditional",
-        "is_abstract",
+        "abstract_status",
         "original_def",
         "deco_line",
     )
@@ -788,7 +793,7 @@ def __init__(
         self._name = name
         self.is_decorated = False
         self.is_conditional = False  # Defined conditionally (within block)?
-        self.is_abstract = False
+        self.abstract_status = NOT_ABSTRACT
         self.is_final = False
         # Original conditional definition
         self.original_def: Union[None, FuncDef, Var, Decorator] = None
@@ -817,6 +822,7 @@ def serialize(self) -> JsonDict:
             "arg_kinds": [int(x.value) for x in self.arg_kinds],
             "type": None if self.type is None else self.type.serialize(),
             "flags": get_flags(self, FUNCDEF_FLAGS),
+            "abstract_status": self.abstract_status,
             # TODO: Do we need expanded, original_def?
         }
 
@@ -839,6 +845,7 @@ def deserialize(cls, data: JsonDict) -> "FuncDef":
         # NOTE: ret.info is set in the fixup phase.
         ret.arg_names = data["arg_names"]
         ret.arg_kinds = [ArgKind(x) for x in data["arg_kinds"]]
+        ret.abstract_status = data["abstract_status"]
         # Leave these uninitialized so that future uses will trigger an error
         del ret.arguments
         del ret.max_pos
@@ -2674,7 +2681,7 @@ class is generic then it will be a type constructor of higher kind.
     is_abstract: bool  # Does the class have any abstract attributes?
     is_protocol: bool  # Is this a protocol class?
     runtime_protocol: bool  # Does this protocol support isinstance checks?
-    abstract_attributes: List[str]
+    abstract_attributes: List[Tuple[str, int]]
     deletable_attributes: List[str]  # Used by mypyc only
     # Does this type have concrete `__slots__` defined?
     # If class does not have `__slots__` defined then it is `None`,
@@ -3034,7 +3041,7 @@ def deserialize(cls, data: JsonDict) -> "TypeInfo":
         ti = TypeInfo(names, defn, module_name)
         ti._fullname = data["fullname"]
         # TODO: Is there a reason to reconstruct ti.subtypes?
-        ti.abstract_attributes = data["abstract_attributes"]
+        ti.abstract_attributes = [(attr[0], attr[1]) for attr in data["abstract_attributes"]]
         ti.type_vars = data["type_vars"]
         ti.has_param_spec_type = data["has_param_spec_type"]
         ti.bases = [mypy.types.Instance.deserialize(b) for b in data["bases"]]