diff --git a/crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md b/crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md index 7063c58dda..25fadb9905 100644 --- a/crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md +++ b/crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md @@ -252,12 +252,8 @@ def _(): ## Load before `global` declaration -This should be an error, but it's not yet. - -TODO implement `SemanticSyntaxContext::global` - ```py def f(): x = 1 - global x + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" ``` diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/global.md b/crates/ty_python_semantic/resources/mdtest/scopes/global.md index ef5c1b6a72..3e4a884c6d 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/global.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/global.md @@ -32,8 +32,14 @@ def f(): y = "" global x - # TODO: error: [invalid-assignment] "Object of type `Literal[""]` is not assignable to `int`" + # error: [invalid-assignment] "Object of type `Literal[""]` is not assignable to `int`" x = "" + + global z + # error: [invalid-assignment] "Object of type `Literal[""]` is not assignable to `int`" + z = "" + +z: int ``` ## Nested intervening scope @@ -48,8 +54,7 @@ def outer(): def inner(): global x - # TODO: revealed: int - reveal_type(x) # revealed: str + reveal_type(x) # revealed: int ``` ## Narrowing @@ -87,8 +92,7 @@ def f(): ```py def f(): global x - # TODO this should also not be an error - y = x # error: [unresolved-reference] "Name `x` used when not defined" + y = x x = 1 # No error. x = 2 @@ -99,79 +103,111 @@ x = 2 Using a name prior to its `global` declaration in the same scope is a syntax error. ```py -x = 1 - def f(): - print(x) # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x + print(x) + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" print(x) def f(): global x - print(x) # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x + print(x) + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" print(x) def f(): - print(x) # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x, y + print(x) + global x, y # error: [invalid-syntax] "name `x` is used prior to global declaration" print(x) def f(): global x, y - print(x) # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x, y + print(x) + global x, y # error: [invalid-syntax] "name `x` is used prior to global declaration" print(x) def f(): - x = 1 # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x + x = 1 + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" x = 1 def f(): global x - x = 1 # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x + x = 1 + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" x = 1 def f(): - del x # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x, y + del x + global x, y # error: [invalid-syntax] "name `x` is used prior to global declaration" del x def f(): global x, y - del x # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x, y + del x + global x, y # error: [invalid-syntax] "name `x` is used prior to global declaration" del x def f(): - del x # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x + del x + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" del x def f(): global x - del x # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x + del x + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" del x def f(): - del x # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x, y + del x + global x, y # error: [invalid-syntax] "name `x` is used prior to global declaration" del x def f(): global x, y - del x # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x, y + del x + global x, y # error: [invalid-syntax] "name `x` is used prior to global declaration" del x def f(): - print(f"{x=}") # TODO: error: [invalid-syntax] name `x` is used prior to global declaration - global x + print(f"{x=}") + global x # error: [invalid-syntax] "name `x` is used prior to global declaration" # still an error in module scope -x = None # TODO: error: [invalid-syntax] name `x` is used prior to global declaration -global x +x = None +global x # error: [invalid-syntax] "name `x` is used prior to global declaration" +``` + +## Local bindings override preceding `global` bindings + +```py +x = 42 + +def f(): + global x + reveal_type(x) # revealed: Unknown | Literal[42] + x = "56" + reveal_type(x) # revealed: Literal["56"] +``` + +## Local assignment prevents falling back to the outer scope + +```py +x = 42 + +def f(): + # error: [unresolved-reference] "Name `x` used when not defined" + reveal_type(x) # revealed: Unknown + x = "56" + reveal_type(x) # revealed: Literal["56"] +``` + +## Annotating a `global` binding is a syntax error + +```py +x: int = 1 + +def f(): + global x + x: str = "foo" # TODO: error: [invalid-syntax] "annotated name 'x' can't be global" ``` diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index 42956f0bfe..85c9342d5b 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -176,6 +176,9 @@ pub(crate) struct SemanticIndex<'db> { /// Map from the file-local [`FileScopeId`] to the salsa-ingredient [`ScopeId`]. scope_ids_by_scope: IndexVec>, + /// Map from the file-local [`FileScopeId`] to the set of explicit-global symbols it contains. + globals_by_scope: FxHashMap>, + /// Use-def map for each scope in this file. use_def_maps: IndexVec>>, @@ -255,6 +258,16 @@ impl<'db> SemanticIndex<'db> { self.scope_ids_by_scope.iter().copied() } + pub(crate) fn symbol_is_global_in_scope( + &self, + symbol: ScopedSymbolId, + scope: FileScopeId, + ) -> bool { + self.globals_by_scope + .get(&scope) + .is_some_and(|globals| globals.contains(&symbol)) + } + /// Returns the id of the parent scope. pub(crate) fn parent_scope_id(&self, scope_id: FileScopeId) -> Option { let scope = self.scope(scope_id); diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 2ef7966713..a619145822 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -12,7 +12,7 @@ use ruff_python_ast::name::Name; use ruff_python_ast::visitor::{walk_expr, walk_pattern, walk_stmt, Visitor}; use ruff_python_ast::{self as ast, PySourceType, PythonVersion}; use ruff_python_parser::semantic_errors::{ - SemanticSyntaxChecker, SemanticSyntaxContext, SemanticSyntaxError, + SemanticSyntaxChecker, SemanticSyntaxContext, SemanticSyntaxError, SemanticSyntaxErrorKind, }; use ruff_text_size::TextRange; @@ -106,6 +106,7 @@ pub(super) struct SemanticIndexBuilder<'db> { use_def_maps: IndexVec>, scopes_by_node: FxHashMap, scopes_by_expression: FxHashMap, + globals_by_scope: FxHashMap>, definitions_by_node: FxHashMap>, expressions_by_node: FxHashMap>, imported_modules: FxHashSet, @@ -144,6 +145,7 @@ impl<'db> SemanticIndexBuilder<'db> { scopes_by_node: FxHashMap::default(), definitions_by_node: FxHashMap::default(), expressions_by_node: FxHashMap::default(), + globals_by_scope: FxHashMap::default(), imported_modules: FxHashSet::default(), generator_functions: FxHashSet::default(), @@ -1085,6 +1087,7 @@ impl<'db> SemanticIndexBuilder<'db> { self.scopes_by_node.shrink_to_fit(); self.generator_functions.shrink_to_fit(); self.eager_snapshots.shrink_to_fit(); + self.globals_by_scope.shrink_to_fit(); SemanticIndex { symbol_tables, @@ -1093,6 +1096,7 @@ impl<'db> SemanticIndexBuilder<'db> { definitions_by_node: self.definitions_by_node, expressions_by_node: self.expressions_by_node, scope_ids_by_scope: self.scope_ids_by_scope, + globals_by_scope: self.globals_by_scope, ast_ids, scopes_by_expression: self.scopes_by_expression, scopes_by_node: self.scopes_by_node, @@ -1898,7 +1902,38 @@ where // Everything in the current block after a terminal statement is unreachable. self.mark_unreachable(); } - + ast::Stmt::Global(ast::StmtGlobal { range: _, names }) => { + for name in names { + let symbol_id = self.add_symbol(name.id.clone()); + let symbol_table = self.current_symbol_table(); + let symbol = symbol_table.symbol(symbol_id); + if symbol.is_bound() || symbol.is_declared() || symbol.is_used() { + self.report_semantic_error(SemanticSyntaxError { + kind: SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration { + name: name.to_string(), + start: name.range.start(), + }, + range: name.range, + python_version: self.python_version, + }); + } + let scope_id = self.current_scope(); + self.globals_by_scope + .entry(scope_id) + .or_default() + .insert(symbol_id); + } + walk_stmt(self, stmt); + } + ast::Stmt::Delete(ast::StmtDelete { targets, range: _ }) => { + for target in targets { + if let ast::Expr::Name(ast::ExprName { id, .. }) = target { + let symbol_id = self.add_symbol(id.clone()); + self.current_symbol_table().mark_symbol_used(symbol_id); + } + } + walk_stmt(self, stmt); + } _ => { walk_stmt(self, stmt); } @@ -2387,7 +2422,8 @@ impl SemanticSyntaxContext for SemanticIndexBuilder<'_> { self.source_text().as_str() } - // TODO(brent) handle looking up `global` bindings + // We handle the one syntax error that relies on this method (`LoadBeforeGlobalDeclaration`) + // directly in `visit_stmt`, so this just returns a placeholder value. fn global(&self, _name: &str) -> Option { None } diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 628d385402..a6bc58d923 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -56,7 +56,7 @@ use crate::semantic_index::definition::{ use crate::semantic_index::expression::{Expression, ExpressionKind}; use crate::semantic_index::narrowing_constraints::ConstraintKey; use crate::semantic_index::symbol::{ - FileScopeId, NodeWithScopeKind, NodeWithScopeRef, ScopeId, ScopeKind, + FileScopeId, NodeWithScopeKind, NodeWithScopeRef, ScopeId, ScopeKind, ScopedSymbolId, }; use crate::semantic_index::{semantic_index, EagerSnapshotResult, SemanticIndex}; use crate::symbol::{ @@ -1387,9 +1387,29 @@ impl<'db> TypeInferenceBuilder<'db> { .kind(self.db()) .category(self.context.in_stub()) .is_binding()); - let use_def = self.index.use_def_map(binding.file_scope(self.db())); - let declarations = use_def.declarations_at_binding(binding); + + let file_scope_id = binding.file_scope(self.db()); + let symbol_table = self.index.symbol_table(file_scope_id); + let use_def = self.index.use_def_map(file_scope_id); let mut bound_ty = ty; + let symbol_id = binding.symbol(self.db()); + + let global_use_def_map = self.index.use_def_map(FileScopeId::global()); + let declarations = if self.skip_non_global_scopes(file_scope_id, symbol_id) { + let symbol_name = symbol_table.symbol(symbol_id).name(); + match self + .index + .symbol_table(FileScopeId::global()) + .symbol_id_by_name(symbol_name) + { + Some(id) => global_use_def_map.public_declarations(id), + // This case is a syntax error (load before global declaration) but ignore that here + None => use_def.declarations_at_binding(binding), + } + } else { + use_def.declarations_at_binding(binding) + }; + let declared_ty = symbol_from_declarations(self.db(), declarations) .map(|SymbolAndQualifiers { symbol, .. }| { symbol.ignore_possibly_unbound().unwrap_or(Type::unknown()) @@ -1415,6 +1435,19 @@ impl<'db> TypeInferenceBuilder<'db> { self.types.bindings.insert(binding, bound_ty); } + /// Returns `true` if `symbol_id` should be looked up in the global scope, skipping intervening + /// local scopes. + fn skip_non_global_scopes( + &self, + file_scope_id: FileScopeId, + symbol_id: ScopedSymbolId, + ) -> bool { + !file_scope_id.is_global() + && self + .index + .symbol_is_global_in_scope(symbol_id, file_scope_id) + } + fn add_declaration( &mut self, node: AnyNodeRef, @@ -5256,6 +5289,20 @@ impl<'db> TypeInferenceBuilder<'db> { } }; + let current_file = self.file(); + + let skip_non_global_scopes = symbol_table + .symbol_id_by_name(symbol_name) + .is_some_and(|symbol_id| self.skip_non_global_scopes(file_scope_id, symbol_id)); + + if skip_non_global_scopes { + return symbol( + db, + FileScopeId::global().to_scope_id(db, current_file), + symbol_name, + ); + } + // If it's a function-like scope and there is one or more binding in this scope (but // none of those bindings are visible from where we are in the control flow), we cannot // fallback to any bindings in enclosing scopes. As such, we can immediately short-circuit @@ -5273,8 +5320,6 @@ impl<'db> TypeInferenceBuilder<'db> { constraint_keys.push((file_scope_id, ConstraintKey::UseId(use_id))); } - let current_file = self.file(); - // Walk up parent scopes looking for a possible enclosing scope that may have a // definition of this name visible to us (would be `LOAD_DEREF` at runtime.) // Note that we skip the scope containing the use that we are resolving, since we