Implement TRY302 - raise after except

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

@@ -1,3 +1,8 @@
+"""
+Violation:
+
+Checks for `raise` statements within `try` blocks.
+"""
 class MyException(Exception):
     pass
 

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

@@ -0,0 +1,49 @@
+"""
+Violation:
+
+Checks for uses of `raise` directly after a `rescue`.
+"""
+class MyException(Exception):
+    pass
+
+def bad():
+    try:
+        process()
+    except Exception:
+        raise
+
+def bad():
+    try:
+        process()
+    except:
+        # I am a comment, not a statement!
+        raise
+
+def fine():
+    try:
+        process()
+    except Exception:
+        print("initiating rapid unscheduled disassembly of program")
+
+def fine():
+    try:
+        process()
+    except MyException:
+        print("oh no!")
+        raise
+
+def fine():
+    try:
+        process()
+    except Exception:
+        if True:
+            raise
+
+
+def fine():
+    try:
+        process()
+    finally:
+        # I am a comment, not a statement!
+        print("but i am a statement")
+        raise

crates/ruff/src/checkers/ast/mod.rs

@@ -1689,6 +1689,9 @@ where
                 if self.settings.rules.enabled(Rule::RaiseWithinTry) {
                     tryceratops::rules::raise_within_try(self, body, handlers);
                 }
+                if self.settings.rules.enabled(Rule::PointlessRaise) {
+                    tryceratops::rules::pointless_raise(self, handlers);
+                }
                 if self.settings.rules.enabled(Rule::ErrorInsteadOfException) {
                     tryceratops::rules::error_instead_of_exception(self, handlers);
                 }

crates/ruff/src/codes.rs

@@ -677,6 +677,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Tryceratops, "201") => Rule::VerboseRaise,
         (Tryceratops, "300") => Rule::TryConsiderElse,
         (Tryceratops, "301") => Rule::RaiseWithinTry,
+        (Tryceratops, "302") => Rule::PointlessRaise,
         (Tryceratops, "400") => Rule::ErrorInsteadOfException,
         (Tryceratops, "401") => Rule::VerboseLogMessage,
 

crates/ruff/src/registry.rs

