From 82812e004d78444229adaeeb1bef0411ffbe32f7 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 20 Feb 2025 18:01:01 +0100 Subject: [PATCH] Do the same for `Scope::node` --- .../src/semantic_index/symbol.rs | 23 ++++++++--- crates/red_knot_python_semantic/src/types.rs | 40 ++++++++++++------- .../src/types/call/bind.rs | 2 +- .../src/types/infer.rs | 6 +-- 4 files changed, 47 insertions(+), 24 deletions(-) diff --git a/crates/red_knot_python_semantic/src/semantic_index/symbol.rs b/crates/red_knot_python_semantic/src/semantic_index/symbol.rs index 5e678c2526..8c691591d9 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/symbol.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/symbol.rs @@ -110,21 +110,34 @@ pub struct ScopeId<'db> { } impl<'db> ScopeId<'db> { - pub(crate) fn is_function_like(self, db: &'db dyn Db) -> bool { - self.node(db).scope_kind().is_function_like() + pub(crate) fn is_function_like(self, db: &'db dyn Db, query_file: File) -> bool { + self.node(db, query_file).scope_kind().is_function_like() } - pub(crate) fn node(self, db: &dyn Db) -> &NodeWithScopeKind { + #[inline] + pub(crate) fn node(self, db: &dyn Db, query_file: File) -> &NodeWithScopeKind { + debug_assert_eq!(self.file(db), query_file); + self.node_unchecked(db) + } + + /// Returns the scope's node without checking if the query's file matches the scope's file + /// (which is desired to avoid cross-module query dependencies). + /// + /// Use this method when in situations where it's okay to add a cross-module dependency. + /// For example, when emitting diagnostics. + #[inline] + pub(crate) fn node_unchecked(self, db: &dyn Db) -> &NodeWithScopeKind { self.scope(db).node() } - pub(crate) fn scope(self, db: &dyn Db) -> &Scope { + fn scope(self, db: &dyn Db) -> &Scope { semantic_index(db, self.file(db)).scope(self.file_scope_id(db)) } #[cfg(test)] pub(crate) fn name(self, db: &'db dyn Db) -> &'db str { - match self.node(db) { + // Use `self.node` if this ever becomes a non-testing function. + match self.node_unchecked(db) { NodeWithScopeKind::Module => "", NodeWithScopeKind::Class(class) | NodeWithScopeKind::ClassTypeParameters(class) => { class.name.as_str() diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 625a48689f..9d4faa8fd0 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -3144,7 +3144,8 @@ impl<'db> FunctionType<'db> { /// would depend on the function's AST and rerun for every change in that file. #[salsa::tracked(return_ref)] pub fn signature(self, db: &'db dyn Db) -> Signature<'db> { - let function_stmt_node = self.body_scope(db).node(db).expect_function(); + let body_scope = self.body_scope(db); + let function_stmt_node = body_scope.node(db, body_scope.file(db)).expect_function(); let internal_signature = self.internal_signature(db); if function_stmt_node.decorator_list.is_empty() { return internal_signature; @@ -3165,8 +3166,10 @@ impl<'db> FunctionType<'db> { /// scope. fn internal_signature(self, db: &'db dyn Db) -> Signature<'db> { let scope = self.body_scope(db); - let function_stmt_node = scope.node(db).expect_function(); - let definition = semantic_index(db, scope.file(db)).definition(function_stmt_node); + let file = scope.file(db); + + let function_stmt_node = scope.node(db, file).expect_function(); + let definition = semantic_index(db, file).definition(function_stmt_node); Signature::from_function(db, definition, function_stmt_node) } @@ -3490,9 +3493,10 @@ impl<'db> Class<'db> { #[salsa::tracked(return_ref)] fn explicit_bases_query(self, db: &'db dyn Db) -> Box<[Type<'db>]> { - let class_stmt = self.node(db); + let file = self.file(db); + let class_stmt = self.node(db, file); - let class_definition = semantic_index(db, self.file(db)).definition(class_stmt); + let class_definition = semantic_index(db, file).definition(class_stmt); class_stmt .bases() @@ -3510,18 +3514,20 @@ impl<'db> Class<'db> { /// ## Note /// Only call this function from queries in the same file or your /// query depends on the AST of another file (bad!). - fn node(self, db: &'db dyn Db) -> &'db ast::StmtClassDef { - self.body_scope(db).node(db).expect_class() + #[inline] + fn node(self, db: &'db dyn Db, query_file: File) -> &'db ast::StmtClassDef { + self.body_scope(db).node(db, query_file).expect_class() } /// Return the types of the decorators on this class #[salsa::tracked(return_ref)] fn decorators(self, db: &'db dyn Db) -> Box<[Type<'db>]> { - let class_stmt = self.node(db); + let file = self.file(db); + let class_stmt = self.node(db, file); if class_stmt.decorator_list.is_empty() { return Box::new([]); } - let class_definition = semantic_index(db, self.file(db)).definition(class_stmt); + let class_definition = semantic_index(db, file).definition(class_stmt); class_stmt .decorator_list .iter() @@ -3577,14 +3583,17 @@ impl<'db> Class<'db> { /// ## Note /// Only call this function from queries in the same file or your /// query depends on the AST of another file (bad!). - fn explicit_metaclass(self, db: &'db dyn Db) -> Option> { - let class_stmt = self.node(db); + fn explicit_metaclass(self, db: &'db dyn Db, query_file: File) -> Option> { + let file = self.file(db); + debug_assert_eq!(query_file, file); + + let class_stmt = self.node(db, file); let metaclass_node = &class_stmt .arguments .as_ref()? .find_keyword("metaclass")? .value; - let class_definition = semantic_index(db, self.file(db)).definition(class_stmt); + let class_definition = semantic_index(db, file).definition(class_stmt); let metaclass_ty = definition_expression_type(db, class_definition, metaclass_node); Some(metaclass_ty) } @@ -3608,7 +3617,7 @@ impl<'db> Class<'db> { return Ok(SubclassOfType::subclass_of_unknown()); } - let explicit_metaclass = self.explicit_metaclass(db); + let explicit_metaclass = self.explicit_metaclass(db, self.file(db)); let (metaclass, class_metaclass_was_from) = if let Some(metaclass) = explicit_metaclass { (metaclass, self) } else if let Some(base_class) = base_classes.next() { @@ -4015,9 +4024,10 @@ impl<'db> TypeAliasType<'db> { #[salsa::tracked] pub fn value_type(self, db: &'db dyn Db) -> Type<'db> { let scope = self.rhs_scope(db); + let file = scope.file(db); - let type_alias_stmt_node = scope.node(db).expect_type_alias(); - let definition = semantic_index(db, scope.file(db)).definition(type_alias_stmt_node); + let type_alias_stmt_node = scope.node(db, file).expect_type_alias(); + let definition = semantic_index(db, file).definition(type_alias_stmt_node); definition_expression_type(db, definition, &type_alias_stmt_node.value) } diff --git a/crates/red_knot_python_semantic/src/types/call/bind.rs b/crates/red_knot_python_semantic/src/types/call/bind.rs index 44c0d298c2..77720378a6 100644 --- a/crates/red_knot_python_semantic/src/types/call/bind.rs +++ b/crates/red_knot_python_semantic/src/types/call/bind.rs @@ -315,7 +315,7 @@ impl<'db> CallBindingError<'db> { if let Some(func_lit) = callable_ty.into_function_literal() { let func_lit_scope = func_lit.body_scope(context.db()); let mut span = Span::from(func_lit_scope.file(context.db())); - let node = func_lit_scope.node(context.db()); + let node = func_lit_scope.node_unchecked(context.db()); if let Some(func_def) = node.as_function() { let range = func_def .parameters diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 62a338acf9..17cf12c4b7 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -547,7 +547,7 @@ impl<'db> TypeInferenceBuilder<'db> { } fn infer_region_scope(&mut self, scope: ScopeId<'db>) { - let node = scope.node(self.db()); + let node = scope.node(self.db(), self.file()); match node { NodeWithScopeKind::Module => { let parsed = parsed_module(self.db().upcast(), self.file()); @@ -3539,7 +3539,7 @@ impl<'db> TypeInferenceBuilder<'db> { // a local variable or not in function-like scopes. If a variable has any bindings in a // function-like scope, it is considered a local variable; it never references another // scope. (At runtime, it would use the `LOAD_FAST` opcode.) - if has_bindings_in_this_scope && scope.is_function_like(db) { + if has_bindings_in_this_scope && scope.is_function_like(db, self.file()) { return Symbol::Unbound; } @@ -3554,7 +3554,7 @@ impl<'db> TypeInferenceBuilder<'db> { // scope differently (because an unbound name there falls back to builtins), so // check only function-like scopes. let enclosing_scope_id = enclosing_scope_file_id.to_scope_id(db, current_file); - if !enclosing_scope_id.is_function_like(db) { + if !enclosing_scope_id.is_function_like(db, self.file()) { continue; }