fix(js_formatter): Match Prettier's breaking strategy for ArrowChain …

…layouts (#934)

crates/biome_formatter/src/formatter.rs

@@ -114,7 +114,7 @@ impl<'buf, Context> Formatter<'buf, Context> {
     /// Specialized version of [crate::Formatter::join_with] for joining SyntaxNodes separated by a space, soft
     /// line break or empty line depending on the input file.
     ///
-    /// This functions inspects the input source and separates consecutive elements with either
+    /// This function inspects the input source and separates consecutive elements with either
     /// a [crate::builders::soft_line_break_or_space] or [crate::builders::empty_line] depending on how many line breaks were
     /// separating the elements in the original file.
     pub fn join_nodes_with_soft_line<'a>(
@@ -126,13 +126,26 @@ impl<'buf, Context> Formatter<'buf, Context> {
     /// Specialized version of [crate::Formatter::join_with] for joining SyntaxNodes separated by one or more
     /// line breaks depending on the input file.
     ///
-    /// This functions inspects the input source and separates consecutive elements with either
+    /// This function inspects the input source and separates consecutive elements with either
     /// a [crate::builders::hard_line_break] or [crate::builders::empty_line] depending on how many line breaks were separating the
     /// elements in the original file.
     pub fn join_nodes_with_hardline<'a>(&'a mut self) -> JoinNodesBuilder<'a, 'buf, Line, Context> {
         JoinNodesBuilder::new(hard_line_break(), self)
     }
 
+    /// Specialized version of [crate::Formatter::join_with] for joining SyntaxNodes separated by a simple space.
+    ///
+    /// This function *disregards* the input source and always separates consecutive elements with a plain
+    /// [crate::builders::space], forcing a flat layout regardless of any line breaks or spaces were separating
+    /// the elements in the original file.
+    ///
+    /// This function should likely only be used in a `best_fitting!` context, where one variant attempts to
+    /// force a list of nodes onto a single line without any possible breaks, then falls back to a broken
+    /// out variant if the content does not fit.
+    pub fn join_nodes_with_space<'a>(&'a mut self) -> JoinNodesBuilder<'a, 'buf, Space, Context> {
+        JoinNodesBuilder::new(space(), self)
+    }
+
     /// Concatenates a list of [crate::Format] objects with spaces and line breaks to fit
     /// them on as few lines as possible. Each element introduces a conceptual group. The printer
     /// first tries to print the item in flat mode but then prints it in expanded mode if it doesn't fit.

crates/biome_js_formatter/src/js/bindings/constructor_parameters.rs

@@ -3,12 +3,18 @@ use crate::prelude::*;
 use crate::js::bindings::parameters::FormatAnyJsParameters;
 use biome_js_syntax::JsConstructorParameters;
 
+use super::parameters::{AnyJsParameters, FormatJsParametersOptions};
+
 #[derive(Debug, Clone, Default)]
 pub(crate) struct FormatJsConstructorParameters;
 
 impl FormatNodeRule<JsConstructorParameters> for FormatJsConstructorParameters {
     fn fmt_fields(&self, node: &JsConstructorParameters, f: &mut JsFormatter) -> FormatResult<()> {
-        FormatAnyJsParameters::from(node.clone()).fmt(f)
+        FormatAnyJsParameters::new(
+            AnyJsParameters::JsConstructorParameters(node.clone()),
+            FormatJsParametersOptions::default(),
+        )
+        .fmt(f)
     }
 
     fn fmt_dangling_comments(

crates/biome_js_formatter/src/js/bindings/formal_parameter.rs

@@ -3,9 +3,12 @@ use biome_formatter::write;
 
 use crate::utils::FormatInitializerClause;
 
-use crate::js::bindings::parameters::{should_hug_function_parameters, FormatAnyJsParameters};
-use biome_js_syntax::JsFormalParameter;
+use crate::js::bindings::parameters::{
+    should_hug_function_parameters, AnyJsParameters, FormatAnyJsParameters,
+    FormatJsParametersOptions,
+};
 use biome_js_syntax::JsFormalParameterFields;
+use biome_js_syntax::{JsFormalParameter, JsParameters};
 
 #[derive(Debug, Clone, Default)]
 pub(crate) struct FormatJsFormalParameter;
@@ -34,9 +37,16 @@ impl FormatNodeRule<JsFormalParameter> for FormatJsFormalParameter {
         let is_hug_parameter = node
             .syntax()
             .grand_parent()
-            .and_then(FormatAnyJsParameters::cast)
+            .and_then(JsParameters::cast)
             .map_or(false, |parameters| {
-                should_hug_function_parameters(&parameters, f.comments()).unwrap_or(false)
+                should_hug_function_parameters(
+                    &FormatAnyJsParameters::new(
+                        AnyJsParameters::JsParameters(parameters),
+                        FormatJsParametersOptions::default(),
+                    ),
+                    f.comments(),
+                )
+                .unwrap_or(false)
             });
 
         if is_hug_parameter && decorators.is_empty() {

crates/biome_js_formatter/src/js/bindings/parameters.rs

@@ -1,5 +1,5 @@
 use crate::prelude::*;
-use biome_formatter::{write, CstFormatContext};
+use biome_formatter::{write, CstFormatContext, FormatRuleWithOptions};
 
 use crate::js::expressions::arrow_function_expression::can_avoid_parentheses;
 use crate::js::lists::parameter_list::FormatJsAnyParameterList;
@@ -8,16 +8,40 @@ use biome_js_syntax::parameter_ext::{AnyJsParameterList, AnyParameter};
 use biome_js_syntax::{
     AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsExpression,
     AnyJsFormalParameter, AnyJsParameter, AnyTsType, JsArrowFunctionExpression,
-    JsConstructorParameters, JsParameters, JsSyntaxToken,
+    JsConstructorParameters, JsParameters, JsSyntaxNode, JsSyntaxToken,
 };
-use biome_rowan::{declare_node_union, SyntaxResult};
+use biome_rowan::{declare_node_union, AstNode, SyntaxResult};
 
-#[derive(Debug, Clone, Default)]
-pub(crate) struct FormatJsParameters;
+#[derive(Debug, Copy, Clone, Default)]
+pub(crate) struct FormatJsParameters {
+    options: FormatJsParametersOptions,
+}
+
+#[derive(Debug, Clone, Copy, Default)]
+pub(crate) struct FormatJsParametersOptions {
+    /// Whether the parameters should include soft line breaks to allow them to
+    /// break naturally over multiple lines when they can't fit on one line.
+    ///
+    /// This is particularly important for arrow chains passed as arguments in
+    /// call expressions, where it must be set to false to avoid having the
+    /// parameters break onto lines before the entire expression expands.
+    ///
+    /// When `true`, parameters will _not_ include any soft line break points.
+    pub prevent_soft_line_breaks: bool,
+}
+
+impl FormatRuleWithOptions<JsParameters> for FormatJsParameters {
+    type Options = FormatJsParametersOptions;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.options = options;
+        self
+    }
+}
 
 impl FormatNodeRule<JsParameters> for FormatJsParameters {
     fn fmt_fields(&self, node: &JsParameters, f: &mut JsFormatter) -> FormatResult<()> {
-        FormatAnyJsParameters::from(node.clone()).fmt(f)
+        FormatAnyJsParameters::new(AnyJsParameters::JsParameters(node.clone()), self.options).fmt(f)
     }
 
     fn fmt_dangling_comments(&self, _: &JsParameters, _: &mut JsFormatter) -> FormatResult<()> {
@@ -27,7 +51,21 @@ impl FormatNodeRule<JsParameters> for FormatJsParameters {
 }
 
 declare_node_union! {
-    pub(crate) FormatAnyJsParameters = JsParameters | JsConstructorParameters
+    pub(crate) AnyJsParameters = JsParameters | JsConstructorParameters
+}
+
+pub(crate) struct FormatAnyJsParameters {
+    pub(crate) parameters: AnyJsParameters,
+    pub(crate) options: FormatJsParametersOptions,
+}
+
+impl FormatAnyJsParameters {
+    pub(crate) fn new(parameters: AnyJsParameters, options: FormatJsParametersOptions) -> Self {
+        Self {
+            parameters,
+            options,
+        }
+    }
 }
 
 impl Format<JsFormatContext> for FormatAnyJsParameters {
@@ -43,6 +81,8 @@ impl Format<JsFormatContext> for FormatAnyJsParameters {
             ParameterLayout::NoParameters
         } else if can_hug || self.is_in_test_call()? {
             ParameterLayout::Hug
+        } else if self.options.prevent_soft_line_breaks {
+            ParameterLayout::Compact
         } else {
             ParameterLayout::Default
         };
@@ -60,7 +100,7 @@ impl Format<JsFormatContext> for FormatAnyJsParameters {
                     f,
                     [
                         l_paren_token.format(),
-                        format_dangling_comments(self.syntax()).with_soft_block_indent(),
+                        format_dangling_comments(self.parameters_syntax()).with_soft_block_indent(),
                         r_paren_token.format()
                     ]
                 )
@@ -99,7 +139,7 @@ impl Format<JsFormatContext> for FormatAnyJsParameters {
                     f,
                     [soft_block_indent(&FormatJsAnyParameterList::with_layout(
                         &list,
-                        ParameterLayout::Default
+                        ParameterLayout::Default,
                     ))]
                 )?;
 
@@ -109,6 +149,29 @@ impl Format<JsFormatContext> for FormatAnyJsParameters {
                     write!(f, [format_removed(&r_paren_token)])?;
                 }
 
+                Ok(())
+            }
+            ParameterLayout::Compact => {
+                if !parentheses_not_needed {
+                    write!(f, [l_paren_token.format()])?;
+                } else {
+                    write!(f, [format_removed(&l_paren_token)])?;
+                }
+
+                write!(
+                    f,
+                    [FormatJsAnyParameterList::with_layout(
+                        &list,
+                        ParameterLayout::Compact
+                    )]
+                )?;
+
+                if !parentheses_not_needed {
+                    write!(f, [r_paren_token.format()])?;
+                } else {
+                    write!(f, [format_removed(&r_paren_token)])?;
+                }
+
                 Ok(())
             }
         }
@@ -117,54 +180,57 @@ impl Format<JsFormatContext> for FormatAnyJsParameters {
 
 impl FormatAnyJsParameters {
     fn l_paren_token(&self) -> SyntaxResult<JsSyntaxToken> {
-        match self {
-            FormatAnyJsParameters::JsParameters(parameters) => parameters.l_paren_token(),
-            FormatAnyJsParameters::JsConstructorParameters(parameters) => {
-                parameters.l_paren_token()
-            }
+        match &self.parameters {
+            AnyJsParameters::JsParameters(parameters) => parameters.l_paren_token(),
+            AnyJsParameters::JsConstructorParameters(parameters) => parameters.l_paren_token(),
         }
     }
 
     fn list(&self) -> AnyJsParameterList {
-        match self {
-            FormatAnyJsParameters::JsParameters(parameters) => {
+        match &self.parameters {
+            AnyJsParameters::JsParameters(parameters) => {
                 AnyJsParameterList::from(parameters.items())
             }
-            FormatAnyJsParameters::JsConstructorParameters(parameters) => {
+            AnyJsParameters::JsConstructorParameters(parameters) => {
                 AnyJsParameterList::from(parameters.parameters())
             }
         }
     }
 
     fn r_paren_token(&self) -> SyntaxResult<JsSyntaxToken> {
-        match self {
-            FormatAnyJsParameters::JsParameters(parameters) => parameters.r_paren_token(),
-            FormatAnyJsParameters::JsConstructorParameters(parameters) => {
-                parameters.r_paren_token()
-            }
+        match &self.parameters {
+            AnyJsParameters::JsParameters(parameters) => parameters.r_paren_token(),
+            AnyJsParameters::JsConstructorParameters(parameters) => parameters.r_paren_token(),
         }
     }
 
     /// Returns `true` for function parameters if the function is an argument of a [test `CallExpression`](is_test_call_expression).
     fn is_in_test_call(&self) -> SyntaxResult<bool> {
-        let result = match self {
-            FormatAnyJsParameters::JsParameters(parameters) => match parameters.syntax().parent() {
+        let result = match &self.parameters {
+            AnyJsParameters::JsParameters(parameters) => match parameters.syntax().parent() {
                 Some(function) => is_test_call_argument(&function)?,
                 None => false,
             },
-            FormatAnyJsParameters::JsConstructorParameters(_) => false,
+            AnyJsParameters::JsConstructorParameters(_) => false,
         };
 
         Ok(result)
     }
 
     fn as_arrow_function_expression(&self) -> Option<JsArrowFunctionExpression> {
-        match self {
-            FormatAnyJsParameters::JsParameters(parameters) => parameters
+        match &self.parameters {
+            AnyJsParameters::JsParameters(parameters) => parameters
                 .syntax()
                 .parent()
                 .and_then(JsArrowFunctionExpression::cast),
-            FormatAnyJsParameters::JsConstructorParameters(_) => None,
+            AnyJsParameters::JsConstructorParameters(_) => None,
+        }
+    }
+
+    fn parameters_syntax(&self) -> &JsSyntaxNode {
+        match &self.parameters {
+            AnyJsParameters::JsParameters(parameters) => parameters.syntax(),
+            AnyJsParameters::JsConstructorParameters(parameters) => parameters.syntax(),
         }
     }
 }
@@ -190,7 +256,8 @@ pub enum ParameterLayout {
     Hug,
 
     /// The default layout formats all parameters on the same line if they fit or breaks after the `(`
-    /// and before the `(`.
+    /// and before the `)`.
+    ///
     /// ```javascript
     /// function test(
     ///     firstParameter,
@@ -199,6 +266,17 @@ pub enum ParameterLayout {
     /// ) {}
     /// ```
     Default,
+
+    /// Compact layout forces all parameters to try to render on the same line,
+    /// with no breaks added around the brackets. This should likely only be
+    /// used in a `best_fitting!` context where one variant attempts to fit the
+    /// parameters on a single line, and a default expanded version that is
+    /// used in case that does not fit.
+    ///
+    /// ```javascript
+    /// function test(firstParameter, secondParameter, thirdParameter, evenOverlyLong) {}
+    /// ```
+    Compact,
 }
 
 pub(crate) fn should_hug_function_parameters(