Improve error messages for except* (B025, B029, B030, B904) #14791 (#…

…14815) Improves error message for [except*](https://peps.python.org/pep-0654/) (Rules: B025, B029, B030, B904) Example python snippet: ```python try: a = 1 except* ValueError: a = 2 except* ValueError: a = 2 try: pass except* (): pass try: pass except* 1: # error pass try: raise ValueError except* ValueError: raise UserWarning ``` Error messages Before: ``` $ ruff check --select=B foo.py foo.py:6:9: B025 try-except block with duplicate exception `ValueError` foo.py:11:1: B029 Using `except ():` with an empty tuple does not catch anything; add exceptions to handle foo.py:16:9: B030 `except` handlers should only be exception classes or tuples of exception classes foo.py:22:5: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling Found 4 errors. ``` After: ``` $ ruff check --select=B foo.py foo.py:6:9: B025 try-except* block with duplicate exception `ValueError` foo.py:11:1: B029 Using `except* ():` with an empty tuple does not catch anything; add exceptions to handle foo.py:16:9: B030 `except*` handlers should only be exception classes or tuples of exception classes foo.py:22:5: B904 Within an `except*` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling Found 4 errors. ``` Closes #14791 --------- Co-authored-by: Micha Reiser <[email protected]>
astral-sh · Dec 8, 2024 · 8540209 · 8540209
1 parent 269e47b
commit 8540209
Show file tree

Hide file tree

Showing 12 changed files with 312 additions and 24 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B025.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B025.py
@@ -1,6 +1,6 @@
 """
 Should emit:
-B025 - on lines 15, 22, 31
+B025 - on lines 15, 22, 31, 40, 47, 56
 """
 
 import pickle
@@ -36,3 +36,28 @@
     a = 2
 except (OSError, TypeError):
     a = 2
+
+try:
+    a = 1
+except* ValueError:
+    a = 2
+except* ValueError:
+    a = 2
+
+try:
+    a = 1
+except* pickle.PickleError:
+    a = 2
+except* ValueError:
+    a = 2
+except* pickle.PickleError:
+    a = 2
+
+try:
+    a = 1
+except* (ValueError, TypeError):
+    a = 2
+except* ValueError:
+    a = 2
+except* (OSError, TypeError):
+    a = 2
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B029.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B029.py
@@ -1,6 +1,6 @@
 """
 Should emit:
-B029 - on lines 8 and 13
+B029 - on lines 8, 13, 18 and 23
 """
 
 try:
@@ -11,4 +11,14 @@
 try:
     pass
 except () as e:
-    pass
+    pass
+
+try:
+    pass
+except* ():
+    pass
+
+try:
+    pass
+except* () as e:
+    pass
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B030.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B030.py
@@ -130,3 +130,23 @@ def what_to_catch():
     pass
 except (a, b) * (c, d):  # B030
     pass
+
+try:
+    pass
+except* 1:  # Error
+    pass
+
+try:
+    pass
+except* (1, ValueError):  # Error
+    pass
+
+try:
+    pass
+except* (ValueError, (RuntimeError, (KeyError, TypeError))):  # Error
+    pass
+
+try:
+    pass
+except* (a, b) * (c, d):  # B030
+    pass
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B904.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B904.py
@@ -1,6 +1,6 @@
 """
 Should emit:
-B904 - on lines 10, 11, 16, 62, and 64
+B904 - on lines 10, 11, 16, 62, 64, 79, 81, 87, 88, 93 and 97
 """
 
 try:
@@ -71,3 +71,29 @@ def context_switch():
     match 0:
         case 0:
             raise RuntimeError("boom!")
+
+try:
+    ...
+except* Exception as e:
+    if ...:
+        raise RuntimeError("boom!")
+    else:
+        raise RuntimeError("bang!")
+
+try:
+    raise ValueError
+except* ValueError:
+    if "abc":
+        raise TypeError
+    raise UserWarning
+except* AssertionError:
+    raise  # Bare `raise` should not be an error
+except* Exception as err:
+    assert err
+    raise Exception("No cause here...")
+except* BaseException as err:
+    raise err
+except* BaseException as err:
+    raise some_other_err
+finally:
+    raise Exception("Nothing to chain from, so no warning here")
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs
@@ -42,13 +42,18 @@ use crate::registry::Rule;
 #[derive(ViolationMetadata)]
 pub(crate) struct DuplicateTryBlockException {
     name: String,
+    is_star: bool,
 }
 
 impl Violation for DuplicateTryBlockException {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let DuplicateTryBlockException { name } = self;
-        format!("try-except block with duplicate exception `{name}`")
+        let DuplicateTryBlockException { name, is_star } = self;
+        if *is_star {
+            format!("try-except* block with duplicate exception `{name}`")
+        } else {
+            format!("try-except block with duplicate exception `{name}`")
+        }
     }
 }
 
