Avoid flagging bare exception issues when exception is re-raised (#1124)

This commit is contained in:
Charlie Marsh 2022-12-07 10:37:08 -05:00 committed by GitHub
parent 9c6e8c7644
commit 9579faffa8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 145 additions and 110 deletions

View file

@ -631,7 +631,7 @@ For more, see [flake8-blind-except](https://pypi.org/project/flake8-blind-except
| Code | Name | Message | Fix | | Code | Name | Message | Fix |
| ---- | ---- | ------- | --- | | ---- | ---- | ------- | --- |
| BLE001 | BlindExcept | Blind except Exception: statement | | | BLE001 | BlindExcept | Do not catch blind exception: `Exception` | |
### flake8-boolean-trap (FBT) ### flake8-boolean-trap (FBT)

View file

@ -53,3 +53,11 @@ try:
raise e raise e
except Exception: except Exception:
pass pass
try:
pass
except Exception as e:
raise bad
except BaseException:
pass

View file

@ -1077,9 +1077,6 @@ where
if self.settings.enabled.contains(&CheckCode::B013) { if self.settings.enabled.contains(&CheckCode::B013) {
flake8_bugbear::plugins::redundant_tuple_in_exception_handler(self, handlers); flake8_bugbear::plugins::redundant_tuple_in_exception_handler(self, handlers);
} }
if self.settings.enabled.contains(&CheckCode::BLE001) {
flake8_blind_except::plugins::blind_except(self, handlers);
}
} }
StmtKind::Assign { targets, value, .. } => { StmtKind::Assign { targets, value, .. } => {
if self.settings.enabled.contains(&CheckCode::E731) { if self.settings.enabled.contains(&CheckCode::E731) {
@ -2347,16 +2344,25 @@ where
ExcepthandlerKind::ExceptHandler { ExcepthandlerKind::ExceptHandler {
type_, name, body, .. type_, name, body, ..
} => { } => {
if self.settings.enabled.contains(&CheckCode::E722) && type_.is_none() { if self.settings.enabled.contains(&CheckCode::E722) {
self.add_check(Check::new( if let Some(check) = pycodestyle::checks::do_not_use_bare_except(
CheckKind::DoNotUseBareExcept, type_.as_deref(),
body,
Range::from_located(excepthandler), Range::from_located(excepthandler),
)); ) {
self.add_check(check);
}
} }
if self.settings.enabled.contains(&CheckCode::B904) { if self.settings.enabled.contains(&CheckCode::B904) {
{
flake8_bugbear::plugins::raise_without_from_inside_except(self, body); flake8_bugbear::plugins::raise_without_from_inside_except(self, body);
} }
if self.settings.enabled.contains(&CheckCode::BLE001) {
flake8_blind_except::plugins::blind_except(
self,
type_.as_deref(),
name.as_ref().map(String::as_str),
body,
);
} }
match name { match name {
Some(name) => { Some(name) => {

View file

@ -631,7 +631,7 @@ pub enum CheckKind {
BuiltinArgumentShadowing(String), BuiltinArgumentShadowing(String),
BuiltinAttributeShadowing(String), BuiltinAttributeShadowing(String),
// flake8-blind-except // flake8-blind-except
BlindExcept, BlindExcept(String),
// flake8-bugbear // flake8-bugbear
AbstractBaseClassWithoutAbstractMethod(String), AbstractBaseClassWithoutAbstractMethod(String),
AssignmentToOsEnviron, AssignmentToOsEnviron,
@ -1045,7 +1045,7 @@ impl CheckCode {
CheckCode::YTT302 => CheckKind::SysVersionCmpStr10, CheckCode::YTT302 => CheckKind::SysVersionCmpStr10,
CheckCode::YTT303 => CheckKind::SysVersionSlice1Referenced, CheckCode::YTT303 => CheckKind::SysVersionSlice1Referenced,
// flake8-blind-except // flake8-blind-except
CheckCode::BLE001 => CheckKind::BlindExcept, CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
// pyupgrade // pyupgrade
CheckCode::UP001 => CheckKind::UselessMetaclassType, CheckCode::UP001 => CheckKind::UselessMetaclassType,
CheckCode::UP003 => CheckKind::TypeOfPrimitive(Primitive::Str), CheckCode::UP003 => CheckKind::TypeOfPrimitive(Primitive::Str),
@ -1529,7 +1529,7 @@ impl CheckKind {
CheckKind::UselessContextlibSuppress => &CheckCode::B022, CheckKind::UselessContextlibSuppress => &CheckCode::B022,
CheckKind::UselessExpression => &CheckCode::B018, CheckKind::UselessExpression => &CheckCode::B018,
// flake8-blind-except // flake8-blind-except
CheckKind::BlindExcept => &CheckCode::BLE001, CheckKind::BlindExcept(_) => &CheckCode::BLE001,
// flake8-comprehensions // flake8-comprehensions
CheckKind::UnnecessaryGeneratorList => &CheckCode::C400, CheckKind::UnnecessaryGeneratorList => &CheckCode::C400,
CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401, CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401,
@ -2460,7 +2460,7 @@ impl CheckKind {
format!("Possible hardcoded password: `\"{string}\"`") format!("Possible hardcoded password: `\"{string}\"`")
} }
// flake8-blind-except // flake8-blind-except
CheckKind::BlindExcept => "Blind except Exception: statement".to_string(), CheckKind::BlindExcept(name) => format!("Do not catch blind exception: `{name}`"),
// mccabe // mccabe
CheckKind::FunctionIsTooComplex(name, complexity) => { CheckKind::FunctionIsTooComplex(name, complexity) => {
format!("`{name}` is too complex ({complexity})") format!("`{name}` is too complex ({complexity})")

View file

@ -1,21 +1,42 @@
use rustpython_ast::{Excepthandler, ExcepthandlerKind, ExprKind}; use rustpython_ast::{Expr, ExprKind, Stmt, StmtKind};
use crate::ast::types::Range; use crate::ast::types::Range;
use crate::check_ast::Checker; use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind}; use crate::checks::{Check, CheckKind};
pub fn blind_except(checker: &mut Checker, handlers: &[Excepthandler]) { /// BLE001
for handler in handlers { pub fn blind_except(
let ExcepthandlerKind::ExceptHandler { type_: Some(type_), .. } = &handler.node else { checker: &mut Checker,
continue; type_: Option<&Expr>,
name: Option<&str>,
body: &[Stmt],
) {
let Some(type_) = type_ else {
return;
}; };
let ExprKind::Name { id, .. } = &type_.node else { let ExprKind::Name { id, .. } = &type_.node else {
continue; return;
}; };
for exception in ["BaseException", "Exception"] { for exception in ["BaseException", "Exception"] {
if id == exception { if id == exception && checker.is_builtin(exception) {
// If the exception is re-raised, don't flag an error.
if !body.iter().any(|stmt| {
if let StmtKind::Raise { exc, .. } = &stmt.node {
if let Some(exc) = exc {
if let ExprKind::Name { id, .. } = &exc.node {
name.map_or(false, |name| name == id)
} else {
false
}
} else {
true
}
} else {
false
}
}) {
checker.add_check(Check::new( checker.add_check(Check::new(
CheckKind::BlindExcept, CheckKind::BlindExcept(id.to_string()),
Range::from_located(type_), Range::from_located(type_),
)); ));
} }

View file

@ -2,31 +2,8 @@
source: src/flake8_blind_except/mod.rs source: src/flake8_blind_except/mod.rs
expression: checks expression: checks
--- ---
- kind: BlindExcept - kind:
location: BlindExcept: BaseException
row: 5
column: 7
end_location:
row: 5
column: 16
fix: ~
- kind: BlindExcept
location:
row: 13
column: 7
end_location:
row: 13
column: 20
fix: ~
- kind: BlindExcept
location:
row: 23
column: 7
end_location:
row: 23
column: 16
fix: ~
- kind: BlindExcept
location: location:
row: 25 row: 25
column: 7 column: 7
@ -34,7 +11,8 @@ expression: checks
row: 25 row: 25
column: 20 column: 20
fix: ~ fix: ~
- kind: BlindExcept - kind:
BlindExcept: Exception
location: location:
row: 31 row: 31
column: 7 column: 7
@ -42,15 +20,8 @@ expression: checks
row: 31 row: 31
column: 16 column: 16
fix: ~ fix: ~
- kind: BlindExcept - kind:
location: BlindExcept: Exception
row: 36
column: 11
end_location:
row: 36
column: 24
fix: ~
- kind: BlindExcept
location: location:
row: 42 row: 42
column: 7 column: 7
@ -58,7 +29,8 @@ expression: checks
row: 42 row: 42
column: 16 column: 16
fix: ~ fix: ~
- kind: BlindExcept - kind:
BlindExcept: BaseException
location: location:
row: 45 row: 45
column: 11 column: 11
@ -66,15 +38,8 @@ expression: checks
row: 45 row: 45
column: 24 column: 24
fix: ~ fix: ~
- kind: BlindExcept - kind:
location: BlindExcept: Exception
row: 52
column: 11
end_location:
row: 52
column: 24
fix: ~
- kind: BlindExcept
location: location:
row: 54 row: 54
column: 7 column: 7
@ -82,4 +47,22 @@ expression: checks
row: 54 row: 54
column: 16 column: 16
fix: ~ fix: ~
- kind:
BlindExcept: Exception
location:
row: 60
column: 7
end_location:
row: 60
column: 16
fix: ~
- kind:
BlindExcept: BaseException
location:
row: 62
column: 7
end_location:
row: 62
column: 20
fix: ~

View file

@ -1,11 +1,65 @@
use itertools::izip; use itertools::izip;
use rustpython_ast::Location; use rustpython_ast::{Location, Stmt, StmtKind};
use rustpython_parser::ast::{Cmpop, Expr, ExprKind}; use rustpython_parser::ast::{Cmpop, Expr, ExprKind};
use crate::ast::types::Range; use crate::ast::types::Range;
use crate::checks::{Check, CheckKind}; use crate::checks::{Check, CheckKind};
use crate::source_code_locator::SourceCodeLocator; use crate::source_code_locator::SourceCodeLocator;
/// E721
pub fn type_comparison(ops: &[Cmpop], comparators: &[Expr], location: Range) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
for (op, right) in izip!(ops, comparators) {
if !matches!(op, Cmpop::Is | Cmpop::IsNot | Cmpop::Eq | Cmpop::NotEq) {
continue;
}
match &right.node {
ExprKind::Call { func, args, .. } => {
if let ExprKind::Name { id, .. } = &func.node {
// Ex) type(False)
if id == "type" {
if let Some(arg) = args.first() {
// Allow comparison for types which are not obvious.
if !matches!(arg.node, ExprKind::Name { .. }) {
checks.push(Check::new(CheckKind::TypeComparison, location));
}
}
}
}
}
ExprKind::Attribute { value, .. } => {
if let ExprKind::Name { id, .. } = &value.node {
// Ex) types.IntType
if id == "types" {
checks.push(Check::new(CheckKind::TypeComparison, location));
}
}
}
_ => {}
}
}
checks
}
/// E722
pub fn do_not_use_bare_except(
type_: Option<&Expr>,
body: &[Stmt],
location: Range,
) -> Option<Check> {
if type_.is_none()
&& !body
.iter()
.any(|stmt| matches!(stmt.node, StmtKind::Raise { exc: None, .. }))
{
Some(Check::new(CheckKind::DoNotUseBareExcept, location))
} else {
None
}
}
fn is_ambiguous_name(name: &str) -> bool { fn is_ambiguous_name(name: &str) -> bool {
name == "l" || name == "I" || name == "O" name == "l" || name == "I" || name == "O"
} }
@ -46,43 +100,6 @@ pub fn ambiguous_function_name(name: &str, location: Range) -> Option<Check> {
} }
} }
/// E721
pub fn type_comparison(ops: &[Cmpop], comparators: &[Expr], location: Range) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
for (op, right) in izip!(ops, comparators) {
if !matches!(op, Cmpop::Is | Cmpop::IsNot | Cmpop::Eq | Cmpop::NotEq) {
continue;
}
match &right.node {
ExprKind::Call { func, args, .. } => {
if let ExprKind::Name { id, .. } = &func.node {
// Ex) type(False)
if id == "type" {
if let Some(arg) = args.first() {
// Allow comparison for types which are not obvious.
if !matches!(arg.node, ExprKind::Name { .. }) {
checks.push(Check::new(CheckKind::TypeComparison, location));
}
}
}
}
}
ExprKind::Attribute { value, .. } => {
if let ExprKind::Name { id, .. } = &value.node {
// Ex) types.IntType
if id == "types" {
checks.push(Check::new(CheckKind::TypeComparison, location));
}
}
}
_ => {}
}
}
checks
}
// See: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals // See: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals
const VALID_ESCAPE_SEQUENCES: &[char; 23] = &[ const VALID_ESCAPE_SEQUENCES: &[char; 23] = &[
'\n', '\\', '\'', '"', 'a', 'b', 'f', 'n', 'r', 't', 'v', '0', '1', '2', '3', '4', '5', '6', '\n', '\\', '\'', '"', 'a', 'b', 'f', 'n', 'r', 't', 'v', '0', '1', '2', '3', '4', '5', '6',