fix(js_formatter): Use fluid assignment layout when left side is brea…

…kable (#1021)

crates/biome_formatter/src/format_element.rs

@@ -232,6 +232,10 @@ impl FormatElement {
     pub const fn is_space(&self) -> bool {
         matches!(self, FormatElement::Space)
     }
+
+    pub const fn is_line(&self) -> bool {
+        matches!(self, FormatElement::Line(_))
+    }
 }
 
 impl FormatElements for FormatElement {
@@ -253,6 +257,10 @@ impl FormatElements for FormatElement {
         }
     }
 
+    fn may_directly_break(&self) -> bool {
+        matches!(self, FormatElement::Line(_))
+    }
+
     fn has_label(&self, label_id: LabelId) -> bool {
         match self {
             FormatElement::Tag(Tag::StartLabelled(actual)) => *actual == label_id,
@@ -341,6 +349,15 @@ pub trait FormatElements {
     /// lines if this element is part of a group and the group doesn't fit on a single line.
     fn will_break(&self) -> bool;
 
+    /// Returns true if this [FormatElement] has the potential to break across multiple lines when printed.
+    /// This is the case _only_ if this format element recursively contains a [FormatElement::Line].
+    ///
+    /// It's possible for [FormatElements::will_break] to return true while this function returns false,
+    /// such as when the group contains a [crate::builders::expand_parent] or some text within the group
+    /// contains a newline. Neither of those cases directly contain a [FormatElement::Line], and so they
+    /// do not _directly_ break.
+    fn may_directly_break(&self) -> bool;
+
     /// Returns true if the element has the given label.
     fn has_label(&self, label: LabelId) -> bool;
 

crates/biome_formatter/src/format_element/document.rs

@@ -576,6 +576,38 @@ impl FormatElements for [FormatElement] {
         false
     }
 
+    fn may_directly_break(&self) -> bool {
+        use Tag::*;
+        let mut ignore_depth = 0usize;
+
+        for element in self {
+            match element {
+                // Line suffix
+                // Ignore if any of its content breaks
+                FormatElement::Tag(StartLineSuffix) => {
+                    ignore_depth += 1;
+                }
+                FormatElement::Tag(EndLineSuffix) => {
+                    ignore_depth -= 1;
+                }
+                FormatElement::Interned(interned) if ignore_depth == 0 => {
+                    if interned.may_directly_break() {
+                        return true;
+                    }
+                }
+
+                element if ignore_depth == 0 && element.may_directly_break() => {
+                    return true;
+                }
+                _ => continue,
+            }
+        }
+
+        debug_assert_eq!(ignore_depth, 0, "Unclosed start container");
+
+        false
+    }
+
     fn has_label(&self, expected: LabelId) -> bool {
         self.first()
             .map_or(false, |element| element.has_label(expected))

crates/biome_js_formatter/src/utils/assignment_like.rs

@@ -658,6 +658,7 @@ impl AnyJsAssignmentLike {
     fn layout(
         &self,
         is_left_short: bool,
+        left_may_break: bool,
         f: &mut Formatter<JsFormatContext>,
     ) -> FormatResult<AssignmentLikeLayout> {
         if self.has_only_left_hand_side() {
@@ -726,17 +727,19 @@ impl AnyJsAssignmentLike {
             return Ok(AssignmentLikeLayout::BreakAfterOperator);
         }
 
-        if matches!(
-            right_expression,
-            Some(
-                AnyJsExpression::JsClassExpression(_)
-                    | AnyJsExpression::JsTemplateExpression(_)
-                    | AnyJsExpression::AnyJsLiteralExpression(
-                        AnyJsLiteralExpression::JsBooleanLiteralExpression(_)
-                            | AnyJsLiteralExpression::JsNumberLiteralExpression(_)
-                    )
+        if !left_may_break
+            && matches!(
+                right_expression,
+                Some(
+                    AnyJsExpression::JsClassExpression(_)
+                        | AnyJsExpression::JsTemplateExpression(_)
+                        | AnyJsExpression::AnyJsLiteralExpression(
+                            AnyJsLiteralExpression::JsBooleanLiteralExpression(_)
+                                | AnyJsLiteralExpression::JsNumberLiteralExpression(_)
+                        )
+                )
             )
-        ) {
+        {
             return Ok(AssignmentLikeLayout::NeverBreakAfterOperator);
         }
 
@@ -1011,11 +1014,12 @@ impl Format<JsFormatContext> for AnyJsAssignmentLike {
             let mut buffer = VecBuffer::new(f.state_mut());
             let is_left_short = self.write_left(&mut Formatter::new(&mut buffer))?;
             let formatted_left = buffer.into_vec();
+            let left_may_break = formatted_left.may_directly_break();
 
             // Compare name only if we are in a position of computing it.
             // If not (for example, left is not an identifier), then let's fallback to false,
             // so we can continue the chain of checks
-            let layout = self.layout(is_left_short, f)?;
+            let layout = self.layout(is_left_short, left_may_break, f)?;
 
             let left = format_once(|f| f.write_elements(formatted_left));
             let right = format_with(|f| self.write_right(f, layout));

crates/biome_js_formatter/src/utils/format_modifiers.rs

@@ -26,6 +26,35 @@ where
         let modifiers = sort_modifiers_by_precedence(&self.list);
         let should_expand = should_expand_decorators(&self.list);
 
+        // Returning early here is important, because otherwise this node
+        // returns a group that always has a soft line break, which causes
+        // `may_directly_break` to return true, even if there is no
+        // possibility for the break to be used, since it's at the start of
+        // a line with no content before it. An example case this affects is:
+        //
+        // ```js
+        // class Test {
+        //   prop1 = // comment
+        //     true;
+        // }
+        // ```
+        //
+        // Here, the modifier list is present before `prop1`, but part of the
+        // statement as a whole. The statement checks if it _may_ break when
+        // determining how to position the trailing comment. If it does break,
+        // the comment is placed on a new line and the value on a line after
+        // that. But if it doesn't break, then the whole statement can remnain
+        // on a single line, which is desirable and important for preserving
+        // semantics of things like ignore comments.
+        //
+        // ```js
+        // class Test {
+        //   prop1 = true; // comment
+        // }
+        if self.list.is_empty() {
+            return Ok(());
+        }
+
         // need to use peek the iterator to check if the current node is a decorator and don't advance the iterator
         let mut iter = modifiers.into_iter().peekable();
         let decorators = format_once(|f| {