feat(rome_js_formatter): Break long object literal members over two l…

…ines #2425 (#2627)

 This is the initial draft for issue "Break long object literal members over two lines".
 Prettier uses [`printAssigment`](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L25-L75) for formatting [AssignmentExpression](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L77), [VariableDeclarator](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L89), [ClassProperty](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/class.js#L195) and [Property](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/property.js#L92).  `printAssigment` can return different IR swhich are based on a layout variants from [`chooseLayout`](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L93) function.
 
-  `"break-after-operator"`
- `"never-break-after-operator"`
- `"fluid"`
- `"break-lhs"`
 - `"chain"`
 - `"chain-tail"`
  - `"chain-tail-arrow-chain"`
   - `"only-left"`

We're going to focus on [`chooseLayout`](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L93-L174) function and branches which are relevant for object literal members. 

- ["only-left"](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L97-L99) branch isn't relevant for `JsPropertyObjectMember` because when object member has only left part it's `JsShorthandPropertyObjectMember`.  

- ["chain", "chain-tail-arrow-chain", "chain-tail"](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L105-L121) these variants can be only if `shouldUseChainFormatting` is true. It seems that `shouldUseChainFormatting` is true only for match `isAssignmentOrVariableDeclarator`.

- ["break-after-operator"](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L122-L129) I'm not sure about this conditional because technically it can be relevant for object member, but I looked prettier [commit](prettier/prettier@0a647a0) when this condition was added and it was for assignment.

- ["never-break-after-operator"](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L131-L139) Prettier doesn't add break line if right node is require call. it works for object member. [Prettier playground](https://prettier.io/playground/#N4Igxg9gdgLgprEAuc0DOMAEEBGArTAXk2AB0pNMdqbbqlMoIYAlOARwFcBLAJzgAUASgA05SnUk4G-Ln0GjyAXwDcIESAgAHGN3TJQAQ168IAdwAKxhGmQhDAGzOGAnrY05ehsAGs4MAGVDAFs4ABluKDhkADNHNDgPL19-AK1vSIBzZBheTkSQOGCcOAATUrKwwyhMzkNMuAAxCF5gwxhdGrtDThgIdRAACxhghwB1Qe54NHSwOACbKe4ANymXOzA0dxBIhN4YCy9Mttj4grw0AA8ArIc4AEVOZmikOIcEjXTePbscQxKHAMtLxIjAxtxSjBBsgAEwABk+pgSYy8WjswLge2W0Q0XGeh20thQhjQAFoomUygNZDx+Id6idXmcNAlgtwcnkCmhbg8nvBTu8CjB-uDIdCkDCNLlDNwHFkAMIQYKMwpoACsA04CQAKv8iW8PiBlvkAJJQCqwAJgEE6ACC5oCMBcdwFCSUSiAA). I haven't found test for this case. 

- ["break-lhs"](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L141-L148) this variant is for [destructuring](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L227), types ([alias](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L252), [annotation](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L278)) and [arrow function declaration](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L300).

- ["break-after-operator"](https://github.com/prettier/prettier/blob/feb6b519ee0e23eeb272227825ad84ecf1af05ab/src/language-js/print/assignment.js#L176-L223) It uses for right nodes which are:

    1.  `isBinaryish` and `!shouldInlineLogicalExpression`. I haven't added `shouldInlineLogicalExpression` checking but there is a difference in IR between [them](https://prettier.io/playground/#N4Igxg9gdgLgprEAuc0DOMAEBDTBeTYAHSkx2woBNqbbqKGlMYAnAVzkwDIvMAKADyUAlgDcA9AD4AlABoSZBtjorKS7E1YduvAIwAmAMzHj8qAF8QskBAAOMYemShsLFhADuABVcI0yEGwAGw9sAE9-awAjFmwwAGs4GABlbABbOAAZYSg4ZAAzYLQ4aNiEpOTbOJyAc2QtEpA4NKi4ajbM7CgatmwauAAxCBY07BgHboDsNhgIKxAACxg0oIB1BeF4NCqwOGS-TbFNsICwNEiQHOKWGC9YmtGCosaAKzQAD2TaoLgARTYIPAnkFitYqixrgEothWkF5rYWDkYKthJQYAtkAAWAAMYPcxVWsVsAQRcGuojy1gAjgD4Hc7P4UNg0ABaXJtNrzFhwGnCbl3PqPJCFEGNYppYT1dhi75-Wl5YXPawwGEotEYpD6ZWxYRBWoAYQgaSFTTQAFZ5mxigAVGGMkWgkCiDgASSglAQKTAiPsAEF3ckYGEfsDiuZzEA). 
    2. `StringLiteralTypeAnnotation`,  
    3. `SequenceExpression`
    4. `ConditionalExpression` + `isBinaryish` and `!shouldInlineLogicalExpression`.
    5. `StringLiteral`
    6. [isPoorlyBreakableMemberOrCallChain](https://github.com/prettier/prettier/blob/feb6b519ee0e23eeb272227825ad84ecf1af05ab/src/language-js/print/assignment.js#L329-L370)



- ["never-break-after-operator"](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L162-L171) the variant is for `TemplateLiteral`, `TaggedTemplateExpression`, `BooleanLiteral`, `NumericLiteral`, `ClassExpression` or for a member which key is [short](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L412-L430).

- ["fluid"](https://github.com/prettier/prettier/blob/67c2df0ffe8c1e8bf1425fdd248ee5dd4eb5699f/src/language-js/print/assignment.js#L173) for other cases.

crates/rome_js_formatter/Cargo.toml

@@ -10,6 +10,7 @@ rome_js_syntax = { path = "../rome_js_syntax" }
 rome_formatter = { path = "../rome_formatter" }
 rome_rowan = { path = "../rome_rowan" }
 tracing = { version = "0.1.31", default-features = false, features = ["std"] }
+unicode-width = "0.1.9"
 
 [dev-dependencies]
 rome_fs = { path = "../rome_fs" }

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

@@ -1,10 +1,11 @@
 use crate::prelude::*;
 use crate::utils::{is_simple_expression, FormatPrecedence};
 
+use crate::utils::JsAnyBinaryLikeExpression;
 use crate::FormatNodeFields;
 use rome_js_syntax::{
     JsAnyExpression, JsAnyLiteralExpression, JsParenthesizedExpression,
-    JsParenthesizedExpressionFields, JsStringLiteralExpression, JsSyntaxKind, JsSyntaxNode,
+    JsParenthesizedExpressionFields, JsStringLiteralExpression, JsSyntaxKind,
 };
 use rome_rowan::{AstNode, SyntaxResult};
 
@@ -155,22 +156,17 @@ fn parenthesis_can_be_omitted(node: &JsParenthesizedExpression) -> SyntaxResult<
             let left = expression.left()?;
             let right = expression.right()?;
 
-            Ok(not_binaryish_expression(left.syntax()) && not_binaryish_expression(right.syntax()))
+            Ok(!JsAnyBinaryLikeExpression::can_cast(left.syntax().kind())
+                && !JsAnyBinaryLikeExpression::can_cast(right.syntax().kind()))
         }
 
         JsAnyExpression::JsLogicalExpression(expression) => {
             let left = expression.left()?;
             let right = expression.right()?;
 
-            Ok(not_binaryish_expression(left.syntax()) && not_binaryish_expression(right.syntax()))
+            Ok(!JsAnyBinaryLikeExpression::can_cast(left.syntax().kind())
+                && !JsAnyBinaryLikeExpression::can_cast(right.syntax().kind()))
         }
         _ => Ok(false),
     }
 }
-
-fn not_binaryish_expression(node: &JsSyntaxNode) -> bool {
-    !matches!(
-        node.kind(),
-        JsSyntaxKind::JS_BINARY_EXPRESSION | JsSyntaxKind::JS_LOGICAL_EXPRESSION
-    )
-}

crates/rome_js_formatter/src/js/objects/property_object_member.rs

@@ -1,8 +1,14 @@
 use crate::prelude::*;
+
 use crate::utils::FormatMemberName;
+use crate::utils::JsAnyBinaryLikeExpression;
 use crate::FormatNodeFields;
+use rome_js_syntax::JsAnyExpression;
+use rome_js_syntax::JsAnyLiteralExpression;
 use rome_js_syntax::JsPropertyObjectMember;
 use rome_js_syntax::JsPropertyObjectMemberFields;
+use rome_rowan::{AstNode, SyntaxResult};
+use unicode_width::UnicodeWidthStr;
 
 impl FormatNodeFields<JsPropertyObjectMember> for FormatNodeRule<JsPropertyObjectMember> {
     fn format_fields(
@@ -15,9 +21,179 @@ impl FormatNodeFields<JsPropertyObjectMember> for FormatNodeRule<JsPropertyObjec
             value,
         } = node.as_fields();
 
-        let key = FormatMemberName::from(name?);
-        let colon = colon_token.format();
-        let value = value.format();
-        formatted![formatter, [key, colon, space_token(), value]]
+        let name = name?;
+
+        let (format_name, name_width) =
+            FormatMemberName::from(name.clone()).format_member_name(formatter)?;
+        let name_width = name_width.unwrap_or_else(|| name.text().width());
+
+        let layout = property_object_member_layout(formatter, name_width, &value.clone()?)?;
+
+        let formatted = match layout {
+            PropertyObjectMemberLayout::Fluid => {
+                let group_id = formatter.group_id("property_object_member");
+
+                let value = formatted![formatter, [value.format()]]?;
+                formatted![
+                    formatter,
+                    [
+                        group_elements(format_name),
+                        colon_token.format(),
+                        group_elements_with_options(
+                            indent(soft_line_break_or_space()),
+                            GroupElementsOptions {
+                                group_id: Some(group_id),
+                            }
+                        ),
+                        line_suffix_boundary(),
+                        if_group_with_id_breaks(indent(value.clone()), group_id),
+                        if_group_with_id_fits_on_line(value, group_id),
+                    ]
+                ]
+            }
+            PropertyObjectMemberLayout::BreakAfterColon => {
+                formatted![
+                    formatter,
+                    [
+                        group_elements(format_name),
+                        colon_token.format(),
+                        space_token(),
+                        group_elements(formatted![
+                            formatter,
+                            [indent(formatted![
+                                formatter,
+                                [soft_line_break_or_space(), value.format()]
+                            ]?)]
+                        ]?),
+                    ]
+                ]
+            }
+            PropertyObjectMemberLayout::NeverBreakAfterColon => formatted![
+                formatter,
+                [
+                    group_elements(format_name),
+                    colon_token.format(),
+                    space_token(),
+                    value.format(),
+                ]
+            ],
+        };
+
+        Ok(group_elements(formatted?))
+    }
+}
+
+/// Determines how a property object member should be formatted
+enum PropertyObjectMemberLayout {
+    /// First break right-hand side, then after operator.
+    /// ```js
+    /// {
+    ///   "array-key": [
+    ///     {
+    ///       "nested-key-1": 1,
+    ///       "nested-key-2": 2,
+    ///     },
+    ///   ]
+    /// }
+    /// ```
+    Fluid,
+    /// First break after operator, then the sides are broken independently on their own lines.
+    /// There is a soft line break after colon token.
+    /// ```js
+    /// {
+    ///     "enough-long-key-to-break-line":
+    ///         1 + 2,
+    ///     "not-long-enough-key":
+    ///         "but long enough string to break line",
+    /// }
+    /// ```
+    BreakAfterColon,
+    /// First break right-hand side, then left-hand side. There are not any soft line breaks
+    /// between property name and property value
+    /// ```js
+    /// {
+    ///     key1: "123",
+    ///     key2: 123,
+    ///     key3: class MyClass {
+    ///        constructor() {},
+    ///     },
+    /// }
+    /// ```
+    NeverBreakAfterColon,
+}
+
+const MIN_OVERLAP_FOR_BREAK: u8 = 3;
+
+/// Returns the layout variant for an object member depending on value expression and name length
+/// [Prettier applies]: https://github.com/prettier/prettier/blob/main/src/language-js/print/assignment.js
+fn property_object_member_layout(
+    formatter: &JsFormatter,
+    name_width: usize,
+    value: &JsAnyExpression,
+) -> FormatResult<PropertyObjectMemberLayout> {
+    let text_width_for_break = (formatter.context().tab_width() + MIN_OVERLAP_FOR_BREAK) as usize;
+    let is_name_short = name_width < text_width_for_break;
+
+    if is_break_after_colon(value)? {
+        return Ok(PropertyObjectMemberLayout::BreakAfterColon);
+    }
+
+    if is_name_short {
+        return Ok(PropertyObjectMemberLayout::NeverBreakAfterColon);
+    } else if matches!(
+        value,
+        JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::JsStringLiteralExpression(
+            _
+        ))
+    ) {
+        return Ok(PropertyObjectMemberLayout::BreakAfterColon);
+    }
+
+    if is_never_break_after_colon(value)? {
+        return Ok(PropertyObjectMemberLayout::NeverBreakAfterColon);
+    }
+
+    Ok(PropertyObjectMemberLayout::Fluid)
+}
+
+fn is_break_after_colon(value: &JsAnyExpression) -> SyntaxResult<bool> {
+    if JsAnyBinaryLikeExpression::can_cast(value.syntax().kind()) {
+        return Ok(true);
+    }
+
+    if matches!(value, JsAnyExpression::JsSequenceExpression(_)) {
+        return Ok(true);
+    }
+
+    if let JsAnyExpression::JsConditionalExpression(conditional) = &value {
+        if JsAnyBinaryLikeExpression::can_cast(conditional.test()?.syntax().kind()) {
+            return Ok(true);
+        }
+    }
+
+    Ok(false)
+}
+
+fn is_never_break_after_colon(value: &JsAnyExpression) -> SyntaxResult<bool> {
+    if let JsAnyExpression::JsCallExpression(call_expression) = &value {
+        if call_expression.callee()?.syntax().text() == "require" {
+            return Ok(true);
+        }
+    }
+
+    if matches!(
+        value,
+        JsAnyExpression::JsClassExpression(_)
+            | JsAnyExpression::JsTemplate(_)
+            | JsAnyExpression::JsAnyLiteralExpression(
+                JsAnyLiteralExpression::JsBooleanLiteralExpression(_),
+            )
+            | JsAnyExpression::JsAnyLiteralExpression(
+                JsAnyLiteralExpression::JsNumberLiteralExpression(_)
+            )
+    ) {
+        return Ok(true);
     }
+
+    Ok(false)
 }

