feat(rome_js_formatter): Heuristic for fill arrays

crates/rome_js_formatter/src/js/declarations/for_variable_declaration.rs

@@ -1,6 +1,6 @@
 use crate::prelude::*;
 
-use rome_formatter::write;
+use rome_formatter::{format_args, write};
 use rome_js_syntax::JsForVariableDeclaration;
 use rome_js_syntax::JsForVariableDeclarationFields;
 
@@ -14,6 +14,13 @@ impl FormatNodeRule<JsForVariableDeclaration> for FormatJsForVariableDeclaration
             declarator,
         } = node.as_fields();
 
-        write![f, [kind_token.format(), space(), declarator.format(),]]
+        write![
+            f,
+            [group(&format_args![
+                kind_token.format(),
+                space(),
+                declarator.format()
+            ])]
+        ]
     }
 }

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

@@ -2,8 +2,11 @@ use crate::prelude::*;
 
 use crate::parentheses::NeedsParentheses;
 use rome_formatter::write;
-use rome_js_syntax::JsArrayExpressionFields;
+use rome_js_syntax::{
+    JsAnyArrayElement, JsAnyExpression, JsArrayElementList, JsArrayExpressionFields,
+};
 use rome_js_syntax::{JsArrayExpression, JsSyntaxNode};
+use rome_rowan::SyntaxResult;
 
 #[derive(Debug, Clone, Default)]
 pub struct FormatJsArrayExpression;
@@ -16,24 +19,78 @@ impl FormatNodeRule<JsArrayExpression> for FormatJsArrayExpression {
             r_brack_token,
         } = node.as_fields();
 
-        let group_id = f.group_id("array");
+        if should_break(&elements)? {
+            write!(
+                f,
+                [
+                    format_delimited(&l_brack_token?, &elements.format(), &r_brack_token?)
+                        .block_indent()
+                ]
+            )
+        } else {
+            let group_id = f.group_id("array");
 
-        let elements = elements.format().with_options(Some(group_id));
+            let elements = elements.format().with_options(Some(group_id));
 
-        write!(
-            f,
-            [
-                format_delimited(&l_brack_token?, &elements, &r_brack_token?)
-                    .soft_block_indent_with_group_id(Some(group_id))
-            ]
-        )
+            write!(
+                f,
+                [
+                    format_delimited(&l_brack_token?, &elements, &r_brack_token?)
+                        .soft_block_indent_with_group_id(Some(group_id))
+                ]
+            )
+        }
     }
 
     fn needs_parentheses(&self, item: &JsArrayExpression) -> bool {
         item.needs_parentheses()
     }
 }
 
