mirror of
https://github.com/astral-sh/ruff
synced 2026-01-21 05:20:49 -05:00
[ty] Avoid including property in subclasses properties (#22088)
## Summary As-is, the following rejects `return self.value` in `def other` in the subclass ([link](https://play.ty.dev/f55b47b2-313e-45d1-ba45-fde410bed32e)) because `self.value` is resolving to `Unknown | int | float | property`: ```python class Base: _value: float = 0.0 @property def value(self) -> float: return self._value @value.setter def value(self, v: float) -> None: self._value = v @property def other(self) -> float: return self.value @other.setter def other(self, v: float) -> None: self.value = v class Derived(Base): @property def other(self) -> float: return self.value @other.setter def other(self, v: float) -> None: reveal_type(self.value) # revealed: int | float self.value = v ``` I believe the root cause is that we're not excluding properties when searching for class methods, so we're treating the `other` setter as a classmethod. I don't fully understand how that ends up materializing as `| property` on the union though.
This commit is contained in:
@@ -525,6 +525,42 @@ c.name = None
|
||||
c.name = 42
|
||||
```
|
||||
|
||||
### Overriding properties in subclasses
|
||||
|
||||
When a subclass overrides a property, accessing other inherited properties from within the
|
||||
overriding property methods should still work correctly.
|
||||
|
||||
```py
|
||||
class Base:
|
||||
_value: float = 0.0
|
||||
|
||||
@property
|
||||
def value(self) -> float:
|
||||
return self._value
|
||||
|
||||
@value.setter
|
||||
def value(self, v: float) -> None:
|
||||
self._value = v
|
||||
|
||||
@property
|
||||
def other(self) -> float:
|
||||
return self.value
|
||||
|
||||
@other.setter
|
||||
def other(self, v: float) -> None:
|
||||
self.value = v
|
||||
|
||||
class Derived(Base):
|
||||
@property
|
||||
def other(self) -> float:
|
||||
return self.value
|
||||
|
||||
@other.setter
|
||||
def other(self, v: float) -> None:
|
||||
reveal_type(self.value) # revealed: int | float
|
||||
self.value = v
|
||||
```
|
||||
|
||||
### Properties with no setters
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
@@ -3479,16 +3479,27 @@ impl<'db> ClassLiteral<'db> {
|
||||
let is_valid_scope = |method_scope: &Scope| {
|
||||
if let Some(method_def) = method_scope.node().as_function() {
|
||||
let method_name = method_def.node(&module).name.as_str();
|
||||
if let Some(Type::FunctionLiteral(method_type)) =
|
||||
class_member(db, class_body_scope, method_name)
|
||||
.inner
|
||||
.place
|
||||
.ignore_possibly_undefined()
|
||||
match class_member(db, class_body_scope, method_name)
|
||||
.inner
|
||||
.place
|
||||
.ignore_possibly_undefined()
|
||||
{
|
||||
let method_decorator = MethodDecorator::try_from_fn_type(db, method_type);
|
||||
if method_decorator != Ok(target_method_decorator) {
|
||||
return false;
|
||||
Some(Type::FunctionLiteral(method_type)) => {
|
||||
let method_decorator = MethodDecorator::try_from_fn_type(db, method_type);
|
||||
if method_decorator != Ok(target_method_decorator) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
Some(Type::PropertyInstance(_)) => {
|
||||
// Property getters and setters have their own scopes. They take `self`
|
||||
// as the first parameter (like regular instance methods), so they're
|
||||
// included when looking for `MethodDecorator::None`. However, they're
|
||||
// not classmethods or staticmethods, so exclude them for those cases.
|
||||
if target_method_decorator != MethodDecorator::None {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
true
|
||||
|
||||
Reference in New Issue
Block a user