diff --git a/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py b/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py index baa823565b5990..154ee23bec74ac 100644 --- a/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py +++ b/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py @@ -17,3 +17,30 @@ def f(): def f(): nonlocal y + + +def f(): + x = 1 + + def g(): + nonlocal x + + del x + + +def f(): + def g(): + nonlocal x + + del x + + +def f(): + try: + pass + except Exception as x: + pass + + def g(): + nonlocal x + x = 2 diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 2eda953a019a10..25b2e30af02cc4 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -307,9 +307,18 @@ where stmt.range(), ExecutionContext::Runtime, ); - } else { - // Ensure that every nonlocal has an existing binding from a parent scope. - if self.enabled(Rule::NonlocalWithoutBinding) { + } + + // Ensure that every nonlocal has an existing binding from a parent scope. + if self.enabled(Rule::NonlocalWithoutBinding) { + if self + .semantic_model + .scopes + .ancestors(self.semantic_model.scope_id) + .skip(1) + .take_while(|scope| !scope.kind.is_module()) + .all(|scope| !scope.declares(name.as_str())) + { self.diagnostics.push(Diagnostic::new( pylint::rules::NonlocalWithoutBinding { name: name.to_string(), @@ -4039,7 +4048,7 @@ where let name_range = helpers::excepthandler_name_range(excepthandler, self.locator).unwrap(); - if self.semantic_model.scope().defines(name) { + if self.semantic_model.scope().has(name) { self.handle_node_store( name, &Expr::Name(ast::ExprName { @@ -4064,7 +4073,7 @@ where if let Some(binding_id) = { let scope = self.semantic_model.scope_mut(); - scope.remove(name) + scope.delete(name) } { if !self.semantic_model.is_used(binding_id) { if self.enabled(Rule::UnusedVariable) { @@ -4694,19 +4703,16 @@ impl<'a> Checker<'a> { } let scope = self.semantic_model.scope_mut(); - if scope.remove(id.as_str()).is_some() { - return; - } - if !self.enabled(Rule::UndefinedName) { - return; + if scope.delete(id.as_str()).is_none() { + if self.enabled(Rule::UndefinedName) { + self.diagnostics.push(Diagnostic::new( + pyflakes::rules::UndefinedName { + name: id.to_string(), + }, + expr.range(), + )); + } } - - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedName { - name: id.to_string(), - }, - expr.range(), - )); } fn check_deferred_future_type_definitions(&mut self) { @@ -4974,7 +4980,7 @@ impl<'a> Checker<'a> { .collect(); if !sources.is_empty() { for (name, range) in &exports { - if !scope.defines(name) { + if !scope.has(name) { diagnostics.push(Diagnostic::new( pyflakes::rules::UndefinedLocalWithImportStarUsage { name: (*name).to_string(), diff --git a/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs b/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs index 07ed2835176444..e4edea697da136 100644 --- a/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs +++ b/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs @@ -51,7 +51,7 @@ impl Violation for UndefinedExport { pub(crate) fn undefined_export(name: &str, range: TextRange, scope: &Scope) -> Vec { let mut diagnostics = Vec::new(); if !scope.uses_star_imports() { - if !scope.defines(name) { + if !scope.has(name) { diagnostics.push(Diagnostic::new( UndefinedExport { name: (*name).to_string(), diff --git a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs index 1479fdb945404e..20c8fdef62d78c 100644 --- a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs +++ b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs @@ -47,7 +47,7 @@ impl Violation for UndefinedLocal { pub(crate) fn undefined_local(checker: &mut Checker, name: &str) { // If the name hasn't already been defined in the current scope... let current = checker.semantic_model().scope(); - if !current.kind.is_any_function() || current.defines(name) { + if !current.kind.is_any_function() || current.has(name) { return; } diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index 09cadf47cf8b9c..be3edffe052d23 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -23,6 +23,8 @@ pub struct Scope<'a> { bindings: FxHashMap<&'a str, BindingId>, /// A map from binding ID to binding ID that it shadows. shadowed_bindings: HashMap>, + /// A list of all names that have been deleted in this scope. + deleted_symbols: Vec<&'a str>, /// Index into the globals arena, if the scope contains any globally-declared symbols. globals_id: Option, } @@ -36,6 +38,7 @@ impl<'a> Scope<'a> { star_imports: Vec::default(), bindings: FxHashMap::default(), shadowed_bindings: IntMap::default(), + deleted_symbols: Vec::default(), globals_id: None, } } @@ -48,6 +51,7 @@ impl<'a> Scope<'a> { star_imports: Vec::default(), bindings: FxHashMap::default(), shadowed_bindings: IntMap::default(), + deleted_symbols: Vec::default(), globals_id: None, } } @@ -67,14 +71,23 @@ impl<'a> Scope<'a> { } } - /// Returns `true` if this scope defines a binding with the given name. - pub fn defines(&self, name: &str) -> bool { + /// Removes the binding with the given name. + pub fn delete(&mut self, name: &'a str) -> Option { + self.deleted_symbols.push(name); + self.bindings.remove(name) + } + + /// Returns `true` if this scope has a binding with the given name. + pub fn has(&self, name: &str) -> bool { self.bindings.contains_key(name) } - /// Removes the binding with the given name - pub fn remove(&mut self, name: &str) -> Option { - self.bindings.remove(name) + /// Returns `true` if the scope declares a symbol with the given name. + /// + /// Unlike [`Scope::has`], the name may no longer be bound to a value (e.g., it could be + /// deleted). + pub fn declares(&self, name: &str) -> bool { + self.has(name) || self.deleted_symbols.contains(&name) } /// Returns the ids of all bindings defined in this scope.