Repository: python/mypy
Pull request: Allow stubtest to raise errors on abstract state mismatch
Closes #13322

Problem statement:
`stubtest` produce no error when `@abstractmethod` is omitted from stubs
If I have this runtime implementation:

```python
from abc import abstractmethod

class A:
    @abstractmethod
    def some(self) -> None:
        raise NotImplementedError
```

And this stub:

```python
class A:
    def some(self) -> None: ...
```

There's no error. 

Why is that important?
Because with the stub like above, mypy will allow to create an instance of `A`.
But, mypy should not be allow to do that.

I think it is a bug that should be fixed. And it is a pretty easy one.

Hints from issue discussion:
I used a stubtest patch along these lines to work on this typeshed PR in March:
- https://github.com/python/typeshed/pull/7443

But I never got round to writing tests and submitting a PR to mypy. I'm glad you're picking it up :D

Commit message: Add abstract `@property` tests

Patch to review:
diff --git a/mypy/stubtest.py b/mypy/stubtest.py
index ebc7fa12857d..499ba4a01887 100644
--- a/mypy/stubtest.py
+++ b/mypy/stubtest.py
@@ -825,6 +825,13 @@ def verify_funcitem(
         if not callable(runtime):
             return
 
+    if isinstance(stub, nodes.FuncDef):
+        stub_abstract = stub.abstract_status == nodes.IS_ABSTRACT
+        runtime_abstract = getattr(runtime, "__isabstractmethod__", False)
+        # The opposite can exist: some implementations omit `@abstractmethod` decorators
+        if runtime_abstract and not stub_abstract:
+            yield Error(object_path, "runtime method is abstract, but stub is not", stub, runtime)
+
     for message in _verify_static_class_methods(stub, runtime, object_path):
         yield Error(object_path, "is inconsistent, " + message, stub, runtime)
 
diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py
index 3de0e3fd5fc6..be953ae17615 100644
--- a/mypy/test/teststubtest.py
+++ b/mypy/test/teststubtest.py
@@ -1264,6 +1264,104 @@ def test_type_var(self) -> Iterator[Case]:
             )
             yield Case(stub="C = ParamSpec('C')", runtime="C = ParamSpec('C')", error=None)
 
+    @collect_cases
+    def test_abstract_methods(self) -> Iterator[Case]:
+        yield Case(
+            stub="from abc import abstractmethod",
+            runtime="from abc import abstractmethod",
+            error=None,
+        )
+        yield Case(
+            stub="""
+            class A1:
+                def some(self) -> None: ...
+            """,
+            runtime="""
+            class A1:
+                @abstractmethod
+                def some(self) -> None: ...
+            """,
+            error="A1.some",
+        )
+        yield Case(
+            stub="""
+            class A2:
+                @abstractmethod
+                def some(self) -> None: ...
+            """,
+            runtime="""
+            class A2:
+                @abstractmethod
+                def some(self) -> None: ...
+            """,
+            error=None,
+        )
+        # Runtime can miss `@abstractmethod`:
+        yield Case(
+            stub="""
+            class A3:
+                @abstractmethod
+                def some(self) -> None: ...
+            """,
+            runtime="""
+            class A3:
+                def some(self) -> None: ...
+            """,
+            error=None,
+        )
+
+    @collect_cases
+    def test_abstract_properties(self) -> Iterator[Case]:
+        yield Case(
+            stub="from abc import abstractmethod",
+            runtime="from abc import abstractmethod",
+            error=None,
+        )
+        # Ensure that `@property` also can be abstract:
+        yield Case(
+            stub="""
+            class AP1:
+                def some(self) -> int: ...
+            """,
+            runtime="""
+            class AP1:
+                @property
+                @abstractmethod
+                def some(self) -> int: ...
+            """,
+            error="AP1.some",
+        )
+        yield Case(
+            stub="""
+            class AP2:
+                @property
+                @abstractmethod
+                def some(self) -> int: ...
+            """,
+            runtime="""
+            class AP2:
+                @property
+                @abstractmethod
+                def some(self) -> int: ...
+            """,
+            error=None,
+        )
+        # Runtime can miss `@abstractmethod`:
+        yield Case(
+            stub="""
+            class AP3:
+                @property
+                @abstractmethod
+                def some(self) -> int: ...
+            """,
+            runtime="""
+            class AP3:
+                @property
+                def some(self) -> int: ...
+            """,
+            error=None,
+        )
+
 
 def remove_color_code(s: str) -> str:
     return re.sub("\\x1b.*?m", "", s)  # this works!


Code review comments:
diff_hunk: @@ -825,6 +825,13 @@ def verify_funcitem(
         if not callable(runtime):
             return
 
+    if isinstance(stub, nodes.FuncDef):
+        stub_abstract = stub.abstract_status == nodes.IS_ABSTRACT
+        runtime_abstract = getattr(runtime, "__isabstractmethod__", False)
+        # The opposite can exist: some implementations omit `@abstractmethod` decorators
+        if runtime_abstract and not stub_abstract:
+            yield Error(object_path, "runtime method is abstract, but stub is not", stub, runtime)
path: mypy/stubtest.py
text: @user1:
The signatures of the runtime and the stub are irrelevant for this check, so the default error message here won't be very good. Maybe something like this would be better?

```suggestion
            yield Error(object_path, "runtime method is abstract, but stub is not", stub, runtime, stub_desc="A concrete method", runtime_desc="An abstract method")
```

@author:
Why not? 🙂