Fix panic when formatting binary expression with two implicit concate…

…nated string operands (#7287)
astral-sh · Sep 12, 2023 · 08f1922 · 08f1922
1 parent 1e6df19
commit 08f1922
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 37 deletions.
diff --git a/...s/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_implicit_string.py b/...s/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_implicit_string.py
@@ -169,3 +169,23 @@ def test3():
      # test trailing operator comment
      b
      )
+
+c = ("a" "b" +
+     # test leading binary comment
+     "a" "b"
+ )
+
+(
+    b + c + d +
+    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+    "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +
+    "cccccccccccccccccccccccccc"
+    "dddddddddddddddddddddddddd"
+    % aaaaaaaaaaaa
+    + x
+)
+
+"a" "b" "c" + "d" "e" + "f" "g" + "h" "i" "j"
+class EC2REPATH:
+    f.write ("Pathway name" + "\t" "Database Identifier" + "\t" "Source database" + "\n")
+
diff --git a/crates/ruff_python_formatter/src/expression/binary_like.rs b/crates/ruff_python_formatter/src/expression/binary_like.rs
@@ -317,46 +317,58 @@ impl Format<PyFormatContext<'_>> for BinaryLike<'_> {
                     //     ^^^^^^ this part or ^^^^^^^ this part
                     // ```
                     if let Some(left_operator_index) = index.left_operator() {
-                        // Everything between the last implicit concatenated string and the left operator
-                        // right before the implicit concatenated string:
+                        // Handles the case where the left and right side of a binary expression are both
+                        // implicit concatenated strings. In this case, the left operator has already been written
+                        // by the preceding implicit concatenated string. It is only necessary to finish the group,
+                        // wrapping the soft line break and operator.
+                        //
                         // ```python
-                        // a + b + "c" "d"
-                        //       ^--- left_operator
-                        // ^^^^^-- left
+                        // "a" "b" + "c" "d"
                         // ```
-                        let left =
-                            flat_binary.between_operators(last_operator_index, left_operator_index);
-                        let left_operator = &flat_binary[left_operator_index];
-
-                        if let Some(leading) = left.first_operand().leading_binary_comments() {
-                            leading_comments(leading).fmt(f)?;
-                        }
-
-                        // Write the left, the left operator, and the space before the right side
-                        write!(
-                            f,
-                            [
-                                left,
-                                left.last_operand()
-                                    .trailing_binary_comments()
-                                    .map(trailing_comments),
-                                in_parentheses_only_soft_line_break_or_space(),
-                                left_operator,
-                            ]
-                        )?;
-
-                        // Finish the left-side group (the group was started before the loop or by the
-                        // previous iteration)
-                        write_in_parentheses_only_group_end_tag(f);
-
-                        if operand.has_unparenthesized_leading_comments(
-                            f.context().comments(),
-                            f.context().source(),
-                        ) || left_operator.has_trailing_comments()
-                        {
-                            hard_line_break().fmt(f)?;
+                        if last_operator_index == Some(left_operator_index) {
+                            write_in_parentheses_only_group_end_tag(f);
                         } else {
-                            space().fmt(f)?;
+                            // Everything between the last implicit concatenated string and the left operator
+                            // right before the implicit concatenated string:
+                            // ```python
+                            // a + b + "c" "d"
+                            //       ^--- left_operator
+                            // ^^^^^-- left
+                            // ```
+                            let left = flat_binary
+                                .between_operators(last_operator_index, left_operator_index);
+                            let left_operator = &flat_binary[left_operator_index];
+
+                            if let Some(leading) = left.first_operand().leading_binary_comments() {
+                                leading_comments(leading).fmt(f)?;
+                            }
+
+                            // Write the left, the left operator, and the space before the right side
+                            write!(
+                                f,
+                                [
+                                    left,
+                                    left.last_operand()
+                                        .trailing_binary_comments()
+                                        .map(trailing_comments),
+                                    in_parentheses_only_soft_line_break_or_space(),
+                                    left_operator,
+                                ]
+                            )?;
+
+                            // Finish the left-side group (the group was started before the loop or by the
+                            // previous iteration)
+                            write_in_parentheses_only_group_end_tag(f);
+
+                            if operand.has_unparenthesized_leading_comments(
+                                f.context().comments(),
+                                f.context().source(),
+                            ) || left_operator.has_trailing_comments()
+                            {
+                                hard_line_break().fmt(f)?;
+                            } else {
+                                space().fmt(f)?;
+                            }
                         }
 
                         write!(

diff --git a/...s/ruff_python_formatter/tests/snapshots/format@expression__binary_implicit_string.py.snap b/...s/ruff_python_formatter/tests/snapshots/format@expression__binary_implicit_string.py.snap
@@ -175,6 +175,26 @@ c = (a
      # test trailing operator comment
      b
      )
+
+c = ("a" "b" +
+     # test leading binary comment
+     "a" "b"
+ )
+
+(
+    b + c + d +
+    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+    "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +
+    "cccccccccccccccccccccccccc"
+    "dddddddddddddddddddddddddd"
+    % aaaaaaaaaaaa
+    + x
+)
+
+"a" "b" "c" + "d" "e" + "f" "g" + "h" "i" "j"
+class EC2REPATH:
+    f.write ("Pathway name" + "\t" "Database Identifier" + "\t" "Source database" + "\n")
+
 ```
 
 ## Output
@@ -363,6 +383,26 @@ c = (
     # test trailing operator comment
     b
 )
+
+c = (
+    "a"
+    "b" +
+    # test leading binary comment
+    "a"
+    "b"
+)
+
+(
+    b + c + d + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+    "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccccccc"
+    "dddddddddddddddddddddddddd" % aaaaaaaaaaaa + x
+)
+
+"a" "b" "c" + "d" "e" + "f" "g" + "h" "i" "j"
+
+
+class EC2REPATH:
+    f.write("Pathway name" + "\t" "Database Identifier" + "\t" "Source database" + "\n")
 ```