Skip to content

Commit e9986d8

Browse files
[ty] Reject using properties with Never setters or deleters (#24510)
## Summary We currently error if `__setattr__` returns `Never` and the user attempts to assign to an attribute, and same for deletion. This PR extends that behavior to properties with setters and deleters. For example, this is now forbidden: ```python from typing import NoReturn class NoReturnSetter: @Property def x(self) -> int: return 1 @x.setter def x(self, value: int) -> NoReturn: raise RuntimeError no_return_setter = NoReturnSetter() # error: [invalid-assignment] "Cannot assign to attribute `x` on type `NoReturnSetter` whose `__set__` method returns `Never`/`NoReturn`" no_return_setter.x = 1 ```
1 parent 9cf212f commit e9986d8

5 files changed

Lines changed: 116 additions & 1 deletion

File tree

crates/ty_python_semantic/resources/mdtest/attributes.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,21 @@ C.declared_and_bound = "overwritten on class"
8585
c_instance.declared_and_bound = 1
8686
```
8787

88+
Assignments to ordinary annotated instance attributes should remain valid even when the annotation
89+
is `Never`/`NoReturn`; they should not be mistaken for non-returning descriptors.
90+
91+
```py
92+
from typing import NoReturn
93+
94+
class ClassA:
95+
x: NoReturn
96+
y: list[NoReturn]
97+
98+
def __init__(self, x: NoReturn, y: list[NoReturn]) -> None:
99+
self.x = x
100+
self.y = y
101+
```
102+
88103
#### Variable declared in class body and not bound anywhere
89104

90105
If a variable is declared in the class body but not bound anywhere, we consider it to be accessible

crates/ty_python_semantic/resources/mdtest/del.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ supports_delete = SupportsDelete()
212212
del supports_delete.x
213213

214214
rejects_descriptor_delete = RejectsDescriptorDelete()
215-
# TODO: this should be an error once properties with `Never`/`NoReturn` deleters are rejected
215+
# error: [invalid-assignment] "Cannot delete attribute `x` on type `RejectsDescriptorDelete` whose `__delete__` method returns `Never`/`NoReturn`"
216216
del rejects_descriptor_delete.x
217217

218218
explicit_none_deleter = ExplicitNoneDeleter()

crates/ty_python_semantic/resources/mdtest/properties.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,25 @@ c.attr = 1
248248
reveal_type(c.attr) # revealed: Never
249249
```
250250

251+
### Non-returning setter
252+
253+
```py
254+
from typing import NoReturn
255+
256+
class NoReturnSetter:
257+
@property
258+
def x(self) -> int:
259+
return 1
260+
261+
@x.setter
262+
def x(self, value: int) -> NoReturn:
263+
raise RuntimeError
264+
265+
no_return_setter = NoReturnSetter()
266+
# error: [invalid-assignment] "Cannot assign to attribute `x` on type `NoReturnSetter` whose `__set__` method returns `Never`/`NoReturn`"
267+
no_return_setter.x = 1
268+
```
269+
251270
### Wrong setter signature
252271

253272
```py

crates/ty_python_semantic/src/types/call.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ impl<'db> Type<'db> {
104104
pub(crate) struct CallError<'db>(pub(crate) CallErrorKind, pub(crate) Box<Bindings<'db>>);
105105

106106
impl<'db> CallError<'db> {
107+
pub(crate) fn return_type(&self, db: &'db dyn Db) -> Type<'db> {
108+
self.1.return_type(db)
109+
}
110+
107111
/// Returns `Some(property)` if the call error was caused by an attempt to set a property
108112
/// that has no setter, and `None` otherwise.
109113
pub(crate) fn as_attempt_to_set_property_with_no_setter(

crates/ty_python_semantic/src/types/infer/builder.rs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2030,6 +2030,39 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
20302030
}
20312031
}
20322032

2033+
/// Returns `true` if `property_ty` is a property whose setter returns `Never`/`NoReturn`
2034+
/// when called for an assignment to `object_ty` with `value_ty`.
2035+
fn property_setter_returns_never(
2036+
&self,
2037+
property_ty: Type<'db>,
2038+
object_ty: Type<'db>,
2039+
value_ty: Type<'db>,
2040+
) -> bool {
2041+
let db = self.db();
2042+
property_ty.as_property_instance().is_some_and(|property| {
2043+
property.setter(db).is_some_and(|setter| {
2044+
match setter.try_call(db, &CallArguments::positional([object_ty, value_ty])) {
2045+
Ok(result) => result.return_type(db).is_never(),
2046+
Err(err) => err.return_type(db).is_never(),
2047+
}
2048+
})
2049+
})
2050+
}
2051+
2052+
/// Returns `true` if `property_ty` is a property whose deleter returns `Never`/`NoReturn`
2053+
/// when called for deletion on `object_ty`.
2054+
fn property_deleter_returns_never(&self, property_ty: Type<'db>, object_ty: Type<'db>) -> bool {
2055+
let db = self.db();
2056+
property_ty.as_property_instance().is_some_and(|property| {
2057+
property.deleter(db).is_some_and(|deleter| {
2058+
match deleter.try_call(db, &CallArguments::positional([object_ty])) {
2059+
Ok(result) => result.return_type(db).is_never(),
2060+
Err(err) => err.return_type(db).is_never(),
2061+
}
2062+
})
2063+
})
2064+
}
2065+
20332066
/// Make sure that the attribute assignment `obj.attribute = value` is valid.
20342067
///
20352068
/// `target` is the node for the left-hand side, `object_ty` is the type of `obj`, `attribute` is
@@ -2343,6 +2376,21 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
23432376
&CallArguments::positional([meta_attr_ty, object_ty, value_ty]),
23442377
);
23452378

2379+
if self.property_setter_returns_never(meta_attr_ty, object_ty, value_ty)
2380+
{
2381+
if emit_diagnostics
2382+
&& let Some(builder) =
2383+
self.context.report_lint(&INVALID_ASSIGNMENT, target)
2384+
{
2385+
builder.into_diagnostic(format_args!(
2386+
"Cannot assign to attribute `{attribute}` on type `{}` \
2387+
whose `__set__` method returns `Never`/`NoReturn`",
2388+
object_ty.display(db),
2389+
));
2390+
}
2391+
return false;
2392+
}
2393+
23462394
if emit_diagnostics
23472395
&& let Err(dunder_set_failure) = dunder_set_result.as_ref()
23482396
{
@@ -2539,6 +2587,21 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
25392587
&CallArguments::positional([meta_attr_ty, object_ty, value_ty]),
25402588
);
25412589

2590+
if self.property_setter_returns_never(meta_attr_ty, object_ty, value_ty)
2591+
{
2592+
if emit_diagnostics
2593+
&& let Some(builder) =
2594+
self.context.report_lint(&INVALID_ASSIGNMENT, target)
2595+
{
2596+
builder.into_diagnostic(format_args!(
2597+
"Cannot assign to attribute `{attribute}` on type `{}` \
2598+
whose `__set__` method returns `Never`/`NoReturn`",
2599+
object_ty.display(db),
2600+
));
2601+
}
2602+
return false;
2603+
}
2604+
25422605
if emit_diagnostics
25432606
&& let Err(dunder_set_failure) = dunder_set_result.as_ref()
25442607
{
@@ -2871,6 +2934,20 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
28712934
TypeContext::default(),
28722935
);
28732936

2937+
if self.property_deleter_returns_never(attr_ty, object_ty) {
2938+
if emit_diagnostics
2939+
&& let Some(builder) =
2940+
self.context.report_lint(&INVALID_ASSIGNMENT, target)
2941+
{
2942+
builder.into_diagnostic(format_args!(
2943+
"Cannot delete attribute `{attribute}` on type `{}` \
2944+
whose `__delete__` method returns `Never`/`NoReturn`",
2945+
object_ty.display(db),
2946+
));
2947+
}
2948+
return false;
2949+
}
2950+
28742951
match delete_dunder_call_result {
28752952
Ok(_) | Err(CallDunderError::PossiblyUnbound(_)) => return true,
28762953
Err(CallDunderError::CallError(kind, bindings)) => {

0 commit comments

Comments
 (0)