Treat methods with empty bodies in Protocols as abstract (#12118)

Also give a note if return type is compatible with `None` (and strict optional is on).

Author: tmke8

mypy/checker.py

@@ -67,7 +67,9 @@
     CONTRAVARIANT,
     COVARIANT,
     GDEF,
+    IMPLICITLY_ABSTRACT,
     INVARIANT,
+    IS_ABSTRACT,
     LDEF,
     LITERAL_TYPE,
     MDEF,
@@ -115,7 +117,6 @@
     ReturnStmt,
     StarExpr,
     Statement,
-    StrExpr,
     SymbolTable,
     SymbolTableNode,
     TempNode,
@@ -134,7 +135,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 +619,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 in (IS_ABSTRACT, IMPLICITLY_ABSTRACT):
                 num_abstract += 1
         if num_abstract not in (0, len(defn.items)):
             self.fail(message_registry.INCONSISTENT_ABSTRACT_OVERLOAD, defn)
@@ -1171,7 +1172,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 +1340,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,
+    IMPLICITLY_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 implicitly abstract attributes are functions with
+            # None-compatible return types.
+            abstract_attributes: Dict[str, bool] = {}
+            for attr_name, abstract_status in type.abstract_attributes:
+                if abstract_status == IMPLICITLY_ABSTRACT:
+                    abstract_attributes[attr_name] = self.can_return_none(type, attr_name)
+                else:
+                    abstract_attributes[attr_name] = False
+            self.msg.cannot_instantiate_abstract_class(
+                callee.type_object().name, abstract_attributes, context
             )
 
         formal_to_actual = map_actuals_to_formals(
@@ -1335,6 +1345,30 @@ def check_callable_call(
             callee = callee.copy_modified(ret_type=new_ret_type)
         return callee.ret_type, callee
 
+    def can_return_none(self, type: TypeInfo, attr_name: str) -> bool:
+        """Is the given attribute a method with a None-compatible return type?
+
+        Overloads are only checked if there is an implementation.
+        """
+        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:
+                continue
+            node = symnode.node
+            if isinstance(node, OverloadedFuncDef):
+                node = node.impl
+            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)
+        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,24 @@ def cannot_instantiate_abstract_class(
             context,
             code=codes.ABSTRACT,
         )
+        attrs_with_none = [
+            f'"{a}"'
+            for a, implicit_and_can_return_none in abstract_attributes.items()
+            if implicit_and_can_return_none
+        ]
+        if not attrs_with_none:
+            return
+        if len(attrs_with_none) == 1:
+            note = (
+                "The following method was marked implicitly abstract because it has an empty "
+                "function body: {}. If it is not meant to be abstract, explicitly return None."
+            )
+        else:
+            note = (
+                "The following methods were marked implicitly abstract because they have empty "
+                "function bodies: {}. If they are not meant to be abstract, explicitly return None."
+            )
+        self.note(note.format(format_string_list(attrs_with_none)), context, code=codes.ABSTRACT)
 
     def base_class_definitions_incompatible(
         self, name: str, base1: TypeInfo, base2: TypeInfo, context: Context

mypy/nodes.py

@@ -758,7 +758,14 @@ 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
+# Explicitly abstract (with @abstractmethod or overload without implementation)
+IS_ABSTRACT: Final = 1
+# Implicitly abstract: used for functions with trivial bodies defined in Protocols
+IMPLICITLY_ABSTRACT: Final = 2
 
 
 class FuncDef(FuncItem, SymbolNode, Statement):
@@ -771,7 +778,7 @@ class FuncDef(FuncItem, SymbolNode, Statement):
         "_name",
         "is_decorated",
         "is_conditional",
-        "is_abstract",
+        "abstract_status",
         "original_def",
         "deco_line",
     )
@@ -788,7 +795,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 +824,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 +847,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 +2683,9 @@ 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]
+    # List of names of abstract attributes together with their abstract status.
+    # The abstract status must be one of `NOT_ABSTRACT`, `IS_ABSTRACT`, `IMPLICITLY_ABSTRACT`.
+    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 +3045,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"]]