Add required space when fixing UP024 (#7171)

This commit is contained in:
Charlie Marsh 2023-09-05 19:37:09 +02:00 committed by GitHub
parent 37dfb205b1
commit 264d9159f8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 95 additions and 77 deletions

View file

@ -96,3 +96,11 @@ try:
pass pass
except (OSError, KeyError): except (OSError, KeyError):
pass 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)

View file

@ -10,7 +10,7 @@ use ruff_python_trivia::{
has_leading_content, is_python_whitespace, PythonWhitespace, SimpleTokenKind, SimpleTokenizer, has_leading_content, is_python_whitespace, PythonWhitespace, SimpleTokenKind, SimpleTokenizer,
}; };
use ruff_source_file::{Locator, NewlineWithTrailingNewline}; 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; use crate::autofix::codemods;
@ -249,6 +249,44 @@ fn next_stmt_break(semicolon: TextSize, locator: &Locator) -> TextSize {
locator.line_end(start_location) 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)] #[cfg(test)]
mod tests { mod tests {
use anyhow::Result; use anyhow::Result;

View file

@ -1,10 +1,10 @@
use ruff_python_ast::{self as ast, ExceptHandler, Expr};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::map_starred; 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::checkers::ast::Checker;
use crate::registry::AsRule; 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 // Otherwise, the output will be invalid syntax, since we're removing a set of
// parentheses. // 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( diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
if requires_space { pad(
format!(" {content}") checker.generator().expr(elt),
} else { type_.range(),
content checker.locator(),
}, ),
type_.range(), type_.range(),
))); )));
} }

View file

@ -6,6 +6,7 @@ use ruff_python_ast::parenthesize::parenthesized_range;
use ruff_python_ast::{self as ast, Arguments, CmpOp, Comprehension, Expr}; use ruff_python_ast::{self as ast, Arguments, CmpOp, Comprehension, Expr};
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use crate::autofix::edits::pad_end;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::registry::AsRule; use crate::registry::AsRule;
@ -121,19 +122,12 @@ fn key_in_dict(
// ```python // ```python
// if key in foo.keys()and bar: // 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( diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
if requires_space { pad_end(
format!("{value_content} ") value_content.to_string(),
} else { right_range.end(),
value_content.to_string() checker.locator(),
}, ),
right_range, right_range,
))); )));
} }

View file

@ -7,8 +7,9 @@ use ruff_python_ast::{self as ast, CmpOp, Expr, UnaryOp};
use ruff_python_codegen::Stylist; use ruff_python_codegen::Stylist;
use ruff_python_stdlib::str::{self}; use ruff_python_stdlib::str::{self};
use ruff_source_file::Locator; 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::autofix::snippet::SourceCodeSnippet;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::cst::helpers::or_space; 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
}

View file

@ -1,10 +1,12 @@
use memchr::memchr_iter; use memchr::memchr_iter;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::str::{leading_quote, trailing_quote}; use ruff_python_ast::str::{leading_quote, trailing_quote};
use ruff_source_file::Locator; use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use crate::autofix::edits::pad_start;
/// ## What it does /// ## What it does
/// Checks for invalid escape sequences. /// 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`, // If necessary, add a space between any leading keyword (`return`, `yield`,
// `assert`, etc.) and the string. For example, `return"foo"` is valid, but // `assert`, etc.) and the string. For example, `return"foo"` is valid, but
// `returnr"foo"` is not. // `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( diagnostic.set_fix(Fix::automatic(Edit::insertion(
if requires_space { pad_start("r".to_string(), range.start(), locator),
" r".to_string()
} else {
"r".to_string()
},
range.start(), range.start(),
))); )));
} }

View file

@ -1,6 +1,7 @@
use ruff_python_ast::{self as ast, ExceptHandler, Expr, ExprContext}; use ruff_python_ast::{self as ast, ExceptHandler, Expr, ExprContext};
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use crate::autofix::edits::pad;
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::compose_call_path; use ruff_python_ast::call_path::compose_call_path;
@ -122,24 +123,23 @@ fn tuple_diagnostic(checker: &mut Checker, target: &Expr, aliases: &[&Expr]) {
remaining.insert(0, node.into()); remaining.insert(0, node.into());
} }
if remaining.len() == 1 { let content = if remaining.len() == 1 {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement( "OSError".to_string()
"OSError".to_string(),
target.range(),
)));
} else { } else {
let node = ast::ExprTuple { let node = ast::ExprTuple {
elts: remaining, elts: remaining,
ctx: ExprContext::Load, ctx: ExprContext::Load,
range: TextRange::default(), range: TextRange::default(),
}; };
format!("({})", checker.generator().expr(&node.into()))
};
diagnostic.set_fix(Fix::automatic(Edit::range_replacement( diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
format!("({})", checker.generator().expr(&node.into())), pad(content, target.range(), checker.locator()),
target.range(), target.range(),
))); )));
} }
} }
}
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }

View file

@ -264,4 +264,22 @@ UP024_0.py:87:8: UP024 [*] Replace aliased errors with `OSError`
89 89 | 89 89 |
90 90 | try: 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)