Bind excepthandler names (#62)

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)
-    print(e)  # noqa: F821

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

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<V: Visitor + ?Sized>(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<V: Visitor + ?Sized>(visitor: &mut V, stmt: &Stmt) {
                 visitor.visit_alias(alias);
             }
         }
-        StmtKind::ImportFrom { module, names, .. } => {
+        StmtKind::ImportFrom { names, .. } => {
-            if let Some(ident) = module {
-                visitor.visit_ident(ident);
-            }
             for alias in names {
                 visitor.visit_alias(alias);
             }
         }
-        StmtKind::Global { names } => {
+        StmtKind::Global { .. } => {}
-            for ident in names {
+        StmtKind::Nonlocal { .. } => {}
-                visitor.visit_ident(ident)
-            }
-        }
-        StmtKind::Nonlocal { names } => {
-            for ident in names {
-                visitor.visit_ident(ident)
-            }
-        }
         StmtKind::Expr { value } => visitor.visit_expr(value),
         StmtKind::Pass => {}
         StmtKind::Break => {}
@@ -428,8 +409,7 @@ pub fn walk_expr<V: Visitor + ?Sized>(visitor: &mut V, expr: &Expr) {
             visitor.visit_expr(value);
             visitor.visit_expr_context(ctx);
         }
-        ExprKind::Name { id, ctx } => {
+        ExprKind::Name { ctx, .. } => {
-            visitor.visit_ident(id);
             visitor.visit_expr_context(ctx);
         }
         ExprKind::List { elts, ctx } => {
@@ -476,13 +456,10 @@ pub fn walk_comprehension<V: Visitor + ?Sized>(visitor: &mut V, comprehension: &
 
 pub fn walk_excepthandler<V: Visitor + ?Sized>(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<V: Visitor + ?Sized>(visitor: &mut V, pattern: &Pattern) {
                 visitor.visit_pattern(pattern)
             }
         }
-        PatternKind::MatchMapping {
+        PatternKind::MatchMapping { keys, patterns, .. } => {
-            keys,
-            patterns,
-            rest,
-        } => {
             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 } => {
+        PatternKind::MatchStar { .. } => {}
-            if let Some(ident) = name {
+        PatternKind::MatchAs { pattern, .. } => {
-                visitor.visit_ident(ident)
-            }
-        }
-        PatternKind::MatchAs { pattern, name } => {
             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<V: Visitor + ?Sized>(visitor: &mut V, pattern: &Pattern) {
 }
 
 #[allow(unused_variables)]
-pub fn walk_ident<V: Visitor + ?Sized>(visitor: &mut V, ident: &str) {}
+#[inline(always)]
-
-#[allow(unused_variables)]
 pub fn walk_expr_context<V: Visitor + ?Sized>(visitor: &mut V, expr_context: &ExprContext) {}
 
 #[allow(unused_variables)]
+#[inline(always)]
 pub fn walk_boolop<V: Visitor + ?Sized>(visitor: &mut V, boolop: &Boolop) {}
 
 #[allow(unused_variables)]
+#[inline(always)]
 pub fn walk_operator<V: Visitor + ?Sized>(visitor: &mut V, operator: &Operator) {}
 
 #[allow(unused_variables)]
+#[inline(always)]
 pub fn walk_unaryop<V: Visitor + ?Sized>(visitor: &mut V, unaryop: &Unaryop) {}
 
 #[allow(unused_variables)]
+#[inline(always)]
 pub fn walk_cmpop<V: Visitor + ?Sized>(visitor: &mut V, cmpop: &Cmpop) {}
 
 #[allow(unused_variables)]
+#[inline(always)]
 pub fn walk_alias<V: Visitor + ?Sized>(visitor: &mut V, alias: &Alias) {}