Avoid raising TRY200 violations within new scopes (#3275)

crates/ruff/resources/test/fixtures/tryceratops/TRY200.py

@@ -30,3 +30,11 @@ def good():
         raise e  # This is verbose violation, shouldn't trigger no cause
     except Exception:
         raise  # Just re-raising don't need 'from'
+
+
+def good():
+    try:
+        from mod import f
+    except ImportError:
+        def f():
+            raise MyException()  # Raising within a new scope is fine

crates/ruff/src/ast/helpers.rs

@@ -765,7 +765,7 @@ pub fn from_relative_import<'a>(module: &'a [String], name: &'a str) -> CallPath
     call_path
 }
 
-/// A [`Visitor`] that collects all return statements in a function or method.
+/// A [`Visitor`] that collects all `return` statements in a function or method.
 #[derive(Default)]
 pub struct ReturnStatementVisitor<'a> {
     pub returns: Vec<Option<&'a Expr>>,
@@ -786,6 +786,48 @@ where
     }
 }
 
+/// A [`Visitor`] that collects all `raise` statements in a function or method.
+#[derive(Default)]
+pub struct RaiseStatementVisitor<'a> {
+    pub raises: Vec<(Range, Option<&'a Expr>, Option<&'a Expr>)>,
+}
+
+impl<'a, 'b> Visitor<'b> for RaiseStatementVisitor<'b>
+where
+    'b: 'a,
+{
+    fn visit_stmt(&mut self, stmt: &'b Stmt) {
+        match &stmt.node {
+            StmtKind::Raise { exc, cause } => {
+                self.raises
+                    .push((Range::from_located(stmt), exc.as_deref(), cause.as_deref()));
+            }
+            StmtKind::ClassDef { .. }
+            | StmtKind::FunctionDef { .. }
+            | StmtKind::AsyncFunctionDef { .. }
+            | StmtKind::Try { .. }
+            | StmtKind::TryStar { .. } => {}
+            StmtKind::If { body, orelse, .. } => {
+                visitor::walk_body(self, body);
+                visitor::walk_body(self, orelse);
+            }
+            StmtKind::While { body, .. }
+            | StmtKind::With { body, .. }
+            | StmtKind::AsyncWith { body, .. }
+            | StmtKind::For { body, .. }
+            | StmtKind::AsyncFor { body, .. } => {
+                visitor::walk_body(self, body);
+            }
+            StmtKind::Match { cases, .. } => {
+                for case in cases {
+                    visitor::walk_body(self, &case.body);
+                }
+            }
+            _ => {}
+        }
+    }
+}
+
 /// Convert a location within a file (relative to `base`) to an absolute
 /// position.
 pub fn to_absolute(relative: Location, base: Location) -> Location {

crates/ruff/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs

@@ -1,10 +1,10 @@
+use rustpython_parser::ast::{ExprKind, Stmt};
+
 use ruff_macros::{define_violation, derive_message_formats};
 use ruff_python::str::is_lower;
-use rustpython_parser::ast::{ExprKind, Stmt, StmtKind};
 
-use crate::ast::types::Range;
+use crate::ast::helpers::RaiseStatementVisitor;
 use crate::ast::visitor;
-use crate::ast::visitor::Visitor;
 use crate::checkers::ast::Checker;
 use crate::registry::Diagnostic;
 use crate::violation::Violation;
@@ -16,62 +16,32 @@ impl Violation for RaiseWithoutFromInsideExcept {
     #[derive_message_formats]
     fn message(&self) -> String {
         format!(
-            "Within an except clause, raise exceptions with `raise ... from err` or `raise ... \
+            "Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... \
              from None` to distinguish them from errors in exception handling"
         )
     }
 }
 
-struct RaiseVisitor {
-    diagnostics: Vec<Diagnostic>,
-}
+/// B904
+pub fn raise_without_from_inside_except(checker: &mut Checker, body: &[Stmt]) {
+    let raises = {
+        let mut visitor = RaiseStatementVisitor::default();
+        visitor::walk_body(&mut visitor, body);
+        visitor.raises
+    };
 
-impl<'a> Visitor<'a> for RaiseVisitor {
-    fn visit_stmt(&mut self, stmt: &'a Stmt) {
-        match &stmt.node {
-            StmtKind::Raise {
-                exc: Some(exc),
-                cause: None,
-            } => match &exc.node {
-                ExprKind::Name { id, .. } if is_lower(id) => {}
-                _ => {
-                    self.diagnostics.push(Diagnostic::new(
-                        RaiseWithoutFromInsideExcept,
-                        Range::from_located(stmt),
-                    ));
-                }
-            },
-            StmtKind::ClassDef { .. }
-            | StmtKind::FunctionDef { .. }
-            | StmtKind::AsyncFunctionDef { .. }
-            | StmtKind::Try { .. }
-            | StmtKind::TryStar { .. } => {}
-            StmtKind::If { body, orelse, .. } => {
-                visitor::walk_body(self, body);
-                visitor::walk_body(self, orelse);
-            }
-            StmtKind::While { body, .. }
-            | StmtKind::With { body, .. }
-            | StmtKind::AsyncWith { body, .. }
-            | StmtKind::For { body, .. }
-            | StmtKind::AsyncFor { body, .. } => {
-                visitor::walk_body(self, body);
-            }
-            StmtKind::Match { cases, .. } => {
-                for case in cases {
-                    visitor::walk_body(self, &case.body);
+    for (range, exc, cause) in raises {
+        if cause.is_none() {
+            if let Some(exc) = exc {
+                match &exc.node {
+                    ExprKind::Name { id, .. } if is_lower(id) => {}
+                    _ => {
+                        checker
+                            .diagnostics
+                            .push(Diagnostic::new(RaiseWithoutFromInsideExcept, range));
+                    }
                 }
             }
-            _ => {}
         }
     }
 }
-
-/// B904
-pub fn raise_without_from_inside_except(checker: &mut Checker, body: &[Stmt]) {
-    let mut visitor = RaiseVisitor {
-        diagnostics: vec![],
-    };
-    visitor::walk_body(&mut visitor, body);
-    checker.diagnostics.extend(visitor.diagnostics);
-}

crates/ruff/src/rules/tryceratops/rules/reraise_no_cause.rs

@@ -1,8 +1,9 @@
-use ruff_macros::{define_violation, derive_message_formats};
-use rustpython_parser::ast::{ExprKind, Stmt, StmtKind};
+use rustpython_parser::ast::{ExprKind, Stmt};
 
-use crate::ast::types::Range;
-use crate::ast::visitor::{self, Visitor};
+use ruff_macros::{define_violation, derive_message_formats};
+
+use crate::ast::helpers::RaiseStatementVisitor;
+use crate::ast::visitor::Visitor;
 use crate::checkers::ast::Checker;
 use crate::registry::Diagnostic;
 use crate::violation::Violation;
@@ -17,23 +18,6 @@ impl Violation for ReraiseNoCause {
     }
 }
 
-#[derive(Default)]
-struct RaiseStatementVisitor<'a> {
-    raises: Vec<&'a Stmt>,
-}
-
-impl<'a, 'b> Visitor<'b> for RaiseStatementVisitor<'a>
-where
-    'b: 'a,
-{
-    fn visit_stmt(&mut self, stmt: &'b Stmt) {
-        match stmt.node {
-            StmtKind::Raise { .. } => self.raises.push(stmt),
-            _ => visitor::walk_stmt(self, stmt),
-        }
-    }
-}
-
 /// TRY200
 pub fn reraise_no_cause(checker: &mut Checker, body: &[Stmt]) {
     let raises = {
@@ -44,17 +28,11 @@ pub fn reraise_no_cause(checker: &mut Checker, body: &[Stmt]) {
         visitor.raises
     };
 
-    for stmt in raises {
-        if let StmtKind::Raise { exc, cause, .. } = &stmt.node {
-            if exc
-                .as_ref()
-                .map_or(false, |expr| matches!(expr.node, ExprKind::Call { .. }))
-                && cause.is_none()
-            {
-                checker
-                    .diagnostics
-                    .push(Diagnostic::new(ReraiseNoCause, Range::from_located(stmt)));
-            }
+    for (range, exc, cause) in raises {
+        if exc.map_or(false, |expr| matches!(expr.node, ExprKind::Call { .. })) && cause.is_none() {
+            checker
+                .diagnostics
+                .push(Diagnostic::new(ReraiseNoCause, range));
         }
     }
 }