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."
         )
```
This commit is contained in:
Yilei "Dolee" Yang 2023-01-22 05:27:11 -08:00 committed by GitHub
parent eabff673b3
commit a36878eb2f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 0 deletions

View File

@ -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

View File

@ -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]
@ -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

View File

@ -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
@ -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']}'"
)