mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-18 11:41:21 +00:00
[ty] Add support for properties that return Self (#21335)
## Summary Detect usages of implicit `self` in property getters, which allows us to treat their signature as being generic. closes https://github.com/astral-sh/ty/issues/1502 ## Typing conformance Two new type assertions that are succeeding. ## Ecosystem results Mostly look good. There are a few new false positives related to a bug with constrained typevars that is unrelated to the work here. I reported this as https://github.com/astral-sh/ty/issues/1503. ## Test Plan Added regression tests.
This commit is contained in:
parent
a6f2dee33b
commit
ab46c8de0f
3 changed files with 88 additions and 22 deletions
|
|
@ -139,7 +139,7 @@ The first parameter of instance methods always has type `Self`, if it is not exp
|
||||||
The name `self` is not special in any way.
|
The name `self` is not special in any way.
|
||||||
|
|
||||||
```py
|
```py
|
||||||
def some_decorator(f: Callable) -> Callable:
|
def some_decorator[**P, R](f: Callable[P, R]) -> Callable[P, R]:
|
||||||
return f
|
return f
|
||||||
|
|
||||||
class B:
|
class B:
|
||||||
|
|
@ -188,10 +188,10 @@ class B:
|
||||||
reveal_type(B().name_does_not_matter()) # revealed: B
|
reveal_type(B().name_does_not_matter()) # revealed: B
|
||||||
reveal_type(B().positional_only(1)) # revealed: B
|
reveal_type(B().positional_only(1)) # revealed: B
|
||||||
reveal_type(B().keyword_only(x=1)) # revealed: B
|
reveal_type(B().keyword_only(x=1)) # revealed: B
|
||||||
|
# TODO: This should deally be `B`
|
||||||
reveal_type(B().decorated_method()) # revealed: Unknown
|
reveal_type(B().decorated_method()) # revealed: Unknown
|
||||||
|
|
||||||
# TODO: this should be B
|
reveal_type(B().a_property) # revealed: B
|
||||||
reveal_type(B().a_property) # revealed: Unknown
|
|
||||||
|
|
||||||
async def _():
|
async def _():
|
||||||
reveal_type(await B().async_method()) # revealed: B
|
reveal_type(await B().async_method()) # revealed: B
|
||||||
|
|
|
||||||
|
|
@ -49,6 +49,40 @@ c.my_property = 2
|
||||||
c.my_property = "a"
|
c.my_property = "a"
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Properties returning `Self`
|
||||||
|
|
||||||
|
A property that returns `Self` refers to an instance of the class:
|
||||||
|
|
||||||
|
```py
|
||||||
|
from typing_extensions import Self
|
||||||
|
|
||||||
|
class Path:
|
||||||
|
@property
|
||||||
|
def parent(self) -> Self:
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
reveal_type(Path().parent) # revealed: Path
|
||||||
|
```
|
||||||
|
|
||||||
|
This also works when a setter is defined:
|
||||||
|
|
||||||
|
```py
|
||||||
|
class Node:
|
||||||
|
@property
|
||||||
|
def parent(self) -> Self:
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
@parent.setter
|
||||||
|
def parent(self, value: Self) -> None:
|
||||||
|
pass
|
||||||
|
|
||||||
|
root = Node()
|
||||||
|
child = Node()
|
||||||
|
child.parent = root
|
||||||
|
|
||||||
|
reveal_type(child.parent) # revealed: Node
|
||||||
|
```
|
||||||
|
|
||||||
## `property.getter`
|
## `property.getter`
|
||||||
|
|
||||||
`property.getter` can be used to overwrite the getter method of a property. This does not overwrite
|
`property.getter` can be used to overwrite the getter method of a property. This does not overwrite
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@
|
||||||
use std::{collections::HashMap, slice::Iter};
|
use std::{collections::HashMap, slice::Iter};
|
||||||
|
|
||||||
use itertools::{EitherOrBoth, Itertools};
|
use itertools::{EitherOrBoth, Itertools};
|
||||||
|
use ruff_db::parsed::parsed_module;
|
||||||
use ruff_python_ast::ParameterWithDefault;
|
use ruff_python_ast::ParameterWithDefault;
|
||||||
use smallvec::{SmallVec, smallvec_inline};
|
use smallvec::{SmallVec, smallvec_inline};
|
||||||
|
|
||||||
|
|
@ -20,9 +21,9 @@ use super::{
|
||||||
DynamicType, Type, TypeVarVariance, definition_expression_type, infer_definition_types,
|
DynamicType, Type, TypeVarVariance, definition_expression_type, infer_definition_types,
|
||||||
semantic_index,
|
semantic_index,
|
||||||
};
|
};
|
||||||
use crate::semantic_index::definition::Definition;
|
use crate::semantic_index::definition::{Definition, DefinitionKind};
|
||||||
use crate::types::constraints::{ConstraintSet, IteratorConstraintsExtension};
|
use crate::types::constraints::{ConstraintSet, IteratorConstraintsExtension};
|
||||||
use crate::types::function::FunctionType;
|
use crate::types::function::{is_implicit_classmethod, is_implicit_staticmethod};
|
||||||
use crate::types::generics::{
|
use crate::types::generics::{
|
||||||
GenericContext, InferableTypeVars, typing_self, walk_generic_context,
|
GenericContext, InferableTypeVars, typing_self, walk_generic_context,
|
||||||
};
|
};
|
||||||
|
|
@ -36,8 +37,11 @@ use crate::{Db, FxOrderSet};
|
||||||
use ruff_python_ast::{self as ast, name::Name};
|
use ruff_python_ast::{self as ast, name::Name};
|
||||||
|
|
||||||
#[derive(Clone, Copy, Debug)]
|
#[derive(Clone, Copy, Debug)]
|
||||||
|
#[expect(clippy::struct_excessive_bools)]
|
||||||
struct MethodInformation<'db> {
|
struct MethodInformation<'db> {
|
||||||
method: FunctionType<'db>,
|
is_staticmethod: bool,
|
||||||
|
is_classmethod: bool,
|
||||||
|
method_may_be_generic: bool,
|
||||||
class_literal: ClassLiteral<'db>,
|
class_literal: ClassLiteral<'db>,
|
||||||
class_is_generic: bool,
|
class_is_generic: bool,
|
||||||
}
|
}
|
||||||
|
|
@ -46,17 +50,49 @@ fn infer_method_information<'db>(
|
||||||
db: &'db dyn Db,
|
db: &'db dyn Db,
|
||||||
definition: Definition<'db>,
|
definition: Definition<'db>,
|
||||||
) -> Option<MethodInformation<'db>> {
|
) -> Option<MethodInformation<'db>> {
|
||||||
|
let DefinitionKind::Function(function_definition) = definition.kind(db) else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
|
||||||
let class_scope_id = definition.scope(db);
|
let class_scope_id = definition.scope(db);
|
||||||
let file = class_scope_id.file(db);
|
let file = class_scope_id.file(db);
|
||||||
|
let module = parsed_module(db, file).load(db);
|
||||||
let index = semantic_index(db, file);
|
let index = semantic_index(db, file);
|
||||||
|
|
||||||
let class_scope = index.scope(class_scope_id.file_scope_id(db));
|
let class_scope = index.scope(class_scope_id.file_scope_id(db));
|
||||||
let class_node = class_scope.node().as_class()?;
|
let class_node = class_scope.node().as_class()?;
|
||||||
|
|
||||||
let method = infer_definition_types(db, definition)
|
let function_node = function_definition.node(&module);
|
||||||
.declaration_type(definition)
|
let function_name = &function_node.name;
|
||||||
.inner_type()
|
|
||||||
.as_function_literal()?;
|
let mut is_staticmethod = is_implicit_classmethod(function_name);
|
||||||
|
let mut is_classmethod = is_implicit_staticmethod(function_name);
|
||||||
|
|
||||||
|
let inference = infer_definition_types(db, definition);
|
||||||
|
for decorator in &function_node.decorator_list {
|
||||||
|
let decorator_ty = inference.expression_type(&decorator.expression);
|
||||||
|
|
||||||
|
match decorator_ty
|
||||||
|
.as_class_literal()
|
||||||
|
.and_then(|class| class.known(db))
|
||||||
|
{
|
||||||
|
Some(KnownClass::Staticmethod) => {
|
||||||
|
is_staticmethod = true;
|
||||||
|
}
|
||||||
|
Some(KnownClass::Classmethod) => {
|
||||||
|
is_classmethod = true;
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let method_may_be_generic = match inference.declaration_type(definition).inner_type() {
|
||||||
|
Type::FunctionLiteral(f) => f.signature(db).overloads.iter().any(|s| {
|
||||||
|
s.generic_context
|
||||||
|
.is_some_and(|context| context.variables(db).any(|v| v.typevar(db).is_self(db)))
|
||||||
|
}),
|
||||||
|
_ => true,
|
||||||
|
};
|
||||||
|
|
||||||
let class_def = index.expect_single_definition(class_node);
|
let class_def = index.expect_single_definition(class_node);
|
||||||
let (class_literal, class_is_generic) = match infer_definition_types(db, class_def)
|
let (class_literal, class_is_generic) = match infer_definition_types(db, class_def)
|
||||||
|
|
@ -71,7 +107,9 @@ fn infer_method_information<'db>(
|
||||||
};
|
};
|
||||||
|
|
||||||
Some(MethodInformation {
|
Some(MethodInformation {
|
||||||
method,
|
is_staticmethod,
|
||||||
|
is_classmethod,
|
||||||
|
method_may_be_generic,
|
||||||
class_literal,
|
class_literal,
|
||||||
class_is_generic,
|
class_is_generic,
|
||||||
})
|
})
|
||||||
|
|
@ -1270,27 +1308,21 @@ impl<'db> Parameters<'db> {
|
||||||
};
|
};
|
||||||
|
|
||||||
let method_info = infer_method_information(db, definition);
|
let method_info = infer_method_information(db, definition);
|
||||||
let is_static_or_classmethod = method_info
|
let is_static_or_classmethod =
|
||||||
.is_some_and(|f| f.method.is_staticmethod(db) || f.method.is_classmethod(db));
|
method_info.is_some_and(|f| f.is_staticmethod || f.is_classmethod);
|
||||||
|
|
||||||
let inferred_annotation = |arg: &ParameterWithDefault| {
|
let inferred_annotation = |arg: &ParameterWithDefault| {
|
||||||
if let Some(MethodInformation {
|
if let Some(MethodInformation {
|
||||||
method,
|
method_may_be_generic,
|
||||||
class_literal,
|
class_literal,
|
||||||
class_is_generic,
|
class_is_generic,
|
||||||
|
..
|
||||||
}) = method_info
|
}) = method_info
|
||||||
&& !is_static_or_classmethod
|
&& !is_static_or_classmethod
|
||||||
&& arg.parameter.annotation().is_none()
|
&& arg.parameter.annotation().is_none()
|
||||||
&& parameters.index(arg.name().id()) == Some(0)
|
&& parameters.index(arg.name().id()) == Some(0)
|
||||||
{
|
{
|
||||||
let method_has_self_in_generic_context =
|
if method_may_be_generic
|
||||||
method.signature(db).overloads.iter().any(|s| {
|
|
||||||
s.generic_context.is_some_and(|context| {
|
|
||||||
context.variables(db).any(|v| v.typevar(db).is_self(db))
|
|
||||||
})
|
|
||||||
});
|
|
||||||
|
|
||||||
if method_has_self_in_generic_context
|
|
||||||
|| class_is_generic
|
|| class_is_generic
|
||||||
|| class_literal
|
|| class_literal
|
||||||
.known(db)
|
.known(db)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue