Skip to content

Commit 6740325

Browse files
[ty] Restrict uncached raw signature access (#25866)
## Summary This follows up on [the post-merge review](#25761 (comment)) of #25761. `FunctionLiteral::last_definition_raw_signature` bypasses the tracked `FunctionType` query and therefore must only be called for a function defined in the same module. After #25761, the method was visible throughout the `types` module, making it easy for a future caller to accidentally introduce a cross-module AST dependency and over-invalidation. This restores the method's privacy and routes the existing same-module callers through a dedicated `same_module_uncached_raw_signature` wrapper. The wrapper's name and documentation make the restriction explicit, while cross-module callers continue to use the tracked query.
1 parent 970b1bf commit 6740325

3 files changed

Lines changed: 51 additions & 27 deletions

File tree

crates/ty_python_semantic/src/types/function.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,7 @@ impl<'db> FunctionLiteral<'db> {
871871
/// calling query is not in the same file as this function is defined in, then this will create
872872
/// a cross-module dependency directly on the full AST which will lead to cache
873873
/// over-invalidation.
874-
pub(super) fn last_definition_raw_signature(
874+
fn last_definition_raw_signature(
875875
self,
876876
db: &'db dyn Db,
877877
return_callable_typevar_scope: ReturnCallableTypeVarScope,
@@ -953,6 +953,23 @@ impl<'db> FunctionLiteral<'db> {
953953
}
954954
}
955955

956+
/// ## Warning
957+
///
958+
/// This uses the semantic index to find the definition of the function. This means that if the
959+
/// calling query is not in the same file as this function is defined in, then this will create
960+
/// a cross-module dependency directly on the full AST which will lead to cache
961+
/// over-invalidation. Cross-module callers should use the tracked
962+
/// [`FunctionType::last_definition_raw_signature`] query instead.
963+
pub(super) fn same_module_uncached_raw_signature<'db>(
964+
db: &'db dyn Db,
965+
function: FunctionType<'db>,
966+
return_callable_typevar_scope: ReturnCallableTypeVarScope,
967+
) -> Signature<'db> {
968+
function
969+
.literal(db)
970+
.last_definition_raw_signature(db, return_callable_typevar_scope)
971+
}
972+
956973
/// Indicates whether a method is explicitly or implicitly abstract.
957974
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, salsa::Update, get_size2::GetSize)]
958975
pub(super) enum AbstractMethodKind {

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

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ use crate::types::diagnostic::{
6969
use crate::types::enums::{enum_ignored_names, is_enum_class_by_inheritance};
7070
use crate::types::function::{
7171
FunctionDecorators, FunctionType, KnownFunction, report_revealed_type,
72+
same_module_uncached_raw_signature,
7273
};
7374
use crate::types::generics::{
7475
GenericContext, InferableTypeVars, Specialization, SpecializationBuilder, bind_typevar,
@@ -5109,13 +5110,12 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
51095110
// the expected type passed should be the "raw" type,
51105111
// i.e. type variables in the return type are non-inferable,
51115112
// and the return types of async functions are not wrapped in `CoroutineType[...]`.
5112-
let return_ty = func
5113-
.literal(self.db())
5114-
.last_definition_raw_signature(
5115-
self.db(),
5116-
ReturnCallableTypeVarScope::Lexical,
5117-
)
5118-
.return_ty;
5113+
let return_ty = same_module_uncached_raw_signature(
5114+
self.db(),
5115+
func,
5116+
ReturnCallableTypeVarScope::Lexical,
5117+
)
5118+
.return_ty;
51195119

51205120
// For generator functions, the declared return type is e.g.
51215121
// `Generator[YieldType, SendType, ReturnType]`. The type context
@@ -8748,10 +8748,12 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
87488748
let _ = self.infer_optional_expression(value.as_deref(), TypeContext::default());
87498749
return Type::unknown();
87508750
};
8751-
let declared_return_ty = enclosing_function
8752-
.literal(self.db())
8753-
.last_definition_raw_signature(self.db(), ReturnCallableTypeVarScope::Public)
8754-
.return_ty;
8751+
let declared_return_ty = same_module_uncached_raw_signature(
8752+
self.db(),
8753+
enclosing_function,
8754+
ReturnCallableTypeVarScope::Public,
8755+
)
8756+
.return_ty;
87558757
let return_type_span = enclosing_function.spans(self.db()).return_type;
87568758

87578759
let Some(generator_type_params) = declared_return_ty.generator_types(self.db()) else {
@@ -8797,10 +8799,12 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
87978799
let _ = self.infer_expression(value, TypeContext::default());
87988800
return Type::unknown();
87998801
};
8800-
let annotated_return_ty = enclosing_function
8801-
.literal(self.db())
8802-
.last_definition_raw_signature(self.db(), ReturnCallableTypeVarScope::Public)
8803-
.return_ty;
8802+
let annotated_return_ty = same_module_uncached_raw_signature(
8803+
self.db(),
8804+
enclosing_function,
8805+
ReturnCallableTypeVarScope::Public,
8806+
)
8807+
.return_ty;
88048808

88058809
let Some(outer_expected) = annotated_return_ty.generator_types(self.db()) else {
88068810
let _ = self.infer_expression(value, TypeContext::default());

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use crate::{
1414
function::{
1515
FunctionBodyKind, FunctionDecorators, FunctionLiteral, FunctionType, KnownFunction,
1616
OverloadLiteral, function_body_kind, is_implicit_classmethod,
17+
same_module_uncached_raw_signature,
1718
},
1819
generics::{enclosing_generic_contexts, typing_self},
1920
infer::{
@@ -80,18 +81,18 @@ impl<'db> ExpectedReturnType<'db> {
8081

8182
let public = normalize(
8283
db,
83-
function
84-
.literal(db)
85-
.last_definition_raw_signature(db, ReturnCallableTypeVarScope::Public)
84+
same_module_uncached_raw_signature(db, function, ReturnCallableTypeVarScope::Public)
8685
.return_ty,
8786
);
8887
let lexical = function_node.type_params.is_some().then(|| {
8988
normalize(
9089
db,
91-
function
92-
.literal(db)
93-
.last_definition_raw_signature(db, ReturnCallableTypeVarScope::Lexical)
94-
.return_ty,
90+
same_module_uncached_raw_signature(
91+
db,
92+
function,
93+
ReturnCallableTypeVarScope::Lexical,
94+
)
95+
.return_ty,
9596
)
9697
});
9798

@@ -169,10 +170,12 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
169170

170171
let enclosing_function = nearest_enclosing_function(db, self.index, self.scope())
171172
.expect("should be in a function body scope");
172-
let declared_ty = enclosing_function
173-
.literal(db)
174-
.last_definition_raw_signature(db, ReturnCallableTypeVarScope::Public)
175-
.return_ty;
173+
let declared_ty = same_module_uncached_raw_signature(
174+
db,
175+
enclosing_function,
176+
ReturnCallableTypeVarScope::Public,
177+
)
178+
.return_ty;
176179
let expected_return =
177180
ExpectedReturnType::from_function(db, enclosing_function, function);
178181
let expected_ty = expected_return.public();

0 commit comments

Comments
 (0)