@@ -207,9 +212,15 @@ pub(crate) fn duplicate_exceptions(checker: &mut Checker, handlers: &[ExceptHand
     if checker.enabled(Rule::DuplicateTryBlockException) {
         for (name, exprs) in duplicates {
             for expr in exprs {
+                let is_star = checker
+                    .semantic()
+                    .current_statement()
+                    .as_try_stmt()
+                    .is_some_and(|try_stmt| try_stmt.is_star);
                 checker.diagnostics.push(Diagnostic::new(
                     DuplicateTryBlockException {
                         name: name.segments().join("."),
+                        is_star,
                     },
                     expr.range(),
                 ));

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs
@@ -34,13 +34,18 @@ use crate::checkers::ast::Checker;
 /// ## References
 /// - [Python documentation: `except` clause](https://docs.python.org/3/reference/compound_stmts.html#except-clause)
 #[derive(ViolationMetadata)]
-pub(crate) struct ExceptWithEmptyTuple;
+pub(crate) struct ExceptWithEmptyTuple {
+    is_star: bool,
+}
 
 impl Violation for ExceptWithEmptyTuple {
     #[derive_message_formats]
     fn message(&self) -> String {
-        "Using `except ():` with an empty tuple does not catch anything; add exceptions to handle"
-            .to_string()
+        if self.is_star {
+            "Using `except* ():` with an empty tuple does not catch anything; add exceptions to handle".to_string()
+        } else {
+            "Using `except ():` with an empty tuple does not catch anything; add exceptions to handle".to_string()
+        }
     }
 }
 
@@ -54,9 +59,15 @@ pub(crate) fn except_with_empty_tuple(checker: &mut Checker, except_handler: &Ex
     let Expr::Tuple(ast::ExprTuple { elts, .. }) = type_.as_ref() else {
         return;
     };
+
     if elts.is_empty() {
+        let is_star = checker
+            .semantic()
+            .current_statement()
+            .as_try_stmt()
+            .is_some_and(|try_stmt| try_stmt.is_star);
         checker.diagnostics.push(Diagnostic::new(
-            ExceptWithEmptyTuple,
+            ExceptWithEmptyTuple { is_star },
             except_handler.range(),
         ));
     }

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/except_with_non_exception_classes.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/except_with_non_exception_classes.rs
@@ -35,13 +35,20 @@ use crate::checkers::ast::Checker;
 /// - [Python documentation: `except` clause](https://docs.python.org/3/reference/compound_stmts.html#except-clause)
 /// - [Python documentation: Built-in Exceptions](https://docs.python.org/3/library/exceptions.html#built-in-exceptions)
 #[derive(ViolationMetadata)]
-pub(crate) struct ExceptWithNonExceptionClasses;
+pub(crate) struct ExceptWithNonExceptionClasses {
+    is_star: bool,
+}
 
 impl Violation for ExceptWithNonExceptionClasses {
     #[derive_message_formats]
     fn message(&self) -> String {
-        "`except` handlers should only be exception classes or tuples of exception classes"
-            .to_string()
+        if self.is_star {
+            "`except*` handlers should only be exception classes or tuples of exception classes"
+                .to_string()
+        } else {
+            "`except` handlers should only be exception classes or tuples of exception classes"
+                .to_string()
+        }
     }
 }
 
@@ -60,9 +67,15 @@ pub(crate) fn except_with_non_exception_classes(
             expr,
             Expr::Subscript(_) | Expr::Attribute(_) | Expr::Name(_) | Expr::Call(_),
         ) {
-            checker
-                .diagnostics
-                .push(Diagnostic::new(ExceptWithNonExceptionClasses, expr.range()));
+            let is_star = checker
+                .semantic()
+                .current_statement()
+                .as_try_stmt()
+                .is_some_and(|try_stmt| try_stmt.is_star);
+            checker.diagnostics.push(Diagnostic::new(
+                ExceptWithNonExceptionClasses { is_star },
+                expr.range(),
+            ));
         }
     }
 }

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs
@@ -47,14 +47,22 @@ use crate::checkers::ast::Checker;
 /// ## References
 /// - [Python documentation: `raise` statement](https://docs.python.org/3/reference/simple_stmts.html#the-raise-statement)
 #[derive(ViolationMetadata)]
-pub(crate) struct RaiseWithoutFromInsideExcept;
+pub(crate) struct RaiseWithoutFromInsideExcept {
+    is_star: bool,
+}
 
 impl Violation for RaiseWithoutFromInsideExcept {
     #[derive_message_formats]
     fn message(&self) -> String {
-        "Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... \
-             from None` to distinguish them from errors in exception handling"
-            .to_string()
+        if self.is_star {
+            "Within an `except*` clause, raise exceptions with `raise ... from err` or `raise ... \
+                 from None` to distinguish them from errors in exception handling"
+                .to_string()
+        } else {
+            "Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... \
+                 from None` to distinguish them from errors in exception handling"
+                .to_string()
+        }
     }
 }
 
@@ -92,9 +100,16 @@ pub(crate) fn raise_without_from_inside_except(
                     }
                 }
 
-                checker
-                    .diagnostics
-                    .push(Diagnostic::new(RaiseWithoutFromInsideExcept, range));
+                let is_star = checker
+                    .semantic()
+                    .current_statement()
+                    .as_try_stmt()
+                    .is_some_and(|try_stmt| try_stmt.is_star);
+
+                checker.diagnostics.push(Diagnostic::new(
+                    RaiseWithoutFromInsideExcept { is_star },
+                    range,
+                ));
             }
         }
     }

