mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-02 14:51:25 +00:00
Remove empty strings when converting to f-string (UP032
) (#11524)
## Summary This PR brings back the functionality to remove empty strings when converting to an f-string in `UP032`. For context, https://github.com/astral-sh/ruff/pull/8712 added this functionality to remove _trailing_ empty strings but it got removed in https://github.com/astral-sh/ruff/pull/8697 possibly unexpectedly so. There's one difference which is that this PR will remove _any_ empty strings and not just trailing ones. For example, ```diff --- /Users/dhruv/playground/ruff/src/UP032.py +++ /Users/dhruv/playground/ruff/src/UP032.py @@ -1,7 +1,5 @@ ( - "{a}" - "" - "{b}" - "" -).format(a=1, b=1) + f"{1}" + f"{1}" +) ``` ## Test Plan Run `cargo insta test` and update the snapshots.
This commit is contained in:
parent
5dcde88099
commit
9200dfc79f
2 changed files with 52 additions and 58 deletions
|
@ -216,8 +216,10 @@ fn formatted_expr<'a>(expr: &Expr, context: FormatContext, locator: &Locator<'a>
|
||||||
|
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
enum FStringConversion {
|
enum FStringConversion {
|
||||||
/// The format string only contains literal parts.
|
/// The format string only contains literal parts and is empty.
|
||||||
Literal,
|
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
|
/// The format call uses arguments with side effects which are repeated within the
|
||||||
/// format string. For example: `"{x} {x}".format(x=foo())`.
|
/// format string. For example: `"{x} {x}".format(x=foo())`.
|
||||||
SideEffects,
|
SideEffects,
|
||||||
|
@ -263,7 +265,7 @@ impl FStringConversion {
|
||||||
|
|
||||||
// If the format string is empty, it doesn't need to be converted.
|
// If the format string is empty, it doesn't need to be converted.
|
||||||
if contents.is_empty() {
|
if contents.is_empty() {
|
||||||
return Ok(Self::Literal);
|
return Ok(Self::EmptyLiteral);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Parse the format string.
|
// Parse the format string.
|
||||||
|
@ -275,7 +277,7 @@ impl FStringConversion {
|
||||||
.iter()
|
.iter()
|
||||||
.all(|part| matches!(part, FormatPart::Literal(..)))
|
.all(|part| matches!(part, FormatPart::Literal(..)))
|
||||||
{
|
{
|
||||||
return Ok(Self::Literal);
|
return Ok(Self::NonEmptyLiteral);
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut converted = String::with_capacity(contents.len());
|
let mut converted = String::with_capacity(contents.len());
|
||||||
|
@ -406,7 +408,7 @@ pub(crate) fn f_strings(checker: &mut Checker, call: &ast::ExprCall, summary: &F
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
let mut patches: Vec<(TextRange, String)> = vec![];
|
let mut patches: Vec<(TextRange, FStringConversion)> = vec![];
|
||||||
let mut lex = lexer::lex_starts_at(
|
let mut lex = lexer::lex_starts_at(
|
||||||
checker.locator().slice(call.func.range()),
|
checker.locator().slice(call.func.range()),
|
||||||
Mode::Expression,
|
Mode::Expression,
|
||||||
|
@ -431,18 +433,14 @@ pub(crate) fn f_strings(checker: &mut Checker, call: &ast::ExprCall, summary: &F
|
||||||
}
|
}
|
||||||
Some((Tok::String { .. }, range)) => {
|
Some((Tok::String { .. }, range)) => {
|
||||||
match FStringConversion::try_convert(range, &mut summary, checker.locator()) {
|
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,
|
// If the format string contains side effects that would need to be repeated,
|
||||||
// we can't convert it to an f-string.
|
// we can't convert it to an f-string.
|
||||||
Ok(FStringConversion::SideEffects) => return,
|
Ok(FStringConversion::SideEffects) => return,
|
||||||
// If any of the segments fail to convert, then we can't convert the entire
|
// If any of the segments fail to convert, then we can't convert the entire
|
||||||
// expression.
|
// expression.
|
||||||
Err(_) => return,
|
Err(_) => return,
|
||||||
|
// Otherwise, push the conversion to be processed later.
|
||||||
|
Ok(conversion) => patches.push((range, conversion)),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Some(_) => continue,
|
Some(_) => continue,
|
||||||
|
@ -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 contents = String::with_capacity(checker.locator().slice(call).len());
|
||||||
let mut prev_end = call.start();
|
let mut prev_end = call.start();
|
||||||
for (range, fstring) in patches {
|
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())
|
||||||
|
}
|
||||||
|
// We handled this in the previous loop.
|
||||||
|
FStringConversion::SideEffects => unreachable!(),
|
||||||
|
};
|
||||||
|
if let Some(fstring) = fstring {
|
||||||
contents.push_str(
|
contents.push_str(
|
||||||
checker
|
checker
|
||||||
.locator()
|
.locator()
|
||||||
.slice(TextRange::new(prev_end, range.start())),
|
.slice(TextRange::new(prev_end, range.start())),
|
||||||
);
|
);
|
||||||
contents.push_str(&fstring);
|
contents.push_str(&fstring);
|
||||||
|
}
|
||||||
prev_end = range.end();
|
prev_end = range.end();
|
||||||
}
|
}
|
||||||
|
contents.push_str(checker.locator().slice(TextRange::new(prev_end, 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
|
|
||||||
}
|
|
||||||
Tok::String { value, .. } => value.is_empty(),
|
|
||||||
_ => false,
|
|
||||||
})
|
|
||||||
{
|
|
||||||
contents.push_str(rest);
|
|
||||||
}
|
|
||||||
|
|
||||||
// If necessary, add a space between any leading keyword (`return`, `yield`, `assert`, etc.)
|
// 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.
|
// and the string. For example, `return"foo"` is valid, but `returnf"foo"` is not.
|
||||||
|
|
|
@ -767,16 +767,16 @@ UP032_0.py:86:1: UP032 [*] Use f-string instead of `format` call
|
||||||
85 85 |
|
85 85 |
|
||||||
86 86 | (
|
86 86 | (
|
||||||
87 |- "{a}"
|
87 |- "{a}"
|
||||||
87 |+ f"{1}"
|
88 |- ""
|
||||||
88 88 | ""
|
|
||||||
89 |- "{b}"
|
89 |- "{b}"
|
||||||
89 |+ f"{1}"
|
90 |- ""
|
||||||
90 90 | ""
|
|
||||||
91 |-).format(a=1, b=1)
|
91 |-).format(a=1, b=1)
|
||||||
91 |+)
|
87 |+ f"{1}"
|
||||||
92 92 |
|
88 |+ f"{1}"
|
||||||
93 93 | (
|
89 |+)
|
||||||
94 94 | (
|
92 90 |
|
||||||
|
93 91 | (
|
||||||
|
94 92 | (
|
||||||
|
|
||||||
UP032_0.py:94:5: UP032 [*] Use f-string instead of `format` call
|
UP032_0.py:94:5: UP032 [*] Use f-string instead of `format` call
|
||||||
|
|
|
|
||||||
|
@ -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.
|
211 211 | # When fixing, trim the trailing empty string.
|
||||||
212 |-raise ValueError("Conflicting configuration dicts: {!r} {!r}"
|
212 |-raise ValueError("Conflicting configuration dicts: {!r} {!r}"
|
||||||
213 |- "".format(new_dict, d))
|
213 |- "".format(new_dict, d))
|
||||||
212 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}"
|
212 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}")
|
||||||
213 |+ "")
|
214 213 |
|
||||||
214 214 |
|
215 214 | # When fixing, trim the trailing empty string.
|
||||||
215 215 | # When fixing, trim the trailing empty string.
|
216 215 | raise ValueError("Conflicting configuration dicts: {!r} {!r}"
|
||||||
216 216 | raise ValueError("Conflicting configuration dicts: {!r} {!r}"
|
|
||||||
|
|
||||||
UP032_0.py:216:18: UP032 [*] Use f-string instead of `format` call
|
UP032_0.py:216:18: UP032 [*] Use f-string instead of `format` call
|
||||||
|
|
|
|
||||||
|
@ -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.
|
215 215 | # When fixing, trim the trailing empty string.
|
||||||
216 |-raise ValueError("Conflicting configuration dicts: {!r} {!r}"
|
216 |-raise ValueError("Conflicting configuration dicts: {!r} {!r}"
|
||||||
217 |- .format(new_dict, d))
|
217 |- .format(new_dict, d))
|
||||||
216 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}")
|
216 |+raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}"
|
||||||
218 217 |
|
217 |+ )
|
||||||
219 218 | raise ValueError(
|
218 218 |
|
||||||
220 219 | "Conflicting configuration dicts: {!r} {!r}"
|
219 219 | raise ValueError(
|
||||||
|
220 220 | "Conflicting configuration dicts: {!r} {!r}"
|
||||||
|
|
||||||
UP032_0.py:220:5: UP032 [*] Use f-string instead of `format` call
|
UP032_0.py:220:5: UP032 [*] Use f-string instead of `format` call
|
||||||
|
|
|
|
||||||
|
@ -1136,10 +1136,9 @@ UP032_0.py:220:5: UP032 [*] Use f-string instead of `format` call
|
||||||
220 |- "Conflicting configuration dicts: {!r} {!r}"
|
220 |- "Conflicting configuration dicts: {!r} {!r}"
|
||||||
221 |- "".format(new_dict, d)
|
221 |- "".format(new_dict, d)
|
||||||
220 |+ f"Conflicting configuration dicts: {new_dict!r} {d!r}"
|
220 |+ f"Conflicting configuration dicts: {new_dict!r} {d!r}"
|
||||||
221 |+ ""
|
222 221 | )
|
||||||
222 222 | )
|
223 222 |
|
||||||
223 223 |
|
224 223 | raise ValueError(
|
||||||
224 224 | raise ValueError(
|
|
||||||
|
|
||||||
UP032_0.py:225:5: UP032 [*] Use f-string instead of `format` call
|
UP032_0.py:225:5: UP032 [*] Use f-string instead of `format` call
|
||||||
|
|
|
|
||||||
|
@ -1160,10 +1159,9 @@ UP032_0.py:225:5: UP032 [*] Use f-string instead of `format` call
|
||||||
225 |- "Conflicting configuration dicts: {!r} {!r}"
|
225 |- "Conflicting configuration dicts: {!r} {!r}"
|
||||||
226 |- "".format(new_dict, d)
|
226 |- "".format(new_dict, d)
|
||||||
225 |+ f"Conflicting configuration dicts: {new_dict!r} {d!r}"
|
225 |+ f"Conflicting configuration dicts: {new_dict!r} {d!r}"
|
||||||
226 |+ ""
|
227 226 |
|
||||||
227 227 |
|
228 227 | )
|
||||||
228 228 | )
|
229 228 |
|
||||||
229 229 |
|
|
||||||
|
|
||||||
UP032_0.py:231:1: UP032 [*] Use f-string instead of `format` call
|
UP032_0.py:231:1: UP032 [*] Use f-string instead of `format` call
|
||||||
|
|
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue