From c8d8a14516113343d241cec46534445d7dddcdcd Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 4 Mar 2022 18:18:27 +0000 Subject: [PATCH 1/5] stubtest: error if an attribute is a read-only property at runtime, but not in the stub --- mypy/stubtest.py | 27 ++++++++++++++++--- mypy/test/teststubtest.py | 56 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index a08b0aac1097..0b0df08bc656 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -770,6 +770,19 @@ def verify_var( yield Error(object_path, "is not present at runtime", stub, runtime) return + if ( + stub.is_initialized_in_class + and isinstance(runtime, property) + and runtime.fset is None + and (stub.is_settable_property or not stub.is_property) + ): + yield Error( + object_path, + "is read-only at runtime but not in the stub", + stub, + runtime + ) + runtime_type = get_mypy_type_of_runtime_value(runtime) if ( runtime_type is not None @@ -802,7 +815,15 @@ def verify_overloadedfuncdef( return if stub.is_property: - # We get here in cases of overloads from property.setter + # Any property with a setter is represented as an OverloadedFuncDef + if isinstance(runtime, property): + if runtime.fset is None: + yield Error( + object_path, + "is read-only at runtime but not in the stub", + stub, + runtime + ) return if not is_probably_a_function(runtime): @@ -845,7 +866,7 @@ def verify_typevarexpr( yield None -def _verify_property(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: +def _verify_readonly_property(stub: nodes.Decorator, runtime: Any) -> Iterator[str]: assert stub.func.is_property if isinstance(runtime, property): return @@ -920,7 +941,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_readonly_property(stub, runtime): yield Error(object_path, message, stub, runtime) return diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index fa7505e37191..bc8706871f04 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -593,6 +593,36 @@ class BadReadOnly: """, error="BadReadOnly.f", ) + yield Case( + stub=""" + class X: + @property + def read_only_attr(self) -> int: ... + """, + runtime=""" + class X: + @property + def read_only_attr(self): return 5 + """, + error=None, + ) + yield Case( + stub=""" + class Z: + @property + def read_write_attr(self) -> int: ... + @read_write_attr.setter + def read_write_attr(self, val: int) -> None: ... + """, + runtime=""" + class Z: + @property + def read_write_attr(self): return self._val + @read_write_attr.setter + def read_write_attr(self, val): self._val = val + """, + error=None, + ) @collect_cases def test_var(self) -> Iterator[Case]: @@ -631,6 +661,32 @@ def __init__(self): """, error=None, ) + yield Case( + stub=""" + class Y: + read_only_attr: int + """, + runtime=""" + class Y: + @property + def read_only_attr(self): return 5 + """, + error="Y.read_only_attr", + ) + yield Case( + stub=""" + class Z: + read_write_attr: int + """, + runtime=""" + class Z: + @property + def read_write_attr(self): return self._val + @read_write_attr.setter + def read_write_attr(self, val): self._val = val + """, + error=None, + ) @collect_cases def test_type_alias(self) -> Iterator[Case]: From 481cea5252c6490bdd8ce602cd3c32f697939887 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 4 Mar 2022 22:13:43 +0000 Subject: [PATCH 2/5] Simplify --- mypy/stubtest.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 0b0df08bc656..f8afaeb4386e 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -772,8 +772,7 @@ def verify_var( if ( stub.is_initialized_in_class - and isinstance(runtime, property) - and runtime.fset is None + and is_read_only_property(runtime) and (stub.is_settable_property or not stub.is_property) ): yield Error( @@ -816,14 +815,13 @@ def verify_overloadedfuncdef( if stub.is_property: # Any property with a setter is represented as an OverloadedFuncDef - if isinstance(runtime, property): - if runtime.fset is None: - yield Error( - object_path, - "is read-only at runtime but not in the stub", - stub, - runtime - ) + if is_read_only_property(runtime): + yield Error( + object_path, + "is read-only at runtime but not in the stub", + stub, + runtime + ) return if not is_probably_a_function(runtime): @@ -1061,6 +1059,10 @@ def is_probably_a_function(runtime: Any) -> bool: ) +def is_read_only_property(runtime: object) -> bool: + return isinstance(runtime, property) and property.fset is None + + def safe_inspect_signature(runtime: Any) -> Optional[inspect.Signature]: try: return inspect.signature(runtime) From 8134f4f49e03701a63c58c5b18e6ea78b9bd9672 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 4 Mar 2022 22:38:56 +0000 Subject: [PATCH 3/5] Fix typo --- mypy/stubtest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index f8afaeb4386e..3024b74182d6 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -1060,7 +1060,7 @@ def is_probably_a_function(runtime: Any) -> bool: def is_read_only_property(runtime: object) -> bool: - return isinstance(runtime, property) and property.fset is None + return isinstance(runtime, property) and runtime.fset is None def safe_inspect_signature(runtime: Any) -> Optional[inspect.Signature]: From d2db2f54e975b1798417df583a18e198dbef7c34 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 5 Mar 2022 16:55:19 +0000 Subject: [PATCH 4/5] Add another test case --- mypy/test/teststubtest.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index bc8706871f04..f623100e43c6 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -606,6 +606,21 @@ def read_only_attr(self): return 5 """, error=None, ) + yield Case( + stub=""" + class Y: + @property + def read_only_attr(self) -> int: ... + @read_only_attr.setter + def read_only_attr(self, val: int) -> None: ... + """, + runtime=""" + class Y: + @property + def read_only_attr(self): return 5 + """, + error="Y.read_only_attr", + ) yield Case( stub=""" class Z: From 841aa1baa5b4230fb3e43ac68eba164362e66436 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 21 Mar 2022 08:19:48 +0100 Subject: [PATCH 5/5] Remove duplication in test cases --- mypy/test/teststubtest.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index f623100e43c6..0310ea04c78a 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -548,12 +548,12 @@ def test_property(self) -> Iterator[Case]: stub=""" class Good: @property - def f(self) -> int: ... + def read_only_attr(self) -> int: ... """, runtime=""" class Good: @property - def f(self) -> int: return 1 + def read_only_attr(self): return 1 """, error=None, ) @@ -593,19 +593,6 @@ class BadReadOnly: """, error="BadReadOnly.f", ) - yield Case( - stub=""" - class X: - @property - def read_only_attr(self) -> int: ... - """, - runtime=""" - class X: - @property - def read_only_attr(self): return 5 - """, - error=None, - ) yield Case( stub=""" class Y: