From 0a710e0012e3748d64e5aebf0b20da7d29cac6a3 Mon Sep 17 00:00:00 2001 From: MeGaGiGaGon <107241144+megagigagon@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:10:20 -0700 Subject: [PATCH 1/5] Prevents string merges from changing f-string quotes --- src/black/trans.py | 8 ++++++++ tests/data/cases/preview_fstring.py | 2 ++ tests/data/cases/preview_long_strings.py | 14 -------------- .../cases/preview_long_strings__regression.py | 18 ------------------ 4 files changed, 10 insertions(+), 32 deletions(-) create mode 100644 tests/data/cases/preview_fstring.py diff --git a/src/black/trans.py b/src/black/trans.py index b44e3cdf0e7..81e2ac35785 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -806,6 +806,7 @@ 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 tyes. - 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 @@ -832,6 +833,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 @@ -854,6 +857,11 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]: set_of_prefixes.add(prefix) + if "f" in prefix and leaf.value[-1] != QUOTE: + return TErr( + "StringMerger does NOT merge f-strings with different quote types." + ) + if id(leaf) in line.comments: num_of_inline_string_comments += 1 if contains_pragma_comment(line.comments[id(leaf)]): diff --git a/tests/data/cases/preview_fstring.py b/tests/data/cases/preview_fstring.py new file mode 100644 index 00000000000..e0a3eacfa20 --- /dev/null +++ b/tests/data/cases/preview_fstring.py @@ -0,0 +1,2 @@ +# flags: --unstable +f"{''=}" f'{""=}' \ No newline at end of file diff --git a/tests/data/cases/preview_long_strings.py b/tests/data/cases/preview_long_strings.py index 86fa1b0c7e1..79d2b8c6854 100644 --- a/tests/data/cases/preview_long_strings.py +++ b/tests/data/cases/preview_long_strings.py @@ -309,16 +309,12 @@ def foo(): log.info(f"Skipping: {desc['db_id']=} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']=} {desc['exposure_max']=}") -log.info(f'Skipping: {desc["db_id"]} {foo("bar",x=123)} {"foo" != "bar"} {(x := "abc=")} {pos_share=} {desc["status"]} {desc["exposure_max"]}') - log.info(f'Skipping: {desc["db_id"]} {desc["ms_name"]} {money=} {(x := "abc=")=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') log.info(f'Skipping: {desc["db_id"]} {foo("bar",x=123)=} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') log.info(f'Skipping: {foo("asdf")=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') -log.info(f'Skipping: {"a" == "b" == "c" == "d"} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') - log.info(f'Skipping: {"a" == "b" == "c" == "d"=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') log.info(f'Skipping: { longer_longer_longer_longer_longer_longer_name [ "db_id" ] [ "another_key" ] = : .3f }') @@ -880,11 +876,6 @@ def foo(): f" {desc['db_id']=} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']=} {desc['exposure_max']=}" ) -log.info( - "Skipping:" - f" {desc['db_id']} {foo('bar',x=123)} {'foo' != 'bar'} {(x := 'abc=')} {pos_share=} {desc['status']} {desc['exposure_max']}" -) - log.info( "Skipping:" f' {desc["db_id"]} {desc["ms_name"]} {money=} {(x := "abc=")=} {pos_share=} {desc["status"]} {desc["exposure_max"]}' @@ -900,11 +891,6 @@ def foo(): f' {foo("asdf")=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}' ) -log.info( - "Skipping:" - f" {'a' == 'b' == 'c' == 'd'} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']} {desc['exposure_max']}" -) - log.info( "Skipping:" f' {"a" == "b" == "c" == "d"=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}' diff --git a/tests/data/cases/preview_long_strings__regression.py b/tests/data/cases/preview_long_strings__regression.py index afe2b311cf4..e4b726c543d 100644 --- a/tests/data/cases/preview_long_strings__regression.py +++ b/tests/data/cases/preview_long_strings__regression.py @@ -551,16 +551,6 @@ 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 @@ -1237,11 +1227,3 @@ 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']}'" -) From c2437e7fa0666e0981f9b1fda029c8501e9b78ff Mon Sep 17 00:00:00 2001 From: MeGaGiGaGon <107241144+megagigagon@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:42:22 -0700 Subject: [PATCH 2/5] Changelog and docs updates --- CHANGES.md | 2 ++ docs/the_black_code_style/future_style.md | 9 +++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e2cc70f1661..c68c3cf7ffd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,6 +17,8 @@ +- Fix/remove string merging changing f-string quotes (#4498) + ### Configuration diff --git a/docs/the_black_code_style/future_style.md b/docs/the_black_code_style/future_style.md index cd4fb12bd51..309cbfe455c 100644 --- a/docs/the_black_code_style/future_style.md +++ b/docs/the_black_code_style/future_style.md @@ -132,10 +132,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 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)= From 7bb170e5bc27ce730e54bc8e668fe991240e4dc9 Mon Sep 17 00:00:00 2001 From: MeGaGiGaGon <107241144+megagigagon@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:43:42 -0700 Subject: [PATCH 3/5] typo fix --- src/black/trans.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/trans.py b/src/black/trans.py index 81e2ac35785..55c72ac0d08 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -806,7 +806,7 @@ 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 tyes. + - The string group would merge f-strings with different quote types. - 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 From 559de44fed15c3953e8949281e059cac0f6d0027 Mon Sep 17 00:00:00 2001 From: MeGaGiGaGon <107241144+MeGaGiGaGon@users.noreply.github.com> Date: Thu, 24 Oct 2024 08:53:39 -0700 Subject: [PATCH 4/5] Update outdated sentences in trans.py --- src/black/trans.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/black/trans.py b/src/black/trans.py index 5781d3240f2..3886097de79 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -794,7 +794,7 @@ 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. + - The string group merge would change f-string quote style. - 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 @@ -847,7 +847,7 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]: if "f" in prefix and leaf.value[-1] != QUOTE: return TErr( - "StringMerger does NOT merge f-strings with different quote types." + "StringMerger does NOT change f-string quote style on merge." ) if id(leaf) in line.comments: From bcfb0fd00300934e6c682836ecbd6757962fcd62 Mon Sep 17 00:00:00 2001 From: MeGaGiGaGon <107241144+megagigagon@users.noreply.github.com> Date: Tue, 5 Nov 2024 15:18:54 -0800 Subject: [PATCH 5/5] Relax condition for rejecting string merge --- CHANGES.md | 3 ++- docs/the_black_code_style/future_style.md | 10 ++++---- src/black/trans.py | 15 ++++++++--- tests/data/cases/preview_long_strings.py | 16 +++++++++++- .../cases/preview_long_strings__regression.py | 25 +++++++++++++++++++ 5 files changed, 59 insertions(+), 10 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c68c3cf7ffd..22af100099d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,7 +17,8 @@ -- Fix/remove string merging changing f-string quotes (#4498) +- Fix/remove string merging changing f-string quotes on f-strings with internal quotes + (#4498) ### Configuration diff --git a/docs/the_black_code_style/future_style.md b/docs/the_black_code_style/future_style.md index 309cbfe455c..262a1cd2afd 100644 --- a/docs/the_black_code_style/future_style.md +++ b/docs/the_black_code_style/future_style.md @@ -132,11 +132,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. f-strings will not be merged if 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). +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)= diff --git a/src/black/trans.py b/src/black/trans.py index 55c72ac0d08..ff1ea68a640 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -806,7 +806,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. + - 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 @@ -857,9 +858,17 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]: set_of_prefixes.add(prefix) - if "f" in prefix and leaf.value[-1] != QUOTE: + 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." + "StringMerger does NOT merge f-strings with different quote types" + "and internal quotes." ) if id(leaf) in line.comments: diff --git a/tests/data/cases/preview_long_strings.py b/tests/data/cases/preview_long_strings.py index 79d2b8c6854..ba48c19d542 100644 --- a/tests/data/cases/preview_long_strings.py +++ b/tests/data/cases/preview_long_strings.py @@ -309,12 +309,16 @@ def foo(): log.info(f"Skipping: {desc['db_id']=} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']=} {desc['exposure_max']=}") +log.info(f'Skipping: {desc["db_id"]} {foo("bar",x=123)} {"foo" != "bar"} {(x := "abc=")} {pos_share=} {desc["status"]} {desc["exposure_max"]}') + log.info(f'Skipping: {desc["db_id"]} {desc["ms_name"]} {money=} {(x := "abc=")=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') log.info(f'Skipping: {desc["db_id"]} {foo("bar",x=123)=} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') log.info(f'Skipping: {foo("asdf")=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') +log.info(f'Skipping: {"a" == "b" == "c" == "d"} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') + log.info(f'Skipping: {"a" == "b" == "c" == "d"=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}') log.info(f'Skipping: { longer_longer_longer_longer_longer_longer_name [ "db_id" ] [ "another_key" ] = : .3f }') @@ -876,6 +880,11 @@ def foo(): f" {desc['db_id']=} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']=} {desc['exposure_max']=}" ) +log.info( + "Skipping:" + f' {desc["db_id"]} {foo("bar",x=123)} {"foo" != "bar"} {(x := "abc=")} {pos_share=} {desc["status"]} {desc["exposure_max"]}' +) + log.info( "Skipping:" f' {desc["db_id"]} {desc["ms_name"]} {money=} {(x := "abc=")=} {pos_share=} {desc["status"]} {desc["exposure_max"]}' @@ -891,6 +900,11 @@ def foo(): f' {foo("asdf")=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}' ) +log.info( + "Skipping:" + f' {"a" == "b" == "c" == "d"} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}' +) + log.info( "Skipping:" f' {"a" == "b" == "c" == "d"=} {desc["ms_name"]} {money=} {dte=} {pos_share=} {desc["status"]} {desc["exposure_max"]}' @@ -911,4 +925,4 @@ def foo(): log.info( f"""Skipping: {'a' == 'b'} {desc['ms_name']} {money=} {dte=} {pos_share=} {desc['status']} {desc['exposure_max']}""" -) +) \ No newline at end of file diff --git a/tests/data/cases/preview_long_strings__regression.py b/tests/data/cases/preview_long_strings__regression.py index e4b726c543d..123342f575c 100644 --- a/tests/data/cases/preview_long_strings__regression.py +++ b/tests/data/cases/preview_long_strings__regression.py @@ -551,6 +551,17 @@ async def foo(self): ("item1", "item2", "item3"), } +# 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']}" + ' 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 @@ -1227,3 +1238,17 @@ 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. +# 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"]}\'' +) \ No newline at end of file