diff --git a/...les/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B025_B025.py.snap b/...les/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B025_B025.py.snap
@@ -38,3 +38,40 @@ B025.py:37:18: B025 try-except block with duplicate exception `TypeError`
    |                  ^^^^^^^^^ B025
 38 |     a = 2
    |
+
+B025.py:44:9: B025 try-except* block with duplicate exception `ValueError`
+   |
+42 | except* ValueError:
+43 |     a = 2
+44 | except* ValueError:
+   |         ^^^^^^^^^^ B025
+45 |     a = 2
+   |
+
+B025.py:53:9: B025 try-except* block with duplicate exception `pickle.PickleError`
+   |
+51 | except* ValueError:
+52 |     a = 2
+53 | except* pickle.PickleError:
+   |         ^^^^^^^^^^^^^^^^^^ B025
+54 |     a = 2
+   |
+
+B025.py:60:9: B025 try-except* block with duplicate exception `ValueError`
+   |
+58 | except* (ValueError, TypeError):
+59 |     a = 2
+60 | except* ValueError:
+   |         ^^^^^^^^^^ B025
+61 |     a = 2
+62 | except* (OSError, TypeError):
+   |
+
+B025.py:62:19: B025 try-except* block with duplicate exception `TypeError`
+   |
+60 | except* ValueError:
+61 |     a = 2
+62 | except* (OSError, TypeError):
+   |                   ^^^^^^^^^ B025
+63 |     a = 2
+   |
diff --git a/...les/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B029_B029.py.snap b/...les/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B029_B029.py.snap
@@ -20,4 +20,26 @@ B029.py:13:1: B029 Using `except ():` with an empty tuple does not catch anythin
 13 | / except () as e:
 14 | |     pass
    | |________^ B029
+15 |   
+16 |   try:
+   |
+
+B029.py:18:1: B029 Using `except* ():` with an empty tuple does not catch anything; add exceptions to handle
+   |
+16 |   try:
+17 |       pass
+18 | / except* ():
+19 | |     pass
+   | |________^ B029
+20 |   
+21 |   try:
+   |
+
+B029.py:23:1: B029 Using `except* ():` with an empty tuple does not catch anything; add exceptions to handle
+   |
+21 |   try:
+22 |       pass
+23 | / except* () as e:
+24 | |     pass
+   | |________^ B029
    |