Skip to content

Commit 9fb5ff6

Browse files
ilevkivskyiJukkaL
authored andcommitted
Fix properties with setters after deleters (#19248)
Fixes #19224 Note we must add an additional attribute on `OverloadedFuncDef` since decorator expressions are not serialized.
1 parent c20fd78 commit 9fb5ff6

File tree

5 files changed

+50
-9
lines changed

5 files changed

+50
-9
lines changed

mypy/checker.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -675,11 +675,9 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
675675
assert isinstance(defn.items[0], Decorator)
676676
self.visit_decorator(defn.items[0])
677677
if defn.items[0].var.is_settable_property:
678-
# TODO: here and elsewhere we assume setter immediately follows getter.
679-
assert isinstance(defn.items[1], Decorator)
680678
# Perform a reduced visit just to infer the actual setter type.
681-
self.visit_decorator_inner(defn.items[1], skip_first_item=True)
682-
setter_type = defn.items[1].var.type
679+
self.visit_decorator_inner(defn.setter, skip_first_item=True)
680+
setter_type = defn.setter.var.type
683681
# Check if the setter can accept two positional arguments.
684682
any_type = AnyType(TypeOfAny.special_form)
685683
fallback_setter_type = CallableType(
@@ -690,7 +688,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
690688
fallback=self.named_type("builtins.function"),
691689
)
692690
if setter_type and not is_subtype(setter_type, fallback_setter_type):
693-
self.fail("Invalid property setter signature", defn.items[1].func)
691+
self.fail("Invalid property setter signature", defn.setter.func)
694692
setter_type = self.extract_callable_type(setter_type, defn)
695693
if not isinstance(setter_type, CallableType) or len(setter_type.arg_types) != 2:
696694
# TODO: keep precise type for callables with tricky but valid signatures.
@@ -2149,7 +2147,7 @@ def check_setter_type_override(self, defn: OverloadedFuncDef, base: TypeInfo) ->
21492147
assert typ is not None and original_type is not None
21502148

21512149
if not is_subtype(original_type, typ):
2152-
self.msg.incompatible_setter_override(defn.items[1], typ, original_type, base)
2150+
self.msg.incompatible_setter_override(defn.setter, typ, original_type, base)
21532151

21542152
def check_method_override_for_base_with_name(
21552153
self, defn: FuncDef | OverloadedFuncDef | Decorator, name: str, base: TypeInfo

mypy/checkmember.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,8 @@ def analyze_instance_member_access(
341341
assert isinstance(method, OverloadedFuncDef)
342342
getter = method.items[0]
343343
assert isinstance(getter, Decorator)
344-
if mx.is_lvalue and (len(items := method.items) > 1):
345-
mx.chk.warn_deprecated(items[1], mx.context)
344+
if mx.is_lvalue and getter.var.is_settable_property:
345+
mx.chk.warn_deprecated(method.setter, mx.context)
346346
return analyze_var(name, getter.var, typ, mx)
347347

348348
if mx.is_lvalue and not mx.suppress_errors:

mypy/nodes.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,19 +550,28 @@ class OverloadedFuncDef(FuncBase, SymbolNode, Statement):
550550
Overloaded variants must be consecutive in the source file.
551551
"""
552552

553-
__slots__ = ("items", "unanalyzed_items", "impl", "deprecated", "_is_trivial_self")
553+
__slots__ = (
554+
"items",
555+
"unanalyzed_items",
556+
"impl",
557+
"deprecated",
558+
"setter_index",
559+
"_is_trivial_self",
560+
)
554561

555562
items: list[OverloadPart]
556563
unanalyzed_items: list[OverloadPart]
557564
impl: OverloadPart | None
558565
deprecated: str | None
566+
setter_index: int | None
559567

560568
def __init__(self, items: list[OverloadPart]) -> None:
561569
super().__init__()
562570
self.items = items
563571
self.unanalyzed_items = items.copy()
564572
self.impl = None
565573
self.deprecated = None
574+
self.setter_index = None
566575
self._is_trivial_self: bool | None = None
567576
if items:
568577
# TODO: figure out how to reliably set end position (we don't know the impl here).
@@ -598,6 +607,17 @@ def is_trivial_self(self) -> bool:
598607
self._is_trivial_self = True
599608
return True
600609

610+
@property
611+
def setter(self) -> Decorator:
612+
# Do some consistency checks first.
613+
first_item = self.items[0]
614+
assert isinstance(first_item, Decorator)
615+
assert first_item.var.is_settable_property
616+
assert self.setter_index is not None
617+
item = self.items[self.setter_index]
618+
assert isinstance(item, Decorator)
619+
return item
620+
601621
def accept(self, visitor: StatementVisitor[T]) -> T:
602622
return visitor.visit_overloaded_func_def(self)
603623

@@ -610,6 +630,7 @@ def serialize(self) -> JsonDict:
610630
"impl": None if self.impl is None else self.impl.serialize(),
611631
"flags": get_flags(self, FUNCBASE_FLAGS),
612632
"deprecated": self.deprecated,
633+
"setter_index": self.setter_index,
613634
}
614635

615636
@classmethod
@@ -630,6 +651,7 @@ def deserialize(cls, data: JsonDict) -> OverloadedFuncDef:
630651
res._fullname = data["fullname"]
631652
set_flags(res, data["flags"])
632653
res.deprecated = data["deprecated"]
654+
res.setter_index = data["setter_index"]
633655
# NOTE: res.info will be set in the fixup phase.
634656
return res
635657

mypy/semanal.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,7 @@ def analyze_property_with_multi_part_definition(
15581558
)
15591559
assert isinstance(setter_func_type, CallableType)
15601560
bare_setter_type = setter_func_type
1561+
defn.setter_index = i + 1
15611562
if first_node.name == "deleter":
15621563
item.func.abstract_status = first_item.func.abstract_status
15631564
for other_node in item.decorators[1:]:

test-data/unit/check-classes.test

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8739,3 +8739,23 @@ class NoopPowerResource:
87398739
def hardware_type(self) -> None: # E: Invalid property setter signature
87408740
self.hardware_type = None # Note: intentionally recursive
87418741
[builtins fixtures/property.pyi]
8742+
8743+
[case testPropertyAllowsDeleterBeforeSetter]
8744+
class C:
8745+
@property
8746+
def foo(self) -> str: ...
8747+
@foo.deleter
8748+
def foo(self) -> None: ...
8749+
@foo.setter
8750+
def foo(self, val: int) -> None: ...
8751+
8752+
@property
8753+
def bar(self) -> int: ...
8754+
@bar.deleter
8755+
def bar(self) -> None: ...
8756+
@bar.setter
8757+
def bar(self, value: int, val: int) -> None: ... # E: Invalid property setter signature
8758+
8759+
C().foo = "no" # E: Incompatible types in assignment (expression has type "str", variable has type "int")
8760+
C().bar = "fine"
8761+
[builtins fixtures/property.pyi]

0 commit comments

Comments
 (0)