From 52329f07cde0b39be8c24b52d65b1501c17f01ea Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 12 Sep 2022 01:11:29 +0300 Subject: [PATCH 1/3] stubtest: Detect abstract properties mismatches --- mypy/stubtest.py | 30 ++++++++++++++++++------------ mypy/test/teststubtest.py | 14 ++++++++++++++ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index ff59a8f682e6..a614a1764e11 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -870,16 +870,8 @@ def verify_funcitem( 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, - "is inconsistent, runtime method is abstract but stub is not", - stub, - runtime, - ) + for error_text in _verify_abstract_status(stub, runtime): + yield Error(object_path, error_text, stub, runtime) for message in _verify_static_class_methods(stub, runtime, object_path): yield Error(object_path, "is inconsistent, " + message, stub, runtime) @@ -1044,8 +1036,13 @@ def verify_paramspecexpr( return -def _verify_readonly_property(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: +def _verify_property(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: assert stub.func.is_property + yield from _verify_readonly_property(stub, runtime) + yield from _verify_abstract_status(stub.func, runtime) + + +def _verify_readonly_property(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: if isinstance(runtime, property): return if inspect.isdatadescriptor(runtime): @@ -1066,6 +1063,15 @@ def _verify_readonly_property(stub: nodes.Decorator, runtime: Any) -> Iterator[s yield "is inconsistent, cannot reconcile @property on stub with runtime object" +def _verify_abstract_status(stub: nodes.FuncDef, runtime: Any) -> Iterator[str]: + 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: + item_type = "property" if stub.is_property else "method" + yield f"is inconsistent, runtime {item_type} is abstract but stub is not" + + def _resolve_funcitem_from_decorator(dec: nodes.OverloadPart) -> nodes.FuncItem | None: """Returns a FuncItem that corresponds to the output of the decorator. @@ -1122,7 +1128,7 @@ def verify_decorator( yield Error(object_path, "is not present at runtime", stub, runtime) return if stub.func.is_property: - for message in _verify_readonly_property(stub, runtime): + for message in _verify_property(stub, runtime): yield Error(object_path, message, stub, runtime) return diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index 8e78966363d4..19c245e8fa30 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -1391,6 +1391,7 @@ def test_abstract_properties(self) -> Iterator[Case]: yield Case( stub=""" class AP1: + @property def some(self) -> int: ... """, runtime=""" @@ -1401,6 +1402,19 @@ def some(self) -> int: ... """, error="AP1.some", ) + yield Case( + stub=""" + class AP1_2: + def some(self) -> int: ... # missing `@property` decorator + """, + runtime=""" + class AP1_2: + @property + @abstractmethod + def some(self) -> int: ... + """, + error="AP1_2.some", + ) yield Case( stub=""" class AP2: From ab3fc7f115c73688c176a33ceeafef241ba2d819 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 12 Sep 2022 16:37:24 +0300 Subject: [PATCH 2/3] Rename new function --- mypy/stubtest.py | 4 ++-- mypy/test/teststubtest.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index a614a1764e11..dd680b0ba25d 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -1036,7 +1036,7 @@ def verify_paramspecexpr( return -def _verify_property(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: +def _verify_property_decorator(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: assert stub.func.is_property yield from _verify_readonly_property(stub, runtime) yield from _verify_abstract_status(stub.func, runtime) @@ -1128,7 +1128,7 @@ def verify_decorator( yield Error(object_path, "is not present at runtime", stub, runtime) return if stub.func.is_property: - for message in _verify_property(stub, runtime): + for message in _verify_property_decorator(stub, runtime): yield Error(object_path, message, stub, runtime) return diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index 19c245e8fa30..d74949fde783 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -1382,6 +1382,7 @@ def some(self) -> None: ... @collect_cases def test_abstract_properties(self) -> Iterator[Case]: + # TODO: test abstract properties with setters yield Case( stub="from abc import abstractmethod", runtime="from abc import abstractmethod", From ed298906c63d3083c2333d899026e27b0f1da4eb Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 12 Sep 2022 17:14:01 +0300 Subject: [PATCH 3/3] Address code review --- mypy/stubtest.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index dd680b0ba25d..e51aa6dc6e53 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -1036,13 +1036,8 @@ def verify_paramspecexpr( return -def _verify_property_decorator(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: - assert stub.func.is_property - yield from _verify_readonly_property(stub, runtime) - yield from _verify_abstract_status(stub.func, runtime) - - def _verify_readonly_property(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: + assert stub.func.is_property if isinstance(runtime, property): return if inspect.isdatadescriptor(runtime): @@ -1128,7 +1123,9 @@ def verify_decorator( yield Error(object_path, "is not present at runtime", stub, runtime) return if stub.func.is_property: - for message in _verify_property_decorator(stub, runtime): + for message in _verify_readonly_property(stub, runtime): + yield Error(object_path, message, stub, runtime) + for message in _verify_abstract_status(stub.func, runtime): yield Error(object_path, message, stub, runtime) return