Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

WIP: move the InvalidNonlocal check to SemanticIndexBuilder 

This makes one test case fail, basically this:

```py
def f():
    def g():
        nonlocal x  # allowed!
    x = 1
```
This commit is contained in:
Jack O'Connor 2025-07-10 12:05:14 -07:00
parent 664a9a28dc
commit 05cf7c3458
4 changed files with 98 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
crates/ruff_linter/src/checkers/ast/mod.rs
View File

@ -674,7 +674,8 @@ impl SemanticSyntaxContext for Checker<'_> {
| SemanticSyntaxErrorKind::LoadBeforeNonlocalDeclaration { .. }
| SemanticSyntaxErrorKind::NonlocalAndGlobal(_)
| SemanticSyntaxErrorKind::AnnotatedGlobal(_)
| SemanticSyntaxErrorKind::AnnotatedNonlocal(_) => {
| SemanticSyntaxErrorKind::AnnotatedNonlocal(_)
| SemanticSyntaxErrorKind::InvalidNonlocal(_) => {
self.semantic_errors.borrow_mut().push(error);
}
}

29
crates/ruff_python_parser/src/semantic_errors.rs
View File

@ -989,6 +989,9 @@ impl Display for SemanticSyntaxError {
SemanticSyntaxErrorKind::AnnotatedNonlocal(name) => {
write!(f, "annotated name `{name}` can't be nonlocal")
}
SemanticSyntaxErrorKind::InvalidNonlocal(name) => {
write!(f, "no binding for nonlocal `{name}` found")
}
}
}
}
@ -1346,6 +1349,32 @@ pub enum SemanticSyntaxErrorKind {
/// Represents a type annotation on a variable that's been declared nonlocal
AnnotatedNonlocal(String),
/// Represents a nonlocal declaration with no definition in an enclosing scope
///
/// ## Examples
///
/// ```python
/// def f():
/// nonlocal x # error
///
/// # Global variables don't count.
/// x = 1
/// def f():
/// nonlocal x # error
///
/// def f():
/// x = 1
/// def g():
/// nonlocal x # allowed
///
/// # The definition can come later.
/// def f():
/// def g():
/// nonlocal x # allowed
/// x = 1
/// ```
InvalidNonlocal(String),
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize)]

74
crates/ty_python_semantic/src/semantic_index/builder.rs
View File

@ -1912,10 +1912,11 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
names,
}) => {
for name in names {
let symbol_id = self.add_symbol(name.id.clone());
let symbol = self.current_place_table().place_expr(symbol_id);
let local_scoped_place_id = self.add_symbol(name.id.clone());
let local_place = self.current_place_table().place_expr(local_scoped_place_id);
// Check whether the variable has already been accessed in this scope.
if symbol.is_bound() || symbol.is_declared() || symbol.is_used() {
if local_place.is_bound() || local_place.is_declared() || local_place.is_used()
{
self.report_semantic_error(SemanticSyntaxError {
kind: SemanticSyntaxErrorKind::LoadBeforeNonlocalDeclaration {
name: name.to_string(),
@ -1926,24 +1927,79 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
});
}
// Check whether the variable has also been declared global.
if symbol.is_marked_global() {
if local_place.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`:
// The name is required to exist in an enclosing scope, but that definition
// might come later. For example, this is example legal:
//
// ```py
// def f():
// def g():
// nonlocal x
// x = 1
// ```
self.current_place_table_mut()
.mark_place_nonlocal(symbol_id);
//
// To handle cases like this, we have to walk `x = 1` before we walk `nonlocal
// x`. In other words, walking function bodies must be "deferred" to the end of
// the scope where they're defined. See the `FunctionDef` branch above.
let name_expr = PlaceExpr::name(name.id.clone());
let mut found_matching_definition = false;
for enclosing_scope_info in self.scope_stack.iter().rev().skip(1) {
let enclosing_scope = &self.scopes[enclosing_scope_info.file_scope_id];
if !enclosing_scope.kind().is_function_like() {
// Skip over class scopes and the global scope.
continue;
}
let enclosing_place_table =
&self.place_tables[enclosing_scope_info.file_scope_id];
let Some(enclosing_scoped_place_id) =
enclosing_place_table.place_id_by_expr(&name_expr)
else {
// This name isn't defined in this scope. Keep going.
continue;
};
let enclosing_place =
enclosing_place_table.place_expr(enclosing_scoped_place_id);
// 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 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 variables in enclosing functions that are declared
// `global` (also a `SyntaxError`).
if enclosing_place.is_marked_global() {
// 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, and we've checked that that place isn't declared
// `global` in its 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 report an error for each one.
found_matching_definition = true;
self.current_place_table_mut()
.mark_place_nonlocal(local_scoped_place_id);
break;
}
if !found_matching_definition {
// There's no matching definition in an enclosing scope. This `nonlocal`
// statement is invalid.
self.report_semantic_error(SemanticSyntaxError {
kind: SemanticSyntaxErrorKind::InvalidNonlocal(name.to_string()),
range: name.range,
python_version: self.python_version,
});
}
}
walk_stmt(self, stmt);
}

68
crates/ty_python_semantic/src/types/infer.rs
View File

@ -35,7 +35,6 @@
//! 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};
@ -2257,12 +2256,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::Global(_)
| ast::Stmt::Nonlocal(_) => {
// No-op
}
}
@ -4662,69 +4661,6 @@ 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<Type<'db>> {
resolve_module(self.db(), module_name)
.map(|module| Type::module_literal(self.db(), self.file(), &module))