Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-bugbear] Allow tuples of exceptions (B030) #10437

Merged
merged 11 commits into from Mar 18, 2024
78 changes: 66 additions & 12 deletions crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B030.py
Expand Up @@ -9,62 +9,69 @@

try:
pass
except 1: # error
except 1: # Error
pass

try:
pass
except (1, ValueError): # error
except (1, ValueError): # Error
pass

try:
pass
except (ValueError, (RuntimeError, (KeyError, TypeError))): # error
except (ValueError, (RuntimeError, (KeyError, TypeError))): # Error
pass

try:
pass
except (ValueError, *(RuntimeError, (KeyError, TypeError))): # error
except (ValueError, *(RuntimeError, (KeyError, TypeError))): # Error
pass


try:
pass
except (*a, *(RuntimeError, (KeyError, TypeError))): # error
except (*a, *(RuntimeError, (KeyError, TypeError))): # Error
pass


try:
pass
except* a + (RuntimeError, (KeyError, TypeError)): # Error
pass


try:
pass
except (ValueError, *(RuntimeError, TypeError)): # ok
except (ValueError, *(RuntimeError, TypeError)): # OK
pass

try:
pass
except (ValueError, *[RuntimeError, *(TypeError,)]): # ok
except (ValueError, *[RuntimeError, *(TypeError,)]): # OK
pass


try:
pass
except (*a, *b): # ok
except (*a, *b): # OK
pass


try:
pass
except (*a, *(RuntimeError, TypeError)): # ok
except (*a, *(RuntimeError, TypeError)): # OK
pass


try:
pass
except (*a, *(b, c)): # ok
except (*a, *(b, c)): # OK
pass


try:
pass
except (*a, *(*b, *c)): # ok
except (*a, *(*b, *c)): # OK
pass


Expand All @@ -74,5 +81,52 @@ def what_to_catch():

try:
pass
except what_to_catch(): # ok
except what_to_catch(): # OK
pass


try:
pass
except (a, b) + (c, d): # OK
pass


try:
pass
except* (a, b) + (c, d): # OK
pass


try:
pass
except* (a, (b) + (c)): # OK
pass


try:
pass
except (a, b) + (c, d) + (e, f): # OK
pass


try:
pass
except a + (b, c): # OK
pass


try:
pass
except (ValueError, *(RuntimeError, TypeError), *((ArithmeticError,) + (EOFError,))):
pass
Comment on lines +118 to +121
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@autinerd added this test case



try:
pass
except ((a, b) + (c, d)) + ((e, f) + (g)): # OK
pass

try:
pass
except (a, b) * (c, d): # B030
pass
@@ -1,6 +1,6 @@
use std::collections::VecDeque;

use ruff_python_ast::{self as ast, ExceptHandler, Expr};
use ruff_python_ast::{self as ast, ExceptHandler, Expr, Operator};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
Expand Down Expand Up @@ -44,30 +44,6 @@ impl Violation for ExceptWithNonExceptionClasses {
}
}

/// Given an [`Expr`], flatten any [`Expr::Starred`] expressions.
/// This should leave any unstarred iterables alone (subsequently raising a
/// warning for B029).
fn flatten_starred_iterables(expr: &Expr) -> Vec<&Expr> {
let Expr::Tuple(ast::ExprTuple { elts, .. }) = expr else {
return vec![expr];
};
let mut flattened_exprs: Vec<&Expr> = Vec::with_capacity(elts.len());
let mut exprs_to_process: VecDeque<&Expr> = elts.iter().collect();
while let Some(expr) = exprs_to_process.pop_front() {
match expr {
Expr::Starred(ast::ExprStarred { value, .. }) => match value.as_ref() {
Expr::Tuple(ast::ExprTuple { elts, .. })
| Expr::List(ast::ExprList { elts, .. }) => {
exprs_to_process.append(&mut elts.iter().collect());
}
_ => flattened_exprs.push(value),
},
_ => flattened_exprs.push(expr),
}
}
flattened_exprs
}

