fix(js_formatter): fix invalid formatting of long arrow function for …

…AsNeeded arrow parens
biomejs · Mar 2, 2024 · ee02ecc · ee02ecc
1 parent f490b15
commit ee02ecc
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -292,6 +292,8 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 - Fix [#1218](https://github.com/biomejs/biome/issues/1218), by correctly preserving empty lines in member chains.
   Contributed by @ah-yu
 
+- Fix [#1934](https://github.com/biomejs/biome/pull/1934). Fix invalid formatting of long arrow function for AsNeeded arrow parens Contributed by @fireairforce
+
 ### JavaScript APIs
 
 ### Linter

diff --git a/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs b/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs
@@ -36,7 +36,7 @@ impl FormatNodeRule<JsFormalParameter> for FormatJsFormalParameter {
             .grand_parent()
             .and_then(FormatAnyJsParameters::cast)
             .map_or(false, |parameters| {
-                should_hug_function_parameters(&parameters, f.comments()).unwrap_or(false)
+                should_hug_function_parameters(&parameters, f.comments(), false).unwrap_or(false)
             });
 
         if is_hug_parameter && decorators.is_empty() {

diff --git a/crates/biome_js_formatter/src/js/bindings/parameters.rs b/crates/biome_js_formatter/src/js/bindings/parameters.rs
@@ -33,9 +33,12 @@ impl Format<JsFormatContext> for FormatAnyJsParameters {
     fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
         let list = self.list();
 
+        let parentheses_not_needed = self
+        .as_arrow_function_expression()
+        .map_or(false, |expression| can_avoid_parentheses(&expression, f));
         let has_any_decorated_parameter = list.has_any_decorated_parameter();
 
-        let can_hug = should_hug_function_parameters(self, f.context().comments())?
+        let can_hug = should_hug_function_parameters(self, f.context().comments(), parentheses_not_needed)?
             && !has_any_decorated_parameter;
 
         let layout = if list.is_empty() {
@@ -49,10 +52,6 @@ impl Format<JsFormatContext> for FormatAnyJsParameters {
         let l_paren_token = self.l_paren_token()?;
         let r_paren_token = self.r_paren_token()?;
 
-        let parentheses_not_needed = self
-            .as_arrow_function_expression()
-            .map_or(false, |expression| can_avoid_parentheses(&expression, f));
-
         match layout {
             ParameterLayout::NoParameters => {
                 write!(
@@ -211,10 +210,14 @@ pub enum ParameterLayout {
 pub(crate) fn should_hug_function_parameters(
     parameters: &FormatAnyJsParameters,
     comments: &JsComments,
+    parentheses_not_needed: bool
 ) -> FormatResult<bool> {
     /// Returns true if the first parameter should be forced onto the same line as the `(` and `)` parentheses.
     /// See the `[ParameterLayout::Hug] documentation.
-    fn hug_formal_parameter(parameter: &self::AnyJsFormalParameter) -> FormatResult<bool> {
+    fn hug_formal_parameter(
+        parameter: &self::AnyJsFormalParameter,
+        should_hug_formal_parameter: bool,
+    ) -> FormatResult<bool> {
         let result = match parameter {
             AnyJsFormalParameter::JsFormalParameter(parameter) => {
                 match parameter.initializer() {
@@ -223,15 +226,26 @@ pub(crate) fn should_hug_function_parameters(
                             // always true for `[a]` or `{a}`
                             AnyJsBindingPattern::JsArrayBindingPattern(_)
                             | AnyJsBindingPattern::JsObjectBindingPattern(_) => true,
-                            // only if the type parameter is an object type
+                            // if the type parameter is an object type
                             // `a: { prop: string }`
+                            // or parameter is an arrow function parameter
+                            // (a) => {}
                             AnyJsBindingPattern::AnyJsBinding(
                                 AnyJsBinding::JsIdentifierBinding(_),
-                            ) => parameter
-                                .type_annotation()
-                                .map_or(false, |type_annotation| {
-                                    matches!(type_annotation.ty(), Ok(AnyTsType::TsObjectType(_)))
-                                }),
+                            ) => {
+                                if should_hug_formal_parameter {
+                                    true
+                                } else {
+                                    parameter
+                                        .type_annotation()
+                                        .map_or(false, |type_annotation| {
+                                            matches!(
+                                                type_annotation.ty(),
+                                                Ok(AnyTsType::TsObjectType(_))
+                                            )
+                                        })
+                                }
+                            }
                             AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsBogusBinding(_)) => {
                                 return Err(FormatError::SyntaxError);
                             }
@@ -267,7 +281,6 @@ pub(crate) fn should_hug_function_parameters(
     }
 
     let list = parameters.list();
-
     if list.len() != 1 {
         return Ok(false);
     }
@@ -278,11 +291,15 @@ pub(crate) fn should_hug_function_parameters(
     if comments.has_comments(only_parameter.syntax()) {
         return Ok(false);
     }
+
+    let has_parentheses = parameters.l_paren_token().is_ok() && parameters.r_paren_token().is_ok();
+    let from_arrow_function = parameters.as_arrow_function_expression().is_some();
+    let should_hug_formal_parameter = has_parentheses && from_arrow_function && parentheses_not_needed;
 
     let result = match only_parameter {
         AnyParameter::AnyJsParameter(parameter) => match parameter {
             AnyJsParameter::AnyJsFormalParameter(formal_parameter) => {
-                hug_formal_parameter(&formal_parameter)?
+                hug_formal_parameter(&formal_parameter, should_hug_formal_parameter)?
             }
             AnyJsParameter::JsRestParameter(_) => false,
             AnyJsParameter::TsThisParameter(this) => {
@@ -294,7 +311,7 @@ pub(crate) fn should_hug_function_parameters(
         AnyParameter::AnyJsConstructorParameter(constructor_parameter) => {
             match constructor_parameter {
                 AnyJsConstructorParameter::AnyJsFormalParameter(formal_parameter) => {
-                    hug_formal_parameter(&formal_parameter)?
+                    hug_formal_parameter(&formal_parameter, should_hug_formal_parameter)?
                 }
                 AnyJsConstructorParameter::JsRestParameter(_)
                 | AnyJsConstructorParameter::TsPropertyParameter(_) => false,

diff --git a/crates/biome_js_formatter/src/utils/object_pattern_like.rs b/crates/biome_js_formatter/src/utils/object_pattern_like.rs
@@ -130,7 +130,7 @@ impl JsObjectPatternLike {
                 .and_then(|parameter| parameter.syntax().grand_parent())
                 .and_then(FormatAnyJsParameters::cast)
                 .map_or(false, |parameters| {
-                    should_hug_function_parameters(&parameters, comments).unwrap_or(false)
+                    should_hug_function_parameters(&parameters, comments, false).unwrap_or(false)
                 }),
         }
     }

diff --git a/crates/biome_js_formatter/tests/quick_test.rs b/crates/biome_js_formatter/tests/quick_test.rs
@@ -14,33 +14,15 @@ mod language {
 // use this test check if your snippet prints as you wish, without using a snapshot
 fn quick_test() {
     let src = r#"
-   import React from "react";
-
-const Component = () => (
-  <div>
-    <div data-a="1">
-      Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-    </div>
-
-    <div data-a="1" data-b="2" data-c="3">
-      Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-    </div>
-
-    <div data-a="Lorem ipsum dolor sit amet" data-b="Lorem ipsum dolor sit amet" data-c="Lorem ipsum dolor sit amet">
-      Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-    </div>
-
-    <div data-long-attribute-a="1" data-long-attribute-b="2" data-long-attribute-c="3">
-      Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-    </div>
-
-    <img src="/images/foo.png" />
-
-    <img src="/images/foo.png" alt="bar" />
-
-    <img src="/images/foo.png" alt="Lorem ipsum dolor sit amet, consectetur adipiscing elit." />
-  </div>
-);
+type InstanceID = string;
+type MaybeCardWithAttachment = string;
+function outerFunctionToForceIndent() {
+    const cardWithAttachment: (id: InstanceID) => MaybeCardWithAttachment = (
+        id
+    ) => {
+        return `${id}test`;
+    };
+}
 
     "#;
     let source_type = JsFileSource::tsx();
@@ -51,11 +33,11 @@ const Component = () => (
     );
     let options = JsFormatOptions::new(source_type)
         .with_indent_style(IndentStyle::Space)
-        .with_line_width(LineWidth::try_from(120).unwrap())
+        .with_line_width(LineWidth::try_from(80).unwrap())
         .with_semicolons(Semicolons::Always)
         .with_quote_style(QuoteStyle::Double)
         .with_jsx_quote_style(QuoteStyle::Single)
-        .with_arrow_parentheses(ArrowParentheses::Always)
+        .with_arrow_parentheses(ArrowParentheses::AsNeeded)
         .with_attribute_position(AttributePosition::Multiline);
 
     let doc = format_node(options.clone(), &tree.syntax()).unwrap();

diff --git a/crates/biome_js_formatter/tests/specs/ts/arrow/long_arrow_parentheses_with_line_break.ts b/crates/biome_js_formatter/tests/specs/ts/arrow/long_arrow_parentheses_with_line_break.ts
@@ -0,0 +1,7 @@
+const this_is_a_very_long_function_name_so_need_to_break_a_new_line_here = (
+  id
+) => {
+  return id;
+};
+
+
diff --git a/...es/biome_js_formatter/tests/specs/ts/arrow/long_arrow_parentheses_with_line_break.ts.snap b/...es/biome_js_formatter/tests/specs/ts/arrow/long_arrow_parentheses_with_line_break.ts.snap
@@ -0,0 +1,75 @@
+---
+source: crates/biome_formatter_test/src/snapshot_builder.rs
+info: ts/arrow/long_arrow_parentheses_with_line_break.ts
+---
+
+# Input
+
+```ts
+const this_is_a_very_long_function_name_so_need_to_break_a_new_line_here = (
+  id
+) => {
+  return id;
+};
+
+
+
+```
+
+
+=============================
+
+# Outputs
+
+## Output 1
+
+-----
+Indent style: Tab
+Indent width: 2
+Line ending: LF
+Line width: 80
+Quote style: Double Quotes
+JSX quote style: Double Quotes
+Quote properties: As needed
+Trailing comma: All
+Semicolons: Always
+Arrow parentheses: Always
+Bracket spacing: true
+Bracket same line: false
+Attribute Position: Auto
+-----
+
+```ts
+const this_is_a_very_long_function_name_so_need_to_break_a_new_line_here = (
+	id,
+) => {
+	return id;
+};
+```
+
+## Output 2
+
+-----
+Indent style: Tab
+Indent width: 2
+Line ending: LF
+Line width: 80
+Quote style: Double Quotes
+JSX quote style: Double Quotes
+Quote properties: As needed
+Trailing comma: All
+Semicolons: Always
+Arrow parentheses: As needed
+Bracket spacing: true
+Bracket same line: false
+Attribute Position: Auto
+-----
+
+```ts
+const this_is_a_very_long_function_name_so_need_to_break_a_new_line_here =
+	id => {
+		return id;
+	};
+```
+
+
diff --git a/website/src/content/docs/internals/changelog.md b/website/src/content/docs/internals/changelog.md
@@ -298,6 +298,8 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 - Fix [#1218](https://github.com/biomejs/biome/issues/1218), by correctly preserving empty lines in member chains.
   Contributed by @ah-yu
 
+- Fix [#1934](https://github.com/biomejs/biome/pull/1934). Fix invalid formatting of long arrow function for AsNeeded arrow parens Contributed by @fireairforce
+
 ### JavaScript APIs
 
 ### Linter