fix(js_formatter): Allow function expressions to group and break as c…

…all arguments (biomejs#1003)

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

@@ -375,3 +375,46 @@ pub(crate) fn should_hug_function_parameters(
 
     Ok(result)
 }
+
+/// Tests if all of the parameters of `expression` are simple enough to allow
+/// a function to group.
+pub(crate) fn has_only_simple_parameters(
+    parameters: &JsParameters,
+    allow_type_annotations: bool,
+) -> bool {
+    parameters
+        .items()
+        .into_iter()
+        .flatten()
+        .all(|parameter| is_simple_parameter(&parameter, allow_type_annotations))
+}
+
+/// Tests if the single parameter is "simple", as in a plain identifier with no
+/// explicit type annotation and no initializer:
+///
+/// Examples:
+/// foo             => true
+/// foo?            => true
+/// foo = 'bar'     => false
+/// foo: string     => false
+/// {a, b}          => false
+/// {a, b} = {}     => false
+/// [a, b]          => false
+///
+pub(crate) fn is_simple_parameter(
+    parameter: &AnyJsParameter,
+    allow_type_annotations: bool,
+) -> bool {
+    match parameter {
+        AnyJsParameter::AnyJsFormalParameter(AnyJsFormalParameter::JsFormalParameter(param)) => {
+            matches!(
+                param.binding(),
+                Ok(AnyJsBindingPattern::AnyJsBinding(
+                    AnyJsBinding::JsIdentifierBinding(_)
+                ))
+            ) && (allow_type_annotations || param.type_annotation().is_none())
+                && param.initializer().is_none()
+        }
+        _ => false,
+    }
+}

crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs

@@ -1,4 +1,4 @@
-use crate::js::bindings::parameters::FormatJsParametersOptions;
+use crate::js::bindings::parameters::{has_only_simple_parameters, FormatJsParametersOptions};
 use crate::prelude::*;
 use biome_formatter::{
     format_args, write, CstFormatContext, FormatRuleWithOptions, RemoveSoftLinesBuffer,
@@ -317,8 +317,9 @@ fn format_signature(
 /// Returns a `true` result if the arrow function contains any elements which
 /// should force the chain to break onto multiple lines. This includes any kind
 /// of return type annotation if the function also takes parameters (e.g.,
-/// `(a, b): bool => ...`), or any kind of rest/object/array binding parameter
-/// (e.g., `({a, b: foo}) => ...`).
+/// `(a, b): bool => ...`), any kind of rest/object/array binding parameter
+/// (e.g., `({a, b: foo}) => ...`), and any kind of initializer for a parameter
+/// (e.g., `(a = 2) => ...`).
 ///
 /// The complexity of these expressions limits their legibility when printed
 /// inline, so they force the chain to break to preserve clarity. Any other
@@ -332,17 +333,20 @@ fn should_break_chain(arrow: &JsArrowFunctionExpression) -> SyntaxResult<bool> {
 
     let has_parameters = match &parameters {
         AnyJsArrowFunctionParameters::AnyJsBinding(_) => true,
-        AnyJsArrowFunctionParameters::JsParameters(parameters) => !parameters.items().is_empty(),
+        AnyJsArrowFunctionParameters::JsParameters(parameters) => {
+            // This matches Prettier, which allows type annotations when
+            // grouping arrow expressions, but disallows them when grouping
+            // normal function expressions.
+            if !has_only_simple_parameters(parameters, true) {
+                return Ok(true);
+            }
+            !parameters.items().is_empty()
+        }
     };
 
-    if arrow.return_type_annotation().is_some() && has_parameters {
-        return Ok(true);
-    }
-
-    // Break if the function has any rest, object, or array parameter
-    let result = has_rest_object_or_array_parameter(&parameters);
+    let has_type_and_parameters = arrow.return_type_annotation().is_some() && has_parameters;
 
-    Ok(result)
+    Ok(has_type_and_parameters || has_rest_object_or_array_parameter(&parameters))
 }
 
 fn should_add_parens(body: &AnyJsFunctionBody) -> bool {

crates/biome_js_formatter/src/js/expressions/call_arguments.rs

@@ -1,4 +1,5 @@
 use crate::context::trailing_comma::FormatTrailingComma;
+use crate::js::bindings::parameters::has_only_simple_parameters;
 use crate::js::declarations::function_declaration::FormatFunctionOptions;
 use crate::js::expressions::arrow_function_expression::{
     is_multiline_template_starting_on_same_line, FormatJsArrowFunctionExpressionOptions,
@@ -359,7 +360,7 @@ fn write_grouped_arguments(
                 grouped_arg.cache_function_body(f);
             }
             AnyJsCallArgument::AnyJsExpression(AnyJsExpression::JsFunctionExpression(function))
-                if !other_args.is_empty() && !has_no_parameters(function) =>
+                if !other_args.is_empty() || function_has_only_simple_parameters(function) =>
             {
                 grouped_arg.cache_function_body(f);
             }
@@ -604,7 +605,7 @@ impl Format<JsFormatContext> for FormatGroupedLastArgument<'_> {
         // to remove any soft line breaks.
         match element.node()? {
             AnyJsCallArgument::AnyJsExpression(JsFunctionExpression(function))
-                if !self.is_only && !has_no_parameters(function) =>
+                if !self.is_only || function_has_only_simple_parameters(function) =>
             {
                 with_token_tracking_disabled(f, |f| {
                     write!(
@@ -667,13 +668,10 @@ fn with_token_tracking_disabled<F: FnOnce(&mut JsFormatter) -> R, R>(
     result
 }
 
-/// Tests if `expression` has an empty parameters list.
-fn has_no_parameters(expression: &JsFunctionExpression) -> bool {
-    match expression.parameters() {
-        // Use default formatting for expressions without parameters, will return `Err` anyway
-        Err(_) => true,
-        Ok(parameters) => parameters.items().is_empty(),
-    }
+fn function_has_only_simple_parameters(expression: &JsFunctionExpression) -> bool {
+    expression.parameters().map_or(true, |parameters| {
+        has_only_simple_parameters(&parameters, false)
+    })
 }
 
 /// Helper for formatting a grouped call argument (see [should_group_first_argument] and [should_group_last_argument]).

crates/biome_js_formatter/tests/quick_test.rs

@@ -9,12 +9,23 @@ mod language {
     include!("language.rs");
 }
 
-#[ignore]
 #[test]
 // use this test check if your snippet prints as you wish, without using a snapshot
 fn quick_test() {
     let src = r#"
-    foo( /*hi*/(a) => (b) => {})
+    
+const makeSomeFunction =
+(services = {logger:null}) =>
+  (a, b, c) =>
+    services.logger(a,b,c)
+
+const makeSomeFunction2 =
+(services = {
+  logger: null
+}) =>
+  (a, b, c) =>
+    services.logger(a, b, c)
+
     "#;
     let syntax = JsFileSource::tsx();
     let tree = parse(

crates/biome_js_formatter/tests/specs/js/module/call_expression.js

@@ -45,4 +45,20 @@ test.expect(t => {
     t.true(a)
 }, false,
     // comment
-    )
+    )
+
+// should group start of function expression onto the same line
+const Button1 = forwardRef(function Button(props, ref) {
+        return <button ref={ref} />;
+    }
+    );
+
+const Button2 = forwardRef(function (props, ref) { return <button ref={ref} />; }
+    );
+
+// should break whole call before breaking parameter list
+const FilterButton = forwardRefWithLongFunctionName(function FilterButton(props, ref) {
+      return <button ref={ref} />;
+    }
+  );
+

crates/biome_js_formatter/tests/specs/js/module/call_expression.js.snap

@@ -54,6 +54,22 @@ test.expect(t => {
 }, false,
     // comment
     )
+
+// should group start of function expression onto the same line
+const Button1 = forwardRef(function Button(props, ref) {
+        return <button ref={ref} />;
+    }
+    );
+
+const Button2 = forwardRef(function (props, ref) { return <button ref={ref} />; }
+    );
+
+// should break whole call before breaking parameter list
+const FilterButton = forwardRefWithLongFunctionName(function FilterButton(props, ref) {
+      return <button ref={ref} />;
+    }
+  );
+
 ```
 
 
@@ -128,6 +144,22 @@ test.expect(
 	false,
 	// comment
 );
+
+// should group start of function expression onto the same line
+const Button1 = forwardRef(function Button(props, ref) {
+	return <button ref={ref} />;
+});
+
+const Button2 = forwardRef(function (props, ref) {
+	return <button ref={ref} />;
+});
+
+// should break whole call before breaking parameter list
+const FilterButton = forwardRefWithLongFunctionName(
+	function FilterButton(props, ref) {
+		return <button ref={ref} />;
+	},
+);
 ```