diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 0fc24dc8e2..e05c807c2d 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -670,7 +670,11 @@ impl SemanticSyntaxContext for Checker<'_> { | SemanticSyntaxErrorKind::InvalidStarExpression | SemanticSyntaxErrorKind::AsyncComprehensionInSyncComprehension(_) | SemanticSyntaxErrorKind::DuplicateParameter(_) - | SemanticSyntaxErrorKind::NonlocalDeclarationAtModuleLevel => { + | SemanticSyntaxErrorKind::NonlocalDeclarationAtModuleLevel + | SemanticSyntaxErrorKind::LoadBeforeNonlocalDeclaration { .. } + | SemanticSyntaxErrorKind::NonlocalAndGlobal(_) + | SemanticSyntaxErrorKind::AnnotatedGlobal(_) + | SemanticSyntaxErrorKind::AnnotatedNonlocal(_) => { self.semantic_errors.borrow_mut().push(error); } } diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index 0bade7b9a6..6a3ce2a850 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/crates/ruff_python_parser/src/semantic_errors.rs @@ -952,6 +952,9 @@ impl Display for SemanticSyntaxError { SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration { name, start: _ } => { write!(f, "name `{name}` is used prior to global declaration") } + SemanticSyntaxErrorKind::LoadBeforeNonlocalDeclaration { name, start: _ } => { + write!(f, "name `{name}` is used prior to nonlocal declaration") + } SemanticSyntaxErrorKind::InvalidStarExpression => { f.write_str("Starred expression cannot be used here") } @@ -977,6 +980,15 @@ impl Display for SemanticSyntaxError { SemanticSyntaxErrorKind::NonlocalDeclarationAtModuleLevel => { write!(f, "nonlocal declaration not allowed at module level") } + SemanticSyntaxErrorKind::NonlocalAndGlobal(name) => { + write!(f, "name `{name}` is nonlocal and global") + } + SemanticSyntaxErrorKind::AnnotatedGlobal(name) => { + write!(f, "annotated name `{name}` can't be global") + } + SemanticSyntaxErrorKind::AnnotatedNonlocal(name) => { + write!(f, "annotated name `{name}` can't be nonlocal") + } } } } @@ -1207,6 +1219,24 @@ pub enum SemanticSyntaxErrorKind { /// [#111123]: https://github.com/python/cpython/issues/111123 LoadBeforeGlobalDeclaration { name: String, start: TextSize }, + /// Represents the use of a `nonlocal` variable before its `nonlocal` declaration. + /// + /// ## Examples + /// + /// ```python + /// def f(): + /// counter = 0 + /// def increment(): + /// print(f"Adding 1 to {counter}") + /// nonlocal counter # SyntaxError: name 'counter' is used prior to nonlocal declaration + /// counter += 1 + /// ``` + /// + /// ## Known Issues + /// + /// See [`LoadBeforeGlobalDeclaration`][Self::LoadBeforeGlobalDeclaration]. + LoadBeforeNonlocalDeclaration { name: String, start: TextSize }, + /// Represents the use of a starred expression in an invalid location, such as a `return` or /// `yield` statement. /// @@ -1307,6 +1337,15 @@ pub enum SemanticSyntaxErrorKind { /// Represents a nonlocal declaration at module level NonlocalDeclarationAtModuleLevel, + + /// Represents the same variable declared as both nonlocal and global + NonlocalAndGlobal(String), + + /// Represents a type annotation on a variable that's been declared global + AnnotatedGlobal(String), + + /// Represents a type annotation on a variable that's been declared nonlocal + AnnotatedNonlocal(String), } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize)] diff --git a/crates/ty_python_semantic/resources/mdtest/import/star.md b/crates/ty_python_semantic/resources/mdtest/import/star.md index 8ac7ea4e02..d84257e032 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/star.md +++ b/crates/ty_python_semantic/resources/mdtest/import/star.md @@ -1304,7 +1304,7 @@ scope of the name that was declared `global`, can add a symbol to the global nam def f(): global g, h - g: bool = True + g = True f() ``` diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/global.md b/crates/ty_python_semantic/resources/mdtest/scopes/global.md index 3e4a884c6d..f02907c8c4 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/global.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/global.md @@ -83,7 +83,7 @@ def f(): x = 1 def g() -> None: nonlocal x - global x # TODO: error: [invalid-syntax] "name 'x' is nonlocal and global" + global x # error: [invalid-syntax] "name `x` is nonlocal and global" x = None ``` @@ -209,5 +209,18 @@ x: int = 1 def f(): global x - x: str = "foo" # TODO: error: [invalid-syntax] "annotated name 'x' can't be global" + x: str = "foo" # error: [invalid-syntax] "annotated name `x` can't be global" +``` + +## Global declarations affect the inferred type of the binding + +Even if the `global` declaration isn't used in an assignment, we conservatively assume it could be: + +```py +x = 1 + +def f(): + global x + +# TODO: reveal_type(x) # revealed: Unknown | Literal["1"] ``` diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md b/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md index 862757ce95..a30bfa219a 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md @@ -43,3 +43,321 @@ def f(): def h(): reveal_type(x) # revealed: Unknown | Literal[1] ``` + +## The `nonlocal` keyword + +Without the `nonlocal` keyword, bindings in an inner scope shadow variables of the same name in +enclosing scopes. This example isn't a type error, because the inner `x` shadows the outer one: + +```py +def f(): + x: int = 1 + def g(): + x = "hello" # allowed +``` + +With `nonlocal` it is a type error, because `x` refers to the same place in both scopes: + +```py +def f(): + x: int = 1 + def g(): + nonlocal x + x = "hello" # error: [invalid-assignment] "Object of type `Literal["hello"]` is not assignable to `int`" +``` + +## Local variable bindings "look ahead" to any assignment in the current scope + +The binding `x = 2` in `g` causes the earlier read of `x` to refer to `g`'s not-yet-initialized +binding, rather than to `x = 1` in `f`'s scope: + +```py +def f(): + x = 1 + def g(): + if x == 1: # error: [unresolved-reference] "Name `x` used when not defined" + x = 2 +``` + +The `nonlocal` keyword makes this example legal (and makes the assignment `x = 2` affect the outer +scope): + +```py +def f(): + x = 1 + def g(): + nonlocal x + if x == 1: + x = 2 +``` + +For the same reason, using the `+=` operator in an inner scope is an error without `nonlocal` +(unless you shadow the outer variable first): + +```py +def f(): + x = 1 + def g(): + x += 1 # error: [unresolved-reference] "Name `x` used when not defined" + +def f(): + x = 1 + def g(): + x = 1 + x += 1 # allowed, but doesn't affect the outer scope + +def f(): + x = 1 + def g(): + nonlocal x + x += 1 # allowed, and affects the outer scope +``` + +## `nonlocal` declarations must match an outer binding + +`nonlocal x` isn't allowed when there's no binding for `x` in an enclosing scope: + +```py +def f(): + def g(): + nonlocal x # error: [invalid-syntax] "no binding for nonlocal `x` found" + +def f(): + x = 1 + def g(): + nonlocal x, y # error: [invalid-syntax] "no binding for nonlocal `y` found" +``` + +A global `x` doesn't work. The target must be in a function-like scope: + +```py +x = 1 + +def f(): + def g(): + nonlocal x # error: [invalid-syntax] "no binding for nonlocal `x` found" + +def f(): + global x + def g(): + nonlocal x # error: [invalid-syntax] "no binding for nonlocal `x` found" +``` + +A class-scoped `x` also doesn't work: + +```py +class Foo: + x = 1 + @staticmethod + def f(): + nonlocal x # error: [invalid-syntax] "no binding for nonlocal `x` found" +``` + +However, class-scoped bindings don't break the `nonlocal` chain the way `global` declarations do: + +```py +def f(): + x: int = 1 + + class Foo: + x: str = "hello" + + @staticmethod + def g(): + # Skips the class scope and reaches the outer function scope. + nonlocal x + x = 2 # allowed + x = "goodbye" # error: [invalid-assignment] +``` + +## `nonlocal` uses the closest binding + +```py +def f(): + x = 1 + def g(): + x = 2 + def h(): + nonlocal x + reveal_type(x) # revealed: Unknown | Literal[2] +``` + +## `nonlocal` "chaining" + +Multiple `nonlocal` statements can "chain" through nested scopes: + +```py +def f(): + x = 1 + def g(): + nonlocal x + def h(): + nonlocal x + reveal_type(x) # revealed: Unknown | Literal[1] +``` + +And the `nonlocal` chain can skip over a scope that doesn't bind the variable: + +```py +def f1(): + x = 1 + def f2(): + nonlocal x + def f3(): + # No binding; this scope gets skipped. + def f4(): + nonlocal x + reveal_type(x) # revealed: Unknown | Literal[1] +``` + +But a `global` statement breaks the chain: + +```py +def f(): + x = 1 + def g(): + global x + def h(): + nonlocal x # error: [invalid-syntax] "no binding for nonlocal `x` found" +``` + +## `nonlocal` bindings respect declared types from the defining scope, even without a binding + +```py +def f(): + x: int + def g(): + nonlocal x + x = "string" # error: [invalid-assignment] "Object of type `Literal["string"]` is not assignable to `int`" +``` + +## A complicated mixture of `nonlocal` chaining, empty scopes, class scopes, and the `global` keyword + +```py +def f1(): + # The original bindings of `x`, `y`, and `z` with type declarations. + x: int = 1 + y: int = 2 + z: int = 3 + + def f2(): + # This scope doesn't touch `x`, `y`, or `z` at all. + + class Foo: + # This class scope is totally ignored. + x: str = "a" + y: str = "b" + z: str = "c" + + @staticmethod + def f3(): + # This scope declares `x` nonlocal and `y` as global, and it shadows `z` without + # giving it a type declaration. + nonlocal x + x = 4 + y = 5 + global z + z = 6 + + def f4(): + # This scope sees `x` from `f1` and `y` from `f3`. It *can't* declare `z` + # nonlocal, because of the global statement above, but it *can* load `z` as a + # "free" variable, in which case it sees the global value. + nonlocal x, y, z # error: [invalid-syntax] "no binding for nonlocal `z` found" + x = "string" # error: [invalid-assignment] + y = "string" # allowed, because `f3`'s `y` is untyped + reveal_type(z) # revealed: Unknown | Literal[6] +``` + +## TODO: `nonlocal` affects the inferred type in the outer scope + +Without `nonlocal`, `g` can't write to `x`, and the inferred type of `x` in `f`'s scope isn't +affected by `g`: + +```py +def f(): + x = 1 + def g(): + reveal_type(x) # revealed: Unknown | Literal[1] + reveal_type(x) # revealed: Literal[1] +``` + +But with `nonlocal`, `g` could write to `x`, and that affects its inferred type in `f`. That's true +regardless of whether `g` actually writes to `x`. With a write: + +```py +def f(): + x = 1 + def g(): + nonlocal x + reveal_type(x) # revealed: Unknown | Literal[1] + x += 1 + reveal_type(x) # revealed: Unknown | Literal[2] + # TODO: should be `Unknown | Literal[1]` + reveal_type(x) # revealed: Literal[1] +``` + +Without a write: + +```py +def f(): + x = 1 + def g(): + nonlocal x + reveal_type(x) # revealed: Unknown | Literal[1] + # TODO: should be `Unknown | Literal[1]` + reveal_type(x) # revealed: Literal[1] +``` + +## Annotating a `nonlocal` binding is a syntax error + +```py +def f(): + x: int = 1 + def g(): + nonlocal x + x: str = "foo" # error: [invalid-syntax] "annotated name `x` can't be nonlocal" +``` + +## Use before `nonlocal` + +Using a name prior to its `nonlocal` declaration in the same scope is a syntax error: + +```py +def f(): + x = 1 + def g(): + x = 2 + nonlocal x # error: [invalid-syntax] "name `x` is used prior to nonlocal declaration" +``` + +This is true even if there are multiple `nonlocal` declarations of the same variable, as long as any +of them come after the usage: + +```py +def f(): + x = 1 + def g(): + nonlocal x + x = 2 + nonlocal x # error: [invalid-syntax] "name `x` is used prior to nonlocal declaration" + +def f(): + x = 1 + def g(): + nonlocal x + nonlocal x + x = 2 # allowed +``` + +## `nonlocal` before outer initialization + +`nonlocal x` works even if `x` isn't bound in the enclosing scope until afterwards: + +```py +def f(): + def g(): + # This is allowed, because of the subsequent definition of `x`. + nonlocal x + x = 1 +``` diff --git a/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md b/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md index 57f64cd875..13c5272928 100644 --- a/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md +++ b/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md @@ -147,8 +147,7 @@ def nonlocal_use(): X: Final[int] = 1 def inner(): nonlocal X - # TODO: this should be an error - X = 2 + X = 2 # error: [invalid-assignment] "Reassignment of `Final` symbol `X` is not allowed: Reassignment of `Final` symbol" ``` `main.py`: diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index 0c1901b591..490ec547b2 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -1421,7 +1421,7 @@ impl RequiresExplicitReExport { /// ```py /// def _(): /// x = 1 -/// +/// /// x = 2 /// /// if flag(): diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index b6e6755f0f..5cb8b00238 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -217,9 +217,6 @@ 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>, @@ -308,9 +305,19 @@ impl<'db> SemanticIndex<'db> { symbol: ScopedPlaceId, scope: FileScopeId, ) -> bool { - self.globals_by_scope - .get(&scope) - .is_some_and(|globals| globals.contains(&symbol)) + self.place_table(scope) + .place_expr(symbol) + .is_marked_global() + } + + pub(crate) fn symbol_is_nonlocal_in_scope( + &self, + symbol: ScopedPlaceId, + scope: FileScopeId, + ) -> bool { + self.place_table(scope) + .place_expr(symbol) + .is_marked_nonlocal() } /// Returns the id of the parent scope. diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 4d2cb57c84..5d862bfa0f 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -103,7 +103,6 @@ pub(super) struct SemanticIndexBuilder<'db, 'ast> { 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, @@ -141,7 +140,6 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { 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(), @@ -349,7 +347,12 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { popped_scope_id } - fn current_place_table(&mut self) -> &mut PlaceTableBuilder { + fn current_place_table(&self) -> &PlaceTableBuilder { + let scope_id = self.current_scope(); + &self.place_tables[scope_id] + } + + fn current_place_table_mut(&mut self) -> &mut PlaceTableBuilder { let scope_id = self.current_scope(); &mut self.place_tables[scope_id] } @@ -389,7 +392,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { /// Add a symbol to the place table and the use-def map. /// Return the [`ScopedPlaceId`] that uniquely identifies the symbol in both. fn add_symbol(&mut self, name: Name) -> ScopedPlaceId { - let (place_id, added) = self.current_place_table().add_symbol(name); + let (place_id, added) = self.current_place_table_mut().add_symbol(name); if added { self.current_use_def_map_mut().add_place(place_id); } @@ -399,7 +402,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { /// Add a place to the place table and the use-def map. /// Return the [`ScopedPlaceId`] that uniquely identifies the place in both. fn add_place(&mut self, place_expr: PlaceExprWithFlags) -> ScopedPlaceId { - let (place_id, added) = self.current_place_table().add_place(place_expr); + let (place_id, added) = self.current_place_table_mut().add_place(place_expr); if added { self.current_use_def_map_mut().add_place(place_id); } @@ -407,15 +410,15 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { } fn mark_place_bound(&mut self, id: ScopedPlaceId) { - self.current_place_table().mark_place_bound(id); + self.current_place_table_mut().mark_place_bound(id); } fn mark_place_declared(&mut self, id: ScopedPlaceId) { - self.current_place_table().mark_place_declared(id); + self.current_place_table_mut().mark_place_declared(id); } fn mark_place_used(&mut self, id: ScopedPlaceId) { - self.current_place_table().mark_place_used(id); + self.current_place_table_mut().mark_place_used(id); } fn add_entry_for_definition_key(&mut self, key: DefinitionNodeKey) -> &mut Definitions<'db> { @@ -1042,7 +1045,6 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { 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 { place_tables, @@ -1050,7 +1052,6 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { 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, @@ -1418,6 +1419,29 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { self.visit_expr(value); } + if let ast::Expr::Name(name) = &*node.target { + let symbol_id = self.add_symbol(name.id.clone()); + let symbol = self.current_place_table().place_expr(symbol_id); + // Check whether the variable has been declared global. + if symbol.is_marked_global() { + self.report_semantic_error(SemanticSyntaxError { + kind: SemanticSyntaxErrorKind::AnnotatedGlobal(name.id.as_str().into()), + range: name.range, + python_version: self.python_version, + }); + } + // Check whether the variable has been declared nonlocal. + if symbol.is_marked_nonlocal() { + self.report_semantic_error(SemanticSyntaxError { + kind: SemanticSyntaxErrorKind::AnnotatedNonlocal( + name.id.as_str().into(), + ), + range: name.range, + python_version: self.python_version, + }); + } + } + // See https://docs.python.org/3/library/ast.html#ast.AnnAssign if matches!( *node.target, @@ -1858,8 +1882,8 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { }) => { for name in names { let symbol_id = self.add_symbol(name.id.clone()); - let symbol_table = self.current_place_table(); - let symbol = symbol_table.place_expr(symbol_id); + let symbol = self.current_place_table().place_expr(symbol_id); + // Check whether the variable has already been accessed in this scope. if symbol.is_bound() || symbol.is_declared() || symbol.is_used() { self.report_semantic_error(SemanticSyntaxError { kind: SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration { @@ -1870,11 +1894,56 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { python_version: self.python_version, }); } - let scope_id = self.current_scope(); - self.globals_by_scope - .entry(scope_id) - .or_default() - .insert(symbol_id); + // Check whether the variable has also been declared nonlocal. + if symbol.is_marked_nonlocal() { + self.report_semantic_error(SemanticSyntaxError { + kind: SemanticSyntaxErrorKind::NonlocalAndGlobal(name.to_string()), + range: name.range, + python_version: self.python_version, + }); + } + self.current_place_table_mut().mark_place_global(symbol_id); + } + walk_stmt(self, stmt); + } + ast::Stmt::Nonlocal(ast::StmtNonlocal { + range: _, + node_index: _, + names, + }) => { + for name in names { + let symbol_id = self.add_symbol(name.id.clone()); + let symbol = self.current_place_table().place_expr(symbol_id); + // Check whether the variable has already been accessed in this scope. + if symbol.is_bound() || symbol.is_declared() || symbol.is_used() { + self.report_semantic_error(SemanticSyntaxError { + kind: SemanticSyntaxErrorKind::LoadBeforeNonlocalDeclaration { + name: name.to_string(), + start: name.range.start(), + }, + range: name.range, + python_version: self.python_version, + }); + } + // Check whether the variable has also been declared global. + if symbol.is_marked_global() { + self.report_semantic_error(SemanticSyntaxError { + kind: SemanticSyntaxErrorKind::NonlocalAndGlobal(name.to_string()), + range: name.range, + python_version: self.python_version, + }); + } + // The variable is required to exist in an enclosing scope, but that definition + // might come later. For example, this is example legal, but we can't check + // that here, because we haven't gotten to `x = 1`: + // ```py + // def f(): + // def g(): + // nonlocal x + // x = 1 + // ``` + self.current_place_table_mut() + .mark_place_nonlocal(symbol_id); } walk_stmt(self, stmt); } @@ -1888,7 +1957,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { for target in targets { if let Ok(target) = PlaceExpr::try_from(target) { let place_id = self.add_place(PlaceExprWithFlags::new(target)); - self.current_place_table().mark_place_used(place_id); + self.current_place_table_mut().mark_place_used(place_id); self.delete_binding(place_id); } } diff --git a/crates/ty_python_semantic/src/semantic_index/place.rs b/crates/ty_python_semantic/src/semantic_index/place.rs index 5f0d281a8c..39c6dcc7cf 100644 --- a/crates/ty_python_semantic/src/semantic_index/place.rs +++ b/crates/ty_python_semantic/src/semantic_index/place.rs @@ -330,6 +330,16 @@ impl PlaceExprWithFlags { self.flags.contains(PlaceFlags::IS_DECLARED) } + /// Is the place `global` its containing scope? + pub fn is_marked_global(&self) -> bool { + self.flags.contains(PlaceFlags::MARKED_GLOBAL) + } + + /// Is the place `nonlocal` its containing scope? + pub fn is_marked_nonlocal(&self) -> bool { + self.flags.contains(PlaceFlags::MARKED_NONLOCAL) + } + pub(crate) fn as_name(&self) -> Option<&Name> { self.expr.as_name() } @@ -397,9 +407,7 @@ bitflags! { const IS_USED = 1 << 0; const IS_BOUND = 1 << 1; const IS_DECLARED = 1 << 2; - /// TODO: This flag is not yet set by anything const MARKED_GLOBAL = 1 << 3; - /// TODO: This flag is not yet set by anything const MARKED_NONLOCAL = 1 << 4; const IS_INSTANCE_ATTRIBUTE = 1 << 5; } @@ -663,7 +671,7 @@ impl PlaceTable { } /// Returns the place named `name`. - #[allow(unused)] // used in tests + #[cfg(test)] pub(crate) fn place_by_name(&self, name: &str) -> Option<&PlaceExprWithFlags> { let id = self.place_id_by_name(name)?; Some(self.place_expr(id)) @@ -814,6 +822,14 @@ impl PlaceTableBuilder { self.table.places[id].insert_flags(PlaceFlags::IS_USED); } + pub(super) fn mark_place_global(&mut self, id: ScopedPlaceId) { + self.table.places[id].insert_flags(PlaceFlags::MARKED_GLOBAL); + } + + pub(super) fn mark_place_nonlocal(&mut self, id: ScopedPlaceId) { + self.table.places[id].insert_flags(PlaceFlags::MARKED_NONLOCAL); + } + pub(super) fn places(&self) -> impl Iterator { self.table.places() } diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index e49c294285..62077774ec 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -35,6 +35,7 @@ //! be considered a bug.) use itertools::{Either, Itertools}; +use ruff_db::diagnostic::{Annotation, DiagnosticId, Severity}; use ruff_db::files::File; use ruff_db::parsed::{ParsedModuleRef, parsed_module}; use ruff_python_ast::visitor::{Visitor, walk_expr}; @@ -1562,6 +1563,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { 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_expr(place_id); let skip_non_global_scopes = self.skip_non_global_scopes(file_scope_id, place_id); @@ -1572,9 +1574,58 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .place_id_by_expr(&place.expr) { Some(id) => (global_use_def_map.end_of_scope_declarations(id), false), - // This case is a syntax error (load before global declaration) but ignore that here + // 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(place_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_place_id) = enclosing_place_table.place_id_by_expr(&place.expr) + else { + // This ancestor scope doesn't have a binding. Keep going. + continue; + }; + if self + .index + .symbol_is_nonlocal_in_scope(enclosing_place_id, enclosing_scope_file_id) + { + // The variable is `nonlocal` in this ancestor scope. Keep going. + continue; + } + if self + .index + .symbol_is_global_in_scope(enclosing_place_id, enclosing_scope_file_id) + { + // 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 (unlike in `infer_place_load`) this + // does *not* need to be a binding. It could be just `x: int`. + nonlocal_use_def_map = self.index.use_def_map(enclosing_scope_file_id); + declarations = nonlocal_use_def_map.end_of_scope_declarations(enclosing_place_id); + is_local = false; + break; + } + (declarations, is_local) } else { (use_def.declarations_at_binding(binding), true) }; @@ -2204,12 +2255,12 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { ast::Stmt::Raise(raise) => self.infer_raise_statement(raise), ast::Stmt::Return(ret) => self.infer_return_statement(ret), ast::Stmt::Delete(delete) => self.infer_delete_statement(delete), + ast::Stmt::Nonlocal(nonlocal) => self.infer_nonlocal_statement(nonlocal), ast::Stmt::Break(_) | ast::Stmt::Continue(_) | ast::Stmt::Pass(_) | ast::Stmt::IpyEscapeCommand(_) - | ast::Stmt::Global(_) - | ast::Stmt::Nonlocal(_) => { + | ast::Stmt::Global(_) => { // No-op } } @@ -4609,6 +4660,69 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } } + fn infer_nonlocal_statement(&mut self, nonlocal: &ast::StmtNonlocal) { + let ast::StmtNonlocal { + node_index: _, + range, + names, + } = nonlocal; + let db = self.db(); + let scope = self.scope(); + let file_scope_id = scope.file_scope_id(db); + let current_file = self.file(); + 'names: for name in names { + // Walk up parent scopes looking for a possible enclosing scope that may have a + // definition of this name visible to us. Note that we skip the scope containing the + // use that we are resolving, since we already looked for the place there up above. + for (enclosing_scope_file_id, _) in self.index.ancestor_scopes(file_scope_id).skip(1) { + // Class scopes are not visible to nested scopes, and `nonlocal` cannot refer to + // globals, 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) { + continue; + } + let enclosing_place_table = self.index.place_table(enclosing_scope_file_id); + let Some(enclosing_place_id) = enclosing_place_table.place_id_by_name(name) else { + // This scope doesn't define this name. Keep going. + continue; + }; + // We've found a definition for this name in an enclosing function-like scope. + // Either this definition is the valid place this name refers to, or else we'll + // emit a syntax error. Either way, we won't walk any more enclosing scopes. Note + // that there are differences here compared to `infer_place_load`: A regular load + // (e.g. `print(x)`) is allowed to refer to a global variable (e.g. `x = 1` in the + // global scope), and similarly it's allowed to refer to a local variable in an + // enclosing function that's declared `global` (e.g. `global x`). However, the + // `nonlocal` keyword can't refer to global variables (that's a `SyntaxError`), and + // it also can't refer to local variables in enclosing functions that are declared + // `global` (also a `SyntaxError`). + if self + .index + .symbol_is_global_in_scope(enclosing_place_id, enclosing_scope_file_id) + { + // A "chain" of `nonlocal` statements is "broken" by a `global` statement. Stop + // looping and report that this `nonlocal` statement is invalid. + break; + } + // We found a definition. We've checked that the name isn't `global` in this scope, + // but it's ok if it's `nonlocal`. If a "chain" of `nonlocal` statements fails to + // lead to a valid binding, the outermost one will be an error; we don't need to + // walk the whole chain for each one. + continue 'names; + } + // There's no matching binding in an enclosing scope. This `nonlocal` statement is + // invalid. + if let Some(builder) = self + .context + .report_diagnostic(DiagnosticId::InvalidSyntax, Severity::Error) + { + builder + .into_diagnostic(format_args!("no binding for nonlocal `{name}` found")) + .annotate(Annotation::primary(self.context.span(*range))); + } + } + } + fn module_type_from_name(&self, module_name: &ModuleName) -> Option> { resolve_module(self.db(), module_name) .map(|module| Type::module_literal(self.db(), self.file(), &module)) @@ -5775,13 +5889,15 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let current_file = self.file(); + let mut is_nonlocal_binding = false; if let Some(name) = expr.as_name() { - let skip_non_global_scopes = place_table - .place_id_by_name(name) - .is_some_and(|symbol_id| self.skip_non_global_scopes(file_scope_id, symbol_id)); - - if skip_non_global_scopes { - return global_symbol(self.db(), self.file(), name); + if let Some(symbol_id) = place_table.place_id_by_name(name) { + if self.skip_non_global_scopes(file_scope_id, symbol_id) { + return global_symbol(self.db(), self.file(), name); + } + is_nonlocal_binding = self + .index + .symbol_is_nonlocal_in_scope(symbol_id, file_scope_id); } } @@ -5794,7 +5910,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // 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) && !is_nonlocal_binding { return Place::Unbound.into(); }