Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove empty strings when converting to f-string (UP032) #11524

Merged
merged 2 commits into from
May 27, 2024
Merged

Remove empty strings when converting to f-string (UP032) #11524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6de6fcc
Remove empty strings when converting to f-string
dhruvmanila May 24, 2024
9dfb592
Rename `Literal` -> `NonEmptyLiteral`
dhruvmanila May 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 29 additions & 33 deletions crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,10 @@ fn formatted_expr<'a>(expr: &Expr, context: FormatContext, locator: &Locator<'a>

#[derive(Debug, Clone)]
enum FStringConversion {
/// The format string only contains literal parts.
Literal,
/// The format string only contains literal parts and is empty.
EmptyLiteral,
/// The format string only contains literal parts and is non-empty.
NonEmptyLiteral,
/// The format call uses arguments with side effects which are repeated within the
/// format string. For example: `"{x} {x}".format(x=foo())`.
SideEffects,
Expand Down Expand Up @@ -263,7 +265,7 @@ impl FStringConversion {

// If the format string is empty, it doesn't need to be converted.
if contents.is_empty() {
return Ok(Self::Literal);
return Ok(Self::EmptyLiteral);
}

// Parse the format string.
Expand All @@ -275,7 +277,7 @@ impl FStringConversion {
.iter()
.all(|part| matches!(part, FormatPart::Literal(..)))
{
return Ok(Self::Literal);
return Ok(Self::NonEmptyLiteral);
}

let mut converted = String::with_capacity(contents.len());
Expand Down Expand Up @@ -406,7 +408,7 @@ pub(crate) fn f_strings(checker: &mut Checker, call: &ast::ExprCall, summary: &F
return;
};

let mut patches: Vec<(TextRange, String)> = vec![];
let mut patches: Vec<(TextRange, FStringConversion)> = vec![];
let mut lex = lexer::lex_starts_at(
checker.locator().slice(call.func.range()),
Mode::Expression,
Expand All @@ -431,18 +433,14 @@ pub(crate) fn f_strings(checker: &mut Checker, call: &ast::ExprCall, summary: &F
}
Some((Tok::String { .. }, range)) => {
match FStringConversion::try_convert(range, &mut summary, checker.locator()) {
Ok(FStringConversion::Convert(fstring)) => patches.push((range, fstring)),
// Convert escaped curly brackets e.g. `{{` to `{` in literal string parts
Ok(FStringConversion::Literal) => patches.push((
range,
curly_unescape(checker.locator().slice(range)).to_string(),
)),
// If the format string contains side effects that would need to be repeated,
// we can't convert it to an f-string.
Ok(FStringConversion::SideEffects) => return,
// If any of the segments fail to convert, then we can't convert the entire
// expression.
Err(_) => return,
// Otherwise, push the conversion to be processed later.
Ok(conversion) => patches.push((range, conversion)),
}
}
Some(_) => continue,
Expand All @@ -455,30 +453,28 @@ pub(crate) fn f_strings(checker: &mut Checker, call: &ast::ExprCall, summary: &F

let mut contents = String::with_capacity(checker.locator().slice(call).len());
let mut prev_end = call.start();
for (range, fstring) in patches {
contents.push_str(
checker
.locator()
.slice(TextRange::new(prev_end, range.start())),
);
contents.push_str(&fstring);
prev_end = range.end();
}

// If the remainder is non-empty, add it to the contents.
let rest = checker.locator().slice(TextRange::new(prev_end, end));
if !lexer::lex_starts_at(rest, Mode::Expression, prev_end)
.flatten()
.all(|(token, _)| match token {
Tok::Comment(_) | Tok::Newline | Tok::NonLogicalNewline | Tok::Indent | Tok::Dedent => {
true
for (range, conversion) in patches {
let fstring = match conversion {
FStringConversion::Convert(fstring) => Some(fstring),
FStringConversion::EmptyLiteral => None,
FStringConversion::NonEmptyLiteral => {
// Convert escaped curly brackets e.g. `{{` to `{` in literal string parts
Some(curly_unescape(checker.locator().slice(range)).to_string())
}
Tok::String { value, .. } => value.is_empty(),
_ => false,
})
{
contents.push_str(rest);
// We handled this in the previous loop.
FStringConversion::SideEffects => unreachable!(),
};
if let Some(fstring) = fstring {
contents.push_str(
checker
.locator()
.slice(TextRange::new(prev_end, range.start())),
);
contents.push_str(&fstring);
}
prev_end = range.end();
}
contents.push_str(checker.locator().slice(TextRange::new(prev_end, end)));

// If necessary, add a space between any leading keyword (`return`, `yield`, `assert`, etc.)
// and the string. For example, `return"foo"` is valid, but `returnf"foo"` is not.
Expand Down
48 changes: 23 additions & 25 deletions ...inter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,16 +767,16 @@ UP032_0.py:86:1: UP032 [*] Use f-string instead of `format` call
85 85 |
86 86 | (
87 |- "{a}"
87 |+ f"{1}"
88 88 | ""
88 |- ""
89 |- "{b}"
89 |+ f"{1}"
90 90 | ""
90 |- ""
91 |-).format(a=1, b=1)
91 |+)
92 92 |
93 93 | (
94 94 | (
87 |+ f"{1}"
88 |+ f"{1}"
89 |+)
92 90 |
93 91 | (
94 92 | (

UP032_0.py:94:5: UP032 [*] Use f-string instead of `format` call
|
Expand Down Expand Up @@ -1089,11 +1089,10 @@ UP032_0.py:212:18: UP032 [*] Use f-string instead of `format` call
211 211 | # When fixing, trim the trailing empty string.
212 |-raise ValueError("Conflicting configuration dicts: {!r} {!r}"
213 |- "".format(new_dict, d))
212 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}"
213 |+ "")
214 214 |
215 215 | # When fixing, trim the trailing empty string.
216 216 | raise ValueError("Conflicting configuration dicts: {!r} {!r}"
212 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}")
214 213 |
215 214 | # When fixing, trim the trailing empty string.
216 215 | raise ValueError("Conflicting configuration dicts: {!r} {!r}"

UP032_0.py:216:18: UP032 [*] Use f-string instead of `format` call
|
Expand All @@ -1113,10 +1112,11 @@ UP032_0.py:216:18: UP032 [*] Use f-string instead of `format` call
215 215 | # When fixing, trim the trailing empty string.
216 |-raise ValueError("Conflicting configuration dicts: {!r} {!r}"
217 |- .format(new_dict, d))
216 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}")
218 217 |
219 218 | raise ValueError(
220 219 | "Conflicting configuration dicts: {!r} {!r}"
216 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}"
217 |+ )
218 218 |
219 219 | raise ValueError(
220 220 | "Conflicting configuration dicts: {!r} {!r}"

UP032_0.py:220:5: UP032 [*] Use f-string instead of `format` call
|
Expand All @@ -1136,10 +1136,9 @@ UP032_0.py:220:5: UP032 [*] Use f-string instead of `format` call
220 |- "Conflicting configuration dicts: {!r} {!r}"
221 |- "".format(new_dict, d)
220 |+ f"Conflicting configuration dicts: {new_dict!r} {d!r}"
221 |+ ""
222 222 | )
223 223 |
224 224 | raise ValueError(
222 221 | )
223 222 |
224 223 | raise ValueError(

UP032_0.py:225:5: UP032 [*] Use f-string instead of `format` call
|
Expand All @@ -1160,10 +1159,9 @@ UP032_0.py:225:5: UP032 [*] Use f-string instead of `format` call
225 |- "Conflicting configuration dicts: {!r} {!r}"
226 |- "".format(new_dict, d)
225 |+ f"Conflicting configuration dicts: {new_dict!r} {d!r}"
226 |+ ""
227 227 |
228 228 | )
229 229 |
227 226 |
228 227 | )
229 228 |

UP032_0.py:231:1: UP032 [*] Use f-string instead of `format` call
|
Expand Down
Loading