Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[ty] Defer all parameter and return type annotations (#21906) 

As described in astral-sh/ty#1729, we previously had a salsa cycle when
inferring the signature of many function definitions.

The most obvious case happened when (a) the function was decorated, (b)
it had no PEP-695 type params, and (c) annotations were not always
deferred (e.g. in a stub file). We currently evaluate and apply function
decorators eagerly, as part of `infer_function_definition`. Applying a
decorator requires knowing the signature of the function being
decorated. There were two places where signature construction called
`infer_definition_types` cyclically.

The simpler case was that we were looking up the generic context and
decorator list of the function to determine whether it has an implicit
`self` parameter. Before, we used `infer_definition_types` to determine
that information. But since we're in the middle of signature
construction for the function, we can just thread the information
through directly.

The harder case is that signature construction requires knowing the
inferred parameter and return type annotations. When (b) and (c) hold,
those type annotations are inferred in `infer_function_definition`! (In
theory, we've already finished that by the time we start applying
decorators, but signature construction doesn't know that.)

If annotations are deferred, the params/return annotations are inferred
in `infer_deferred_types`; if there are PEP-695 type params, they're
inferred in `infer_function_type_params`. Both of those are different
salsa queries, and don't induce this cycle.

So the quick fix here is to always defer inference of the function
params/return, so that they are always inferred under a different salsa
query.

A more principled fix would be to apply decorators lazily, just like we
construct signatures lazily. But that is a more invasive fix.

Fixes astral-sh/ty#1729

---------

Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
This commit is contained in:
Douglas Creager 2025-12-11 15:00:18 -05:00 committed by GitHub
parent d442433e93
commit c8851ecf70
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 269 additions and 216 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
crates/ty_python_semantic/resources/mdtest/annotations/self.md
View File

@ -282,8 +282,10 @@ reveal_type(C().method()) # revealed: C
## Class Methods
### Explicit
```py
from typing import Self, TypeVar
from typing import Self
class Shape:
def foo(self: Self) -> Self:
@ -298,6 +300,64 @@ class Circle(Shape): ...
reveal_type(Shape().foo()) # revealed: Shape
reveal_type(Shape.bar()) # revealed: Shape
reveal_type(Circle().foo()) # revealed: Circle
reveal_type(Circle.bar()) # revealed: Circle
```
### Implicit
```py
from typing import Self
class Shape:
def foo(self) -> Self:
return self
@classmethod
def bar(cls) -> Self:
reveal_type(cls) # revealed: type[Self@bar]
return cls()
class Circle(Shape): ...
reveal_type(Shape().foo()) # revealed: Shape
reveal_type(Shape.bar()) # revealed: Shape
reveal_type(Circle().foo()) # revealed: Circle
reveal_type(Circle.bar()) # revealed: Circle
```
### Implicit in generic class
```py
from typing import Self
class GenericShape[T]:
def foo(self) -> Self:
return self
@classmethod
def bar(cls) -> Self:
reveal_type(cls) # revealed: type[Self@bar]
return cls()
@classmethod
def baz[U](cls, u: U) -> "GenericShape[U]":
reveal_type(cls) # revealed: type[Self@baz]
return cls()
class GenericCircle[T](GenericShape[T]): ...
reveal_type(GenericShape().foo()) # revealed: GenericShape[Unknown]
reveal_type(GenericShape.bar()) # revealed: GenericShape[Unknown]
reveal_type(GenericShape[int].bar()) # revealed: GenericShape[int]
reveal_type(GenericShape.baz(1)) # revealed: GenericShape[Literal[1]]
reveal_type(GenericCircle().foo()) # revealed: GenericCircle[Unknown]
reveal_type(GenericCircle.bar()) # revealed: GenericCircle[Unknown]
reveal_type(GenericCircle[int].bar()) # revealed: GenericCircle[int]
reveal_type(GenericCircle.baz(1)) # revealed: GenericShape[Literal[1]]
```
## Attributes

5
crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md
View File

@ -417,16 +417,13 @@ The `converter` function act as a decorator here:
def f3(x: int, y: str) -> int:
return 1
# TODO: This should reveal `(x: int, y: str) -> bool` but there's a cycle: https://github.com/astral-sh/ty/issues/1729
reveal_type(f3) # revealed: ((x: int, y: str) -> bool) | ((x: Divergent, y: Divergent) -> bool)
reveal_type(f3) # revealed: (x: int, y: str) -> bool
reveal_type(f3(1, "a")) # revealed: bool
reveal_type(f3(x=1, y="a")) # revealed: bool
reveal_type(f3(1, y="a")) # revealed: bool
reveal_type(f3(y="a", x=1)) # revealed: bool
# TODO: There should only be one error but the type of `f3` is a union: https://github.com/astral-sh/ty/issues/1729
# error: [missing-argument] "No argument provided for required parameter `y`"
# error: [missing-argument] "No argument provided for required parameter `y`"
f3(1)
# error: [invalid-argument-type] "Argument is incorrect: Expected `int`, found `Literal["a"]`"

4
crates/ty_python_semantic/src/types/context.rs
View File

@ -177,8 +177,8 @@ impl<'db, 'ast> InferContext<'db, 'ast> {
std::mem::replace(&mut self.multi_inference, multi_inference)
}
pub(super) fn set_in_no_type_check(&mut self, no_type_check: InNoTypeCheck) {
self.no_type_check = no_type_check;
pub(super) fn set_in_no_type_check(&mut self, no_type_check: InNoTypeCheck) -> InNoTypeCheck {
std::mem::replace(&mut self.no_type_check, no_type_check)
}
fn is_in_no_type_check(&self) -> bool {

90
crates/ty_python_semantic/src/types/function.rs
View File

@ -73,7 +73,8 @@ use crate::types::diagnostic::{
report_runtime_check_against_non_runtime_checkable_protocol,
};
use crate::types::display::DisplaySettings;
use crate::types::generics::{GenericContext, InferableTypeVars};
use crate::types::generics::{GenericContext, InferableTypeVars, typing_self};
use crate::types::infer::nearest_enclosing_class;
use crate::types::list_members::all_members;
use crate::types::narrow::ClassInfoConstraintFunction;
use crate::types::signatures::{CallableSignature, Signature};
@ -82,8 +83,9 @@ use crate::types::{
ApplyTypeMappingVisitor, BoundMethodType, BoundTypeVarInstance, CallableType, CallableTypeKind,
ClassBase, ClassLiteral, ClassType, DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor,
HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType,
NormalizedVisitor, SpecialFormType, Truthiness, Type, TypeContext, TypeMapping, TypeRelation,
UnionBuilder, binding_type, definition_expression_type, walk_signature,
NormalizedVisitor, SpecialFormType, SubclassOfInner, SubclassOfType, Truthiness, Type,
TypeContext, TypeMapping, TypeRelation, UnionBuilder, binding_type, definition_expression_type,
infer_definition_types, walk_signature,
};
use crate::{Db, FxOrderSet, ModuleName, resolve_module};
@ -499,13 +501,91 @@ impl<'db> OverloadLiteral<'db> {
index,
);
Signature::from_function(
let mut raw_signature = Signature::from_function(
db,
pep695_ctx,
definition,
function_stmt_node,
has_implicitly_positional_first_parameter,
)
);
let generic_context = raw_signature.generic_context;
raw_signature.add_implicit_self_annotation(db, || {
if self.is_staticmethod(db) {
return None;
}
// We have not yet added an implicit annotation to the `self` parameter, so any
// typevars that currently appear in the method's generic context come from explicit
// annotations.
let method_has_explicit_self = generic_context
.is_some_and(|context| context.variables(db).any(|v| v.typevar(db).is_self(db)));
let class_scope_id = definition.scope(db);
let class_scope = index.scope(class_scope_id.file_scope_id(db));
let class_node = class_scope.node().as_class()?;
let class_def = index.expect_single_definition(class_node);
let Type::ClassLiteral(class_literal) = infer_definition_types(db, class_def)
.declaration_type(class_def)
.inner_type()
else {
return None;
};
let class_is_generic = class_literal.generic_context(db).is_some();
let class_is_fallback = class_literal
.known(db)
.is_some_and(KnownClass::is_fallback_class);
// Normally we implicitly annotate `self` or `cls` with `Self` or `type[Self]`, and
// create a `Self` typevar that we then have to solve for whenever this method is
// called. As an optimization, we can skip creating that typevar in certain situations:
//
// - The method cannot use explicit `Self` in any other parameter annotations,
// or in its return type. If it does, then we really do need specialization
// inference at each call site to see which specific instance type should be
// used in those other parameters / return type.
//
// - The class cannot be generic. If it is, then we might need an actual `Self`
// typevar to help carry through constraints that relate the instance type to
// other typevars in the method signature.
//
// - The class cannot be a "fallback class". A fallback class is used like a mixin,
// and so we need specialization inference to determine the "real" class that the
// fallback is augmenting. (See KnownClass::is_fallback_class for more details.)
if method_has_explicit_self || class_is_generic || class_is_fallback {
let scope_id = definition.scope(db);
let typevar_binding_context = Some(definition);
let index = semantic_index(db, scope_id.file(db));
let class = nearest_enclosing_class(db, index, scope_id).unwrap();
let typing_self = typing_self(db, scope_id, typevar_binding_context, class).expect(
"We should always find the surrounding class \
for an implicit self: Self annotation",
);
if self.is_classmethod(db) {
Some(SubclassOfType::from(
db,
SubclassOfInner::TypeVar(typing_self),
))
} else {
Some(Type::TypeVar(typing_self))
}
} else {
// If skip creating the typevar, we use "instance of class" or "subclass of
// class" as the implicit annotation instead.
if self.is_classmethod(db) {
Some(SubclassOfType::from(
db,
SubclassOfInner::Class(ClassType::NonGeneric(class_literal)),
))
} else {
Some(class_literal.to_non_generic_instance(db))
}
}
});
raw_signature
}
pub(crate) fn parameter_span(

38
crates/ty_python_semantic/src/types/infer/builder.rs
View File

@ -2241,7 +2241,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
name,
type_params,
parameters,
returns,
returns: _,
body: _,
decorator_list,
} = function;
@ -2288,21 +2288,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
self.infer_expression(default, TypeContext::default());
}
// If there are type params, parameters and returns are evaluated in that scope, that is, in
// `infer_function_type_params`, rather than here.
// If there are type params, parameters and returns are evaluated in that scope. Otherwise,
// we always defer the inference of the parameters and returns. That ensures that we do not
// add any spurious salsa cycles when applying decorators below. (Applying a decorator
// requires getting the signature of this function definition, which in turn requires
// (lazily) inferring the parameter and return types.)
if type_params.is_none() {
if self.defer_annotations() {
self.deferred.insert(definition, self.multi_inference_state);
} else {
let previous_typevar_binding_context =
self.typevar_binding_context.replace(definition);
self.infer_return_type_annotation(
returns.as_deref(),
DeferredExpressionState::None,
);
self.infer_parameters(parameters);
self.typevar_binding_context = previous_typevar_binding_context;
}
self.deferred.insert(definition, self.multi_inference_state);
}
let known_function =
@ -2946,10 +2938,24 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
definition: Definition<'db>,
function: &ast::StmtFunctionDef,
) {
let mut prev_in_no_type_check = self.context.set_in_no_type_check(InNoTypeCheck::Yes);
for decorator in &function.decorator_list {
let decorator_type = self.infer_decorator(decorator);
if let Type::FunctionLiteral(function) = decorator_type
&& let Some(KnownFunction::NoTypeCheck) = function.known(self.db())
{
// If the function is decorated with the `no_type_check` decorator,
// we need to suppress any errors that come after the decorators.
prev_in_no_type_check = InNoTypeCheck::Yes;
break;
}
}
self.context.set_in_no_type_check(prev_in_no_type_check);
let previous_typevar_binding_context = self.typevar_binding_context.replace(definition);
self.infer_return_type_annotation(
function.returns.as_deref(),
DeferredExpressionState::Deferred,
self.defer_annotations().into(),
);
self.infer_parameters(function.parameters.as_ref());
self.typevar_binding_context = previous_typevar_binding_context;

286
crates/ty_python_semantic/src/types/signatures.rs
View File

@ -13,107 +13,46 @@
use std::{collections::HashMap, slice::Iter};
use itertools::{EitherOrBoth, Itertools};
use ruff_db::parsed::parsed_module;
use ruff_python_ast::ParameterWithDefault;
use smallvec::{SmallVec, smallvec_inline};
use super::{
ClassType, DynamicType, Type, TypeVarVariance, definition_expression_type,
infer_definition_types, semantic_index,
};
use crate::semantic_index::definition::{Definition, DefinitionKind};
use super::{DynamicType, Type, TypeVarVariance, definition_expression_type, semantic_index};
use crate::semantic_index::definition::Definition;
use crate::types::constraints::{ConstraintSet, IteratorConstraintsExtension};
use crate::types::function::{is_implicit_classmethod, is_implicit_staticmethod};
use crate::types::generics::{
GenericContext, InferableTypeVars, typing_self, walk_generic_context,
};
use crate::types::infer::nearest_enclosing_class;
use crate::types::generics::{GenericContext, InferableTypeVars, walk_generic_context};
use crate::types::infer::{infer_deferred_types, infer_scope_types};
use crate::types::{
ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, CallableTypeKind, ClassLiteral,
ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, CallableTypeKind,
FindLegacyTypeVarsVisitor, HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor,
KnownClass, MaterializationKind, NormalizedVisitor, ParamSpecAttrKind, SubclassOfInner,
SubclassOfType, TypeContext, TypeMapping, TypeRelation, VarianceInferable, todo_type,
KnownClass, MaterializationKind, NormalizedVisitor, ParamSpecAttrKind, TypeContext,
TypeMapping, TypeRelation, VarianceInferable, todo_type,
};
use crate::{Db, FxOrderSet};
use ruff_python_ast::{self as ast, name::Name};
#[derive(Clone, Copy, Debug)]
#[expect(clippy::struct_excessive_bools)]
struct MethodInformation<'db> {
is_staticmethod: bool,
is_classmethod: bool,
method_may_be_generic: bool,
class_literal: ClassLiteral<'db>,
class_is_generic: bool,
}
fn infer_method_information<'db>(
/// Infer the type of a parameter or return annotation in a function signature.
///
/// This is very similar to [`definition_expression_type`], but knows that `TypeInferenceBuilder`
/// will always infer the parameters and return of a function in its PEP-695 typevar scope, if
/// there is one; otherwise they will be inferred in the function definition scope, but will always
/// be deferred. (This prevents spurious salsa cycles when we need the signature of the function
/// while in the middle of inferring its definition scope — for instance, when applying
/// decorators.)
fn function_signature_expression_type<'db>(
db: &'db dyn Db,
definition: Definition<'db>,
) -> Option<MethodInformation<'db>> {
let DefinitionKind::Function(function_definition) = definition.kind(db) else {
return None;
};
let class_scope_id = definition.scope(db);
let file = class_scope_id.file(db);
let module = parsed_module(db, file).load(db);
expression: &ast::Expr,
) -> Type<'db> {
let file = definition.file(db);
let index = semantic_index(db, file);
let class_scope = index.scope(class_scope_id.file_scope_id(db));
let class_node = class_scope.node().as_class()?;
let function_node = function_definition.node(&module);
let function_name = &function_node.name;
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 file_scope = index.expression_scope_id(expression);
let scope = file_scope.to_scope_id(db, file);
if scope == definition.scope(db) {
// expression is in the function definition scope, but always deferred
infer_deferred_types(db, definition).expression_type(expression)
} else {
// expression is in the PEP-695 type params sub-scope
infer_scope_types(db, scope).expression_type(expression)
}
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_literal, class_is_generic) = match infer_definition_types(db, class_def)
.declaration_type(class_def)
.inner_type()
{
Type::ClassLiteral(class_literal) => {
(class_literal, class_literal.generic_context(db).is_some())
}
Type::GenericAlias(alias) => (alias.origin(db), true),
_ => return None,
};
Some(MethodInformation {
is_staticmethod,
is_classmethod,
method_may_be_generic,
class_literal,
class_is_generic,
})
}
/// The signature of a single callable. If the callable is overloaded, there is a separate
@ -615,7 +554,7 @@ impl<'db> Signature<'db> {
let return_ty = function_node
.returns
.as_ref()
.map(|returns| definition_expression_type(db, definition, returns.as_ref()));
.map(|returns| function_signature_expression_type(db, definition, returns.as_ref()));
let legacy_generic_context =
GenericContext::from_function_params(db, definition, &parameters, return_ty);
let full_generic_context = GenericContext::merge_pep695_and_legacy(
@ -771,6 +710,57 @@ impl<'db> Signature<'db> {
&self.parameters
}
/// Adds an implicit annotation to the first parameter of this signature, if that parameter is
/// positional and does not already have an annotation. We do not check whether that's the
/// right thing to do! The caller must determine whether the first parameter is actually a
/// `self` or `cls` parameter, and must determine the correct type to use as the implicit
/// annotation.
pub(crate) fn add_implicit_self_annotation(
&mut self,
db: &'db dyn Db,
self_type: impl FnOnce() -> Option<Type<'db>>,
) {
if let Some(first_parameter) = self.parameters.value.first_mut()
&& first_parameter.is_positional()
&& first_parameter.annotated_type.is_none()
&& let Some(self_type) = self_type()
{
first_parameter.annotated_type = Some(self_type);
first_parameter.inferred_annotation = true;
// If we've added an implicit `self` annotation, we might need to update the
// signature's generic context, too. (The generic context should include any synthetic
// typevars created for `typing.Self`, even if the `typing.Self` annotation was added
// implicitly.)
let self_typevar = match self_type {
Type::TypeVar(self_typevar) => Some(self_typevar),
Type::SubclassOf(subclass_of) => subclass_of.into_type_var(),
_ => None,
};
if let Some(self_typevar) = self_typevar {
match self.generic_context.as_mut() {
Some(generic_context)
if generic_context
.binds_typevar(db, self_typevar.typevar(db))
.is_some() => {}
Some(generic_context) => {
*generic_context = GenericContext::from_typevar_instances(
db,
std::iter::once(self_typevar).chain(generic_context.variables(db)),
);
}
None => {
self.generic_context = Some(GenericContext::from_typevar_instances(
db,
std::iter::once(self_typevar),
));
}
}
}
}
}
/// Return the definition associated with this signature, if any.
pub(crate) fn definition(&self) -> Option<Definition<'db>> {
self.definition
@ -1674,84 +1664,16 @@ impl<'db> Parameters<'db> {
})
};
let method_info = infer_method_information(db, definition);
let is_staticmethod = method_info.is_some_and(|f| f.is_staticmethod);
let is_classmethod = method_info.is_some_and(|f| f.is_classmethod);
let inferred_annotation = |arg: &ParameterWithDefault| {
if let Some(MethodInformation {
method_may_be_generic,
class_literal,
class_is_generic,
..
}) = method_info
&& !is_staticmethod
&& arg.parameter.annotation().is_none()
&& parameters.index(arg.name().id()) == Some(0)
{
if method_may_be_generic
|| class_is_generic
|| class_literal
.known(db)
.is_some_and(KnownClass::is_fallback_class)
{
let scope_id = definition.scope(db);
let typevar_binding_context = Some(definition);
let index = semantic_index(db, scope_id.file(db));
let class = nearest_enclosing_class(db, index, scope_id).unwrap();
let typing_self = typing_self(db, scope_id, typevar_binding_context, class)
.expect("We should always find the surrounding class for an implicit self: Self annotation");
if is_classmethod {
Some(SubclassOfType::from(
db,
SubclassOfInner::TypeVar(typing_self),
))
} else {
Some(Type::TypeVar(typing_self))
}
} else {
// For methods of non-generic classes that are not otherwise generic (e.g. return `Self` or
// have additional type parameters), the implicit `Self` type of the `self`, or the implicit
// `type[Self]` type of the `cls` parameter, would be the only type variable, so we can just
// use the class directly.
if is_classmethod {
Some(SubclassOfType::from(
db,
SubclassOfInner::Class(ClassType::NonGeneric(class_literal)),
))
} else {
Some(class_literal.to_non_generic_instance(db))
}
}
} else {
None
}
};
let pos_only_param = |param: &ast::ParameterWithDefault| {
if let Some(inferred_annotation_type) = inferred_annotation(param) {
Parameter {
annotated_type: Some(inferred_annotation_type),
inferred_annotation: true,
kind: ParameterKind::PositionalOnly {
name: Some(param.parameter.name.id.clone()),
default_type: default_type(param),
},
form: ParameterForm::Value,
}
} else {
Parameter::from_node_and_kind(
db,
definition,
&param.parameter,
ParameterKind::PositionalOnly {
name: Some(param.parameter.name.id.clone()),
default_type: default_type(param),
},
)
}
Parameter::from_node_and_kind(
db,
definition,
&param.parameter,
ParameterKind::PositionalOnly {
name: Some(param.parameter.name.id.clone()),
default_type: default_type(param),
},
)
};
let mut positional_only: Vec<Parameter> = posonlyargs.iter().map(pos_only_param).collect();
@ -1775,27 +1697,15 @@ impl<'db> Parameters<'db> {
}
let positional_or_keyword = pos_or_keyword_iter.map(|arg| {
if let Some(inferred_annotation_type) = inferred_annotation(arg) {
Parameter {
annotated_type: Some(inferred_annotation_type),
inferred_annotation: true,
kind: ParameterKind::PositionalOrKeyword {
name: arg.parameter.name.id.clone(),
default_type: default_type(arg),
},
form: ParameterForm::Value,
}
} else {
Parameter::from_node_and_kind(
db,
definition,
&arg.parameter,
ParameterKind::PositionalOrKeyword {
name: arg.parameter.name.id.clone(),
default_type: default_type(arg),
},
)
}
Parameter::from_node_and_kind(
db,
definition,
&arg.parameter,
ParameterKind::PositionalOrKeyword {
name: arg.parameter.name.id.clone(),
default_type: default_type(arg),
},
)
});
let variadic = vararg.as_ref().map(|arg| {
@ -2212,7 +2122,7 @@ impl<'db> Parameter<'db> {
Self {
annotated_type: parameter
.annotation()
.map(|annotation| definition_expression_type(db, definition, annotation)),
.map(|annotation| function_signature_expression_type(db, definition, annotation)),
kind,
form: ParameterForm::Value,
inferred_annotation: false,