crates/rome_js_formatter/src/utils/mod.rs

@@ -433,6 +433,26 @@ impl From<JsAnyObjectMemberName> for FormatMemberName {
     }
 }
 
+impl FormatMemberName {
+    pub fn format_member_name(
+        &self,
+        formatter: &JsFormatter,
+    ) -> FormatResult<(FormatElement, Option<usize>)> {
+        match self {
+            FormatMemberName::ComputedMemberName(node) => {
+                formatted![formatter, [node.format()]].map(|element| (element, None))
+            }
+            FormatMemberName::PrivateClassMemberName(node) => {
+                formatted![formatter, [node.format()]].map(|element| (element, None))
+            }
+            FormatMemberName::LiteralMemberName(literal) => {
+                FormatLiteralStringToken::new(&literal.value()?, StringLiteralParentKind::Member)
+                    .format_token(formatter)
+            }
+        }
+    }
+}
+
 impl From<JsLiteralMemberName> for FormatMemberName {
     fn from(literal: JsLiteralMemberName) -> Self {
         Self::LiteralMemberName(literal)
@@ -443,19 +463,6 @@ impl Format for FormatMemberName {
     type Context = JsFormatContext;
 
     fn format(&self, formatter: &JsFormatter) -> FormatResult<FormatElement> {
-        let name = match self {
-            FormatMemberName::ComputedMemberName(node) => {
-                formatted![formatter, [node.format()]]
-            }
-            FormatMemberName::PrivateClassMemberName(node) => {
-                formatted![formatter, [node.format()]]
-            }
-            FormatMemberName::LiteralMemberName(literal) => {
-                FormatLiteralStringToken::new(&literal.value()?, StringLiteralParentKind::Member)
-                    .format(formatter)
-            }
-        };
-
-        name
+        self.format_member_name(formatter).map(|result| result.0)
     }
 }

crates/rome_js_formatter/src/utils/string_utils.rs

@@ -4,6 +4,7 @@ use crate::utils::string_utils::CharSignal::AlreadyPrinted;
 use rome_js_syntax::JsSyntaxKind::JS_STRING_LITERAL;
 use rome_js_syntax::{JsSyntaxToken, SourceType};
 use std::borrow::Cow;
+use unicode_width::UnicodeWidthStr;
 
 pub trait ToAsciiLowercaseCow {
     /// Returns the same value as String::to_lowercase. The only difference
@@ -70,27 +71,39 @@ impl<'token> FormatLiteralStringToken<'token> {
     pub fn token(&self) -> &'token JsSyntaxToken {
         self.token
     }
-}
-
-impl Format for FormatLiteralStringToken<'_> {
-    type Context = JsFormatContext;
 
-    fn format(&self, formatter: &JsFormatter) -> FormatResult<FormatElement> {
+    /// Returns the format element for the string literal
+    /// and the new text width if the string literal has been normalized
+    pub fn format_token(
+        &self,
+        formatter: &JsFormatter,
+    ) -> FormatResult<(FormatElement, Option<usize>)> {
         let token = self.token();
         // tokens that are don't hold any strings don't need to be processed any further
         if token.kind() != JS_STRING_LITERAL {
-            return formatted![formatter, [self.token.format()]];
+            return formatted![formatter, [self.token.format()]].map(|element| (element, None));
         }
         let chosen_quote_style = formatter.context().quote_style();
         let mut string_cleaner = LiteralStringNormaliser::new(self, chosen_quote_style);
 
         let content = string_cleaner.normalise_text(formatter.context().source_type.into());
+        let normalized_text_width = content.width();
 
-        Ok(formatter.format_replaced(
+        let element = formatter.format_replaced(
             token,
             Token::from_syntax_token_cow_slice(content, token, token.text_trimmed_range().start())
                 .into(),
-        ))
+        );
+
+        Ok((element, Some(normalized_text_width)))
+    }
+}
+
+impl Format for FormatLiteralStringToken<'_> {
+    type Context = JsFormatContext;
+
+    fn format(&self, formatter: &JsFormatter) -> FormatResult<FormatElement> {
+        self.format_token(formatter).map(|result| result.0)
     }
 }
 

crates/rome_js_formatter/tests/specs/js/module/expression/sequence_expression.js.snap

@@ -1,6 +1,5 @@
 ---
 source: crates/rome_js_formatter/tests/spec_test.rs
-assertion_line: 244
 expression: sequence_expression.js
 ---
 # Input