From c47837ab2d99d894a6d8aec6355055359873d1a6 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sat, 7 Jun 2025 17:42:28 +0100 Subject: [PATCH 1/2] Fix properties with setters after deleters --- mypy/checker.py | 10 ++++------ mypy/checkmember.py | 4 ++-- mypy/nodes.py | 20 +++++++++++++++++++- mypy/semanal.py | 1 + test-data/unit/check-classes.test | 20 ++++++++++++++++++++ 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 6929543db24e..49f1bc15f583 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -697,11 +697,9 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: assert isinstance(defn.items[0], Decorator) self.visit_decorator(defn.items[0]) if defn.items[0].var.is_settable_property: - # TODO: here and elsewhere we assume setter immediately follows getter. - assert isinstance(defn.items[1], Decorator) # Perform a reduced visit just to infer the actual setter type. - self.visit_decorator_inner(defn.items[1], skip_first_item=True) - setter_type = defn.items[1].var.type + self.visit_decorator_inner(defn.setter, skip_first_item=True) + setter_type = defn.setter.var.type # Check if the setter can accept two positional arguments. any_type = AnyType(TypeOfAny.special_form) fallback_setter_type = CallableType( @@ -712,7 +710,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: fallback=self.named_type("builtins.function"), ) if setter_type and not is_subtype(setter_type, fallback_setter_type): - self.fail("Invalid property setter signature", defn.items[1].func) + self.fail("Invalid property setter signature", defn.setter.func) setter_type = self.extract_callable_type(setter_type, defn) if not isinstance(setter_type, CallableType) or len(setter_type.arg_types) != 2: # TODO: keep precise type for callables with tricky but valid signatures. @@ -2171,7 +2169,7 @@ def check_setter_type_override(self, defn: OverloadedFuncDef, base: TypeInfo) -> assert typ is not None and original_type is not None if not is_subtype(original_type, typ): - self.msg.incompatible_setter_override(defn.items[1], typ, original_type, base) + self.msg.incompatible_setter_override(defn.setter, typ, original_type, base) def check_method_override_for_base_with_name( self, defn: FuncDef | OverloadedFuncDef | Decorator, name: str, base: TypeInfo diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 50eaf42a9934..beb3c1397c11 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -345,8 +345,8 @@ def analyze_instance_member_access( assert isinstance(method, OverloadedFuncDef) getter = method.items[0] assert isinstance(getter, Decorator) - if mx.is_lvalue and (len(items := method.items) > 1): - mx.chk.warn_deprecated(items[1], mx.context) + if mx.is_lvalue and getter.var.is_settable_property: + mx.chk.warn_deprecated(method.setter, mx.context) return analyze_var(name, getter.var, typ, mx) if mx.is_lvalue and not mx.suppress_errors: diff --git a/mypy/nodes.py b/mypy/nodes.py index 7db32240c33e..32bca6ad2a65 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -538,12 +538,20 @@ class OverloadedFuncDef(FuncBase, SymbolNode, Statement): Overloaded variants must be consecutive in the source file. """ - __slots__ = ("items", "unanalyzed_items", "impl", "deprecated", "_is_trivial_self") + __slots__ = ( + "items", + "unanalyzed_items", + "impl", + "deprecated", + "setter_index", + "_is_trivial_self", + ) items: list[OverloadPart] unanalyzed_items: list[OverloadPart] impl: OverloadPart | None deprecated: str | None + setter_index: int | None def __init__(self, items: list[OverloadPart]) -> None: super().__init__() @@ -551,6 +559,7 @@ def __init__(self, items: list[OverloadPart]) -> None: self.unanalyzed_items = items.copy() self.impl = None self.deprecated = None + self.setter_index = None self._is_trivial_self: bool | None = None if items: # TODO: figure out how to reliably set end position (we don't know the impl here). @@ -586,6 +595,13 @@ def is_trivial_self(self) -> bool: self._is_trivial_self = True return True + @property + def setter(self) -> Decorator: + assert self.setter_index is not None + item = self.items[self.setter_index] + assert isinstance(item, Decorator) + return item + def accept(self, visitor: StatementVisitor[T]) -> T: return visitor.visit_overloaded_func_def(self) @@ -598,6 +614,7 @@ def serialize(self) -> JsonDict: "impl": None if self.impl is None else self.impl.serialize(), "flags": get_flags(self, FUNCBASE_FLAGS), "deprecated": self.deprecated, + "setter_index": self.setter_index, } @classmethod @@ -618,6 +635,7 @@ def deserialize(cls, data: JsonDict) -> OverloadedFuncDef: res._fullname = data["fullname"] set_flags(res, data["flags"]) res.deprecated = data["deprecated"] + res.setter_index = data["setter_index"] # NOTE: res.info will be set in the fixup phase. return res diff --git a/mypy/semanal.py b/mypy/semanal.py index 5cd58966f619..d70abe911fea 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1543,6 +1543,7 @@ def analyze_property_with_multi_part_definition( ) assert isinstance(setter_func_type, CallableType) bare_setter_type = setter_func_type + defn.setter_index = i + 1 if first_node.name == "deleter": item.func.abstract_status = first_item.func.abstract_status for other_node in item.decorators[1:]: diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index c75ede7cc6d5..c7136509729e 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -8736,3 +8736,23 @@ class NoopPowerResource: def hardware_type(self) -> None: # E: Invalid property setter signature self.hardware_type = None # Note: intentionally recursive [builtins fixtures/property.pyi] + +[case testPropertyAllowsDeleterBeforeSetter] +class C: + @property + def foo(self) -> str: ... + @foo.deleter + def foo(self) -> None: ... + @foo.setter + def foo(self, val: int) -> None: ... + + @property + def bar(self) -> int: ... + @bar.deleter + def bar(self) -> None: ... + @bar.setter + def bar(self, value: int, val: int) -> None: ... # E: Invalid property setter signature + +C().foo = "no" # E: Incompatible types in assignment (expression has type "str", variable has type "int") +C().bar = "fine" +[builtins fixtures/property.pyi] From 2b28b570e9959db1166578e509150695ec2c31bd Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sat, 7 Jun 2025 22:57:59 +0100 Subject: [PATCH 2/2] Add some consistency checks --- mypy/nodes.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mypy/nodes.py b/mypy/nodes.py index 32bca6ad2a65..2cec4852f31c 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -597,6 +597,10 @@ def is_trivial_self(self) -> bool: @property def setter(self) -> Decorator: + # Do some consistency checks first. + first_item = self.items[0] + assert isinstance(first_item, Decorator) + assert first_item.var.is_settable_property assert self.setter_index is not None item = self.items[self.setter_index] assert isinstance(item, Decorator)