Use Cow in printf rewrite rule (#7986)

Small thing that bothered me when looking into the regex update.
This commit is contained in:
Charlie Marsh 2023-10-16 12:47:03 -04:00 committed by GitHub
parent 7da4e28a98
commit 84f7391cc5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 31 additions and 31 deletions

View file

@ -1,22 +1,21 @@
use once_cell::sync::Lazy; use once_cell::sync::Lazy;
use regex::{Captures, Regex}; use regex::{Captures, Regex};
use std::borrow::Cow;
static CURLY_BRACES: Lazy<Regex> = Lazy::new(|| Regex::new(r"(\\N\{[^}]+})|([{}])").unwrap()); static CURLY_BRACES: Lazy<Regex> = Lazy::new(|| Regex::new(r"(\\N\{[^}]+})|([{}])").unwrap());
pub(super) fn curly_escape(text: &str) -> String { pub(super) fn curly_escape(text: &str) -> Cow<'_, str> {
// Match all curly braces. This will include named unicode escapes (like // Match all curly braces. This will include named unicode escapes (like
// \N{SNOWMAN}), which we _don't_ want to escape, so take care to preserve them. // \N{SNOWMAN}), which we _don't_ want to escape, so take care to preserve them.
CURLY_BRACES CURLY_BRACES.replace_all(text, |caps: &Captures| {
.replace_all(text, |caps: &Captures| { if let Some(match_) = caps.get(1) {
if let Some(match_) = caps.get(1) { match_.as_str().to_string()
match_.as_str().to_string() } else {
if &caps[2] == "{" {
"{{".to_string()
} else { } else {
if &caps[2] == "{" { "}}".to_string()
"{{".to_string()
} else {
"}}".to_string()
}
} }
}) }
.to_string() })
} }

View file

@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::str::FromStr; use std::str::FromStr;
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
@ -105,7 +106,7 @@ fn simplify_conversion_flag(flags: CConversionFlags) -> String {
} }
/// Convert a [`PercentFormat`] struct into a `String`. /// Convert a [`PercentFormat`] struct into a `String`.
fn handle_part(part: &CFormatPart<String>) -> String { fn handle_part(part: &CFormatPart<String>) -> Cow<'_, str> {
match part { match part {
CFormatPart::Literal(item) => curly_escape(item), CFormatPart::Literal(item) => curly_escape(item),
CFormatPart::Spec(spec) => { CFormatPart::Spec(spec) => {
@ -114,7 +115,7 @@ fn handle_part(part: &CFormatPart<String>) -> String {
// TODO(charlie): What case is this? // TODO(charlie): What case is this?
if spec.format_char == '%' { if spec.format_char == '%' {
format_string.push('%'); format_string.push('%');
return format_string; return Cow::Owned(format_string);
} }
format_string.push('{'); format_string.push('{');
@ -171,26 +172,25 @@ fn handle_part(part: &CFormatPart<String>) -> String {
format_string.push(spec.format_char); format_string.push(spec.format_char);
} }
format_string.push('}'); format_string.push('}');
format_string Cow::Owned(format_string)
} }
} }
} }
/// Convert a [`CFormatString`] into a `String`. /// Convert a [`CFormatString`] into a `String`.
fn percent_to_format(format_string: &CFormatString) -> String { fn percent_to_format(format_string: &CFormatString) -> String {
let mut contents = String::new(); format_string
for (.., format_part) in format_string.iter() { .iter()
contents.push_str(&handle_part(format_part)); .map(|(_, part)| handle_part(part))
} .collect()
contents
} }
/// If a tuple has one argument, remove the comma; otherwise, return it as-is. /// If a tuple has one argument, remove the comma; otherwise, return it as-is.
fn clean_params_tuple(right: &Expr, locator: &Locator) -> String { fn clean_params_tuple<'a>(right: &Expr, locator: &Locator<'a>) -> Cow<'a, str> {
let mut contents = locator.slice(right).to_string();
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = &right { if let Expr::Tuple(ast::ExprTuple { elts, .. }) = &right {
if elts.len() == 1 { if elts.len() == 1 {
if !locator.contains_line_break(right.range()) { if !locator.contains_line_break(right.range()) {
let mut contents = locator.slice(right).to_string();
for (i, character) in contents.chars().rev().enumerate() { for (i, character) in contents.chars().rev().enumerate() {
if character == ',' { if character == ',' {
let correct_index = contents.len() - i - 1; let correct_index = contents.len() - i - 1;
@ -198,10 +198,12 @@ fn clean_params_tuple(right: &Expr, locator: &Locator) -> String {
break; break;
} }
} }
return Cow::Owned(contents);
} }
} }
} }
contents
Cow::Borrowed(locator.slice(right))
} }
/// Converts a dictionary to a function call while preserving as much styling as /// Converts a dictionary to a function call while preserving as much styling as
@ -419,16 +421,16 @@ pub(crate) fn printf_string_formatting(checker: &mut Checker, expr: &Expr, right
// Parse the parameters. // Parse the parameters.
let params_string = match right { let params_string = match right {
Expr::Constant(_) | Expr::FString(_) => { Expr::Constant(_) | Expr::FString(_) => {
format!("({})", checker.locator().slice(right)) Cow::Owned(format!("({})", checker.locator().slice(right)))
} }
Expr::Name(_) | Expr::Attribute(_) | Expr::Subscript(_) | Expr::Call(_) => { Expr::Name(_) | Expr::Attribute(_) | Expr::Subscript(_) | Expr::Call(_) => {
if num_keyword_arguments > 0 { if num_keyword_arguments > 0 {
// If we have _any_ named fields, assume the right-hand side is a mapping. // If we have _any_ named fields, assume the right-hand side is a mapping.
format!("(**{})", checker.locator().slice(right)) Cow::Owned(format!("(**{})", checker.locator().slice(right)))
} else if num_positional_arguments > 1 { } else if num_positional_arguments > 1 {
// If we have multiple fields, but no named fields, assume the right-hand side is a // If we have multiple fields, but no named fields, assume the right-hand side is a
// tuple. // tuple.
format!("(*{})", checker.locator().slice(right)) Cow::Owned(format!("(*{})", checker.locator().slice(right)))
} else { } else {
// Otherwise, if we have a single field, we can't make any assumptions about the // Otherwise, if we have a single field, we can't make any assumptions about the
// right-hand side. It _could_ be a tuple, but it could also be a single value, // right-hand side. It _could_ be a tuple, but it could also be a single value,
@ -444,13 +446,12 @@ pub(crate) fn printf_string_formatting(checker: &mut Checker, expr: &Expr, right
} }
Expr::Tuple(_) => clean_params_tuple(right, checker.locator()), Expr::Tuple(_) => clean_params_tuple(right, checker.locator()),
Expr::Dict(_) => { Expr::Dict(_) => {
if let Some(params_string) = let Some(params_string) =
clean_params_dictionary(right, checker.locator(), checker.stylist()) clean_params_dictionary(right, checker.locator(), checker.stylist())
{ else {
params_string
} else {
return; return;
} };
Cow::Owned(params_string)
} }
_ => return, _ => return,
}; };