/// B030
pub(crate) fn except_with_non_exception_classes(
checker: &mut Checker,
Expand All @@ -78,7 +54,7 @@ pub(crate) fn except_with_non_exception_classes(
let Some(type_) = type_ else {
return;
};
for expr in flatten_starred_iterables(type_) {
for expr in flatten_iterables(type_) {
if !matches!(
expr,
Expr::Subscript(_) | Expr::Attribute(_) | Expr::Name(_) | Expr::Call(_),
Expand All @@ -89,3 +65,61 @@ pub(crate) fn except_with_non_exception_classes(
}
}
}

/// Given an [`Expr`], flatten any [`Expr::Starred`] expressions and any
/// [`Expr::BinOp`] expressions into a flat list of expressions.
///
/// This should leave any unstarred iterables alone (subsequently raising a
/// warning for B029).
fn flatten_iterables(expr: &Expr) -> Vec<&Expr> {
// Unpack the top-level Tuple into queue, otherwise add as-is.
let mut exprs_to_process: VecDeque<&Expr> = match expr {
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().collect(),
_ => vec![expr].into(),
};
let mut flattened_exprs: Vec<&Expr> = Vec::with_capacity(exprs_to_process.len());

while let Some(expr) = exprs_to_process.pop_front() {
match expr {
Expr::Starred(ast::ExprStarred { value, .. }) => match value.as_ref() {
Expr::Tuple(ast::ExprTuple { elts, .. })
| Expr::List(ast::ExprList { elts, .. }) => {
exprs_to_process.append(&mut elts.iter().collect());
}
Expr::BinOp(ast::ExprBinOp {
op: Operator::Add, ..
}) => {
exprs_to_process.push_back(value);
}
_ => flattened_exprs.push(value),
},
Expr::BinOp(ast::ExprBinOp {
left,
right,
op: Operator::Add,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gated these to Operator::Add.

..
}) => {
for expr in [left, right] {
// If left or right are tuples, starred, or binary operators, flatten them.
match expr.as_ref() {
Expr::Tuple(ast::ExprTuple { elts, .. }) => {
exprs_to_process.append(&mut elts.iter().collect());
}
Expr::Starred(ast::ExprStarred { value, .. }) => {
exprs_to_process.push_back(value);
}
Expr::BinOp(ast::ExprBinOp {
op: Operator::Add, ..
}) => {
exprs_to_process.push_back(expr);
}
_ => flattened_exprs.push(expr),
}
}
}
_ => flattened_exprs.push(expr),
}
}

flattened_exprs
}
Expand Up @@ -5,7 +5,7 @@ B030.py:12:8: B030 `except` handlers should only be exception classes or tuples
|
10 | try:
11 | pass
12 | except 1: # error
12 | except 1: # Error
| ^ B030
13 | pass
|
Expand All @@ -14,7 +14,7 @@ B030.py:17:9: B030 `except` handlers should only be exception classes or tuples
|
15 | try:
16 | pass
17 | except (1, ValueError): # error
17 | except (1, ValueError): # Error
| ^ B030
18 | pass
|
Expand All @@ -23,7 +23,7 @@ B030.py:22:21: B030 `except` handlers should only be exception classes or tuples
|
20 | try:
21 | pass
22 | except (ValueError, (RuntimeError, (KeyError, TypeError))): # error
22 | except (ValueError, (RuntimeError, (KeyError, TypeError))): # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B030
23 | pass
|
Expand All @@ -32,7 +32,7 @@ B030.py:27:37: B030 `except` handlers should only be exception classes or tuples
|
25 | try:
26 | pass
27 | except (ValueError, *(RuntimeError, (KeyError, TypeError))): # error
27 | except (ValueError, *(RuntimeError, (KeyError, TypeError))): # Error
| ^^^^^^^^^^^^^^^^^^^^^ B030
28 | pass
|
Expand All @@ -41,9 +41,25 @@ B030.py:33:29: B030 `except` handlers should only be exception classes or tuples
|
31 | try:
32 | pass
33 | except (*a, *(RuntimeError, (KeyError, TypeError))): # error
33 | except (*a, *(RuntimeError, (KeyError, TypeError))): # Error
| ^^^^^^^^^^^^^^^^^^^^^ B030
34 | pass
|

B030.py:39:28: B030 `except` handlers should only be exception classes or tuples of exception classes
|
37 | try:
38 | pass
39 | except* a + (RuntimeError, (KeyError, TypeError)): # Error
| ^^^^^^^^^^^^^^^^^^^^^ B030
40 | pass
|

B030.py:131:8: B030 `except` handlers should only be exception classes or tuples of exception classes
|
129 | try:
130 | pass
131 | except (a, b) * (c, d): # B030
| ^^^^^^^^^^^^^^^ B030
132 | pass
|