From ee42413e107965af26b4b2b19ad7903155e981ac Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 10 Oct 2022 13:18:28 -0400 Subject: [PATCH] Enable autofix for B014 --- src/checks.rs | 15 +++-- src/plugins/duplicate_exceptions.rs | 50 +++++++++++++--- src/snapshots/ruff__linter__tests__b014.snap | 63 ++++++++++++++------ 3 files changed, 97 insertions(+), 31 deletions(-) diff --git a/src/checks.rs b/src/checks.rs index 6feba6b1de..b0ca898458 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -220,7 +220,7 @@ pub enum CheckKind { BuiltinAttributeShadowing(String), // flake8-bugbear DoNotAssertFalse, - DuplicateHandlerException(String), + DuplicateHandlerException(Vec), DuplicateTryBlockException(String), // flake8-comprehensions UnnecessaryGeneratorList, @@ -317,7 +317,7 @@ impl CheckCode { CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()), // flake8-bugbear CheckCode::B011 => CheckKind::DoNotAssertFalse, - CheckCode::B014 => CheckKind::DuplicateHandlerException("Exception".to_string()), + CheckCode::B014 => CheckKind::DuplicateHandlerException(vec!["ValueError".to_string()]), CheckCode::B025 => CheckKind::DuplicateTryBlockException("Exception".to_string()), // flake8-comprehensions CheckCode::C400 => CheckKind::UnnecessaryGeneratorList, @@ -594,8 +594,14 @@ impl CheckKind { "Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`" .to_string() } - CheckKind::DuplicateHandlerException(name) => { - format!("Exception handler with duplicate exception `{name}`") + CheckKind::DuplicateHandlerException(names) => { + if names.len() == 1 { + let name = &names[0]; + format!("Exception handler with duplicate exception: `{name}") + } else { + let names = names.iter().map(|name| format!("`{name}`")).join(", "); + format!("Exception handler with duplicate exceptions: {names}") + } } CheckKind::DuplicateTryBlockException(name) => { format!("try-except block with duplicate exception `{name}`") @@ -704,6 +710,7 @@ impl CheckKind { self, CheckKind::DeprecatedUnittestAlias(_, _) | CheckKind::DoNotAssertFalse + | CheckKind::DuplicateHandlerException(_) | CheckKind::PPrintFound | CheckKind::PrintFound | CheckKind::SuperCallWithParameters diff --git a/src/plugins/duplicate_exceptions.rs b/src/plugins/duplicate_exceptions.rs index 93ec8a628b..467964f32e 100644 --- a/src/plugins/duplicate_exceptions.rs +++ b/src/plugins/duplicate_exceptions.rs @@ -1,37 +1,69 @@ use std::collections::BTreeSet; use itertools::Itertools; -use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt}; +use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Stmt}; use crate::ast::helpers; use crate::ast::types::{CheckLocator, Range}; +use crate::autofix::fixer; use crate::check_ast::Checker; -use crate::checks::{Check, CheckCode, CheckKind}; +use crate::checks::{Check, CheckCode, CheckKind, Fix}; +use crate::code_gen::SourceGenerator; + +fn type_pattern(elts: Vec<&Expr>) -> Expr { + Expr::new( + Default::default(), + Default::default(), + ExprKind::Tuple { + elts: elts.into_iter().cloned().collect(), + ctx: ExprContext::Load, + }, + ) +} pub fn duplicate_handler_exceptions( checker: &mut Checker, - stmt: &Stmt, + expr: &Expr, elts: &Vec, ) -> BTreeSet { let mut seen: BTreeSet = Default::default(); let mut duplicates: BTreeSet = Default::default(); + let mut unique_elts: Vec<&Expr> = Default::default(); for type_ in elts { if let Some(name) = helpers::compose_call_path(type_) { if seen.contains(&name) { duplicates.insert(name); } else { seen.insert(name); + unique_elts.push(type_); } } } if checker.settings.enabled.contains(&CheckCode::B014) { // TODO(charlie): Handle "BaseException" and redundant exception aliases. - for duplicate in duplicates.into_iter().sorted() { - checker.add_check(Check::new( - CheckKind::DuplicateHandlerException(duplicate), - checker.locate_check(Range::from_located(stmt)), - )); + if !duplicates.is_empty() { + let mut check = Check::new( + CheckKind::DuplicateHandlerException( + duplicates.into_iter().sorted().collect::>(), + ), + checker.locate_check(Range::from_located(expr)), + ); + if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { + // TODO(charlie): If we have a single element, remove the tuple. + let mut generator = SourceGenerator::new(); + if let Ok(()) = generator.unparse_expr(&type_pattern(unique_elts), 0) { + if let Ok(content) = generator.generate() { + check.amend(Fix { + content, + location: expr.location, + end_location: expr.end_location, + applied: false, + }) + } + } + } + checker.add_check(check); } } @@ -56,7 +88,7 @@ pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Exce } } ExprKind::Tuple { elts, .. } => { - for name in duplicate_handler_exceptions(checker, stmt, elts) { + for name in duplicate_handler_exceptions(checker, type_, elts) { if seen.contains(&name) { duplicates.insert(name); } else { diff --git a/src/snapshots/ruff__linter__tests__b014.snap b/src/snapshots/ruff__linter__tests__b014.snap index 88fe034dac..02f9ff2de8 100644 --- a/src/snapshots/ruff__linter__tests__b014.snap +++ b/src/snapshots/ruff__linter__tests__b014.snap @@ -3,30 +3,57 @@ source: src/linter.rs expression: checks --- - kind: - DuplicateHandlerException: OSError + DuplicateHandlerException: + - OSError location: - row: 15 - column: 1 + row: 17 + column: 9 end_location: - row: 22 - column: 1 - fix: ~ + row: 17 + column: 25 + fix: + content: "OSError," + location: + row: 17 + column: 9 + end_location: + row: 17 + column: 25 + applied: false - kind: - DuplicateHandlerException: MyError + DuplicateHandlerException: + - MyError location: - row: 26 - column: 1 + row: 28 + column: 9 end_location: - row: 33 - column: 1 - fix: ~ + row: 28 + column: 25 + fix: + content: "MyError," + location: + row: 28 + column: 9 + end_location: + row: 28 + column: 25 + applied: false - kind: - DuplicateHandlerException: re.error + DuplicateHandlerException: + - re.error location: - row: 47 - column: 1 + row: 49 + column: 9 end_location: - row: 54 - column: 1 - fix: ~ + row: 49 + column: 27 + fix: + content: "re.error," + location: + row: 49 + column: 9 + end_location: + row: 49 + column: 27 + applied: false