Skip to content

Commit

Permalink
Use Cow in printf rewrite rule (#7986)
Browse files Browse the repository at this point in the history
Small thing that bothered me when looking into the regex update.
  • Loading branch information
charliermarsh committed Oct 16, 2023
1 parent 7da4e28 commit 84f7391
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 31 deletions.
23 changes: 11 additions & 12 deletions crates/ruff_linter/src/rules/pyupgrade/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
use once_cell::sync::Lazy;
use regex::{Captures, Regex};
use std::borrow::Cow;

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
// \N{SNOWMAN}), which we _don't_ want to escape, so take care to preserve them.
CURLY_BRACES
.replace_all(text, |caps: &Captures| {
if let Some(match_) = caps.get(1) {
match_.as_str().to_string()
CURLY_BRACES.replace_all(text, |caps: &Captures| {
if let Some(match_) = caps.get(1) {
match_.as_str().to_string()
} else {
if &caps[2] == "{" {
"{{".to_string()
} else {
if &caps[2] == "{" {
"{{".to_string()
} else {
"}}".to_string()
}
"}}".to_string()
}
})
.to_string()
}
})
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::str::FromStr;

use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
Expand Down Expand Up @@ -105,7 +106,7 @@ fn simplify_conversion_flag(flags: CConversionFlags) -> 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 {
CFormatPart::Literal(item) => curly_escape(item),
CFormatPart::Spec(spec) => {
Expand All @@ -114,7 +115,7 @@ fn handle_part(part: &CFormatPart<String>) -> String {
// TODO(charlie): What case is this?
if spec.format_char == '%' {
format_string.push('%');
return format_string;
return Cow::Owned(format_string);
}

format_string.push('{');
Expand Down Expand Up @@ -171,37 +172,38 @@ fn handle_part(part: &CFormatPart<String>) -> String {
format_string.push(spec.format_char);
}
format_string.push('}');
format_string
Cow::Owned(format_string)
}
}
}

/// Convert a [`CFormatString`] into a `String`.
fn percent_to_format(format_string: &CFormatString) -> String {
let mut contents = String::new();
for (.., format_part) in format_string.iter() {
contents.push_str(&handle_part(format_part));
}
contents
format_string
.iter()
.map(|(_, part)| handle_part(part))
.collect()
}

/// If a tuple has one argument, remove the comma; otherwise, return it as-is.
fn clean_params_tuple(right: &Expr, locator: &Locator) -> String {
let mut contents = locator.slice(right).to_string();
fn clean_params_tuple<'a>(right: &Expr, locator: &Locator<'a>) -> Cow<'a, str> {
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = &right {
if elts.len() == 1 {
if !locator.contains_line_break(right.range()) {
let mut contents = locator.slice(right).to_string();
for (i, character) in contents.chars().rev().enumerate() {
if character == ',' {
let correct_index = contents.len() - i - 1;
contents.remove(correct_index);
break;
}
}
return Cow::Owned(contents);
}
}
}
contents

Cow::Borrowed(locator.slice(right))
}

/// Converts a dictionary to a function call while preserving as much styling as
Expand Down Expand Up @@ -419,16 +421,16 @@ pub(crate) fn printf_string_formatting(checker: &mut Checker, expr: &Expr, right
// Parse the parameters.
let params_string = match right {
Expr::Constant(_) | Expr::FString(_) => {
format!("({})", checker.locator().slice(right))
Cow::Owned(format!("({})", checker.locator().slice(right)))
}
Expr::Name(_) | Expr::Attribute(_) | Expr::Subscript(_) | Expr::Call(_) => {
if num_keyword_arguments > 0 {
// 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 {
// If we have multiple fields, but no named fields, assume the right-hand side is a
// tuple.
format!("(*{})", checker.locator().slice(right))
Cow::Owned(format!("(*{})", checker.locator().slice(right)))
} else {
// 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,
Expand All @@ -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::Dict(_) => {
if let Some(params_string) =
let Some(params_string) =
clean_params_dictionary(right, checker.locator(), checker.stylist())
{
params_string
} else {
else {
return;
}
};
Cow::Owned(params_string)
}
_ => return,
};
Expand Down

0 comments on commit 84f7391

Please sign in to comment.