From 420365811f27d597ea33a62270667ce9cee1bb5f Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 16 Jan 2025 12:01:42 +0100 Subject: [PATCH] Fix joining of f-strings with different quotes when using quote style `Preserve` (#15524) --- ...n_implicit_concatenated_string_preserve.py | 9 ++++++ .../src/string/normalize.rs | 30 ++++++++++++------- ...licit_concatenated_string_preserve.py.snap | 27 +++++++++++++++++ 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/join_implicit_concatenated_string_preserve.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/join_implicit_concatenated_string_preserve.py index 2492be4703..c8d60d251a 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/join_implicit_concatenated_string_preserve.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/join_implicit_concatenated_string_preserve.py @@ -11,3 +11,12 @@ 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")=}' diff --git a/crates/ruff_python_formatter/src/string/normalize.rs b/crates/ruff_python_formatter/src/string/normalize.rs index 5632324511..f15fe89641 100644 --- a/crates/ruff_python_formatter/src/string/normalize.rs +++ b/crates/ruff_python_formatter/src/string/normalize.rs @@ -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. @@ -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, diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__join_implicit_concatenated_string_preserve.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__join_implicit_concatenated_string_preserve.py.snap index f14ff125f6..688efde6cd 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__join_implicit_concatenated_string_preserve.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__join_implicit_concatenated_string_preserve.py.snap @@ -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 @@ -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")=}' ``` @@ -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")=}" ```