Skip to content

Commit 0b7afda

Browse files
authored
Check property decorators stricter (#19313)
Fixes #19312. Fixes #18327. Only accept `@current_prop_name.{setter,deleter}` as property-related decorators.
1 parent b4d52e1 commit 0b7afda

File tree

4 files changed

+103
-14
lines changed

4 files changed

+103
-14
lines changed

mypy/semanal.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,33 +1527,33 @@ def analyze_property_with_multi_part_definition(
15271527
assert isinstance(first_item, Decorator)
15281528
deleted_items = []
15291529
bare_setter_type = None
1530+
func_name = first_item.func.name
15301531
for i, item in enumerate(items[1:]):
15311532
if isinstance(item, Decorator):
15321533
item.func.accept(self)
15331534
if item.decorators:
15341535
first_node = item.decorators[0]
1535-
if isinstance(first_node, MemberExpr):
1536+
if self._is_valid_property_decorator(first_node, func_name):
1537+
# Get abstractness from the original definition.
1538+
item.func.abstract_status = first_item.func.abstract_status
15361539
if first_node.name == "setter":
15371540
# The first item represents the entire property.
15381541
first_item.var.is_settable_property = True
1539-
# Get abstractness from the original definition.
1540-
item.func.abstract_status = first_item.func.abstract_status
15411542
setter_func_type = function_type(
15421543
item.func, self.named_type("builtins.function")
15431544
)
15441545
assert isinstance(setter_func_type, CallableType)
15451546
bare_setter_type = setter_func_type
15461547
defn.setter_index = i + 1
1547-
if first_node.name == "deleter":
1548-
item.func.abstract_status = first_item.func.abstract_status
15491548
for other_node in item.decorators[1:]:
15501549
other_node.accept(self)
15511550
else:
15521551
self.fail(
1553-
f"Only supported top decorator is @{first_item.func.name}.setter", item
1552+
f'Only supported top decorators are "@{func_name}.setter" and "@{func_name}.deleter"',
1553+
first_node,
15541554
)
15551555
else:
1556-
self.fail(f'Unexpected definition for property "{first_item.func.name}"', item)
1556+
self.fail(f'Unexpected definition for property "{func_name}"', item)
15571557
deleted_items.append(i + 1)
15581558
for i in reversed(deleted_items):
15591559
del items[i]
@@ -1567,6 +1567,23 @@ def analyze_property_with_multi_part_definition(
15671567
)
15681568
return bare_setter_type
15691569

1570+
def _is_valid_property_decorator(
1571+
self, deco: Expression, property_name: str
1572+
) -> TypeGuard[MemberExpr]:
1573+
if not isinstance(deco, MemberExpr):
1574+
return False
1575+
if not isinstance(deco.expr, NameExpr) or deco.expr.name != property_name:
1576+
return False
1577+
if deco.name not in {"setter", "deleter"}:
1578+
# This intentionally excludes getter. While `@prop.getter` is valid at
1579+
# runtime, that would mean replacing the already processed getter type.
1580+
# Such usage is almost definitely a mistake (except for overrides in
1581+
# subclasses but we don't support them anyway) and might be a typo
1582+
# (only one letter away from `setter`), it's likely almost never used,
1583+
# so supporting it properly won't pay off.
1584+
return False
1585+
return True
1586+
15701587
def add_function_to_symbol_table(self, func: FuncDef | OverloadedFuncDef) -> None:
15711588
if self.is_class_scope():
15721589
assert self.type is not None

test-data/unit/check-classes.test

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,7 @@ class A:
772772
class B(A):
773773
@property
774774
def f(self) -> Callable[[object], None]: pass
775-
@func.setter
775+
@f.setter
776776
def f(self, x: object) -> None: pass
777777
[builtins fixtures/property.pyi]
778778

@@ -786,7 +786,7 @@ class A:
786786
class B(A):
787787
@property
788788
def f(self) -> Callable[[object], None]: pass
789-
@func.setter
789+
@f.setter
790790
def f(self, x: object) -> None: pass
791791
[builtins fixtures/property.pyi]
792792

@@ -1622,7 +1622,81 @@ class A:
16221622
self.x = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int")
16231623
return ''
16241624
[builtins fixtures/property.pyi]
1625+
1626+
[case testPropertyNameIsChecked]
1627+
class A:
1628+
@property
1629+
def f(self) -> str: ...
1630+
@not_f.setter # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1631+
def f(self, val: str) -> None: ...
1632+
1633+
a = A()
1634+
reveal_type(a.f) # N: Revealed type is "builtins.str"
1635+
a.f = '' # E: Property "f" defined in "A" is read-only
1636+
1637+
class B:
1638+
@property
1639+
def f(self) -> str: ...
1640+
@not_f.deleter # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1641+
def f(self) -> None: ...
1642+
1643+
class C:
1644+
@property
1645+
def f(self) -> str: ...
1646+
@not_f.setter # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1647+
def f(self, val: str) -> None: ...
1648+
@not_f.deleter # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1649+
def f(self) -> None: ...
1650+
[builtins fixtures/property.pyi]
1651+
1652+
[case testPropertyAttributeIsChecked]
1653+
class A:
1654+
@property
1655+
def f(self) -> str: ...
1656+
@f.unknown # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1657+
def f(self, val: str) -> None: ...
1658+
@f.bad.setter # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1659+
def f(self, val: str) -> None: ...
1660+
@f # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1661+
def f(self, val: str) -> None: ...
1662+
@int # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1663+
def f(self, val: str) -> None: ...
1664+
[builtins fixtures/property.pyi]
1665+
1666+
[case testPropertyNameAndAttributeIsCheckedPretty]
1667+
# flags: --pretty
1668+
class A:
1669+
@property
1670+
def f(self) -> str: ...
1671+
@not_f.setter
1672+
def f(self, val: str) -> None: ...
1673+
@not_f.deleter
1674+
def f(self) -> None: ...
1675+
1676+
class B:
1677+
@property
1678+
def f(self) -> str: ...
1679+
@f.unknown
1680+
def f(self, val: str) -> None: ...
1681+
[builtins fixtures/property.pyi]
16251682
[out]
1683+
main:5: error: Only supported top decorators are "@f.setter" and "@f.deleter"
1684+
@not_f.setter
1685+
^~~~~~~~~~~~
1686+
main:7: error: Only supported top decorators are "@f.setter" and "@f.deleter"
1687+
@not_f.deleter
1688+
^~~~~~~~~~~~~
1689+
main:13: error: Only supported top decorators are "@f.setter" and "@f.deleter"
1690+
@f.unknown
1691+
^~~~~~~~~
1692+
1693+
[case testPropertyGetterDecoratorIsRejected]
1694+
class A:
1695+
@property
1696+
def f(self) -> str: ...
1697+
@f.getter # E: Only supported top decorators are "@f.setter" and "@f.deleter"
1698+
def f(self, val: str) -> None: ...
1699+
[builtins fixtures/property.pyi]
16261700

16271701
[case testDynamicallyTypedProperty]
16281702
import typing
@@ -7739,7 +7813,7 @@ class A:
77397813
def y(self) -> int: ...
77407814
@y.setter
77417815
def y(self, value: int) -> None: ...
7742-
@dec # E: Only supported top decorator is @y.setter
7816+
@dec # E: Only supported top decorators are "@y.setter" and "@y.deleter"
77437817
def y(self) -> None: ...
77447818

77457819
reveal_type(A().y) # N: Revealed type is "builtins.int"

test-data/unit/check-functions.test

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2752,7 +2752,7 @@ class B:
27522752
@property
27532753
@dec
27542754
def f(self) -> int: pass
2755-
@dec # E: Only supported top decorator is @f.setter
2755+
@dec # E: Only supported top decorators are "@f.setter" and "@f.deleter"
27562756
@f.setter
27572757
def f(self, v: int) -> None: pass
27582758

@@ -2764,7 +2764,6 @@ class C:
27642764
@dec
27652765
def f(self, v: int) -> None: pass
27662766
[builtins fixtures/property.pyi]
2767-
[out]
27682767

27692768
[case testInvalidArgCountForProperty]
27702769
from typing import Callable, TypeVar
@@ -2783,7 +2782,6 @@ class A:
27832782
@property
27842783
def h(self, *args, **kwargs) -> int: pass # OK
27852784
[builtins fixtures/property.pyi]
2786-
[out]
27872785

27882786
[case testSubtypingUnionGenericBounds]
27892787
from typing import Callable, TypeVar, Union, Sequence

test-data/unit/semanal-errors.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,7 +1236,7 @@ class A:
12361236
@overload # E: Decorators on top of @property are not supported
12371237
@property
12381238
def f(self) -> int: pass
1239-
@property # E: Only supported top decorator is @f.setter
1239+
@property # E: Only supported top decorators are "@f.setter" and "@f.deleter"
12401240
@overload
12411241
def f(self) -> int: pass
12421242
[builtins fixtures/property.pyi]

0 commit comments

Comments
 (0)