Implement F841 (for exception handlers) (#63)

resources/test/src/F841.py

@@ -0,0 +1,10 @@
+try:
+    1 / 0
+except ValueError as e:
+    pass
+
+
+try:
+    1 / 0
+except ValueError as e:
+    print(e)

resources/test/src/pyproject.toml

@@ -11,5 +11,6 @@ select = [
     "F821",
     "F831",
     "F832",
+    "F841",
     "F901",
 ]

src/check_ast.rs

@@ -42,6 +42,7 @@ impl Scope {
     }
 }
 
+#[derive(Clone)]
 enum BindingKind {
     Argument,
     Assignment,
@@ -54,6 +55,7 @@ enum BindingKind {
     SubmoduleImportation(String),
 }
 
+#[derive(Clone)]
 struct Binding {
     kind: BindingKind,
     location: Location,
@@ -380,6 +382,8 @@ impl Visitor for Checker<'_> {
                         ));
                     }
 
+                    let scope = self.scopes.last().expect("No current scope found.");
+                    let prev_definition = scope.values.get(name).cloned();
                     self.handle_node_store(&Expr::new(
                         excepthandler.location,
                         ExprKind::Name {
@@ -391,7 +395,19 @@ impl Visitor for Checker<'_> {
                     walk_excepthandler(self, excepthandler);
 
                     let scope = self.scopes.last_mut().expect("No current scope found.");
-                    scope.values.remove(name);
+                    if let Some(binding) = scope.values.remove(name) {
+                        if self.settings.select.contains(&CheckCode::F841) && binding.used.is_none()
+                        {
+                            self.checks.push(Check {
+                                kind: CheckKind::UnusedVariable(name.to_string()),
+                                location: excepthandler.location,
+                            });
+                        }
+                    }
+
+                    if let Some(binding) = prev_definition {
+                        scope.values.insert(name.to_string(), binding);
+                    }
                 }
                 None => walk_excepthandler(self, excepthandler),
             },

src/checks.rs

@@ -16,6 +16,7 @@ pub enum CheckCode {
     F821,
     F831,
     F832,
+    F841,
     F901,
 }
 
@@ -33,6 +34,7 @@ impl FromStr for CheckCode {
             "F821" => Ok(CheckCode::F821),
             "F831" => Ok(CheckCode::F831),
             "F832" => Ok(CheckCode::F832),
+            "F841" => Ok(CheckCode::F841),
             "F901" => Ok(CheckCode::F901),
             _ => Err(anyhow::anyhow!("Unknown check code: {s}")),
         }
@@ -51,6 +53,7 @@ impl CheckCode {
             CheckCode::F821 => "F821",
             CheckCode::F831 => "F831",
             CheckCode::F832 => "F832",
+            CheckCode::F841 => "F841",
             CheckCode::F901 => "F901",
         }
     }
@@ -67,6 +70,7 @@ impl CheckCode {
             CheckCode::F821 => &LintSource::AST,
             CheckCode::F831 => &LintSource::AST,
             CheckCode::F832 => &LintSource::AST,
+            CheckCode::F841 => &LintSource::AST,
             CheckCode::F901 => &LintSource::AST,
         }
     }
@@ -89,6 +93,7 @@ pub enum CheckKind {
     ReturnOutsideFunction,
     UndefinedName(String),
     UndefinedLocal(String),
+    UnusedVariable(String),
     UnusedImport(String),
 }
 
@@ -105,6 +110,7 @@ impl CheckKind {
             CheckKind::ReturnOutsideFunction => &CheckCode::F706,
             CheckKind::UndefinedName(_) => &CheckCode::F821,
             CheckKind::UndefinedLocal(_) => &CheckCode::F832,
+            CheckKind::UnusedVariable(_) => &CheckCode::F841,
             CheckKind::UnusedImport(_) => &CheckCode::F401,
         }
     }
@@ -132,6 +138,9 @@ impl CheckKind {
             CheckKind::UndefinedName(name) => {
                 format!("Undefined name `{name}`")
             }
+            CheckKind::UnusedVariable(name) => {
+                format!("Local variable `{name}` is assigned to but never used")
+            }
             CheckKind::UndefinedLocal(name) => {
                 format!("Local variable `{name}` referenced before assignment")
             }

src/linter.rs

@@ -354,6 +354,30 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn f841() -> Result<()> {
+        let actual = check_path(
+            &Path::new("./resources/test/src/F841.py"),
+            &settings::Settings {
+                line_length: 88,
+                exclude: vec![],
+                select: BTreeSet::from([CheckCode::F841]),
+            },
+            &cache::Mode::None,
+        )?;
+        let expected = vec![Message {
+            kind: CheckKind::UnusedVariable("e".to_string()),
+            location: Location::new(3, 1),
+            filename: "./resources/test/src/F841.py".to_string(),
+        }];
+        assert_eq!(actual.len(), expected.len());
+        for i in 0..actual.len() {
+            assert_eq!(actual[i], expected[i]);
+        }
+
+        Ok(())
+    }
+
     #[test]
     fn f901() -> Result<()> {
         let actual = check_path(

src/pyproject.rs

@@ -236,6 +236,7 @@ other-attribute = 1
                     CheckCode::F821,
                     CheckCode::F831,
                     CheckCode::F832,
+                    CheckCode::F841,
                     CheckCode::F901,
                 ])),
             }