From b9e92affb11c091223000968dcf75e8f6445eaa3 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 3 Jan 2023 08:11:18 -0500 Subject: [PATCH] Avoid silently dropping code generator errors (#1598) --- src/flake8_bugbear/plugins/assert_false.rs | 18 ++++--- .../plugins/duplicate_exceptions.rs | 16 ++++--- .../plugins/getattr_with_constant.rs | 16 ++++--- .../redundant_tuple_in_exception_handler.rs | 16 ++++--- src/pyflakes/plugins/strings.rs | 17 ++++--- ...convert_named_tuple_functional_to_class.rs | 7 +-- .../plugins/use_pep604_annotation.rs | 48 +++++++++++-------- 7 files changed, 85 insertions(+), 53 deletions(-) diff --git a/src/flake8_bugbear/plugins/assert_false.rs b/src/flake8_bugbear/plugins/assert_false.rs index 47f9202bed..7c97df7b83 100644 --- a/src/flake8_bugbear/plugins/assert_false.rs +++ b/src/flake8_bugbear/plugins/assert_false.rs @@ -1,3 +1,4 @@ +use log::error; use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location, Stmt, StmtKind}; use crate::ast::types::Range; @@ -53,13 +54,16 @@ pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: Option checker.style.line_ending(), ); generator.unparse_stmt(&assertion_error(msg)); - if let Ok(content) = generator.generate() { - check.amend(Fix::replacement( - content, - stmt.location, - stmt.end_location.unwrap(), - )); - } + match generator.generate() { + Ok(content) => { + check.amend(Fix::replacement( + content, + stmt.location, + stmt.end_location.unwrap(), + )); + } + Err(e) => error!("Failed to rewrite `assert False`: {e}"), + }; } checker.add_check(check); } diff --git a/src/flake8_bugbear/plugins/duplicate_exceptions.rs b/src/flake8_bugbear/plugins/duplicate_exceptions.rs index bb4257dbd6..9bcc015b50 100644 --- a/src/flake8_bugbear/plugins/duplicate_exceptions.rs +++ b/src/flake8_bugbear/plugins/duplicate_exceptions.rs @@ -1,4 +1,5 @@ use itertools::Itertools; +use log::error; use rustc_hash::{FxHashMap, FxHashSet}; use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Location}; @@ -64,12 +65,15 @@ fn duplicate_handler_exceptions<'a>( } else { generator.unparse_expr(&type_pattern(unique_elts), 0); } - if let Ok(content) = generator.generate() { - check.amend(Fix::replacement( - content, - expr.location, - expr.end_location.unwrap(), - )); + match generator.generate() { + Ok(content) => { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + Err(e) => error!("Failed to remove duplicate exceptions: {e}"), } } checker.add_check(check); diff --git a/src/flake8_bugbear/plugins/getattr_with_constant.rs b/src/flake8_bugbear/plugins/getattr_with_constant.rs index 1aea139c83..88984e4e8b 100644 --- a/src/flake8_bugbear/plugins/getattr_with_constant.rs +++ b/src/flake8_bugbear/plugins/getattr_with_constant.rs @@ -1,3 +1,4 @@ +use log::error; use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location}; use crate::ast::types::Range; @@ -52,12 +53,15 @@ pub fn getattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar checker.style.line_ending(), ); generator.unparse_expr(&attribute(obj, value), 0); - if let Ok(content) = generator.generate() { - check.amend(Fix::replacement( - content, - expr.location, - expr.end_location.unwrap(), - )); + match generator.generate() { + Ok(content) => { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + Err(e) => error!("Failed to rewrite `getattr`: {e}"), } } checker.add_check(check); diff --git a/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs b/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs index db1f462ef3..e16ef44ecf 100644 --- a/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs +++ b/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs @@ -1,3 +1,4 @@ +use log::error; use rustpython_ast::{Excepthandler, ExcepthandlerKind, ExprKind}; use crate::ast::types::Range; @@ -29,12 +30,15 @@ pub fn redundant_tuple_in_exception_handler(checker: &mut Checker, handlers: &[E checker.style.line_ending(), ); generator.unparse_expr(elt, 0); - if let Ok(content) = generator.generate() { - check.amend(Fix::replacement( - content, - type_.location, - type_.end_location.unwrap(), - )); + match generator.generate() { + Ok(content) => { + check.amend(Fix::replacement( + content, + type_.location, + type_.end_location.unwrap(), + )); + } + Err(e) => error!("Failed to remove redundant tuple: {e}"), } } checker.add_check(check); diff --git a/src/pyflakes/plugins/strings.rs b/src/pyflakes/plugins/strings.rs index 22fd54246e..a2cabd4d56 100644 --- a/src/pyflakes/plugins/strings.rs +++ b/src/pyflakes/plugins/strings.rs @@ -1,5 +1,6 @@ use std::string::ToString; +use log::error; use rustc_hash::FxHashSet; use rustpython_ast::{Keyword, KeywordData}; use rustpython_parser::ast::{Constant, Expr, ExprKind}; @@ -115,9 +116,11 @@ pub(crate) fn percent_format_extra_named_arguments( location, ); if checker.patch(check.kind.code()) { - if let Ok(fix) = remove_unused_format_arguments_from_dict(&missing, right, checker.locator) - { - check.amend(fix); + match remove_unused_format_arguments_from_dict(&missing, right, checker.locator) { + Ok(fix) => { + check.amend(fix); + } + Err(e) => error!("Failed to remove unused format arguments: {e}"), } } checker.add_check(check); @@ -272,10 +275,12 @@ pub(crate) fn string_dot_format_extra_named_arguments( location, ); if checker.patch(check.kind.code()) { - if let Ok(fix) = - remove_unused_keyword_arguments_from_format_call(&missing, location, checker.locator) + match remove_unused_keyword_arguments_from_format_call(&missing, location, checker.locator) { - check.amend(fix); + Ok(fix) => { + check.amend(fix); + } + Err(e) => error!("Failed to remove unused keyword arguments: {e}"), } } checker.add_check(check); diff --git a/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs b/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs index 3a5718c13a..2c11a8abb0 100644 --- a/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs +++ b/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs @@ -193,8 +193,8 @@ pub fn convert_named_tuple_functional_to_class( return; }; match match_defaults(keywords) { - Ok(defaults) => { - if let Ok(properties) = create_properties_from_args(args, defaults) { + Ok(defaults) => match create_properties_from_args(args, defaults) { + Ok(properties) => { let mut check = Check::new( CheckKind::ConvertNamedTupleFunctionalToClass(typename.to_string()), Range::from_located(stmt), @@ -209,7 +209,8 @@ pub fn convert_named_tuple_functional_to_class( } checker.add_check(check); } - } + Err(err) => error!("Failed to create properties: {err}"), + }, Err(err) => error!("Failed to parse defaults: {err}"), } } diff --git a/src/pyupgrade/plugins/use_pep604_annotation.rs b/src/pyupgrade/plugins/use_pep604_annotation.rs index 8ea9c0592f..eba10229d8 100644 --- a/src/pyupgrade/plugins/use_pep604_annotation.rs +++ b/src/pyupgrade/plugins/use_pep604_annotation.rs @@ -1,3 +1,4 @@ +use log::error; use rustpython_ast::{Constant, Expr, ExprKind, Location, Operator}; use crate::ast::helpers::{collect_call_paths, dealias_call_path}; @@ -71,13 +72,16 @@ pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, s checker.style.line_ending(), ); generator.unparse_expr(&optional(slice), 0); - if let Ok(content) = generator.generate() { - check.amend(Fix::replacement( - content, - expr.location, - expr.end_location.unwrap(), - )); - } + match generator.generate() { + Ok(content) => { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + Err(e) => error!("Failed to rewrite PEP604 annotation: {e}"), + }; } checker.add_check(check); } else if checker.match_typing_call_path(&call_path, "Union") { @@ -94,12 +98,15 @@ pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, s checker.style.line_ending(), ); generator.unparse_expr(&union(elts), 0); - if let Ok(content) = generator.generate() { - check.amend(Fix::replacement( - content, - expr.location, - expr.end_location.unwrap(), - )); + match generator.generate() { + Ok(content) => { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + Err(e) => error!("Failed to rewrite PEP604 annotation: {e}"), } } _ => { @@ -110,12 +117,15 @@ pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, s checker.style.line_ending(), ); generator.unparse_expr(slice, 0); - if let Ok(content) = generator.generate() { - check.amend(Fix::replacement( - content, - expr.location, - expr.end_location.unwrap(), - )); + match generator.generate() { + Ok(content) => { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + Err(e) => error!("Failed to rewrite PEP604 annotation: {e}"), } } }