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.

mypy/checker.py

@@ -19,8 +19,8 @@
     ClassDef, Block, AssignmentStmt, NameExpr, MemberExpr, IndexExpr,
     TupleExpr, ListExpr, ExpressionStmt, ReturnStmt, IfStmt,
     WhileStmt, OperatorAssignmentStmt, WithStmt, AssertStmt,
-    RaiseStmt, TryStmt, ForStmt, DelStmt, CallExpr, IntExpr, StrExpr,
-    UnicodeExpr, OpExpr, UnaryExpr, LambdaExpr, TempNode, SymbolTableNode,
+    RaiseStmt, TryStmt, ForStmt, DelStmt, CallExpr, IntExpr,
+    OpExpr, UnaryExpr, LambdaExpr, TempNode, SymbolTableNode,
     Context, Decorator, PrintStmt, BreakStmt, PassStmt, ContinueStmt,
     ComparisonExpr, StarExpr, EllipsisExpr, RefExpr, PromoteExpr,
     Import, ImportFrom, ImportAll, ImportBase, TypeAlias,
@@ -70,7 +70,7 @@
 from mypy.constraints import SUPERTYPE_OF
 from mypy.maptype import map_instance_to_supertype
 from mypy.typevars import fill_typevars, has_no_typevars, fill_typevars_with_any
-from mypy.semanal import set_callable_name, refers_to_fullname
+from mypy.semanal import set_callable_name, refers_to_fullname, is_trivial_body
 from mypy.mro import calculate_mro, MroError
 from mypy.erasetype import erase_typevars, remove_instance_last_known_values, erase_type
 from mypy.expandtype import expand_type, expand_type_by_instance
@@ -1008,7 +1008,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.
@@ -1152,51 +1152,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, UnicodeExpr))):
-            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/semanal.py

@@ -60,7 +60,7 @@
     ClassDef, Var, GDEF, FuncItem, Import, Expression, Lvalue,
     ImportFrom, ImportAll, Block, LDEF, NameExpr, MemberExpr,
     IndexExpr, TupleExpr, ListExpr, ExpressionStmt, ReturnStmt,
-    RaiseStmt, AssertStmt, OperatorAssignmentStmt, WhileStmt,
+    RaiseStmt, AssertStmt, OperatorAssignmentStmt, WhileStmt, PassStmt,
     ForStmt, BreakStmt, ContinueStmt, IfStmt, TryStmt, WithStmt, DelStmt,
     GlobalDecl, SuperExpr, DictExpr, CallExpr, RefExpr, OpExpr, UnaryExpr,
     SliceExpr, CastExpr, RevealExpr, TypeApplication, Context, SymbolTable,
@@ -669,6 +669,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) and
                 self.wrapped_coro_return_types.get(defn) != defn.type):
@@ -803,6 +813,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,
@@ -876,7 +901,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
@@ -5533,3 +5559,59 @@ 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, UnicodeExpr))):
+        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

@@ -7,7 +7,7 @@
 from typing_extensions import Final
 
 from mypy.nodes import (
-    Node, TypeInfo, Var, Decorator, OverloadedFuncDef, SymbolTable, CallExpr, PromoteExpr,
+    Node, TypeInfo, Var, Decorator, OverloadedFuncDef, SymbolTable, CallExpr, PromoteExpr, FuncDef
 )
 from mypy.types import Instance, Type
 from mypy.errors import Errors
@@ -79,8 +79,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

@@ -2910,3 +2910,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()