Fix an invalid quote escaping bug in f-string expressions (#3509)

Fixes #3506

We can't simply escape the quotes in a naked f-string when merging string groups, because backslashes are invalid.

The quotes in f-string expressions should be toggled (this is safe since quotes can't be reused).

This fix also means implicitly concatenated f-strings with different quotes can now be merged or quote-normalized by changing the quotes used in expressions. e.g.:

```diff
         raise sa_exc.UnboundExecutionError(
             "Could not locate a bind configured on "
-            f'{", ".join(context)} or this Session.'
+            f"{', '.join(context)} or this Session."
         )
```

diff --git a/CHANGES.md b/CHANGES.md
index e2e4b341761eb679b21d8fbb990946bbac304c81..e311c492789ec410da87956072c7c64b48535139 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -39,6 +39,9 @@
 - Wrap multiple context managers in parentheses when targeting Python 3.9+ (#3489)
 - Fix several crashes in preview style with walrus operators used in `with` statements
   or tuples (#3473)
 - Wrap multiple context managers in parentheses when targeting Python 3.9+ (#3489)
 - Fix several crashes in preview style with walrus operators used in `with` statements
   or tuples (#3473)

+- Fix an invalid quote escaping bug in f-string expressions where it produced invalid
+  code. Implicitly concatenated f-strings with different quotes can now be merged or
+  quote-normalized by changing the quotes used in expressions. (#3509)

 
 ### Configuration
 
 
 ### Configuration
 

diff --git a/src/black/trans.py b/src/black/trans.py
index ec07f5eab74f03498f65d5b07624f59bf5e12fb5..2360c13f06a249e17c340a684d9f8a3e9eba02ce 100644 (file)
--- a/src/black/trans.py
+++ b/src/black/trans.py
@@ -572,6 +572,12 @@ class StringMerger(StringTransformer, CustomSplitMapMixin):
                 characters have been escaped.
             """
             assert_is_leaf_string(string)
                 characters have been escaped.
             """
             assert_is_leaf_string(string)

+            if "f" in string_prefix:
+                string = _toggle_fexpr_quotes(string, QUOTE)
+                # After quotes toggling, quotes in expressions won't be escaped
+                # because quotes can't be reused in f-strings. So we can simply
+                # let the escaping logic below run without knowing f-string
+                # expressions.

 
             RE_EVEN_BACKSLASHES = r"(?:(?<!\\)(?:\\\\)*)"
             naked_string = string[len(string_prefix) + 1 : -1]
 
             RE_EVEN_BACKSLASHES = r"(?:(?<!\\)(?:\\\\)*)"
             naked_string = string[len(string_prefix) + 1 : -1]


@@ -1240,6 +1246,30 @@ def fstring_contains_expr(s: str) -> bool:
     return any(iter_fexpr_spans(s))
 
 
     return any(iter_fexpr_spans(s))
 
 

+def _toggle_fexpr_quotes(fstring: str, old_quote: str) -> str:
+    """
+    Toggles quotes used in f-string expressions that are `old_quote`.
+
+    f-string expressions can't contain backslashes, so we need to toggle the
+    quotes if the f-string itself will end up using the same quote. We can
+    simply toggle without escaping because, quotes can't be reused in f-string
+    expressions. They will fail to parse.
+
+    NOTE: If PEP 701 is accepted, above statement will no longer be true.
+    Though if quotes can be reused, we can simply reuse them without updates or
+    escaping, once Black figures out how to parse the new grammar.
+    """
+    new_quote = "'" if old_quote == '"' else '"'
+    parts = []
+    previous_index = 0
+    for start, end in iter_fexpr_spans(fstring):
+        parts.append(fstring[previous_index:start])
+        parts.append(fstring[start:end].replace(old_quote, new_quote))
+        previous_index = end
+    parts.append(fstring[previous_index:])
+    return "".join(parts)
+
+

 class StringSplitter(BaseStringSplitter, CustomSplitMapMixin):
     """
     StringTransformer that splits "atom" strings (i.e. strings which exist on
 class StringSplitter(BaseStringSplitter, CustomSplitMapMixin):
     """
     StringTransformer that splits "atom" strings (i.e. strings which exist on

diff --git a/tests/data/preview/long_strings__regression.py b/tests/data/preview/long_strings__regression.py
index ef9007f4ce11b04f8d72204d876e72e03780f7aa..eead8c204a9fc1ce89dcc680c0bc40df5b064e94 100644 (file)
--- a/tests/data/preview/long_strings__regression.py
+++ b/tests/data/preview/long_strings__regression.py
@@ -550,6 +550,16 @@ a_dict = {
         ("item1", "item2", "item3"),
 }
 
         ("item1", "item2", "item3"),
 }
 

+# Regression test for https://github.com/psf/black/issues/3506.
+s = (
+    "With single quote: ' "
+    f" {my_dict['foo']}"
+    ' With double quote: " '
+    f' {my_dict["bar"]}'
+)
+
+s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry:\'{my_dict["foo"]}\''
+

 
 # output
 
 
 # output
 


@@ -1235,3 +1245,11 @@ a_dict = {
     # And there is a comment before the value
     ("item1", "item2", "item3"),
 }
     # And there is a comment before the value
     ("item1", "item2", "item3"),
 }

+
+# Regression test for https://github.com/psf/black/issues/3506.
+s = f"With single quote: '  {my_dict['foo']} With double quote: \"  {my_dict['bar']}"
+
+s = (
+    "Lorem Ipsum is simply dummy text of the printing and typesetting"
+    f" industry:'{my_dict['foo']}'"
+)