diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP024_0.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP024_0.py index 5d8c07b8bd..523ee451f9 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP024_0.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP024_0.py @@ -96,3 +96,11 @@ try: pass except (OSError, KeyError): pass + + +# Regression test for: https://github.com/astral-sh/ruff/issues/7101 +def get_owner_id_from_mac_address(): + try: + mac_address = get_primary_mac_address() + except(IOError, OSError) as ex: + msg = 'Unable to query URL to get Owner ID: {u}\n{e}'.format(u=owner_id_url, e=ex) diff --git a/crates/ruff/src/autofix/edits.rs b/crates/ruff/src/autofix/edits.rs index 02b18bdf0a..82d8e749cb 100644 --- a/crates/ruff/src/autofix/edits.rs +++ b/crates/ruff/src/autofix/edits.rs @@ -10,7 +10,7 @@ use ruff_python_trivia::{ has_leading_content, is_python_whitespace, PythonWhitespace, SimpleTokenKind, SimpleTokenizer, }; use ruff_source_file::{Locator, NewlineWithTrailingNewline}; -use ruff_text_size::{Ranged, TextLen, TextSize}; +use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use crate::autofix::codemods; @@ -249,6 +249,44 @@ fn next_stmt_break(semicolon: TextSize, locator: &Locator) -> TextSize { locator.line_end(start_location) } +/// Add leading whitespace to a snippet, if it's immediately preceded an identifier or keyword. +pub(crate) fn pad_start(mut content: String, start: TextSize, locator: &Locator) -> String { + // Ex) When converting `except(ValueError,)` from a tuple to a single argument, we need to + // insert a space before the fix, to achieve `except ValueError`. + if locator + .up_to(start) + .chars() + .last() + .is_some_and(|char| char.is_ascii_alphabetic()) + { + content.insert(0, ' '); + } + content +} + +/// Add trailing whitespace to a snippet, if it's immediately followed by an identifier or keyword. +pub(crate) fn pad_end(mut content: String, end: TextSize, locator: &Locator) -> String { + if locator + .after(end) + .chars() + .next() + .is_some_and(|char| char.is_ascii_alphabetic()) + { + content.push(' '); + } + content +} + +/// Add leading or trailing whitespace to a snippet, if it's immediately preceded or followed by +/// an identifier or keyword. +pub(crate) fn pad(content: String, range: TextRange, locator: &Locator) -> String { + pad_start( + pad_end(content, range.end(), locator), + range.start(), + locator, + ) +} + #[cfg(test)] mod tests { use anyhow::Result; diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs b/crates/ruff/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs index 37b10d0843..cabcd9b558 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs @@ -1,10 +1,10 @@ -use ruff_python_ast::{self as ast, ExceptHandler, Expr}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::map_starred; -use ruff_text_size::{Ranged, TextRange}; +use ruff_python_ast::{self as ast, ExceptHandler, Expr}; +use ruff_text_size::Ranged; +use crate::autofix::edits::pad; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -85,19 +85,12 @@ pub(crate) fn redundant_tuple_in_exception_handler( // ``` // Otherwise, the output will be invalid syntax, since we're removing a set of // parentheses. - let requires_space = checker - .locator() - .slice(TextRange::up_to(type_.start())) - .chars() - .last() - .is_some_and(|char| char.is_ascii_alphabetic()); - let content = checker.generator().expr(elt); diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - if requires_space { - format!(" {content}") - } else { - content - }, + pad( + checker.generator().expr(elt), + type_.range(), + checker.locator(), + ), type_.range(), ))); } diff --git a/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs b/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs index 8d6124074f..33e63663ae 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs @@ -6,6 +6,7 @@ use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, Arguments, CmpOp, Comprehension, Expr}; use ruff_text_size::{Ranged, TextRange}; +use crate::autofix::edits::pad_end; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -121,19 +122,12 @@ fn key_in_dict( // ```python // if key in foo.keys()and bar: // ``` - let requires_space = checker - .locator() - .after(right_range.end()) - .chars() - .next() - .is_some_and(|char| char.is_ascii_alphabetic()); - diagnostic.set_fix(Fix::suggested(Edit::range_replacement( - if requires_space { - format!("{value_content} ") - } else { - value_content.to_string() - }, + pad_end( + value_content.to_string(), + right_range.end(), + checker.locator(), + ), right_range, ))); } diff --git a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs b/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs index d2618f8d47..2fc1ae1fb1 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs @@ -7,8 +7,9 @@ use ruff_python_ast::{self as ast, CmpOp, Expr, UnaryOp}; use ruff_python_codegen::Stylist; use ruff_python_stdlib::str::{self}; use ruff_source_file::Locator; -use ruff_text_size::{Ranged, TextRange}; +use ruff_text_size::Ranged; +use crate::autofix::edits::pad; use crate::autofix::snippet::SourceCodeSnippet; use crate::checkers::ast::Checker; use crate::cst::helpers::or_space; @@ -206,29 +207,3 @@ pub(crate) fn yoda_conditions( )); } } - -/// Add leading or trailing whitespace to a snippet, if it's immediately preceded or followed by -/// an identifier or keyword. -fn pad(mut content: String, range: TextRange, locator: &Locator) -> String { - // Ex) `A or(B)>1`. To avoid `A or1<(B)`, insert a space before the fix, to achieve `A or 1<(B)`. - if locator - .up_to(range.start()) - .chars() - .last() - .is_some_and(|char| char.is_ascii_alphabetic()) - { - content.insert(0, ' '); - } - - // Ex) `1>(B)or A`. To avoid `(B)<1or A`, insert a space after the fix, to achieve `(B)<1 or A`. - if locator - .after(range.end()) - .chars() - .next() - .is_some_and(|char| char.is_ascii_alphabetic()) - { - content.push(' '); - } - - content -} diff --git a/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs b/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs index 286344a7ca..8cf0466c5c 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs @@ -1,10 +1,12 @@ use memchr::memchr_iter; -use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::str::{leading_quote, trailing_quote}; use ruff_source_file::Locator; +use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; + +use crate::autofix::edits::pad_start; /// ## What it does /// Checks for invalid escape sequences. @@ -138,18 +140,8 @@ pub(crate) fn invalid_escape_sequence( // If necessary, add a space between any leading keyword (`return`, `yield`, // `assert`, etc.) and the string. For example, `return"foo"` is valid, but // `returnr"foo"` is not. - let requires_space = locator - .slice(TextRange::up_to(range.start())) - .chars() - .last() - .is_some_and(|char| char.is_ascii_alphabetic()); - diagnostic.set_fix(Fix::automatic(Edit::insertion( - if requires_space { - " r".to_string() - } else { - "r".to_string() - }, + pad_start("r".to_string(), range.start(), locator), range.start(), ))); } diff --git a/crates/ruff/src/rules/pyupgrade/rules/os_error_alias.rs b/crates/ruff/src/rules/pyupgrade/rules/os_error_alias.rs index 1e4e659c92..45510ccfc1 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/os_error_alias.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/os_error_alias.rs @@ -1,6 +1,7 @@ use ruff_python_ast::{self as ast, ExceptHandler, Expr, ExprContext}; use ruff_text_size::{Ranged, TextRange}; +use crate::autofix::edits::pad; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::compose_call_path; @@ -122,22 +123,21 @@ fn tuple_diagnostic(checker: &mut Checker, target: &Expr, aliases: &[&Expr]) { remaining.insert(0, node.into()); } - if remaining.len() == 1 { - diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - "OSError".to_string(), - target.range(), - ))); + let content = if remaining.len() == 1 { + "OSError".to_string() } else { let node = ast::ExprTuple { elts: remaining, ctx: ExprContext::Load, range: TextRange::default(), }; - diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - format!("({})", checker.generator().expr(&node.into())), - target.range(), - ))); - } + format!("({})", checker.generator().expr(&node.into())) + }; + + diagnostic.set_fix(Fix::automatic(Edit::range_replacement( + pad(content, target.range(), checker.locator()), + target.range(), + ))); } } checker.diagnostics.push(diagnostic); diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP024_0.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP024_0.py.snap index f77d7d36a6..aa0859fd46 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP024_0.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP024_0.py.snap @@ -264,4 +264,22 @@ UP024_0.py:87:8: UP024 [*] Replace aliased errors with `OSError` 89 89 | 90 90 | try: +UP024_0.py:105:11: UP024 [*] Replace aliased errors with `OSError` + | +103 | try: +104 | mac_address = get_primary_mac_address() +105 | except(IOError, OSError) as ex: + | ^^^^^^^^^^^^^^^^^^ UP024 +106 | msg = 'Unable to query URL to get Owner ID: {u}\n{e}'.format(u=owner_id_url, e=ex) + | + = help: Replace with builtin `OSError` + +ℹ Fix +102 102 | def get_owner_id_from_mac_address(): +103 103 | try: +104 104 | mac_address = get_primary_mac_address() +105 |- except(IOError, OSError) as ex: + 105 |+ except OSError as ex: +106 106 | msg = 'Unable to query URL to get Owner ID: {u}\n{e}'.format(u=owner_id_url, e=ex) +