Skip to content

Commit

Permalink
Restrict SIM105 to try blocks with a body of one simple statement (#1948
Browse files Browse the repository at this point in the history
)

If a `try` block has multiple statements, a compound statement, or
control flow, rewriting it with `contextlib.suppress` would obfuscate
the fact that the exception still short-circuits further statements in
the block.

Fixes #1947.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
  • Loading branch information
andersk committed Jan 18, 2023
1 parent 51b917c commit ea4d54a
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 3 deletions.
18 changes: 18 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM105.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,21 @@ def foo():
pass
finally:
print('bar')

try:
foo()
foo()
except ValueError:
pass

try:
for i in range(3):
foo()
except ValueError:
pass

def bar():
try:
return foo()
except ValueError:
pass
2 changes: 1 addition & 1 deletion src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,7 @@ where
}
if self.settings.rules.enabled(&RuleCode::SIM105) {
flake8_simplify::rules::use_contextlib_suppress(
self, stmt, handlers, orelse, finalbody,
self, stmt, body, handlers, orelse, finalbody,
);
}
if self.settings.rules.enabled(&RuleCode::SIM107) {
Expand Down
22 changes: 20 additions & 2 deletions src/rules/flake8_simplify/rules/use_contextlib_suppress.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Stmt, StmtKind};
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Located, Stmt, StmtKind};

use crate::ast::helpers;
use crate::ast::types::Range;
Expand All @@ -10,11 +10,29 @@ use crate::violations;
pub fn use_contextlib_suppress(
checker: &mut Checker,
stmt: &Stmt,
body: &[Stmt],
handlers: &[Excepthandler],
orelse: &[Stmt],
finalbody: &[Stmt],
) {
if handlers.len() != 1 || !orelse.is_empty() || !finalbody.is_empty() {
if !matches!(
body,
[Located {
node: StmtKind::Delete { .. }
| StmtKind::Assign { .. }
| StmtKind::AugAssign { .. }
| StmtKind::AnnAssign { .. }
| StmtKind::Assert { .. }
| StmtKind::Import { .. }
| StmtKind::ImportFrom { .. }
| StmtKind::Expr { .. }
| StmtKind::Pass,
..
}]
) || handlers.len() != 1
|| !orelse.is_empty()
|| !finalbody.is_empty()
{
return;
}
let handler = &handlers[0];
Expand Down

0 comments on commit ea4d54a

Please sign in to comment.