From b036317b8e51b9d777ecb2f60410df19ee825897 Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Fri, 1 Aug 2025 11:09:49 -0700 Subject: [PATCH] [ty] Use `definition_in_enclosing_scope` instead of walking scopes in `add_binding` --- crates/ty_python_semantic/src/types/infer.rs | 88 +++++--------------- 1 file changed, 20 insertions(+), 68 deletions(-) diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index ce518e5f31..a160cad1e5 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -1785,81 +1785,33 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let place_table = self.index.place_table(file_scope_id); let use_def = self.index.use_def_map(file_scope_id); let mut bound_ty = ty; - - let global_use_def_map = self.index.use_def_map(FileScopeId::global()); - let nonlocal_use_def_map; let place_id = binding.place(self.db()); let place = place_table.place(place_id); - let (declarations, is_local) = if let Some(symbol) = place.as_symbol() { - let symbol_id = place_id.expect_symbol(); - let skip_non_global_scopes = self.skip_non_global_scopes(file_scope_id, symbol_id); - - if skip_non_global_scopes { - match self - .index - .place_table(FileScopeId::global()) - .symbol_id(symbol.name()) + // If the variable is declared `global` or `nonlocal`, but the semantic index doesn't have + // a definition for it, fall back to local declarations. That's almost certainly empty, + // because it's a syntax error to annotate `global` or `nonlocal` variables with a type. + // SemanticIndexBuilder might already have reported an error for other reasons, for example + // if a `nonlocal` keyword doesn't match any binding in any enclosing (function) scope. + let enclosing_use_def; + let mut declarations = use_def.declarations_at_binding(binding); + let mut is_local = true; + if let Some(symbol) = place.as_symbol() { + // We don't need to consider free variables that aren't explicitly declared `global` or + // `nonlocal`, because we're in the middle of handling a binding, and bound variables + // can't refer to an enclosing scope without those keywords. + if symbol.is_global() || symbol.is_nonlocal() { + let symbol_id = place_id.expect_symbol(); + if let Some((enclosing_scope_id, enclosing_scope_symbol_id)) = + place_table.definition_in_enclosing_scope(symbol_id) { - Some(id) => ( - global_use_def_map.end_of_scope_symbol_declarations(id), - false, - ), - // This variable shows up in `global` declarations but doesn't have an explicit - // binding in the global scope. - None => (use_def.declarations_at_binding(binding), true), - } - } else if self - .index - .symbol_is_nonlocal_in_scope(symbol_id, file_scope_id) - { - // If we run out of ancestor scopes without finding a definition, we'll fall back to - // the local scope. This will also be a syntax error in `infer_nonlocal_statement` (no - // binding for `nonlocal` found), but ignore that here. - let mut declarations = use_def.declarations_at_binding(binding); - let mut is_local = true; - // Walk up parent scopes looking for the enclosing scope that has definition of this - // name. `ancestor_scopes` includes the current scope, so skip that one. - for (enclosing_scope_file_id, enclosing_scope) in - self.index.ancestor_scopes(file_scope_id).skip(1) - { - // Ignore class scopes and the global scope. - if !enclosing_scope.kind().is_function_like() { - continue; - } - let enclosing_place_table = self.index.place_table(enclosing_scope_file_id); - let Some(enclosing_symbol_id) = enclosing_place_table.symbol_id(symbol.name()) - else { - // This ancestor scope doesn't have a binding. Keep going. - continue; - }; - - let enclosing_symbol = enclosing_place_table.symbol(enclosing_symbol_id); - if enclosing_symbol.is_nonlocal() { - // The variable is `nonlocal` in this ancestor scope. Keep going. - continue; - } - if enclosing_symbol.is_global() { - // The variable is `global` in this ancestor scope. This breaks the `nonlocal` - // chain, and it's a syntax error in `infer_nonlocal_statement`. Ignore that - // here and just bail out of this loop. - break; - } - // We found the closest definition. Note that (as in `infer_place_load`) this does - // *not* need to be a binding. It could be just a declaration, e.g. `x: int`. - nonlocal_use_def_map = self.index.use_def_map(enclosing_scope_file_id); - declarations = - nonlocal_use_def_map.end_of_scope_symbol_declarations(enclosing_symbol_id); + enclosing_use_def = self.index.use_def_map(enclosing_scope_id); + declarations = enclosing_use_def + .end_of_scope_symbol_declarations(enclosing_scope_symbol_id); is_local = false; - break; } - (declarations, is_local) - } else { - (use_def.declarations_at_binding(binding), true) } - } else { - (use_def.declarations_at_binding(binding), true) - }; + } let (declared_ty, is_modifiable) = place_from_declarations(self.db(), declarations) .and_then(|place_and_quals| {