Restrict SIM105 to try blocks with a body of one simple statement (#1948)

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>
This commit is contained in:
Anders Kaseorg 2023-01-18 00:22:22 -05:00 committed by GitHub
parent 51b917cfbf
commit ea4d54a90f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 3 deletions

View File

@ -41,3 +41,21 @@ except ValueError:
pass pass
finally: finally:
print('bar') 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

View File

@ -1384,7 +1384,7 @@ where
} }
if self.settings.rules.enabled(&RuleCode::SIM105) { if self.settings.rules.enabled(&RuleCode::SIM105) {
flake8_simplify::rules::use_contextlib_suppress( flake8_simplify::rules::use_contextlib_suppress(
self, stmt, handlers, orelse, finalbody, self, stmt, body, handlers, orelse, finalbody,
); );
} }
if self.settings.rules.enabled(&RuleCode::SIM107) { if self.settings.rules.enabled(&RuleCode::SIM107) {

View File

@ -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::helpers;
use crate::ast::types::Range; use crate::ast::types::Range;
@ -10,11 +10,29 @@ use crate::violations;
pub fn use_contextlib_suppress( pub fn use_contextlib_suppress(
checker: &mut Checker, checker: &mut Checker,
stmt: &Stmt, stmt: &Stmt,
body: &[Stmt],
handlers: &[Excepthandler], handlers: &[Excepthandler],
orelse: &[Stmt], orelse: &[Stmt],
finalbody: &[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; return;
} }
let handler = &handlers[0]; let handler = &handlers[0];