+/// Returns `true` for arrays containing at least two elements if:
+/// * all elements are either object or array expressions
+/// * each child array expression has at least two elements, or each child object expression has at least two members.
+fn should_break(elements: &JsArrayElementList) -> SyntaxResult<bool> {
+    if elements.len() < 2 {
+        Ok(false)
+    } else {
+        let mut elements = elements.iter().peekable();
+
+        while let Some(element) = elements.next() {
+            match element? {
+                JsAnyArrayElement::JsAnyExpression(JsAnyExpression::JsArrayExpression(array)) => {
+                    let next_is_array_or_end = matches!(
+                        elements.peek(),
+                        None | Some(Ok(JsAnyArrayElement::JsAnyExpression(
+                            JsAnyExpression::JsArrayExpression(_)
+                        )))
+                    );
+                    if array.elements().len() < 2 || !next_is_array_or_end {
+                        return Ok(false);
+                    }
+                }
+                JsAnyArrayElement::JsAnyExpression(JsAnyExpression::JsObjectExpression(object)) => {
+                    let next_is_object_or_empty = matches!(
+                        elements.peek(),
+                        None | Some(Ok(JsAnyArrayElement::JsAnyExpression(
+                            JsAnyExpression::JsObjectExpression(_)
+                        )))
+                    );
+
+                    if object.members().len() < 2 || !next_is_object_or_empty {
+                        return Ok(false);
+                    }
+                }
+                _ => {
+                    return Ok(false);
+                }
+            }
+        }
+
+        Ok(true)
+    }
+}
+
 impl NeedsParentheses for JsArrayExpression {
     #[inline(always)]
     fn needs_parentheses(&self) -> bool {

crates/rome_js_formatter/src/js/lists/array_element_list.rs

@@ -1,11 +1,10 @@
 use crate::prelude::*;
-use rome_formatter::{FormatRuleWithOptions, GroupId};
-use std::convert::Infallible;
+use rome_formatter::{write, FormatRuleWithOptions, GroupId};
 
 use crate::utils::array::write_array_node;
 
-use crate::utils::has_formatter_trivia;
-use rome_js_syntax::{JsAnyExpression, JsArrayElementList, JsSyntaxKind};
+use crate::utils::{has_token_trailing_line_comment, has_trailing_line_comment};
+use rome_js_syntax::{JsArrayElementList, JsSyntaxKind};
 use rome_rowan::{AstNode, AstSeparatedList};
 
 #[derive(Debug, Clone, Default)]
@@ -26,22 +25,64 @@ impl FormatRule<JsArrayElementList> for FormatJsArrayElementList {
     type Context = JsFormatContext;
 
     fn fmt(&self, node: &JsArrayElementList, f: &mut JsFormatter) -> FormatResult<()> {
-        if !has_formatter_trivia(node.syntax()) && can_print_fill(node) {
-            // Using format_separated is valid in this case as can_print_fill does not allow holes
-            return f
-                .fill()
-                .entries(
-                    &soft_line_break_or_space(),
+        let layout = if can_print_fill(node) {
+            ArrayLayout::Fill
+        } else {
+            ArrayLayout::OnePerLine
+        };
+
+        match layout {
+            ArrayLayout::Fill => {
+                let mut filler = f.fill();
+
+                // Using format_separated is valid in this case as can_print_fill does not allow holes
+                for (element, formatted) in node.iter().zip(
                     node.format_separated(JsSyntaxKind::COMMA)
                         .with_group_id(self.group_id),
-                )
-                .finish();
-        }
+                ) {
+                    filler.entry(
+                        &format_once(|f| {
+                            if get_lines_before(element?.syntax()) > 1 {
+                                write!(f, [empty_line()])
+                            } else {
+                                write!(f, [soft_line_break_or_space()])
+                            }
+                        }),
+                        &formatted,
+                    );
+                }
 
-        write_array_node(node, f)
+                filler.finish()
+            }
+            ArrayLayout::OnePerLine => write_array_node(node, f),
+        }
     }
 }
 
+#[derive(Copy, Clone)]
+enum ArrayLayout {
+    /// Tries to fit as many array elements on a single line as possible.
+    ///
+    /// ```javascript
+    /// [
+    ///     1, 2, 3,
+    ///     5, 6,
+    /// ]
+    /// ```
+    Fill,
+
+    /// Prints every element on a single line if the whole array expression exceeds the line width, or any
+    /// of its elements gets printed in *expanded* mode.
+    /// ```javascript
+    /// [
+    ///     a.b(),
+    ///     4,
+    ///     3,
+    /// ]
+    /// ```
+    OnePerLine,
+}
+
 /// Returns true if the provided JsArrayElementList could
 /// be "fill-printed" instead of breaking each element on
 /// a different line.
@@ -54,39 +95,58 @@ fn can_print_fill(list: &JsArrayElementList) -> bool {
     use rome_js_syntax::JsAnyExpression::*;
     use rome_js_syntax::JsUnaryOperator::*;
 
-    list.iter().all(|item| match item {
-        Ok(JsAnyExpression(JsUnaryExpression(expr))) => {
-            match expr.operator() {
-                Ok(Plus | Minus | BitwiseNot | LogicalNot) => {}
-                _ => return false,
+    if list.is_empty() {
+        return false;
+    }
+
+    list.elements().all(|item| {
+        let separator_has_trailing = item.trailing_separator().map_or(true, |separator| {
+            separator.map_or(false, |separator| {
+                has_token_trailing_line_comment(separator)
+            })
+        });
+
+        if separator_has_trailing {
+            return false;
+        }
+
+        let syntax = match item.into_node() {
+            Ok(JsAnyExpression(JsAnyLiteralExpression(
+                rome_js_syntax::JsAnyLiteralExpression::JsNumberLiteralExpression(literal),
+            ))) => literal.into_syntax(),
+
+            Ok(JsAnyExpression(JsUnaryExpression(expr))) => {
+                let signed = matches!(expr.operator(), Ok(Plus | Minus));
+                let argument = expr.argument();
+
+                match argument {
+                    Ok(JsAnyLiteralExpression(
+                        rome_js_syntax::JsAnyLiteralExpression::JsNumberLiteralExpression(literal),
+                    )) => {
+                        let has_operator_comments = expr
+                            .operator_token()
+                            .map_or(false, |operator| operator.has_trailing_comments());
+
+                        if signed
+                            && !literal.syntax().has_leading_comments()
+                            && !has_operator_comments
+                        {
+                            literal.into_syntax()
+                        } else {
+                            return false;
+                        }
+                    }
+                    _ => {
+                        return false;
+                    }
+                }
             }
 
-            if let Ok(expr) = expr.argument() {
-                is_short_literal(&expr)
-            } else {
-                false
+            _ => {
+                return false;
             }
-        }
-        Ok(JsAnyExpression(expr)) => is_short_literal(&expr),
-        _ => false,
-    })
-}
+        };
 
-/// Returns true if the provided expression is a literal with 10 or less characters
-fn is_short_literal(expr: &JsAnyExpression) -> bool {
-    match expr {
-        JsAnyExpression::JsAnyLiteralExpression(lit) => {
-            let token_len = lit
-                .syntax()
-                .text_trimmed()
-                .try_fold_chunks::<_, _, Infallible>(0, |sum, chunk| {
-                    // Count actual characters instead of byte length
-                    Ok(sum + chunk.chars().count())
-                })
-                .expect("the above fold operation is infallible");
-
-            token_len <= 10
-        }
-        _ => false,
-    }
+        !has_trailing_line_comment(&syntax)
+    })
 }

crates/rome_js_formatter/src/utils/binary_like_expression.rs

@@ -54,7 +54,8 @@
 //! and right side of each Right side.
 
 use crate::prelude::*;
-use rome_formatter::{format_args, write, Buffer, CommentStyle, CstFormatContext};
+use crate::utils::{has_token_trailing_line_comment, has_trailing_line_comment};
+use rome_formatter::{format_args, write, Buffer, CstFormatContext};
 use rome_js_syntax::{
     JsAnyExpression, JsAnyInProperty, JsBinaryExpression, JsBinaryOperator, JsDoWhileStatement,
     JsIfStatement, JsInExpression, JsInstanceofExpression, JsLogicalExpression, JsLogicalOperator,
@@ -66,7 +67,6 @@ use crate::parentheses::{
     is_arrow_function_body, is_callee, is_member_object, is_spread, is_tag, NeedsParentheses,
 };
 
-use crate::context::JsCommentStyle;
 use crate::js::expressions::static_member_expression::JsAnyStaticMemberLike;
 use crate::utils::assignment_like::has_leading_own_line_comment;
 use rome_rowan::{declare_node_union, AstNode, SyntaxResult};
@@ -977,16 +977,3 @@ mod tests {
         );
     }
 }
-
-fn has_trailing_line_comment(node: &JsSyntaxNode) -> bool {
-    node.last_token()
-        .map_or(false, |token| has_token_trailing_line_comment(&token))
-}
-
-fn has_token_trailing_line_comment(token: &JsSyntaxToken) -> bool {
-    token
-        .trailing_trivia()
-        .pieces()
-        .filter_map(|piece| piece.as_comments())
-        .any(|comment| JsCommentStyle.get_comment_kind(&comment).is_line())
-}