feat(rome_js_formatter): Format JSX (#3144)

crates/rome_formatter/src/buffer.rs

@@ -164,10 +164,11 @@ pub struct VecBuffer<'a, Context> {
 
 impl<'a, Context> VecBuffer<'a, Context> {
     pub fn new(state: &'a mut FormatState<Context>) -> Self {
-        Self {
-            state,
-            elements: vec![],
-        }
+        Self::new_with_vec(state, Vec::new())
+    }
+
+    pub fn new_with_vec(state: &'a mut FormatState<Context>, elements: Vec<FormatElement>) -> Self {
+        Self { state, elements }
     }
 
     /// Creates a buffer with the specified capacity

crates/rome_formatter/src/builders.rs

@@ -2212,60 +2212,6 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> {
         self
     }
 
-    /// Adds an iterator of entries to the fill output, flattening any [FormatElement::List]
-    /// entries by adding the list's elements to the fill output.
-    ///
-    /// ## Warning
-    ///
-    /// The usage of this method is highly discouraged and it's better to use
-    /// other APIs on ways: for example progressively format the items based on their type.
-    pub fn flatten_entries<F, I>(
-        &mut self,
-        separator: &dyn Format<Context>,
-        entries: I,
-    ) -> &mut Self
-    where
-        F: Format<Context>,
-        I: IntoIterator<Item = F>,
-    {
-        for entry in entries {
-            self.flatten_entry(separator, &entry);
-        }
-
-        self
-    }
-
-    /// Adds a new entry to the fill output. If the entry is a [FormatElement::List],
-    /// then adds the list's entries to the fill output instead of the list itself.
-    fn flatten_entry(
-        &mut self,
-        separator: &dyn Format<Context>,
-        entry: &dyn Format<Context>,
-    ) -> &mut Self {
-        self.result = self.result.and_then(|_| {
-            let mut buffer = VecBuffer::new(self.fmt.state_mut());
-            write!(buffer, [entry])?;
-
-            let entries = buffer.into_vec();
-            self.items.reserve((entries.len() * 2).saturating_sub(1));
-
-            let mut buffer = VecBuffer::new(self.fmt.state_mut());
-            for item in entries.into_iter() {
-                if !self.items.is_empty() {
-                    write!(buffer, [separator])?;
-
-                    self.items.push(buffer.take_element());
-                }
-
-                self.items.push(item);
-            }
-
-            Ok(())
-        });
-
-        self
-    }
-
     /// Adds a new entry to the fill output. The `separator` isn't written if this is the first element in the list.
     pub fn entry(
         &mut self,

crates/rome_formatter/src/format_element.rs

@@ -224,6 +224,12 @@ pub enum LineMode {
     Empty,
 }
 
+impl LineMode {
+    pub const fn is_hard(&self) -> bool {
+        matches!(self, LineMode::Hard)
+    }
+}
+
 /// A token used to gather a list of elements; see [crate::Formatter::join_with].
 #[derive(Clone, Default, Eq, PartialEq)]
 pub struct List {
@@ -984,9 +990,15 @@ impl Format<IrFormatContext> for FormatElement {
                         [
                             dynamic_text(&std::format!("<interned {index}>"), TextSize::default()),
                             space(),
-                            &interned.0.as_ref()
                         ]
-                    )
+                    )?;
+
+                    match interned.0.as_ref() {
+                        element @ FormatElement::Text(_) | element @ FormatElement::Space => {
+                            write!(f, [text("\""), element, text("\"")])
+                        }
+                        element => element.fmt(f),
+                    }
                 } else {
                     write!(
                         f,
@@ -1017,13 +1029,9 @@ impl<'a> Format<IrFormatContext> for &'a [FormatElement] {
                                 matches!(element, FormatElement::Text(_) | FormatElement::Space);
 
                             if print_as_str {
-                                write!(f, [text("\"")])?;
-                            }
-
-                            write!(f, [group(&element)])?;
-
-                            if print_as_str {
-                                write!(f, [text("\"")])?;
+                                write!(f, [text("\""), &element, text("\"")])?;
+                            } else {
+                                write!(f, [group(&element)])?;
                             }
 
                             if index < len - 1 {

crates/rome_formatter/src/printed_tokens.rs

@@ -38,7 +38,8 @@ impl PrintedTokens {
                     descendant_offset if descendant_offset < *offset => {
                         panic!("token has not been seen by the formatter: {descendant:#?}.\
                         \nUse `format_replaced` if you want to replace a token from the formatted output.\
-                        \nUse `format_removed` if you to remove a token from the formatted output")
+                        \nUse `format_removed` if you want to remove a token from the formatted output.\n\
+                        parent: {:#?}", descendant.parent())
                     }
                     descendant_offset if descendant_offset > *offset => {
                         panic!("tracked offset {offset:?} doesn't match any token of {root:#?}. Have you passed a token from another tree?");

crates/rome_formatter/src/printer/mod.rs

@@ -474,12 +474,31 @@ impl<'a> Printer<'a> {
                         current_fits = next_fits;
                     }
                 }
-                (Some(_), _) => {
-                    // Don't print a trailing separator
+                // Trailing separator
+                (Some(separator), _) => {
+                    let print_mode = if current_fits
+                        && fits_on_line(
+                            [separator],
+                            args.with_print_mode(PrintMode::Flat),
+                            &empty_rest,
+                            self,
+                        ) {
+                        PrintMode::Flat
+                    } else {
+                        PrintMode::Expanded
+                    };
+
+                    self.print_all(queue, &[separator], args.with_print_mode(print_mode));
                 }
-                (None, _) => {
+                (None, None) => {
                     break;
                 }
+                (None, Some(_)) => {
+                    // Unreachable for iterators implementing [FusedIterator] which slice.iter implements.
+                    // Reaching this means that the first `iter.next()` returned `None` but calling `iter.next()`
+                    // again returns `Some`
+                    unreachable!()
+                }
             }
         }
     }

crates/rome_js_formatter/src/jsx/attribute/expression_attribute_value.rs

@@ -1,8 +1,8 @@
 use crate::prelude::*;
 
-use rome_formatter::write;
+use rome_formatter::{format_args, write};
 use rome_js_syntax::{
-    JsAnyExpression, JsxExpressionAttributeValue, JsxExpressionAttributeValueFields,
+    JsAnyExpression, JsxAnyTag, JsxExpressionAttributeValue, JsxExpressionAttributeValueFields,
 };
 
 #[derive(Debug, Clone, Default)]
@@ -19,58 +19,85 @@ impl FormatNodeRule<JsxExpressionAttributeValue> for FormatJsxExpressionAttribut
             expression,
             r_curly_token,
         } = node.as_fields();
-        write!(
-            f,
-            [group(&format_with(|f| {
-                write!(f, [l_curly_token.format()])?;
 
-                let expression = expression.as_ref()?;
+        let expression = expression?;
 
-                // When the inner expression for a prop is an object, array, or call expression, we want to combine the
-                // delimiters of the expression (`{`, `}`, `[`, `]`, or `(`, `)`) with the delimiters of the JSX
-                // attribute (`{`, `}`), so that we don't end up with redundant indents. Therefore we do not
-                // soft indent the expression
-                //
-                // Good:
-                // ```jsx
-                //  <ColorPickerPage
-                //     colors={[
-                //        "blue",
-                //        "brown",
-                //        "green",
-                //        "orange",
-                //        "purple",
-                //     ]} />
-                // ```
-                //
-                // Bad:
-                // ```jsx
-                //  <ColorPickerPage
-                //     colors={
-                //       [
-                //         "blue",
-                //          "brown",
-                //         "green",
-                //         "orange",
-                //         "purple",
-                //       ]
-                //     } />
-                // ```
-                //
-                if matches!(
-                    expression,
-                    JsAnyExpression::JsObjectExpression(_)
-                        | JsAnyExpression::JsArrayExpression(_)
-                        | JsAnyExpression::JsCallExpression(_)
-                        | JsAnyExpression::JsArrowFunctionExpression(_)
-                ) {
-                    write!(f, [expression.format()])?;
-                } else {
-                    write!(f, [soft_block_indent(&expression.format())])?;
-                };
+        let should_inline = should_inline_jsx_expression(&expression);
 
-                write!(f, [line_suffix_boundary(), r_curly_token.format()])
-            }))]
-        )
+        if should_inline {
+            write!(
+                f,
+                [
+                    l_curly_token.format(),
+                    expression.format(),
+                    line_suffix_boundary(),
+                    r_curly_token.format()
+                ]
+            )
+        } else {
+            write!(
+                f,
+                [group(&format_args![
+                    l_curly_token.format(),
+                    soft_block_indent(&expression.format()),
+                    line_suffix_boundary(),
+                    r_curly_token.format()
+                ])]
+            )
+        }
+    }
+}
+
+/// Tests if an expression inside of a [JsxExpressionChild] or [JsxExpressionAttributeValue] should be inlined.
+/// Good:
+/// ```jsx
+///  <ColorPickerPage
+///     colors={[
+///        "blue",
+///        "brown",
+///        "green",
+///        "orange",
+///        "purple",
+///     ]} />
+/// ```
+///
+/// Bad:
+/// ```jsx
+///  <ColorPickerPage
+///     colors={
+///       [
+///         "blue",
+///          "brown",
+///         "green",
+///         "orange",
+///         "purple",
+///       ]
+///     } />
+/// ```
+pub(crate) fn should_inline_jsx_expression(expression: &JsAnyExpression) -> bool {
+    use JsAnyExpression::*;
+
+    if expression.syntax().has_comments_direct() {
+        return false;
+    }
+
+    match expression {
+        JsArrayExpression(_)
+        | JsObjectExpression(_)
+        | JsArrowFunctionExpression(_)
+        | JsCallExpression(_)
+        | JsNewExpression(_)
+        | JsImportCallExpression(_)
+        | ImportMeta(_)
+        | JsFunctionExpression(_)
+        | JsTemplate(_) => true,
+        JsAwaitExpression(await_expression) => match await_expression.argument() {
+            Ok(JsxTagExpression(argument)) => {
+                matches!(argument.tag(), Ok(JsxAnyTag::JsxElement(_)))
+                    && should_inline_jsx_expression(&argument.into())
+            }
+            _ => false,
+        },
+        _ => false,
     }
 }