fix(formatter): Stable member chain printing

crates/rome_js_formatter/src/utils/member_chain/flatten_item.rs

@@ -6,7 +6,6 @@ use rome_js_syntax::{
 };
 use rome_rowan::{AstNode, SyntaxResult};
 use std::fmt::Debug;
-use std::slice;
 
 #[derive(Clone)]
 /// Data structure that holds the node with its formatted version
@@ -35,15 +34,6 @@ impl FlattenItem {
  }
  }
 
- pub(crate) fn as_format_elements(&self) -> &[FormatElement] {
- match self {
- FlattenItem::StaticMember(_, elements) => elements,
- FlattenItem::CallExpression(_, elements) => elements,
- FlattenItem::ComputedExpression(_, elements) => elements,
- FlattenItem::Node(_, element) => slice::from_ref(element),
- }
- }
-
  pub(crate) fn as_syntax(&self) -> &JsSyntaxNode {
  match self {
  FlattenItem::StaticMember(node, _) => node.syntax(),
@@ -53,15 +43,6 @@ impl FlattenItem {
  }
  }
 
- pub(crate) fn has_leading_comments(&self) -> bool {
- match self {
- FlattenItem::StaticMember(node, _) => node.syntax().has_leading_comments(),
- FlattenItem::CallExpression(node, _) => node.syntax().has_leading_comments(),
- FlattenItem::ComputedExpression(node, _) => node.syntax().has_leading_comments(),
- FlattenItem::Node(node, _) => node.has_leading_comments(),
- }
- }
-
  pub(crate) fn has_trailing_comments(&self) -> bool {
  match self {
  FlattenItem::StaticMember(node, _) => node.syntax().has_trailing_comments(),

crates/rome_js_formatter/src/utils/member_chain/groups.rs

@@ -2,7 +2,7 @@ use crate::prelude::*;
 use crate::utils::member_chain::flatten_item::FlattenItem;
 use crate::utils::member_chain::simple_argument::SimpleArgument;
 
-use rome_js_syntax::{JsAnyCallArgument, JsAnyExpression, JsCallExpression};
+use rome_js_syntax::JsCallExpression;
 use rome_rowan::{AstSeparatedList, SyntaxResult};
 use std::mem;
 
@@ -78,48 +78,19 @@ impl<'f> Groups<'f> {
  }
 
  /// It tells if the groups should be break on multiple lines
- pub fn groups_should_break(
- &self,
- calls_count: usize,
- head_group: &HeadGroup,
- ) -> SyntaxResult<bool> {
+ pub fn groups_should_break(&self, calls_count: usize) -> SyntaxResult<bool> {
  // Do not allow the group to break if it only contains a single call expression
  if calls_count <= 1 {
  return Ok(false);
  }
 
- let node_has_comments = self.has_comments() || head_group.has_comments();
  // we want to check the simplicity of the call expressions only if we have at least
  // two of them
  // Check prettier: https://github.com/prettier/prettier/blob/main/src/language-js/print/member-chain.js#L389
- let call_expressions_are_not_simple = if calls_count > 2 {
- self.call_expressions_are_not_simple()?
- } else {
- false
- };
- let last_group_will_break_and_other_calls_have_function_arguments =
- self.last_group_will_break_and_other_calls_have_function_arguments()?;
-
- // This emulates a simplified version of the similar logic found in the
- // printer to force groups to break if they contain any "hard line
- // break" (these not only include hard_line_break elements but also
- // empty_line or tokens containing the "\n" character): The idea is
- // that since any of these will force the group to break when it gets
- // printed, the formatter needs to emit a group element for the call
- // chain in the first place or it will not be printed correctly
- let has_line_breaks = self
- .groups
- .iter()
- .flat_map(|group| group.iter())
- .flat_map(|item| item.as_format_elements())
- .any(|element| element.will_break());
-
- let should_break = has_line_breaks
- || node_has_comments
- || call_expressions_are_not_simple
- || last_group_will_break_and_other_calls_have_function_arguments;
+ let call_expressions_are_not_simple =
+ calls_count > 2 && self.call_expressions_are_not_simple()?;
 
- Ok(should_break)
+ Ok(call_expressions_are_not_simple)
  }
 
  fn into_formatted_groups(self) -> Vec<FormatElement> {
@@ -132,53 +103,16 @@ impl<'f> Groups<'f> {
  /// Format groups on multiple lines
  pub fn into_joined_hard_line_groups(self) -> FormatElement {
  let formatted_groups = self.into_formatted_groups();
- join_elements(hard_line_break(), formatted_groups)
+ concat_elements(formatted_groups)
  }
 
  /// 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, FormatElement) {
+ pub fn into_format_elements(self) -> FormatElement {
  let formatted_groups = self.into_formatted_groups();
- (
- concat_elements(formatted_groups.clone()),
- join_elements(soft_line_break(), formatted_groups),
- )
- }
-
- /// Checks if the groups contain comments.
- pub fn has_comments(&self) -> bool {
- let has_leading_comments = if self.groups.len() > 1 {
- // SAFETY: access guarded by the previous check
- self.groups
- .iter()
- .flat_map(|item| item.iter())
- .skip(1)
- .any(|item| item.has_leading_comments())
- } else {
- false
- };
- let has_trailing_comments = self
- .groups
- .iter()
- .flat_map(|item| item.iter())
- .any(|item| item.has_trailing_comments());
-
- // This check might not be needed... trying to understand why Prettier has it
- let cutoff_has_leading_comments = if self.groups.len() >= self.cutoff as usize {
- self.groups
- .get(self.cutoff as usize)
- .map_or(false, |group| {
- group
- .first()
- .map_or(false, |group| group.has_leading_comments())
- })
- } else {
- false
- };
-
- has_leading_comments || has_trailing_comments || cutoff_has_leading_comments
+ concat_elements(formatted_groups.clone())
  }
 
  /// Filters the stack of [FlattenItem] and return only the ones that
@@ -210,48 +144,6 @@ impl<'f> Groups<'f> {
  }))
  }
 
- /// Checks if the last group will break - by emulating the behaviour of the printer,
- /// or if there's a call expression that contain a function/arrow function as argument
- pub fn last_group_will_break_and_other_calls_have_function_arguments(
- &self,
- ) -> SyntaxResult<bool> {
- let last_group = self.groups.iter().flat_map(|group| group.iter()).last();
-
- if let Some(last_group) = last_group {
- let element = last_group.as_format_elements().last();
- let group_will_break = element.map_or(false, |element| element.will_break());
-
- let is_call_expression = last_group.is_loose_call_expression();
-
- Ok(group_will_break
- && is_call_expression
- && self.call_expressions_have_function_or_arrow_func_as_argument()?)
- } else {
- Ok(false)
- }
- }
-
- /// Checks if any of the call expressions contains arguments that are functions or arrow
- /// functions.
- pub fn call_expressions_have_function_or_arrow_func_as_argument(&self) -> SyntaxResult<bool> {
- for call_expression in self.get_call_expressions() {
- let arguments = call_expression.arguments()?;
- for argument in arguments.args() {
- if matches!(
- argument?,
- JsAnyCallArgument::JsAnyExpression(JsAnyExpression::JsFunctionExpression(_))
- | JsAnyCallArgument::JsAnyExpression(
- JsAnyExpression::JsArrowFunctionExpression(_)
- )
- ) {
- return Ok(true);
- }
- }
- }
-
- Ok(false)
- }
-
  /// This is an heuristic needed to check when the first element of the group
  /// Should be part of the "head" or the "tail".
  fn should_not_wrap(&self, first_group: &HeadGroup) -> SyntaxResult<bool> {
@@ -332,8 +224,4 @@ impl HeadGroup {
  pub fn expand_group(&mut self, group: Vec<FlattenItem>) {
  self.items.extend(group)
  }
-
- fn has_comments(&self) -> bool {
- self.items.iter().any(|item| item.has_trailing_comments())
- }
 }

crates/rome_js_formatter/src/utils/member_chain/mod.rs

@@ -289,7 +289,8 @@ fn format_groups(
  head_group: HeadGroup,
  groups: Groups,
 ) -> FormatResult<FormatElement> {
- if groups.groups_should_break(calls_count, &head_group)? {
+ // TODO use Alternatives once available
+ if groups.groups_should_break(calls_count)? {
  Ok(format_elements![
  head_group.into_format_element(),
  group_elements(indent(format_elements![
@@ -298,13 +299,17 @@ fn format_groups(
  ]),)
  ])
  } else {
- let head_formatted = head_group.into_format_element();
- let (one_line, _) = groups.into_format_elements();
-
- // TODO: this is not the definitive solution, as there are few restrictions due to how the printer works:
- // - groups that contains other groups with hard lines break all the groups
- // - conditionally print one single line is subject to how the printer works (by default, multiline)
- Ok(format_elements![head_formatted, one_line])
+ Ok(format_elements![
+ head_group.into_format_element(),
+ groups.into_format_elements()
+ ])
+ // Ok(format_elements![
+ // head_group.into_format_element(),
+ // group_elements(indent(format_elements![
+ // soft_line_break(),
+ // groups.into_format_elements()
+ // ]))
+ // ])
  }
 }
 

crates/rome_js_formatter/tests/specs/js/module/expression/member-chain/complex_arguments.js.snap

@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/spec_test.rs
-assertion_line: 242
 expression: complex_arguments.js
-
 ---
 # Input
 client.execute(
@@ -21,7 +19,6 @@ Quote style: Double Quotes
 -----
 client.execute(
  Post.selectAll()
- .where(Post.id.eq(42))
- .where(Post.published.eq(true)),
+ .where(Post.id.eq(42)).where(Post.published.eq(true)),
 );
 

crates/rome_js_formatter/tests/specs/js/module/expression/member-chain/computed.js.snap

@@ -1,6 +1,5 @@
 ---
 source: crates/rome_js_formatter/tests/spec_test.rs
-assertion_line: 242
 expression: computed.js
 ---
 # Input
@@ -19,6 +18,13 @@ Line width: 80
 Quote style: Double Quotes
 -----
 nock(/test/)
- .matchHeader("Accept", "application/json")[httpMethodNock(method)]("/foo")
- .reply(200, { foo: "bar" });
+ .matchHeader("Accept", "application/json")[httpMethodNock(method)]("/foo").reply(
+ 200,
+ { foo: "bar" },
+ );
+
+
+## Lines exceeding width of 80 characters
+
+ 2: .matchHeader("Accept", "application/json")[httpMethodNock(method)]("/foo").reply(
 

crates/rome_js_formatter/tests/specs/js/module/expression/member-chain/inline-merge.js.snap

@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/spec_test.rs
-assertion_line: 242
 expression: inline-merge.js
-
 ---
 # Input
 _.flatMap(this.visibilityHandlers, fn => fn())
@@ -27,13 +25,11 @@ Line width: 80
 Quote style: Double Quotes
 -----
 _.flatMap(this.visibilityHandlers, (fn) => fn())
- .concat(this.record.resolved_legacy_visrules)
- .filter(Boolean);
+ .concat(this.record.resolved_legacy_visrules).filter(Boolean);
 
-Object.keys(availableLocales({ test: true }))
- .forEach(
- (locale) => {
- // ...
- },
- );
+Object.keys(availableLocales({ test: true })).forEach(
+ (locale) => {
+ // ...
+ },
+);
 

crates/rome_js_formatter/tests/specs/prettier/js/arrays/holes-in-args.js.snap

@@ -1,6 +1,5 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 175
 expression: holes-in-args.js
 ---
 # Input
@@ -15,9 +14,7 @@ new Test()
 # Output
 ```js
 new Test()
- .test()
- .test([, 0])
- .test();
+ .test().test([, 0]).test();
 
 ```
 

crates/rome_js_formatter/tests/specs/prettier/js/async/inline-await.js.snap

@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 125
 expression: inline-await.js
-
 ---
 # Input
 ```js
@@ -18,10 +16,11 @@ async function f() {
  const admins = (
  await (
  db
- .select("*")
- .from("admins")
- .leftJoin("bla")
- .where("id", "in", [1, 2, 3, 4])
+ .select("*").from("admins").leftJoin("bla").where(
+ "id",
+ "in",
+ [1, 2, 3, 4],
+ )
  )
  ).map(({ id, name }) => ({ id, name }));
 }

crates/rome_js_formatter/tests/specs/prettier/js/binary-expressions/arrow.js.snap

@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 125
 expression: arrow.js
-
 ---
 # Input
 ```js

crates/rome_js_formatter/tests/specs/prettier/js/comments/dangling_array.js.snap

@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 125
 expression: dangling_array.js
-
 ---
 # Input
 ```js
@@ -16,10 +14,9 @@ expect(() => {}).toTriggerReadyStateChanges([
 
 # Output
 ```js
-expect(() => {})
- .toTriggerReadyStateChanges([
- // Nothing.
- ]);
+expect(() => {}).toTriggerReadyStateChanges([
+ // Nothing.
+]);
 
 [1 /* first comment */ , 2 /* second comment */ , 3];