Treat methods with empty bodies in Protocols as abstract

Closes python#8005
Closes python#8926

Methods in Protocols are considered abstract if they have an empty
function body, have a return type that is not compatible with `None`,
and are not in a stub file.

Author: tmke8

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
@@ -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,48 +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

mypy/semanal.py

@@ -145,6 +145,7 @@
     OverloadedFuncDef,
     OverloadPart,
     ParamSpecExpr,
+    PassStmt,
     PlaceholderNode,
     PromoteExpr,
     RaiseStmt,
@@ -837,6 +838,16 @@ def analyze_func_def(self, defn: FuncDef) -> None:
 
         self.analyze_arg_initializers(defn)
         self.analyze_function_body(defn)
+
+        # Mark protocol methods with empty bodies and None-incompatible return types as abstract.
+        if self.is_class_scope() and defn.type is not None:
+            assert self.type is not None and isinstance(defn.type, CallableType)
+            if (not self.is_stub_file and self.type.is_protocol and
+                    (not isinstance(self.scope.function, OverloadedFuncDef)
+                     or defn.is_property) and
+                    not can_be_none(defn.type.ret_type) and is_trivial_body(defn.body)):
+                defn.is_abstract = True
+
         if (
             defn.is_coroutine
             and isinstance(defn.type, CallableType)
@@ -975,6 +986,21 @@ def analyze_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
         # We know this is an overload def. Infer properties and perform some checks.
         self.process_final_in_overload(defn)
         self.process_static_or_class_method_in_overload(defn)
+        if defn.impl:
+            self.process_overload_impl(defn)
+
+    def process_overload_impl(self, defn: OverloadedFuncDef) -> None:
+        """Set flags for an overload implementation.
+
+        Currently, this checks for a trivial body in protocols classes,
+        where it makes the method implicitly abstract.
+        """
+        assert defn.impl is not None
+        impl = defn.impl if isinstance(defn.impl, FuncDef) else defn.impl.func
+        if is_trivial_body(impl.body) and self.is_class_scope() and not self.is_stub_file:
+            assert self.type is not None
+            if self.type.is_protocol:
+                impl.is_abstract = True
 
     def analyze_overload_sigs_and_impl(
         self, defn: OverloadedFuncDef
@@ -1052,7 +1078,8 @@ def handle_missing_overload_implementation(self, defn: OverloadedFuncDef) -> Non
         """Generate error about missing overload implementation (only if needed)."""
         if not self.is_stub_file:
             if self.type and self.type.is_protocol and not self.is_func_scope():
-                # An overloaded protocol method doesn't need an implementation.
+                # An overloaded protocol method doesn't need an implementation,
+                # but if it doesn't have one, then it is considered implicitly abstract.
                 for item in defn.items:
                     if isinstance(item, Decorator):
                         item.func.is_abstract = True
@@ -6033,3 +6060,57 @@ def is_same_symbol(a: Optional[SymbolNode], b: Optional[SymbolNode]) -> bool:
         or (isinstance(a, PlaceholderNode) and isinstance(b, PlaceholderNode))
         or is_same_var_from_getattr(a, b)
     )
+
+
+def is_trivial_body(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 can_be_none(t: Type) -> bool:
+    """Can a variable of the given type be None?"""
+    t = get_proper_type(t)
+    return (
+        isinstance(t, NoneType) or
+        isinstance(t, AnyType) or
+        (isinstance(t, UnionType) and any(can_be_none(ut) for ut in t.items))
+    )

mypy/semanal_classprop.py

@@ -11,6 +11,7 @@
 from mypy.nodes import (
     CallExpr,
     Decorator,
+    FuncDef,
     Node,
     OverloadedFuncDef,
     PromoteExpr,
@@ -69,8 +70,9 @@ def calculate_class_abstract_status(typ: TypeInfo, is_stub_file: bool, errors: E
             else:
                 func = node
             if isinstance(func, Decorator):
-                fdef = func.func
-                if fdef.is_abstract and name not in concrete:
+                func = func.func
+            if isinstance(func, FuncDef):
+                if func.is_abstract and name not in concrete:
                     typ.is_abstract = True
                     abstract.append(name)
                     if base is typ:

test-data/unit/check-protocols.test

@@ -2924,3 +2924,152 @@ class C:
 def round(number: SupportsRound[_T], ndigits: int) -> _T: ...
 
 round(C(), 1)
+
+[case testEmptyBodyImplicitlyAbstractProtocol]
+from typing import Protocol, overload, Union
+
+class P1(Protocol):
+    def meth(self) -> int: ...
+class B1(P1): ...
+class C1(P1):
+    def meth(self) -> int:
+        return 0
+B1()  # E: Cannot instantiate abstract class "B1" with abstract attribute "meth"
+C1()
+
+class P2(Protocol):
+    @classmethod
+    def meth(cls) -> int: ...
+class B2(P2): ...
+class C2(P2):
+    @classmethod
+    def meth(cls) -> int:
+        return 0
+B2()  # E: Cannot instantiate abstract class "B2" with abstract attribute "meth"
+C2()
+
+class P3(Protocol):
+    @overload
+    def meth(self, x: int) -> int: ...
+    @overload
+    def meth(self, x: str) -> str: ...
+class B3(P3): ...
+class C3(P3):
+    @overload
+    def meth(self, x: int) -> int: ...
+    @overload
+    def meth(self, x: str) -> str: ...
+    def meth(self, x: Union[int, str]) -> Union[int, str]:
+        return 0
+B3()  # E: Cannot instantiate abstract class "B3" with abstract attribute "meth"
+C3()
+[builtins fixtures/classmethod.pyi]
+
+[case testEmptyBodyImplicitlyAbstractProtocolProperty]
+from typing import Protocol
+
+class P1(Protocol):
+    @property
+    def attr(self) -> int: ...
+class B1(P1): ...
+class C1(P1):
+    @property
+    def attr(self) -> int:
+        return 0
+B1()  # E: Cannot instantiate abstract class "B1" with abstract attribute "attr"
+C1()
+
+class P2(Protocol):
+    @property
+    def attr(self) -> int: ...
+    @attr.setter
+    def attr(self, value: int) -> None: ...
+class B2(P2): ...
+class C2(P2):
+    @property
+    def attr(self) -> int: return 0
+    @attr.setter
+    def attr(self, value: int) -> None: pass
+B2()  # E: Cannot instantiate abstract class "B2" with abstract attribute "attr"
+C2()
+[builtins fixtures/property.pyi]
+
+[case testEmptyBodyImplicitlyAbstractProtocolStub]
+from stub import P1, P2, P3, P4
+
+class B1(P1): ...
+class B2(P2): ...
+class B3(P3): ...
+class B4(P4): ...
+
+B1()
+B2()
+B3()
+B4()  # E: Cannot instantiate abstract class "B4" with abstract attribute "meth"
+
+[file stub.pyi]
+from typing import Protocol, overload, Union
+from abc import abstractmethod
+
+class P1(Protocol):
+    def meth(self) -> int: ...
+
+class P2(Protocol):
+    @classmethod
+    def meth(cls) -> int: ...
+
+class P3(Protocol):
+    @overload
+    def meth(self, x: int) -> int: ...
+    @overload
+    def meth(self, x: str) -> str: ...
+
+class P4(Protocol):
+    @abstractmethod
+    def meth(self) -> int: ...
+[builtins fixtures/classmethod.pyi]
+
+[case testEmptyBodyVariationsImplicitlyAbstractProtocol]
+from typing import Protocol
+
+class WithPass(Protocol):
+    def meth(self) -> int:
+        pass
+class A(WithPass): ...
+A() # E: Cannot instantiate abstract class "A" with abstract attribute "meth"
+
+class WithEllipses(Protocol):
+    def meth(self) -> int: ...
+class B(WithEllipses): ...
+B() # E: Cannot instantiate abstract class "B" with abstract attribute "meth"
+
+class WithDocstring(Protocol):
+    def meth(self) -> int:
+        """Docstring for meth.
+
+	This is meth."""
+class C(WithDocstring): ...
+C() # E: Cannot instantiate abstract class "C" with abstract attribute "meth"
+
+class WithRaise(Protocol):
+    def meth(self) -> int:
+        """Docstring for meth."""
+        raise NotImplementedError
+class D(WithRaise): ...
+D() # E: Cannot instantiate abstract class "D" with abstract attribute "meth"
+[builtins fixtures/exception.pyi]
+
+[case testEmptyBodyNonAbstractProtocol]
+from typing import Any, Optional, Protocol, Union
+
+class NotAbstract(Protocol):
+    def f(self) -> None: ...
+    def g(self) -> Any: ...
+    def h(self, x: int): ...
+    def j(self): ...
+    def k(self, x): ...
+    def l(self) -> Optional[int]: ...
+    def m(self) -> Union[str, None]: ...
+
+class A(NotAbstract): ...
+A()