Skip to content

Commit 4203658

Browse files
authored
Fix joining of f-strings with different quotes when using quote style Preserve (#15524)
1 parent fc9dd63 commit 4203658

File tree

3 files changed

+55
-11
lines changed

3 files changed

+55
-11
lines changed

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/join_implicit_concatenated_string_preserve.py

+9
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,12 @@
1111

1212
# Already invalid Pre Python 312
1313
f"{'Hy "User"'}" f'{"Hy 'User'"}'
14+
15+
16+
# Regression tests for https://github.com/astral-sh/ruff/issues/15514
17+
params = {}
18+
string = "this is my string with " f'"{params.get("mine")}"'
19+
string = f'"{params.get("mine")} ' f"with {'nested single quoted string'}"
20+
string = f"{'''inner ' '''}" f'{"""inner " """}'
21+
string = f"{10 + len('bar')=}" f"{10 + len('bar')=}"
22+
string = f"{10 + len('bar')=}" f'{10 + len("bar")=}'

crates/ruff_python_formatter/src/string/normalize.rs

+19-11
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,33 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {
4444
let preferred_quote_style = self
4545
.preferred_quote_style
4646
.unwrap_or(self.context.options().quote_style());
47+
let supports_pep_701 = self.context.options().target_version().supports_pep_701();
4748

49+
// For f-strings prefer alternating the quotes unless The outer string is triple quoted and the inner isn't.
50+
if let FStringState::InsideExpressionElement(parent_context) = self.context.f_string_state()
51+
{
52+
let parent_flags = parent_context.f_string().flags();
53+
54+
if !parent_flags.is_triple_quoted() || string.flags().is_triple_quoted() {
55+
// This logic is even necessary when using preserve and the target python version doesn't support PEP701 because
56+
// we might end up joining two f-strings that have different quote styles, in which case we need to alternate the quotes
57+
// for inner strings to avoid a syntax error: `string = "this is my string with " f'"{params.get("mine")}"'`
58+
if !preferred_quote_style.is_preserve() || !supports_pep_701 {
59+
return QuoteStyle::from(parent_flags.quote_style().opposite());
60+
}
61+
}
62+
}
63+
64+
// Leave the quotes unchanged for all other strings.
4865
if preferred_quote_style.is_preserve() {
4966
return QuoteStyle::Preserve;
5067
}
5168

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

80-
// For f-strings prefer alternating the quotes unless The outer string is triple quoted and the inner isn't.
81-
if let FStringState::InsideExpressionElement(parent_context) = self.context.f_string_state()
82-
{
83-
let parent_flags = parent_context.f_string().flags();
84-
85-
if !parent_flags.is_triple_quoted() || string.flags().is_triple_quoted() {
86-
return QuoteStyle::from(parent_flags.quote_style().opposite());
87-
}
88-
}
89-
9098
// Per PEP 8, always prefer double quotes for triple-quoted strings.
9199
if string.flags().is_triple_quoted() {
92100
// ... unless we're formatting a code snippet inside a docstring,

crates/ruff_python_formatter/tests/snapshots/format@expression__join_implicit_concatenated_string_preserve.py.snap

+27
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ a = "different '" 'quote "are fine"' # join
1717
1818
# Already invalid Pre Python 312
1919
f"{'Hy "User"'}" f'{"Hy 'User'"}'
20+
21+
22+
# Regression tests for https://github.com/astral-sh/ruff/issues/15514
23+
params = {}
24+
string = "this is my string with " f'"{params.get("mine")}"'
25+
string = f'"{params.get("mine")} ' f"with {'nested single quoted string'}"
26+
string = f"{'''inner ' '''}" f'{"""inner " """}'
27+
string = f"{10 + len('bar')=}" f"{10 + len('bar')=}"
28+
string = f"{10 + len('bar')=}" f'{10 + len("bar")=}'
2029
```
2130
2231
## Outputs
@@ -49,6 +58,15 @@ a = "different 'quote \"are fine\"" # join
4958
5059
# Already invalid Pre Python 312
5160
f"{'Hy "User"'}{"Hy 'User'"}"
61+
62+
63+
# Regression tests for https://github.com/astral-sh/ruff/issues/15514
64+
params = {}
65+
string = f"this is my string with \"{params.get('mine')}\""
66+
string = f'"{params.get("mine")} with {"nested single quoted string"}'
67+
string = f"{'''inner ' '''}" f'{"""inner " """}'
68+
string = f"{10 + len('bar')=}{10 + len('bar')=}"
69+
string = f"{10 + len('bar')=}" f'{10 + len("bar")=}'
5270
```
5371
5472
@@ -81,4 +99,13 @@ a = "different 'quote \"are fine\"" # join
8199
82100
# Already invalid Pre Python 312
83101
f"{'Hy "User"'}{"Hy 'User'"}"
102+
103+
104+
# Regression tests for https://github.com/astral-sh/ruff/issues/15514
105+
params = {}
106+
string = f"this is my string with \"{params.get("mine")}\""
107+
string = f'"{params.get("mine")} with {'nested single quoted string'}'
108+
string = f"{'''inner ' '''}{"""inner " """}"
109+
string = f"{10 + len('bar')=}{10 + len('bar')=}"
110+
string = f"{10 + len('bar')=}{10 + len("bar")=}"
84111
```

0 commit comments

Comments
 (0)