Skip to content

Commit

Permalink
Fix joining of f-strings with different quotes when using quote style…
Browse files Browse the repository at this point in the history
… `Preserve` (#15524)
  • Loading branch information
MichaReiser authored Jan 16, 2025
1 parent fc9dd63 commit 4203658
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,12 @@

# Already invalid Pre Python 312
f"{'Hy "User"'}" f'{"Hy 'User'"}'


# Regression tests for https://github.com/astral-sh/ruff/issues/15514
params = {}
string = "this is my string with " f'"{params.get("mine")}"'
string = f'"{params.get("mine")} ' f"with {'nested single quoted string'}"
string = f"{'''inner ' '''}" f'{"""inner " """}'
string = f"{10 + len('bar')=}" f"{10 + len('bar')=}"
string = f"{10 + len('bar')=}" f'{10 + len("bar")=}'
30 changes: 19 additions & 11 deletions crates/ruff_python_formatter/src/string/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,33 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {
let preferred_quote_style = self
.preferred_quote_style
.unwrap_or(self.context.options().quote_style());
let supports_pep_701 = self.context.options().target_version().supports_pep_701();

// For f-strings prefer alternating the quotes unless The outer string is triple quoted and the inner isn't.
if let FStringState::InsideExpressionElement(parent_context) = self.context.f_string_state()
{
let parent_flags = parent_context.f_string().flags();

if !parent_flags.is_triple_quoted() || string.flags().is_triple_quoted() {
// This logic is even necessary when using preserve and the target python version doesn't support PEP701 because
// we might end up joining two f-strings that have different quote styles, in which case we need to alternate the quotes
// for inner strings to avoid a syntax error: `string = "this is my string with " f'"{params.get("mine")}"'`
if !preferred_quote_style.is_preserve() || !supports_pep_701 {
return QuoteStyle::from(parent_flags.quote_style().opposite());
}
}
}

// Leave the quotes unchanged for all other strings.
if preferred_quote_style.is_preserve() {
return QuoteStyle::Preserve;
}

// There are cases where it is necessary to preserve the quotes to prevent an invalid f-string.
if let StringLikePart::FString(fstring) = string {
// There are two cases where it's necessary to preserve the quotes if the
// target version is pre 3.12 and the part is an f-string.
if !self.context.options().target_version().supports_pep_701() {
if !supports_pep_701 {
// An f-string expression contains a debug text with a quote character
// because the formatter will emit the debug expression **exactly** the
// same as in the source text.
Expand All @@ -77,16 +95,6 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {
}
}

// For f-strings prefer alternating the quotes unless The outer string is triple quoted and the inner isn't.
if let FStringState::InsideExpressionElement(parent_context) = self.context.f_string_state()
{
let parent_flags = parent_context.f_string().flags();

if !parent_flags.is_triple_quoted() || string.flags().is_triple_quoted() {
return QuoteStyle::from(parent_flags.quote_style().opposite());
}
}

// Per PEP 8, always prefer double quotes for triple-quoted strings.
if string.flags().is_triple_quoted() {
// ... unless we're formatting a code snippet inside a docstring,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ a = "different '" 'quote "are fine"' # join
# Already invalid Pre Python 312
f"{'Hy "User"'}" f'{"Hy 'User'"}'
# Regression tests for https://github.com/astral-sh/ruff/issues/15514
params = {}
string = "this is my string with " f'"{params.get("mine")}"'
string = f'"{params.get("mine")} ' f"with {'nested single quoted string'}"
string = f"{'''inner ' '''}" f'{"""inner " """}'
string = f"{10 + len('bar')=}" f"{10 + len('bar')=}"
string = f"{10 + len('bar')=}" f'{10 + len("bar")=}'
```
## Outputs
Expand Down Expand Up @@ -49,6 +58,15 @@ a = "different 'quote \"are fine\"" # join
# Already invalid Pre Python 312
f"{'Hy "User"'}{"Hy 'User'"}"
# Regression tests for https://github.com/astral-sh/ruff/issues/15514
params = {}
string = f"this is my string with \"{params.get('mine')}\""
string = f'"{params.get("mine")} with {"nested single quoted string"}'
string = f"{'''inner ' '''}" f'{"""inner " """}'
string = f"{10 + len('bar')=}{10 + len('bar')=}"
string = f"{10 + len('bar')=}" f'{10 + len("bar")=}'
```
Expand Down Expand Up @@ -81,4 +99,13 @@ a = "different 'quote \"are fine\"" # join
# Already invalid Pre Python 312
f"{'Hy "User"'}{"Hy 'User'"}"
# Regression tests for https://github.com/astral-sh/ruff/issues/15514
params = {}
string = f"this is my string with \"{params.get("mine")}\""
string = f'"{params.get("mine")} with {'nested single quoted string'}'
string = f"{'''inner ' '''}{"""inner " """}"
string = f"{10 + len('bar')=}{10 + len('bar')=}"
string = f"{10 + len('bar')=}{10 + len("bar")=}"
```

0 comments on commit 4203658

Please sign in to comment.