From 3f739214b41c72a14c2273e86eb28183876af511 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 31 Aug 2022 18:16:37 -0400 Subject: [PATCH] Bind excepthandler names (#62) --- resources/test/src/F821.py | 4 +- src/check_ast.rs | 39 ++++++++++++++++-- src/visitor.rs | 82 +++++++++++--------------------------- 3 files changed, 60 insertions(+), 65 deletions(-) diff --git a/resources/test/src/F821.py b/resources/test/src/F821.py index a672514c96..b00ad2adfc 100644 --- a/resources/test/src/F821.py +++ b/resources/test/src/F821.py @@ -40,9 +40,7 @@ class Class: # TODO(charlie): This should be recognized as a defined variable. Class # noqa: F821 - try: x = 1 / 0 except Exception as e: - # TODO(charlie): This should be recognized as a defined variable. - print(e) # noqa: F821 + print(e) diff --git a/src/check_ast.rs b/src/check_ast.rs index 56ccaa298d..003c95135a 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -2,7 +2,8 @@ use std::collections::{BTreeMap, BTreeSet}; use std::sync::atomic::{AtomicUsize, Ordering}; use rustpython_parser::ast::{ - Arg, Arguments, Constant, Expr, ExprContext, ExprKind, Location, Stmt, StmtKind, Suite, + Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, + Location, Stmt, StmtKind, Suite, }; use rustpython_parser::parser; @@ -11,7 +12,7 @@ use crate::check_ast::ScopeKind::{Class, Function, Generator, Module}; use crate::checks::{Check, CheckCode, CheckKind}; use crate::settings::Settings; use crate::visitor; -use crate::visitor::Visitor; +use crate::visitor::{walk_excepthandler, Visitor}; fn id() -> usize { static COUNTER: AtomicUsize = AtomicUsize::new(1); @@ -301,7 +302,6 @@ impl Visitor for Checker<'_> { ); } } - fn visit_annotation(&mut self, expr: &Expr) { let initial = self.in_annotation; self.in_annotation = true; @@ -365,6 +365,39 @@ impl Visitor for Checker<'_> { }; } + fn visit_excepthandler(&mut self, excepthandler: &Excepthandler) { + match &excepthandler.node { + ExcepthandlerKind::ExceptHandler { name, .. } => match name { + Some(name) => { + let scope = self.scopes.last().expect("No current scope found."); + if scope.values.contains_key(name) { + self.handle_node_store(&Expr::new( + excepthandler.location, + ExprKind::Name { + id: name.to_string(), + ctx: ExprContext::Store, + }, + )); + } + + self.handle_node_store(&Expr::new( + excepthandler.location, + ExprKind::Name { + id: name.to_string(), + ctx: ExprContext::Store, + }, + )); + + walk_excepthandler(self, excepthandler); + + let scope = self.scopes.last_mut().expect("No current scope found."); + scope.values.remove(name); + } + None => walk_excepthandler(self, excepthandler), + }, + } + } + fn visit_arguments(&mut self, arguments: &Arguments) { if self .settings diff --git a/src/visitor.rs b/src/visitor.rs index 463256d9f7..155ba59ffb 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -14,9 +14,6 @@ pub trait Visitor { fn visit_expr(&mut self, expr: &Expr) { walk_expr(self, expr); } - fn visit_ident(&mut self, ident: &str) { - walk_ident(self, ident); - } fn visit_constant(&mut self, constant: &Constant) { walk_constant(self, constant); } @@ -67,53 +64,48 @@ pub trait Visitor { pub fn walk_stmt(visitor: &mut V, stmt: &Stmt) { match &stmt.node { StmtKind::FunctionDef { - name, args, body, decorator_list, returns, .. } => { - visitor.visit_ident(name); visitor.visit_arguments(args); - for stmt in body { - visitor.visit_stmt(stmt) - } for expr in decorator_list { visitor.visit_expr(expr) } for expr in returns { visitor.visit_annotation(expr); } + for stmt in body { + visitor.visit_stmt(stmt) + } } StmtKind::AsyncFunctionDef { - name, args, body, decorator_list, returns, .. } => { - visitor.visit_ident(name); visitor.visit_arguments(args); - for stmt in body { - visitor.visit_stmt(stmt) - } for expr in decorator_list { visitor.visit_expr(expr) } for expr in returns { visitor.visit_annotation(expr); } + for stmt in body { + visitor.visit_stmt(stmt) + } } StmtKind::ClassDef { - name, bases, keywords, body, decorator_list, + .. } => { - visitor.visit_ident(name); for expr in bases { visitor.visit_expr(expr) } @@ -271,24 +263,13 @@ pub fn walk_stmt(visitor: &mut V, stmt: &Stmt) { visitor.visit_alias(alias); } } - StmtKind::ImportFrom { module, names, .. } => { - if let Some(ident) = module { - visitor.visit_ident(ident); - } + StmtKind::ImportFrom { names, .. } => { for alias in names { visitor.visit_alias(alias); } } - StmtKind::Global { names } => { - for ident in names { - visitor.visit_ident(ident) - } - } - StmtKind::Nonlocal { names } => { - for ident in names { - visitor.visit_ident(ident) - } - } + StmtKind::Global { .. } => {} + StmtKind::Nonlocal { .. } => {} StmtKind::Expr { value } => visitor.visit_expr(value), StmtKind::Pass => {} StmtKind::Break => {} @@ -428,8 +409,7 @@ pub fn walk_expr(visitor: &mut V, expr: &Expr) { visitor.visit_expr(value); visitor.visit_expr_context(ctx); } - ExprKind::Name { id, ctx } => { - visitor.visit_ident(id); + ExprKind::Name { ctx, .. } => { visitor.visit_expr_context(ctx); } ExprKind::List { elts, ctx } => { @@ -476,13 +456,10 @@ pub fn walk_comprehension(visitor: &mut V, comprehension: & pub fn walk_excepthandler(visitor: &mut V, excepthandler: &Excepthandler) { match &excepthandler.node { - ExcepthandlerKind::ExceptHandler { type_, name, body } => { + ExcepthandlerKind::ExceptHandler { type_, body, .. } => { if let Some(expr) = type_ { visitor.visit_expr(expr); } - if let Some(ident) = name { - visitor.visit_ident(ident); - } for stmt in body { visitor.visit_stmt(stmt); } @@ -550,50 +527,34 @@ pub fn walk_pattern(visitor: &mut V, pattern: &Pattern) { visitor.visit_pattern(pattern) } } - PatternKind::MatchMapping { - keys, - patterns, - rest, - } => { + PatternKind::MatchMapping { keys, patterns, .. } => { for expr in keys { visitor.visit_expr(expr); } for pattern in patterns { visitor.visit_pattern(pattern); } - if let Some(ident) = rest { - visitor.visit_ident(ident); - } } PatternKind::MatchClass { cls, patterns, - kwd_attrs, kwd_patterns, + .. } => { visitor.visit_expr(cls); for pattern in patterns { visitor.visit_pattern(pattern); } - for ident in kwd_attrs { - visitor.visit_ident(ident); - } + for pattern in kwd_patterns { visitor.visit_pattern(pattern); } } - PatternKind::MatchStar { name } => { - if let Some(ident) = name { - visitor.visit_ident(ident) - } - } - PatternKind::MatchAs { pattern, name } => { + PatternKind::MatchStar { .. } => {} + PatternKind::MatchAs { pattern, .. } => { if let Some(pattern) = pattern { visitor.visit_pattern(pattern) } - if let Some(ident) = name { - visitor.visit_ident(ident) - } } PatternKind::MatchOr { patterns } => { for pattern in patterns { @@ -604,22 +565,25 @@ pub fn walk_pattern(visitor: &mut V, pattern: &Pattern) { } #[allow(unused_variables)] -pub fn walk_ident(visitor: &mut V, ident: &str) {} - -#[allow(unused_variables)] +#[inline(always)] pub fn walk_expr_context(visitor: &mut V, expr_context: &ExprContext) {} #[allow(unused_variables)] +#[inline(always)] pub fn walk_boolop(visitor: &mut V, boolop: &Boolop) {} #[allow(unused_variables)] +#[inline(always)] pub fn walk_operator(visitor: &mut V, operator: &Operator) {} #[allow(unused_variables)] +#[inline(always)] pub fn walk_unaryop(visitor: &mut V, unaryop: &Unaryop) {} #[allow(unused_variables)] +#[inline(always)] pub fn walk_cmpop(visitor: &mut V, cmpop: &Cmpop) {} #[allow(unused_variables)] +#[inline(always)] pub fn walk_alias(visitor: &mut V, alias: &Alias) {}