From b0984a2868d640aa25d6756c4e3dbacc6054240f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 13 Jun 2023 13:45:51 -0400 Subject: [PATCH] Treat exception binding as explicit deletion (#5057) ## Summary This PR corrects a misunderstanding I had related to Python's handling of bound exceptions. Previously, I thought this code ran without error: ```py def f(): x = 1 try: 1 / 0 except Exception as x: pass print(x) ``` My understanding was that `except Exception as x` bound `x` within the `except` block, but then restored the `x = 1` binding after exiting the block. In practice, however, this throws a `UnboundLocalError` error, because `x` becomes "unbound" after exiting the exception handler. It's similar to a `del` statement in this way. This PR removes our behavior to "restore" the previous binding. This could lead to faulty analysis in conditional blocks due to our lack of control flow analysis, but those same problems already exist for `del` statements. --- crates/ruff/src/checkers/ast/mod.rs | 6 ---- crates/ruff/src/rules/pyflakes/mod.rs | 14 ++++++++ ...__tests__print_after_shadowing_except.snap | 32 +++++++++++++++++++ 3 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__print_after_shadowing_except.snap diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 5bfadea96e..acce1cdd4c 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -3944,7 +3944,6 @@ where ); } - let definition = self.semantic_model.scope().get(name); self.handle_node_store( name, &Expr::Name(ast::ExprName { @@ -3979,11 +3978,6 @@ where } } } - - if let Some(binding_id) = definition { - let scope = self.semantic_model.scope_mut(); - scope.add(name, binding_id); - } } None => walk_excepthandler(self, excepthandler), } diff --git a/crates/ruff/src/rules/pyflakes/mod.rs b/crates/ruff/src/rules/pyflakes/mod.rs index 11c6414b7f..ba63fe6ef0 100644 --- a/crates/ruff/src/rules/pyflakes/mod.rs +++ b/crates/ruff/src/rules/pyflakes/mod.rs @@ -311,6 +311,20 @@ mod tests { "#, "del_shadowed_local_import_in_local_scope" )] + #[test_case( + r#" + def f(): + x = 1 + + try: + 1 / 0 + except Exception as x: + pass + + print(x) + "#, + "print_after_shadowing_except" + )] fn contents(contents: &str, snapshot: &str) { let diagnostics = test_snippet(contents, &Settings::for_rules(&Linter::Pyflakes)); assert_messages!(snapshot, diagnostics); diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__print_after_shadowing_except.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__print_after_shadowing_except.snap new file mode 100644 index 0000000000..70f280d1fd --- /dev/null +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__print_after_shadowing_except.snap @@ -0,0 +1,32 @@ +--- +source: crates/ruff/src/rules/pyflakes/mod.rs +--- +:7:25: F841 [*] Local variable `x` is assigned to but never used + | +5 | try: +6 | 1 / 0 +7 | except Exception as x: + | ^ F841 +8 | pass + | + = help: Remove assignment to unused variable `x` + +ℹ Suggested fix +4 4 | +5 5 | try: +6 6 | 1 / 0 +7 |- except Exception as x: + 7 |+ except Exception: +8 8 | pass +9 9 | +10 10 | print(x) + +:10:11: F821 Undefined name `x` + | + 8 | pass + 9 | +10 | print(x) + | ^ F821 + | + +