@@ -611,6 +611,7 @@ ruff_macros::register_rules!(
     rules::tryceratops::rules::ReraiseNoCause,
     rules::tryceratops::rules::VerboseRaise,
     rules::tryceratops::rules::TryConsiderElse,
+    rules::tryceratops::rules::PointlessRaise,
     rules::tryceratops::rules::RaiseWithinTry,
     rules::tryceratops::rules::ErrorInsteadOfException,
     rules::tryceratops::rules::VerboseLogMessage,

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

@@ -22,6 +22,7 @@ mod tests {
     #[test_case(Rule::VerboseRaise, Path::new("TRY201.py"); "TRY201")]
     #[test_case(Rule::TryConsiderElse, Path::new("TRY300.py"); "TRY300")]
     #[test_case(Rule::RaiseWithinTry , Path::new("TRY301.py"); "TRY301")]
+    #[test_case(Rule::PointlessRaise , Path::new("TRY302.py"); "TRY302")]
     #[test_case(Rule::ErrorInsteadOfException, Path::new("TRY400.py"); "TRY400")]
     #[test_case(Rule::VerboseLogMessage, Path::new("TRY401.py"); "TRY401")]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {

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

@@ -1,4 +1,5 @@
 pub(crate) use error_instead_of_exception::{error_instead_of_exception, ErrorInsteadOfException};
+pub(crate) use pointless_raise::{pointless_raise, PointlessRaise};
 pub(crate) use raise_vanilla_args::{raise_vanilla_args, RaiseVanillaArgs};
 pub(crate) use raise_vanilla_class::{raise_vanilla_class, RaiseVanillaClass};
 pub(crate) use raise_within_try::{raise_within_try, RaiseWithinTry};
@@ -11,6 +12,7 @@ pub(crate) use verbose_log_message::{verbose_log_message, VerboseLogMessage};
 pub(crate) use verbose_raise::{verbose_raise, VerboseRaise};
 
 mod error_instead_of_exception;
+mod pointless_raise;
 mod raise_vanilla_args;
 mod raise_vanilla_class;
 mod raise_within_try;

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

@@ -0,0 +1,58 @@
+
+
+use rustpython_parser::ast::{self, Excepthandler, StmtKind, ExcepthandlerKind};
+
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+
+
+use crate::checkers::ast::Checker;
+
+
+/// ## What it does
+/// Checks for uses of `raise` directly after a `rescue`.
+///
+/// ## Why is this bad?
+/// Catching an error just to reraise it is pointless. Instead, remove error-handling and let the error propogate naturally
+///
+/// ## Example
+/// ```python
+///
+/// def foo():
+///     try:
+///         bar()
+///     except NotImplementedError:
+//          raise
+/// ```
+///
+/// Use instead:
+/// ```python
+///
+/// def foo():
+///     bar()
+/// ```
+#[violation]
+pub struct PointlessRaise;
+
+impl Violation for PointlessRaise {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Remove this exception handler as the error is immediately re-raised")
+    }
+}
+
+/// TRY302
+pub(crate) fn pointless_raise(checker: &mut Checker, handlers: &[Excepthandler]) {
+    for handler in handlers {
+        let ExcepthandlerKind::ExceptHandler(handler) = &handler.node;
+        let body = &handler.body;
+
+        if let Some(stmt) = body.first() {
+            if let StmtKind::Raise(ast::StmtRaise { exc: None, .. }) = &stmt.node {
+                checker
+                    .diagnostics
+                    .push(Diagnostic::new(PointlessRaise, stmt.range()));
+            }
+        }
+    }
+}

crates/ruff/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__pointless-raise_TRY302.py.snap

@@ -0,0 +1,24 @@
+---
+source: crates/ruff/src/rules/tryceratops/mod.rs
+---
+TRY302.py:13:9: TRY302 Remove this exception handler as the error is immediately re-raised
+   |
+13 |         process()
+14 |     except Exception:
+15 |         raise
+   |         ^^^^^ TRY302
+16 | 
+17 | def bad():
+   |
+
+TRY302.py:20:9: TRY302 Remove this exception handler as the error is immediately re-raised
+   |
+20 |     except:
+21 |         # I am a comment, not a statement!
+22 |         raise
+   |         ^^^^^ TRY302
+23 | 
+24 | def fine():
+   |
+
+

crates/ruff/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__raise-within-try_TRY301.py.snap

@@ -1,64 +1,64 @@
 ---
 source: crates/ruff/src/rules/tryceratops/mod.rs
 ---
-TRY301.py:9:13: TRY301 Abstract `raise` to an inner function
+TRY301.py:14:13: TRY301 Abstract `raise` to an inner function
    |
- 9 |         a = process()
-10 |         if not a:
-11 |             raise MyException(a)
+14 |         a = process()
+15 |         if not a:
+16 |             raise MyException(a)
    |             ^^^^^^^^^^^^^^^^^^^^ TRY301
-12 | 
-13 |         raise MyException(a)
+17 | 
+18 |         raise MyException(a)
    |
 
-TRY301.py:11:9: TRY301 Abstract `raise` to an inner function
+TRY301.py:16:9: TRY301 Abstract `raise` to an inner function
    |
-11 |             raise MyException(a)
-12 | 
-13 |         raise MyException(a)
+16 |             raise MyException(a)
+17 | 
+18 |         raise MyException(a)
    |         ^^^^^^^^^^^^^^^^^^^^ TRY301
-14 | 
-15 |         try:
+19 | 
+20 |         try:
    |
 
-TRY301.py:16:17: TRY301 Abstract `raise` to an inner function
+TRY301.py:21:17: TRY301 Abstract `raise` to an inner function
    |
-16 |             b = process()
-17 |             if not b:
-18 |                 raise MyException(b)
+21 |             b = process()
+22 |             if not b:
+23 |                 raise MyException(b)
    |                 ^^^^^^^^^^^^^^^^^^^^ TRY301
-19 |         except Exception:
-20 |             logger.exception("something failed")
+24 |         except Exception:
+25 |             logger.exception("something failed")
    |
 
-TRY301.py:27:13: TRY301 Abstract `raise` to an inner function
+TRY301.py:32:13: TRY301 Abstract `raise` to an inner function
    |
-27 |         a = process()
-28 |         if not a:
-29 |             raise MyException(a)
+32 |         a = process()
+33 |         if not a:
+34 |             raise MyException(a)
    |             ^^^^^^^^^^^^^^^^^^^^ TRY301
-30 | 
-31 |         raise MyException(a)
+35 | 
+36 |         raise MyException(a)
    |
 
-TRY301.py:29:9: TRY301 Abstract `raise` to an inner function
+TRY301.py:34:9: TRY301 Abstract `raise` to an inner function
    |
-29 |             raise MyException(a)
-30 | 
-31 |         raise MyException(a)
+34 |             raise MyException(a)
+35 | 
+36 |         raise MyException(a)
    |         ^^^^^^^^^^^^^^^^^^^^ TRY301
-32 | 
-33 |         try:
+37 | 
+38 |         try:
    |
 
-TRY301.py:34:17: TRY301 Abstract `raise` to an inner function
+TRY301.py:39:17: TRY301 Abstract `raise` to an inner function
    |
-34 |             b = process()
-35 |             if not b:
-36 |                 raise MyException(b)
+39 |             b = process()
+40 |             if not b:
+41 |                 raise MyException(b)
    |                 ^^^^^^^^^^^^^^^^^^^^ TRY301
-37 |         except* Exception:
-38 |             logger.exception("something failed")
+42 |         except* Exception:
+43 |             logger.exception("something failed")
    |