diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs index a8a520e90351da..31a23832743495 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs @@ -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, @@ -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. @@ -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()); @@ -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, @@ -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, @@ -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. diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap index ca151a4a6ac235..e5eb8940540144 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap @@ -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 | @@ -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 | @@ -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 | @@ -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 | @@ -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 |