fix(rome_js_formatter): correctly format sequence expressions

crates/rome_js_formatter/src/js/expressions/sequence_expression.rs

@@ -1,60 +1,117 @@
 use crate::prelude::*;
-
-use crate::FormatNodeFields;
-use rome_js_syntax::{JsSequenceExpression, JsSequenceExpressionFields};
+use crate::{FormatElement, FormatNodeFields, FormatNodeRule, Formatter, JsFormatOptions};
+use rome_formatter::{concat_elements, group_elements, soft_block_indent, FormatResult};
+use rome_js_syntax::{JsSequenceExpression, JsSequenceExpressionFields, JsSyntaxKind};
 use rome_rowan::AstNode;
 
 impl FormatNodeFields<JsSequenceExpression> for FormatNodeRule<JsSequenceExpression> {
     fn format_fields(
         node: &JsSequenceExpression,
         formatter: &Formatter<JsFormatOptions>,
     ) -> FormatResult<FormatElement> {
-        let mut current = node.clone();
+        format_sequence_expression(node, formatter)
+    }
+}
+
+pub(crate) fn format_sequence_expression(
+    node: &JsSequenceExpression,
+    formatter: &Formatter<JsFormatOptions>,
+) -> FormatResult<FormatElement> {
+    let mut current = node.clone();
+    let parent = current.syntax().parent();
 
-        // Find the left most sequence expression
-        while let Some(sequence_expression) =
-            JsSequenceExpression::cast(current.left()?.syntax().clone())
-        {
-            current = sequence_expression;
+    let has_already_indentation = parent.map_or(false, |parent| {
+        // Return statement already does the indentation for us
+        // Arrow function body can't have a sequence expression unless it's parenthesized, otherwise
+        // would be a syntax error
+        if matches!(parent.kind(), JsSyntaxKind::JS_RETURN_STATEMENT) {
+            true
+        } else if matches!(parent.kind(), JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION) {
+            // In case we are inside a sequence expression, we have to go up a level and see the great parent.
+            // Arrow function body and return statements applying indentation for us, so we signal the
+            // sequence expression to not add other indentation levels
+            let great_parent = parent.parent().map(|gp| gp.kind());
+
+            matches!(
+                great_parent,
+                Some(
+                    JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION
+                        | JsSyntaxKind::JS_RETURN_STATEMENT
+                        | JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER
+                )
+            )
+        } else {
+            false
         }
+    });
+
+    // Find the left most sequence expression
+    while let Some(sequence_expression) =
+        JsSequenceExpression::cast(current.left()?.syntax().clone())
+    {
+        current = sequence_expression;
+    }
+
+    // Format the left most sequence expression
+    let JsSequenceExpressionFields {
+        left,
+        comma_token,
+        right,
+    } = current.as_fields();
 
-        // Format the left most sequence expression
+    let mut formatted = vec![formatted![
+        formatter,
+        [left.format()?, comma_token.format()?]
+    ]?];
+
+    let mut previous_right = right;
+
+    // Traverse upwards again and concatenate the sequence expression until we find the first non-sequence expression
+    while let Some(parent_sequence) = current
+        .syntax()
+        .parent()
+        .and_then(JsSequenceExpression::cast)
+    {
         let JsSequenceExpressionFields {
-            left,
+            left: _left,
             comma_token,
             right,
-        } = current.as_fields();
+        } = parent_sequence.as_fields();
 
-        let mut formatted = vec![formatted![
-            formatter,
-            [
-                left.format(),
-                comma_token.format(),
-                space_token(),
-                right.format(),
-            ]
-        ]?];
-
-        // Traverse upwards again and concatenate the sequence expression until we find the first non-sequence expression
-        while let Some(parent) = current.syntax().parent() {
-            if let Some(parent_sequence) = JsSequenceExpression::cast(parent) {
-                let JsSequenceExpressionFields {
-                    left: _left,
-                    comma_token,
-                    right,
-                } = parent_sequence.as_fields();
-
-                formatted.push(formatted![
-                    formatter,
-                    [comma_token.format(), space_token(), right.format()]
-                ]?);
-
-                current = parent_sequence;
-            } else {
-                break;
-            }
+        if has_already_indentation {
+            formatted.push(formatted![
+                formatter,
+                [
+                    soft_line_break_or_space(),
+                    previous_right.format()?,
+                    comma_token.format()?,
+                ]
+            ]?);
+        } else {
+            formatted.push(soft_block_indent(formatted![
+                formatter,
+                [
+                    soft_line_break_or_space(),
+                    previous_right.format()?,
+                    comma_token.format()?,
+                ]
+            ]?))
         }
+        previous_right = right;
+        current = parent_sequence;
+    }
 
-        Ok(concat_elements(formatted))
+    if has_already_indentation {
+        formatted.push(formatted![
+            formatter,
+            [soft_line_break_or_space(), previous_right.format()?,]
+        ]?);
+    } else {
+        formatted.push(soft_block_indent(formatted![
+            formatter,
+            [soft_line_break_or_space(), previous_right.format()?,]
+        ]?))
     }
+
+    Ok(group_elements(concat_elements(formatted)))
 }

crates/rome_js_formatter/src/lib.rs

@@ -379,7 +379,9 @@ mod test {
     #[ignore]
     // use this test check if your snippet prints as you wish, without using a snapshot
     fn quick_test() {
-        let src = r#"xyz.a(b!).a(b!).a(b!)
+        let src = r#"(function(){
+	return aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName;
+});
 
 "#;
         let syntax = SourceType::jsx();

crates/rome_js_formatter/tests/specs/js/module/expression/sequence_expression.js

@@ -1 +1,22 @@
 a,b
+
+const f = () => (
+    ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+);
+
+(
+    ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+)
+
+function a() {
+    return     ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+}
+
+const object ={
+    something: (
+        ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+    )
+
+}

crates/rome_js_formatter/tests/specs/js/module/expression/sequence_expression.js.snap

@@ -1,10 +1,31 @@
 ---
 source: crates/rome_js_formatter/tests/spec_test.rs
+assertion_line: 244
 expression: sequence_expression.js
 ---
 # Input
 a,b
 
+const f = () => (
+    ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+);
+
+(
+    ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+)
+
+function a() {
+    return     ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+}
+
+const object ={
+    something: (
+        ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+    )
+
+}
 =============================
 # Outputs
 ## Output 1
@@ -15,3 +36,47 @@ Quote style: Double Quotes
 -----
 a, b;
 
+const f = () => (
+	____________first,
+	____________second,
+	____________third,
+	____________third,
+	____________third,
+	____________third,
+	____________third
+);
+
+(
+	____________first,
+		____________second,
+		____________third,
+		____________third,
+		____________third,
+		____________third,
+		____________third
+);
+
+function a() {
+	return (
+		____________first,
+		____________second,
+		____________third,
+		____________third,
+		____________third,
+		____________third,
+		____________third
+	);
+}
+
+const object = {
+	something: (
+		____________first,
+		____________second,
+		____________third,
+		____________third,
+		____________third,
+		____________third,
+		____________third
+	),
+};
+

crates/rome_js_formatter/tests/specs/prettier/js/objects/expression.js.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
+assertion_line: 181
 expression: expression.js
 ---
 # Input
@@ -57,7 +58,9 @@ const a2 = {
 
 const a3 = {
   someKey: (
-    longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName
+    longLongLongLongLongLongLongLongLongLongLongLongLongLongName,
+    longLongLongLongLongLongLongLongLongLongLongLongLongLongName,
+    longLongLongLongLongLongLongLongLongLongLongLongLongLongName
   ),
 };
 
@@ -80,8 +83,4 @@ error[SyntaxError]: expected `')'` but instead found `:`
 
 ```
 
-# Lines exceeding max width of 80 characters
-```
-   25:     longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName
-```