Skip to content

Commit

Permalink
Prevent f-string merge quote changes with nested quotes (#4498)
Browse files Browse the repository at this point in the history
  • Loading branch information
MeGaGiGaGon authored Dec 4, 2024
1 parent e54f86b commit 3fab5ad
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

<!-- Changes that affect Black's preview style -->

- Fix/remove string merging changing f-string quotes on f-strings with internal quotes
(#4498)
- Remove parentheses around sole list items (#4312)
- Collapse multiple empty lines after an import into one (#4489)

Expand Down
9 changes: 5 additions & 4 deletions docs/the_black_code_style/future_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,11 @@ foo(

_Black_ will split long string literals and merge short ones. Parentheses are used where
appropriate. When split, parts of f-strings that don't need formatting are converted to
plain strings. User-made splits are respected when they do not exceed the line length
limit. Line continuation backslashes are converted into parenthesized strings.
Unnecessary parentheses are stripped. The stability and status of this feature is
tracked in [this issue](https://github.com/psf/black/issues/2188).
plain strings. f-strings will not be merged if they contain internal quotes and it would
change their quotation mark style. User-made splits are respected when they do not
exceed the line length limit. Line continuation backslashes are converted into
parenthesized strings. Unnecessary parentheses are stripped. The stability and status of
this feature istracked in [this issue](https://github.com/psf/black/issues/2188).

(labels/wrap-long-dict-values)=

Expand Down
17 changes: 17 additions & 0 deletions src/black/trans.py
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,8 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]:
- The set of all string prefixes in the string group is of
length greater than one and is not equal to {"", "f"}.
- The string group consists of raw strings.
- The string group would merge f-strings with different quote types
and internal quotes.
- The string group is stringified type annotations. We don't want to
process stringified type annotations since pyright doesn't support
them spanning multiple string values. (NOTE: mypy, pytype, pyre do
Expand All @@ -820,6 +822,8 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]:

i += inc

QUOTE = line.leaves[string_idx].value[-1]

num_of_inline_string_comments = 0
set_of_prefixes = set()
num_of_strings = 0
Expand All @@ -842,6 +846,19 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]:

set_of_prefixes.add(prefix)

if (
"f" in prefix
and leaf.value[-1] != QUOTE
and (
"'" in leaf.value[len(prefix) + 1 : -1]
or '"' in leaf.value[len(prefix) + 1 : -1]
)
):
return TErr(
"StringMerger does NOT merge f-strings with different quote types"
"and internal quotes."
)

if id(leaf) in line.comments:
num_of_inline_string_comments += 1
if contains_pragma_comment(line.comments[id(leaf)]):
Expand Down
2 changes: 2 additions & 0 deletions tests/data/cases/preview_fstring.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# flags: --unstable
f"{''=}" f'{""=}'
6 changes: 3 additions & 3 deletions tests/data/cases/preview_long_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ def foo():

log.info(
"Skipping:"
f" {desc['db_id']} {foo('bar',x=123)} {'foo' != 'bar'} {(x := 'abc=')} {pos_share=} {desc['status']} {desc['exposure_max']}"
f' {desc["db_id"]} {foo("bar",x=123)} {"foo" != "bar"} {(x := "abc=")} {pos_share=} {desc["status"]} {desc["exposure_max"]}'
)

log.info(
Expand All @@ -902,7 +902,7 @@ def foo():

log.info(
"Skipping:"
f" {'a' == 'b' == 'c' == 'd'} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']} {desc['exposure_max']}"
f' {"a" == "b" == "c" == "d"} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}'
)

log.info(
Expand All @@ -925,4 +925,4 @@ def foo():

log.info(
f"""Skipping: {'a' == 'b'} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']} {desc['exposure_max']}"""
)
)
13 changes: 10 additions & 3 deletions tests/data/cases/preview_long_strings__regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ async def foo(self):
}

# Regression test for https://github.com/psf/black/issues/3506.
# Regressed again by https://github.com/psf/black/pull/4498
s = (
"With single quote: ' "
f" {my_dict['foo']}"
Expand Down Expand Up @@ -1239,9 +1240,15 @@ async def foo(self):
}

# Regression test for https://github.com/psf/black/issues/3506.
s = f"With single quote: ' {my_dict['foo']} With double quote: \" {my_dict['bar']}"
# Regressed again by https://github.com/psf/black/pull/4498
s = (
"With single quote: ' "
f" {my_dict['foo']}"
' With double quote: " '
f' {my_dict["bar"]}'
)

s = (
"Lorem Ipsum is simply dummy text of the printing and typesetting"
f" industry:'{my_dict['foo']}'"
)
f' industry:\'{my_dict["foo"]}\''
)

0 comments on commit 3fab5ad

Please sign in to comment.