Merge branch 'main' into 5246_try301_identical_rule

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

@@ -57,3 +57,11 @@ def fine():
         a = process()  # This throws the exception now
     finally:
         print("finally")
+
+
+def fine():
+    try:
+        raise ValueError("a doesn't exist")
+
+    except TypeError: # A different exception is caught
+        print("A different exception is caught")

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

@@ -1,13 +1,17 @@
-use rustpython_parser::ast::{ExceptHandler, Ranged, Stmt};
+use rustpython_parser::ast::{self, ExceptHandler, Expr, Ranged, Stmt};
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
-use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor};
+use ruff_python_ast::{
+    helpers,
+    statement_visitor::{walk_stmt, StatementVisitor},
+};
 
 use crate::checkers::ast::Checker;
 
 /// ## What it does
-/// Checks for `raise` statements within `try` blocks.
+/// Checks for `raise` statements within `try` blocks. The only `raise`s
+/// caught are those that throw exceptions caught by the `try` statement itself.
 ///
 /// ## Why is this bad?
 /// Raising and catching exceptions within the same `try` block is redundant,
@@ -83,9 +87,55 @@ pub(crate) fn raise_within_try(checker: &mut Checker, body: &[Stmt], handlers: &
         visitor.raises
     };
 
+    if raises.is_empty() {
+        return;
+    }
+
+    // The names of exceptions handled by the `try` Stmt. We can't compare `Expr`'s since they have
+    // different ranges, but by virtue of this function's call path we know that the `raise`
+    // statements will always be within the surrounding `try`.
+
+    let handler_ids: Vec<&str> = helpers::extract_handled_exceptions(handlers)
+        .into_iter()
+        .filter_map(|handler| {
+            if let Expr::Name(ast::ExprName { id, .. }) = handler {
+                Some(id.as_str())
+            } else {
+                None
+            }
+        })
+        .collect();
+
     for stmt in raises {
-        checker
+        let Stmt::Raise(ast::StmtRaise { exc: Some(exception), .. }) = stmt else {
-            .diagnostics
+            continue;
-            .push(Diagnostic::new(RaiseWithinTry, stmt.range()));
+        };
+
+        let Some(exc_name) = get_function_name(exception.as_ref()) else {
+            continue;
+        };
+
+        // We can't check exception sub-classes without a type-checker implementation, so let's
+        // just catch the blanket `Exception` for now.
+        if (handler_ids.contains(&"Exception") && checker.semantic().is_builtin("Exception"))
+            || handler_ids.contains(&exc_name)
+        {
+            checker
+                .diagnostics
+                .push(Diagnostic::new(RaiseWithinTry, stmt.range()));
+        }
+    }
+}
+
+/// Get the name of an [`Expr::Call`], if applicable. If the passed [`Expr`] isn't a [`Expr::Call`], return an
+/// empty [`Option`].
+fn get_function_name(expr: &Expr) -> Option<&str> {
+    let Expr::Call(ast::ExprCall { func, .. }) = expr else {
+        return None;
+    };
+
+    match func.as_ref() {
+        Expr::Name(ast::ExprName { id, .. }) => Some(id.as_str()),
+        _ => None,
     }
 }