Skip to content

Commit

Permalink
Fix an invalid quote escaping bug in f-string expressions (#3509)
Browse files Browse the repository at this point in the history
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."
         )
```
  • Loading branch information
yilei committed Jan 22, 2023
1 parent eabff67 commit a36878e
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGES.md
Expand Up @@ -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)
- 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

Expand Down
30 changes: 30 additions & 0 deletions src/black/trans.py
Expand Up @@ -572,6 +572,12 @@ def make_naked(string: str, string_prefix: str) -> str:
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]
Expand Down Expand Up @@ -1240,6 +1246,30 @@ def fstring_contains_expr(s: str) -> bool:
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
Expand Down
18 changes: 18 additions & 0 deletions tests/data/preview/long_strings__regression.py
Expand Up @@ -550,6 +550,16 @@ async def foo(self):
("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

Expand Down Expand Up @@ -1235,3 +1245,11 @@ async def foo(self):
# 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']}'"
)

0 comments on commit a36878e

Please sign in to comment.