refactor(formatter): Member chain: left to right formatting
This PR refactors the member chain formatting into two stages:
* Building the member chain groups
* Formatting the groups
Doing so allows us to format the elements in left-to-right order.
MODIFY
crates/rome_js_formatter/src/utils/member_chain/flatten_item.rs
crates/rome_js_formatter/src/utils/member_chain/flatten_item.rs
use crate::prelude::*;
+use rome_formatter::write;
use rome_js_syntax::{
- JsAnyExpression, JsCallExpression, JsComputedMemberExpression, JsIdentifierExpression,
- JsImportCallExpression, JsNewExpression, JsStaticMemberExpression, JsSyntaxNode,
+ JsAnyExpression, JsCallExpression, JsCallExpressionFields, JsComputedMemberExpression,
+ JsComputedMemberExpressionFields, JsIdentifierExpression, JsImportCallExpression,
+ JsNewExpression, JsStaticMemberExpression, JsStaticMemberExpressionFields, JsSyntaxNode,
JsThisExpression,
};
use rome_rowan::{AstNode, SyntaxResult};
use std::fmt::Debug;
-#[derive(Clone)]
+#[derive(Clone, Debug)]
/// Data structure that holds the node with its formatted version
pub(crate) enum FlattenItem {
/// Holds onto a [rome_js_syntax::JsStaticMemberExpression]
- StaticMember(JsStaticMemberExpression, Vec<FormatElement>),
+ StaticMember(JsStaticMemberExpression),
/// Holds onto a [rome_js_syntax::JsCallExpression]
- CallExpression(JsCallExpression, Vec<FormatElement>),
+ CallExpression(JsCallExpression),
/// Holds onto a [rome_js_syntax::JsComputedMemberExpression]
- ComputedExpression(JsComputedMemberExpression, Vec<FormatElement>),
+ ComputedMember(JsComputedMemberExpression),
/// Any other node that are not [rome_js_syntax::JsCallExpression] or [rome_js_syntax::JsStaticMemberExpression]
/// Are tracked using this variant
- Node(JsSyntaxNode, FormatElement),
+ Node(JsSyntaxNode),
}
impl FlattenItem {
/// checks if the current node is a [rome_js_syntax::JsCallExpression], [rome_js_syntax::JsImportExpression] or a [rome_js_syntax::JsNewExpression]
pub fn is_loose_call_expression(&self) -> bool {
match self {
- FlattenItem::CallExpression(_, _) => true,
- FlattenItem::Node(node, _) => {
+ FlattenItem::CallExpression(_) => true,
+ FlattenItem::Node(node) => {
JsImportCallExpression::can_cast(node.kind())
| JsNewExpression::can_cast(node.kind())
}
@@ -36,36 +38,36 @@ impl FlattenItem {
pub(crate) fn as_syntax(&self) -> &JsSyntaxNode {
match self {
- FlattenItem::StaticMember(node, _) => node.syntax(),
- FlattenItem::CallExpression(node, _) => node.syntax(),
- FlattenItem::ComputedExpression(node, _) => node.syntax(),
- FlattenItem::Node(node, _) => node,
+ FlattenItem::StaticMember(node) => node.syntax(),
+ FlattenItem::CallExpression(node) => node.syntax(),
+ FlattenItem::ComputedMember(node) => node.syntax(),
+ FlattenItem::Node(node) => node,
}
}
pub(crate) fn has_trailing_comments(&self) -> bool {
match self {
- FlattenItem::StaticMember(node, _) => node.syntax().has_trailing_comments(),
- FlattenItem::CallExpression(node, _) => node.syntax().has_trailing_comments(),
- FlattenItem::ComputedExpression(node, _) => node.syntax().has_trailing_comments(),
- FlattenItem::Node(node, _) => node.has_trailing_comments(),
+ FlattenItem::StaticMember(node) => node.syntax().has_trailing_comments(),
+ FlattenItem::CallExpression(node) => node.syntax().has_trailing_comments(),
+ FlattenItem::ComputedMember(node) => node.syntax().has_trailing_comments(),
+ FlattenItem::Node(node) => node.has_trailing_comments(),
}
}
pub fn is_computed_expression(&self) -> bool {
- matches!(self, FlattenItem::ComputedExpression(..))
+ matches!(self, FlattenItem::ComputedMember(..))
}
pub(crate) fn is_this_expression(&self) -> bool {
match self {
- FlattenItem::Node(node, _) => JsThisExpression::can_cast(node.kind()),
+ FlattenItem::Node(node) => JsThisExpression::can_cast(node.kind()),
_ => false,
}
}
pub(crate) fn is_identifier_expression(&self) -> bool {
match self {
- FlattenItem::Node(node, _) => JsIdentifierExpression::can_cast(node.kind()),
+ FlattenItem::Node(node) => JsIdentifierExpression::can_cast(node.kind()),
_ => false,
}
}
@@ -138,32 +140,58 @@ impl FlattenItem {
}
}
-impl Debug for FlattenItem {
- fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl Format<JsFormatContext> for FlattenItem {
+ fn fmt(&self, f: &mut JsFormatter) -> FormatResult<()> {
match self {
- FlattenItem::StaticMember(_, formatted) => {
- std::write!(f, "StaticMember: {:?}", formatted)
- }
- FlattenItem::CallExpression(_, formatted) => {
- std::write!(f, "CallExpression: {:?}", formatted)
- }
- FlattenItem::ComputedExpression(_, formatted) => {
- std::write!(f, "ComputedExpression: {:?}", formatted)
- }
- FlattenItem::Node(node, formatted) => {
- std::write!(f, "{:?} {:?}", node.kind(), formatted)
- }
- }
- }
-}
+ FlattenItem::StaticMember(static_member) => {
+ let JsStaticMemberExpressionFields {
+ // Formatted as part of the previous item
+ object: _,
+ operator_token,
+ member,
+ } = static_member.as_fields();
+ write![f, [operator_token.format(), member.format(),]]
+ }
+ FlattenItem::CallExpression(call_expression) => {
+ let JsCallExpressionFields {
+ // Formatted as part of the previous item
+ callee: _,
+ optional_chain_token,
+ type_arguments,
+ arguments,
+ } = call_expression.as_fields();
-impl From<FlattenItem> for FormatElement {
- fn from(flatten_item: FlattenItem) -> Self {
- match flatten_item {
- FlattenItem::StaticMember(_, formatted) => FormatElement::from_iter(formatted),
- FlattenItem::CallExpression(_, formatted) => FormatElement::from_iter(formatted),
- FlattenItem::ComputedExpression(_, formatted) => FormatElement::from_iter(formatted),
- FlattenItem::Node(_, formatted) => formatted,
+ write!(
+ f,
+ [
+ optional_chain_token.format(),
+ type_arguments.format(),
+ arguments.format()
+ ]
+ )
+ }
+ FlattenItem::ComputedMember(computed_member) => {
+ let JsComputedMemberExpressionFields {
+ // Formatted as part of the previous item
+ object: _,
+ optional_chain_token,
+ l_brack_token,
+ member,
+ r_brack_token,
+ } = computed_member.as_fields();
+ write!(
+ f,
+ [
+ optional_chain_token.format(),
+ l_brack_token.format(),
+ member.format(),
+ r_brack_token.format(),
+ ]
+ )
+ }
+ FlattenItem::Node(node) => {
+ write!(f, [node.format()])
+ }
}
}
}

MODIFY
crates/rome_js_formatter/src/utils/member_chain/groups.rs
crates/rome_js_formatter/src/utils/member_chain/groups.rs
@@ -2,7 +2,6 @@ use crate::prelude::*;
use crate::utils::member_chain::flatten_item::FlattenItem;
use crate::utils::member_chain::simple_argument::SimpleArgument;
-use rome_formatter::{format, Buffer};
use rome_js_syntax::JsCallExpression;
use rome_rowan::{AstSeparatedList, SyntaxResult};
use std::mem;
@@ -93,43 +92,25 @@ impl Groups {
Ok(call_expressions_are_not_simple)
}
- fn into_formatted_groups(self) -> Vec<FormatElement> {
- self.groups
- .into_iter()
- .map(|group| {
- FormatElement::from_iter(group.into_iter().map(|flatten_item| flatten_item.into()))
- })
- .collect()
- }
-
/// Format groups on multiple lines
- pub fn into_joined_hard_line_groups(self) -> FormatElement {
- let elements = format!(
- JsFormatContext::default(),
- [format_once(|f| {
- let formatted_groups = self.into_formatted_groups();
-
- f.join_with(&hard_line_break())
+ pub fn write_joined_with_hard_line_breaks(&self, f: &mut JsFormatter) -> FormatResult<()> {
+ f.join_with(hard_line_break())
.entries(
- formatted_groups
- .into_iter()
- .map(|e| format_once(|f| f.write_element(e))),
+ self.groups
+ .iter()
+ .map(|group| format_with(|f| f.join().entries(group.iter()).finish())),
)
.finish()
- })]
- )
- .unwrap();
-
- elements.into_format_element()
}
/// Creates two different versions of the formatted groups, one that goes in one line
/// and the other one that goes on multiple lines.
///
/// It's up to the printer to decide which one to use.
- pub fn into_format_elements(self) -> FormatElement {
- let formatted_groups = self.into_formatted_groups();
- FormatElement::from_iter(formatted_groups)
+ pub fn write(&self, f: &mut JsFormatter) -> FormatResult<()> {
+ f.join()
+ .entries(self.groups.iter().flat_map(|group| group.iter()))
+ .finish()
}
/// Filters the stack of [FlattenItem] and return only the ones that
@@ -234,11 +215,13 @@ impl HeadGroup {
&self.items
}
- pub fn into_format_element(self) -> FormatElement {
- FormatElement::from_iter(self.items.into_iter().map(FlattenItem::into))
- }
-
pub fn expand_group(&mut self, group: Vec<FlattenItem>) {
self.items.extend(group)
}
}
+
+impl Format<JsFormatContext> for HeadGroup {
+ fn fmt(&self, f: &mut JsFormatter) -> FormatResult<()> {
+ f.join().entries(self.items.iter()).finish()
+ }
+}

MODIFY
crates/rome_js_formatter/src/utils/member_chain/mod.rs
crates/rome_js_formatter/src/utils/member_chain/mod.rs
@@ -5,7 +5,7 @@ mod simple_argument;
use crate::prelude::*;
use crate::utils::member_chain::flatten_item::FlattenItem;
use crate::utils::member_chain::groups::{Groups, HeadGroup};
-use rome_formatter::{format_args, write, Buffer, VecBuffer};
+use rome_formatter::{format_args, write, Buffer, PreambleBuffer};
use rome_js_syntax::{
JsCallExpression, JsComputedMemberExpression, JsExpressionStatement, JsStaticMemberExpression,
};
@@ -124,7 +124,7 @@ pub fn format_call_expression(syntax_node: &JsSyntaxNode, f: &mut JsFormatter) -
JsExpressionStatement::can_cast(parent.kind())
});
- flatten_call_expression(&mut flattened_items, syntax_node.clone(), f)?;
+ flatten_call_expression(&mut flattened_items, syntax_node, f)?;
// Count the number of CallExpression in the chain,
// will be used later to decide on how to format it
@@ -153,7 +153,7 @@ pub fn format_call_expression(syntax_node: &JsSyntaxNode, f: &mut JsFormatter) -
head_group.expand_group(group_to_merge);
}
- format_groups(calls_count, head_group, rest_of_groups, f)
+ write_groups(calls_count, head_group, rest_of_groups, f)
}
/// Retrieves the index where we want to calculate the first group.
@@ -174,7 +174,7 @@ fn compute_first_group_index(flatten_items: &[FlattenItem]) -> usize {
// - `something[1][2][4]`
// - `something[1]()[3]()`
// - `something()[2].something.else[0]`
- FlattenItem::CallExpression(_, _) | FlattenItem::ComputedExpression(_, _) => true,
+ FlattenItem::CallExpression(_) | FlattenItem::ComputedMember(_) => true,
// SAFETY: The check `flatten_items[index + 1]` will never panic at runtime because
// 1. The array will always have at least two items
@@ -205,11 +205,11 @@ fn compute_first_group_index(flatten_items: &[FlattenItem]) -> usize {
// and the next one is a call expression... the `matches!` fails and the loop is stopped.
//
// The last element of the array is always a `CallExpression`, which allows us to avoid the overflow of the array.
- FlattenItem::StaticMember(_, _) => {
+ FlattenItem::StaticMember(_) => {
let next_flatten_item = &flatten_items[index + 1];
matches!(
next_flatten_item,
- FlattenItem::StaticMember(_, _) | FlattenItem::ComputedExpression(_, _)
+ FlattenItem::StaticMember(_) | FlattenItem::ComputedMember(_)
)
}
_ => false,
@@ -239,7 +239,7 @@ fn compute_groups(
let has_trailing_comments = item.as_syntax().has_trailing_comments();
match item {
- FlattenItem::StaticMember(_, _) => {
+ FlattenItem::StaticMember(_) => {
// if we have seen a JsCallExpression, we want to close the group.
// The resultant group will be something like: [ . , then, () ];
// `.` and `then` belong to the previous StaticMemberExpression,
@@ -253,17 +253,17 @@ fn compute_groups(
groups.start_or_continue_group(item);
}
}
- FlattenItem::CallExpression(_, _) => {
+ FlattenItem::CallExpression(_) => {
let is_loose_call_expression = item.is_loose_call_expression();
groups.start_or_continue_group(item);
if is_loose_call_expression {
has_seen_call_expression = true;
}
}
- FlattenItem::ComputedExpression(_, _) => {
+ FlattenItem::ComputedMember(_) => {
groups.start_or_continue_group(item);
}
- FlattenItem::Node(_, _) => groups.continue_group(item),
+ FlattenItem::Node(_) => groups.continue_group(item),
}
// Close the group immediately if the node had any trailing comments to
@@ -281,35 +281,29 @@ fn compute_groups(
}
/// Formats together the first group and the rest of groups
-fn format_groups(
+fn write_groups(
calls_count: usize,
head_group: HeadGroup,
groups: Groups,
f: &mut JsFormatter,
) -> FormatResult<()> {
// TODO use Alternatives once available
- f.write_element(head_group.into_format_element())?;
+ write!(f, [head_group])?;
if groups.groups_should_break(calls_count)? {
write!(
f,
[indent(&format_args!(
hard_line_break(),
- format_once(|f| {
- f.write_element(groups.into_joined_hard_line_groups())?;
- Ok(())
- })
+ format_with(|f| { groups.write_joined_with_hard_line_breaks(f) })
))]
)
} else {
- let chain = groups.into_format_elements();
- if !chain.is_empty() {
// TODO This line suffix boundary shouldn't be needed but currently is because comments
// can move over node boundaries. Follow up when re-working member chain formatting
- write!(f, [line_suffix_boundary()])?;
- }
+ let mut buffer = PreambleBuffer::new(f, line_suffix_boundary());
- f.write_element(chain)
+ write!(buffer, [format_with(|f| { groups.write(f) })])
}
}
@@ -317,72 +311,35 @@ fn format_groups(
/// inside an vector of [FlattenItem]. The first element of the vector is the last one.
fn flatten_call_expression(
queue: &mut Vec<FlattenItem>,
- node: JsSyntaxNode,
+ node: &JsSyntaxNode,
f: &mut JsFormatter,
) -> FormatResult<()> {
match node.kind() {
JsSyntaxKind::JS_CALL_EXPRESSION => {
- let call_expression = JsCallExpression::cast(node).unwrap();
+ let call_expression = JsCallExpression::cast(node.clone()).unwrap();
let callee = call_expression.callee()?;
- flatten_call_expression(queue, callee.syntax().clone(), f)?;
+ flatten_call_expression(queue, callee.syntax(), f)?;
- let mut buffer = VecBuffer::new(f.state_mut());
- write!(
- buffer,
- [
- call_expression.optional_chain_token().format(),
- call_expression.type_arguments().format(),
- call_expression.arguments().format()
- ]
- )?;
-
- queue.push(FlattenItem::CallExpression(
- call_expression,
- buffer.into_vec(),
- ));
+ queue.push(FlattenItem::CallExpression(call_expression));
}
JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION => {
- let static_member = JsStaticMemberExpression::cast(node).unwrap();
+ let static_member = JsStaticMemberExpression::cast(node.clone()).unwrap();
let object = static_member.object()?;
- flatten_call_expression(queue, object.syntax().clone(), f)?;
+ flatten_call_expression(queue, object.syntax(), f)?;
- let mut buffer = VecBuffer::new(f.state_mut());
- let _formatted = write![
- buffer,
- [
- static_member.operator_token().format(),
- static_member.member().format(),
- ]
- ]?;
- queue.push(FlattenItem::StaticMember(static_member, buffer.into_vec()));
+ queue.push(FlattenItem::StaticMember(static_member));
}
JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION => {
- let computed_expression = JsComputedMemberExpression::cast(node).unwrap();
+ let computed_expression = JsComputedMemberExpression::cast(node.clone()).unwrap();
let object = computed_expression.object()?;
- flatten_call_expression(queue, object.syntax().clone(), f)?;
-
- let mut buffer = VecBuffer::new(f.state_mut());
- write!(
- buffer,
- [
- computed_expression.optional_chain_token().format(),
- computed_expression.l_brack_token().format(),
- computed_expression.member().format(),
- computed_expression.r_brack_token().format(),
- ]
- )?;
+ flatten_call_expression(queue, object.syntax(), f)?;
- queue.push(FlattenItem::ComputedExpression(
- computed_expression,
- buffer.into_vec(),
- ));
+ queue.push(FlattenItem::ComputedMember(computed_expression));
}
_ => {
- let mut buffer = VecBuffer::new(f.state_mut());
- write!(buffer, [node.format()])?;
- queue.push(FlattenItem::Node(node, buffer.into_element()));
+ queue.push(FlattenItem::Node(node.clone()));
}
}

refactor(formatter): Member chain: left to right formatting (#2641)

This PR refactors the member chain formatting into two stages:

* Building the member chain groups
* Formatting the groups

Doing so allows us to format the elements in left-to-right order.