Fix a string merging/split issue caused by standalone comments. (#3227)

Fixes #2734: a standalone comment causes strings to be merged into one far too long (and requiring two passes to do so).

Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com>

diff --git a/.github/workflows/diff_shades.yml b/.github/workflows/diff_shades.yml
index 390089eca42a7067d6b0d91b899b3dfd2e56307f..fef9637c92f59d2653da25bfb6312f66c41b1c7c 100644 (file)
--- 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'

diff --git a/CHANGES.md b/CHANGES.md
index cae232684bd5bdc10d8ef96cd92075d51b5a04ed..39db0fb95b81341e4ea01bc8ccbbf2f2b55584ab 100644 (file)
--- 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 dc9c5520d5bdfcaf7e56bd8c93fae537e779f0ed..9e0284cefe3735b04b4b9ae472e4797d1ea4cb83 100644 (file)
--- a/src/black/trans.py
+++ b/src/black/trans.py
@@ -553,6 +553,9 @@ class StringMerger(StringTransformer, CustomSplitMapMixin):
 
             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,18 @@ class StringMerger(StringTransformer, CustomSplitMapMixin):
         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 (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 = 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:
+                # 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 430f760cf0b9dc9a8490974198904b4ff01195ba..26400eea450f8b0b383f5b56511a783676e39e91 100644 (file)
--- a/tests/data/preview/long_strings.py
+++ b/tests/data/preview/long_strings.py
@@ -72,6 +72,25 @@ bad_split_func3(
     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 @@ bad_split_func3(
     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."