From 7f818522e7fbdbfa2d5ea11e3b5d6135db9030d3 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 16 Aug 2022 16:38:29 -0700 Subject: [PATCH 1/6] Fix a string merging/split issue caused by standalone comments. Fixes #2734: a standalone comment causes strings to be merged into one far too long (and requiring two passes to do so). --- CHANGES.md | 2 ++ src/black/trans.py | 17 ++++++++++++++- tests/data/preview/long_strings.py | 35 ++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index fb7a2723b67..32c75796368 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,6 +26,8 @@ normalized as expected (#3168) - When using `--skip-magic-trailing-comma` or `-C`, trailing commas are stripped from subscript expressions with more than 1 element (#3209) +- Fix a string merging/split issue when a comment is present in the middle of + implicitly concatenated strings on its own line (#3227) ### _Blackd_ diff --git a/src/black/trans.py b/src/black/trans.py index dc9c5520d5b..78f0f822737 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -553,6 +553,9 @@ def make_naked(string: str, string_prefix: str) -> str: next_str_idx += 1 + # Take a note on the index of the non-STRING leaf. + non_string_idx = next_str_idx + S_leaf = Leaf(token.STRING, S) if self.normalize_strings: S_leaf.value = normalize_string_quotes(S_leaf.value) @@ -572,7 +575,19 @@ def make_naked(string: str, string_prefix: str) -> str: string_leaf = Leaf(token.STRING, S_leaf.value.replace(BREAK_MARK, "")) if atom_node is not None: - replace_child(atom_node, string_leaf) + # If not all children of the atom node are merged... + if non_string_idx - string_idx < len(atom_node.children): + # We need to replace the old STRING leaves with the new string leaf. + first_child_idx = None + for idx in range(string_idx, non_string_idx): + child_idx = LL[idx].remove() + if first_child_idx is None: + first_child_idx = child_idx + if first_child_idx is not None: + atom_node.insert_child(first_child_idx, string_leaf) + else: + # Else replace the atom node with the new string leaf. + replace_child(atom_node, string_leaf) # Build the final line ('new_line') that this method will later return. new_line = line.clone() diff --git a/tests/data/preview/long_strings.py b/tests/data/preview/long_strings.py index 430f760cf0b..26400eea450 100644 --- a/tests/data/preview/long_strings.py +++ b/tests/data/preview/long_strings.py @@ -72,6 +72,25 @@ zzz, ) +inline_comments_func1( + "if there are inline " + "comments in the middle " + # Here is the standard alone comment. + "of the implicitly concatenated " + "string, we should handle " + "them correctly", + xxx, +) + +inline_comments_func2( + "what if the string is very very very very very very very very very very long and this part does " + "not fit into a single line? " + # Here is the standard alone comment. + "then the string should still be properly handled by merging and splitting " + "it into parts that fit in line length.", + xxx, +) + raw_string = r"This is a long raw string. When re-formatting this string, black needs to make sure it prepends the 'r' onto the new string." fmt_string1 = "We also need to be sure to preserve any and all {} which may or may not be attached to the string in question.".format("method calls") @@ -395,6 +414,22 @@ def foo(): zzz, ) +inline_comments_func1( + "if there are inline comments in the middle " + # Here is the standard alone comment. + "of the implicitly concatenated string, we should handle them correctly", + xxx, +) + +inline_comments_func2( + "what if the string is very very very very very very very very very very long and" + " this part does not fit into a single line? " + # Here is the standard alone comment. + "then the string should still be properly handled by merging and splitting " + "it into parts that fit in line length.", + xxx, +) + raw_string = ( r"This is a long raw string. When re-formatting this string, black needs to make" r" sure it prepends the 'r' onto the new string." From be2546a54490f8314f961a7b378ed9ee976b38c2 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 16 Aug 2022 16:41:16 -0700 Subject: [PATCH 2/6] Fix formatting. --- CHANGES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 32c75796368..4d3c23bf8c5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,8 +26,8 @@ normalized as expected (#3168) - When using `--skip-magic-trailing-comma` or `-C`, trailing commas are stripped from subscript expressions with more than 1 element (#3209) -- Fix a string merging/split issue when a comment is present in the middle of - implicitly concatenated strings on its own line (#3227) +- Fix a string merging/split issue when a comment is present in the middle of implicitly + concatenated strings on its own line (#3227) ### _Blackd_ From fe7d7e8f254c794ad410d377f265774b38835a7d Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 16 Aug 2022 16:43:33 -0700 Subject: [PATCH 3/6] Add a bit more comment. --- src/black/trans.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/black/trans.py b/src/black/trans.py index 78f0f822737..1fa866626cb 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -575,7 +575,8 @@ def make_naked(string: str, string_prefix: str) -> str: string_leaf = Leaf(token.STRING, S_leaf.value.replace(BREAK_MARK, "")) if atom_node is not None: - # If not all children of the atom node are merged... + # If not all children of the atom node are merged (this can happen + # when there is a standalone comment in the middle) ... if non_string_idx - string_idx < len(atom_node.children): # We need to replace the old STRING leaves with the new string leaf. first_child_idx = None From 231391ec2d7b8bd15c4cb78f52c531f1e9c8ca9c Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 16 Aug 2022 16:57:59 -0700 Subject: [PATCH 4/6] Fix mypyc error. --- 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 1fa866626cb..9ad7d7e6dd8 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -579,7 +579,7 @@ def make_naked(string: str, string_prefix: str) -> str: # when there is a standalone comment in the middle) ... if non_string_idx - string_idx < len(atom_node.children): # We need to replace the old STRING leaves with the new string leaf. - first_child_idx = None + first_child_idx: Optional[int] = None for idx in range(string_idx, non_string_idx): child_idx = LL[idx].remove() if first_child_idx is None: From f2f20d6aa198ccc9964b62622704eff89cd5b25e Mon Sep 17 00:00:00 2001 From: Richard Si <63936253+ichard26@users.noreply.github.com> Date: Mon, 22 Aug 2022 20:28:07 -0400 Subject: [PATCH 5/6] =?UTF-8?q?Whee=20this=20fixes=20a=20crash=20?= =?UTF-8?q?=F0=9F=8E=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/diff_shades.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/diff_shades.yml b/.github/workflows/diff_shades.yml index 390089eca42..fef9637c92f 100644 --- a/.github/workflows/diff_shades.yml +++ b/.github/workflows/diff_shades.yml @@ -155,4 +155,3 @@ jobs: if: always() run: > diff-shades show-failed --check --show-log ${{ matrix.target-analysis }} - --check-allow 'sqlalchemy:test/orm/test_relationship_criteria.py' From 987a5a7ae940d55153f1a51085e9f1e79b73c9de Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Mon, 22 Aug 2022 17:49:04 -0700 Subject: [PATCH 6/6] Simplify the implementation. --- src/black/trans.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/black/trans.py b/src/black/trans.py index 9ad7d7e6dd8..9e0284cefe3 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -579,11 +579,9 @@ def make_naked(string: str, string_prefix: str) -> str: # when there is a standalone comment in the middle) ... if non_string_idx - string_idx < len(atom_node.children): # We need to replace the old STRING leaves with the new string leaf. - first_child_idx: Optional[int] = None - for idx in range(string_idx, non_string_idx): - child_idx = LL[idx].remove() - if first_child_idx is None: - first_child_idx = child_idx + first_child_idx = LL[string_idx].remove() + for idx in range(string_idx + 1, non_string_idx): + LL[idx].remove() if first_child_idx is not None: atom_node.insert_child(first_child_idx, string_leaf) else: