From c56f2636185dd8eb6be31a6e7cc9cc19b45348d0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 11 Jan 2023 18:49:25 -0500 Subject: [PATCH] Avoid flagging builtins for OSError rewrites (#1800) Related to (but does not fix) #1790. --- resources/test/fixtures/pyupgrade/UP024_3.py | 8 ++++++++ src/checkers/ast.rs | 6 +++--- src/pyupgrade/mod.rs | 1 + src/pyupgrade/rules/os_error_alias.rs | 20 +++++++++---------- ...f__pyupgrade__tests__UP024_UP024_3.py.snap | 6 ++++++ 5 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 resources/test/fixtures/pyupgrade/UP024_3.py create mode 100644 src/pyupgrade/snapshots/ruff__pyupgrade__tests__UP024_UP024_3.py.snap diff --git a/resources/test/fixtures/pyupgrade/UP024_3.py b/resources/test/fixtures/pyupgrade/UP024_3.py new file mode 100644 index 0000000000..48ebf7edbd --- /dev/null +++ b/resources/test/fixtures/pyupgrade/UP024_3.py @@ -0,0 +1,8 @@ +class SocketError(Exception): + pass + + +try: + raise SocketError() +except SocketError: + pass diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 298335835a..f72885d895 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1207,7 +1207,7 @@ where } if self.settings.enabled.contains(&RuleCode::UP024) { if let Some(item) = exc { - pyupgrade::rules::os_error_alias(self, item); + pyupgrade::rules::os_error_alias(self, &item); } } } @@ -1333,7 +1333,7 @@ where flake8_bugbear::rules::redundant_tuple_in_exception_handler(self, handlers); } if self.settings.enabled.contains(&RuleCode::UP024) { - pyupgrade::rules::os_error_alias(self, handlers); + pyupgrade::rules::os_error_alias(self, &handlers); } if self.settings.enabled.contains(&RuleCode::PT017) { self.diagnostics.extend( @@ -1921,7 +1921,7 @@ where pyupgrade::rules::replace_stdout_stderr(self, expr, keywords); } if self.settings.enabled.contains(&RuleCode::UP024) { - pyupgrade::rules::os_error_alias(self, expr); + pyupgrade::rules::os_error_alias(self, &expr); } // flake8-print diff --git a/src/pyupgrade/mod.rs b/src/pyupgrade/mod.rs index 45866bb299..96ef365950 100644 --- a/src/pyupgrade/mod.rs +++ b/src/pyupgrade/mod.rs @@ -44,6 +44,7 @@ mod tests { #[test_case(RuleCode::UP024, Path::new("UP024_0.py"); "UP024_0")] #[test_case(RuleCode::UP024, Path::new("UP024_1.py"); "UP024_1")] #[test_case(RuleCode::UP024, Path::new("UP024_2.py"); "UP024_2")] + #[test_case(RuleCode::UP024, Path::new("UP024_3.py"); "UP024_3")] #[test_case(RuleCode::UP025, Path::new("UP025.py"); "UP025")] #[test_case(RuleCode::UP026, Path::new("UP026.py"); "UP026")] #[test_case(RuleCode::UP027, Path::new("UP027.py"); "UP027")] diff --git a/src/pyupgrade/rules/os_error_alias.rs b/src/pyupgrade/rules/os_error_alias.rs index 01ecead5a8..ab5139ea84 100644 --- a/src/pyupgrade/rules/os_error_alias.rs +++ b/src/pyupgrade/rules/os_error_alias.rs @@ -1,5 +1,3 @@ -#![allow(clippy::len_zero, clippy::needless_pass_by_value)] - use itertools::Itertools; use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, Located}; @@ -13,8 +11,11 @@ use crate::violations; const ERROR_NAMES: &[&str] = &["EnvironmentError", "IOError", "WindowsError"]; const ERROR_MODULES: &[&str] = &["mmap", "select", "socket"]; -fn get_correct_name(original: &str) -> String { - if ERROR_NAMES.contains(&original) { +fn corrected_name(checker: &Checker, original: &str) -> String { + if ERROR_NAMES.contains(&original) + && checker.is_builtin(original) + && checker.is_builtin("OSError") + { "OSError".to_string() } else { original.to_string() @@ -64,7 +65,7 @@ fn handle_name_or_attribute( replacements.extend(temp_replacements); before_replace.extend(temp_before_replace); if replacements.is_empty() { - let new_name = get_correct_name(id); + let new_name = corrected_name(checker, id); replacements.push(new_name); before_replace.push(id.to_string()); } @@ -102,7 +103,7 @@ fn handle_except_block(checker: &mut Checker, handler: &Located { - let new_name = get_correct_name(id); + let new_name = corrected_name(checker, id); replacements.push(new_name); } ExprKind::Attribute { .. } => { @@ -138,7 +139,7 @@ fn handle_making_changes( before_replace: &[String], replacements: &[String], ) { - if before_replace != replacements && replacements.len() > 0 { + if before_replace != replacements && !replacements.is_empty() { let range = Range::new(target.location, target.end_location.unwrap()); let contents = checker.locator.slice_source_code_range(&range); // Pyyupgrade does not want imports changed if a module only is @@ -171,8 +172,6 @@ fn handle_making_changes( } } -// This is a hacky way to handle the different variable types we get since -// raise and try are very different. Would love input on a cleaner way pub trait OSErrorAliasChecker { fn check_error(&self, checker: &mut Checker) where @@ -181,7 +180,6 @@ pub trait OSErrorAliasChecker { impl OSErrorAliasChecker for &Vec { fn check_error(&self, checker: &mut Checker) { - // Each separate except block is a separate error and fix for handler in self.iter() { handle_except_block(checker, handler); } @@ -233,6 +231,6 @@ impl OSErrorAliasChecker for &Expr { } /// UP024 -pub fn os_error_alias(checker: &mut Checker, handlers: U) { +pub fn os_error_alias(checker: &mut Checker, handlers: &U) { handlers.check_error(checker); } diff --git a/src/pyupgrade/snapshots/ruff__pyupgrade__tests__UP024_UP024_3.py.snap b/src/pyupgrade/snapshots/ruff__pyupgrade__tests__UP024_UP024_3.py.snap new file mode 100644 index 0000000000..ff56b7772a --- /dev/null +++ b/src/pyupgrade/snapshots/ruff__pyupgrade__tests__UP024_UP024_3.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyupgrade/mod.rs +expression: diagnostics +--- +[] +