diff --git a/crates/rome_js_formatter/src/utils/binary_like_expression.rs b/crates/rome_js_formatter/src/utils/binary_like_expression.rs index 2f035ede93b..6ccef7bd59e 100644 --- a/crates/rome_js_formatter/src/utils/binary_like_expression.rs +++ b/crates/rome_js_formatter/src/utils/binary_like_expression.rs @@ -11,6 +11,7 @@ use rome_rowan::{AstNode, SyntaxResult}; use std::cmp::Ordering; use std::fmt::Debug; use std::iter::FusedIterator; +use std::ops::Deref; /// This function is charge to flat binaryish expressions that have the same precedence of their operators /// @@ -100,7 +101,7 @@ pub(crate) fn format_binary_like_expression( f: &mut JsFormatter, ) -> FormatResult<()> { let mut flatten_items = FlattenItems::default(); - let current_node = expression.syntax().clone(); + let current_node = expression.clone(); let post_order_binary_like_expressions = PostorderIterator::new(expression); let mut left: Option = None; @@ -109,24 +110,16 @@ pub(crate) fn format_binary_like_expression( let parent_operator = parent.operator_token()?; if let Some(left) = left { - flatten_items.format_binary_expression_right_hand_side( - left, - Some(parent_operator), - f, - )?; + flatten_items.flatten_binary_expression_right_hand_side(left, Some(parent_operator))?; } else { // Leaf binary like expression. Format the left hand side. // The right hand side gets formatted when traversing upwards in the tree. let left = parent.left()?; - let has_comments = left.syntax().has_comments_direct(); - let mut buffer = VecBuffer::new(f.state_mut()); - write!(buffer, [left])?; - - flatten_items.items.push(FlattenItem::regular( - buffer.into_element(), + flatten_items.items.push(FlattenItem::new( + FlattenedBinaryExpressionPart::Left { expression: left }, Some(parent_operator), has_comments.into(), )); @@ -137,11 +130,17 @@ pub(crate) fn format_binary_like_expression( // Format the top most binary like expression if let Some(root) = left { - flatten_items.format_binary_expression_right_hand_side(root, None, f)?; + flatten_items.flatten_binary_expression_right_hand_side(root, None)?; } - let element = flatten_items.take_format_element(¤t_node, f)?; - f.write_element(element) + let group = FlattenedBinaryExpressionPart::Group { + current: JsAnyBinaryLikeLeftExpression::JsAnyExpression(current_node.into_expression()), + expressions_start: flatten_items.current_group_start, + expressions_end: flatten_items.len(), + parenthesized: false, + }; + + group.write(f, &flatten_items) } /// Small wrapper to identify the operation of an expression and deduce their precedence @@ -170,25 +169,26 @@ enum BinaryLikeOperator { /// first `foo && bar` is computed and its result is then computed against `|| lorem`. /// /// In order to make this distinction more obvious, we wrap `foo && bar` in parenthesis. -fn format_with_or_without_parenthesis( +fn needs_parens( parent_operator: BinaryLikeOperator, - node: &JsSyntaxNode, - formatted_node: FormatElement, - f: &mut JsFormatter, -) -> FormatResult<(FormatElement, bool)> { - let compare_to = match JsAnyExpression::cast(node.clone()) { - Some(JsAnyExpression::JsLogicalExpression(logical)) => { - Some(BinaryLikeOperator::Logical(logical.operator()?)) - } - Some(JsAnyExpression::JsBinaryExpression(binary)) => { - Some(BinaryLikeOperator::Binary(binary.operator()?)) - } - Some(JsAnyExpression::JsInstanceofExpression(_)) => Some(BinaryLikeOperator::Instanceof), - Some(JsAnyExpression::JsInExpression(_)) => Some(BinaryLikeOperator::In), + node: &JsAnyBinaryLikeLeftExpression, +) -> FormatResult { + let compare_to = match node { + JsAnyBinaryLikeLeftExpression::JsAnyExpression(expression) => match expression { + JsAnyExpression::JsLogicalExpression(logical) => { + Some(BinaryLikeOperator::Logical(logical.operator()?)) + } + JsAnyExpression::JsBinaryExpression(binary) => { + Some(BinaryLikeOperator::Binary(binary.operator()?)) + } + JsAnyExpression::JsInstanceofExpression(_) => Some(BinaryLikeOperator::Instanceof), + JsAnyExpression::JsInExpression(_) => Some(BinaryLikeOperator::In), + _ => None, + }, _ => None, }; - let operation_is_higher = if let Some(compare_to) = compare_to { + let result = if let Some(compare_to) = compare_to { match (parent_operator, compare_to) { ( BinaryLikeOperator::Logical(previous_operation), @@ -209,13 +209,37 @@ fn format_with_or_without_parenthesis( false }; - let result = if operation_is_higher { - let (leading, content, trailing) = formatted_node.split_trivia(); + Ok(result) +} + +// False positive, Removing the `+ 'a` lifetime fails to compile with `hidden type for `impl Trait` captures lifetime that does not appear in bounds` +#[allow(clippy::needless_lifetimes)] +fn format_sub_expression<'a>( + parent_operator: BinaryLikeOperator, + sub_expression: &'a JsAnyBinaryLikeLeftExpression, +) -> impl Format + 'a { + format_with(move |f| { + if needs_parens(parent_operator, sub_expression)? { + write!(f, [format_parenthesized(sub_expression)]) + } else { + write!(f, [sub_expression]) + } + }) +} + +fn format_parenthesized<'a, Inner>(inner: Inner) -> impl Format +where + Inner: Format + 'a, +{ + format_with(move |f| { let mut buffer = VecBuffer::new(f.state_mut()); + write!(buffer, [inner])?; + let formatted_node = buffer.into_element(); + let (leading, content, trailing) = formatted_node.split_trivia(); - buffer.write_element(leading)?; + f.write_element(leading)?; write![ - buffer, + f, [group_elements(&format_args![ token("("), soft_block_indent(&format_once(|f| { @@ -224,24 +248,14 @@ fn format_with_or_without_parenthesis( })), token(")") ])] - ]?; - - (buffer.into_element(), true) - } else { - (formatted_node, false) - }; - - Ok(result) + ] + }) } -/// It tells if the expression can be hard grouped -fn can_hard_group(flatten_nodes: &[FlattenItem]) -> bool { +fn keep_on_same_line(flatten_nodes: &[FlattenItem]) -> bool { // We don't want to have 1 + 2 to break, for example. - // If there are any trailing comments, let's break. - flatten_nodes.len() <= 2 - && flatten_nodes - .iter() - .all(|node| !node.has_comments() && !node.is_group()) + // But if there are any trailing comments, break it. + flatten_nodes.len() <= 2 && flatten_nodes.iter().all(|node| !node.has_comments()) } fn is_inside_parenthesis(current_node: &JsSyntaxNode) -> bool { @@ -277,8 +291,8 @@ fn should_not_indent_if_parent_indents(current_node: &JsSyntaxNode) -> bool { /// these cases the decide to actually break on a new line and indent it. /// /// This function checks what the parents adheres to this behaviour -fn should_indent_if_parent_inlines(current_node: &JsSyntaxNode) -> bool { - let parent = current_node.parent(); +fn should_indent_if_parent_inlines(current_node: &JsAnyBinaryLikeLeftExpression) -> bool { + let parent = current_node.syntax().parent(); let grand_parent = parent.as_ref().and_then(|p| p.parent()); match (parent, grand_parent) { @@ -294,22 +308,32 @@ fn should_indent_if_parent_inlines(current_node: &JsSyntaxNode) -> bool { #[derive(Debug, Default)] struct FlattenItems { items: Vec, + + /// Position into `items` where the next group starts. + current_group_start: usize, +} + +impl Deref for FlattenItems { + type Target = [FlattenItem]; + + fn deref(&self) -> &Self::Target { + &self.items + } } impl FlattenItems { /// Formats the right hand side of a binary like expression - fn format_binary_expression_right_hand_side( + fn flatten_binary_expression_right_hand_side( &mut self, expression: JsAnyBinaryLikeExpression, parent_operator: Option, - f: &mut JsFormatter, ) -> FormatResult<()> { let should_flatten = expression.can_flatten()?; if should_flatten { - self.flatten_right_hand_side(expression, parent_operator, f) + self.flatten_right_hand_side(expression, parent_operator) } else { - self.format_new_binary_like_group(expression, parent_operator, f) + self.flatten_new_binary_like_group(expression, parent_operator) } } @@ -318,37 +342,29 @@ impl FlattenItems { &mut self, binary_like_expression: JsAnyBinaryLikeExpression, parent_operator: Option, - f: &mut JsFormatter, ) -> FormatResult<()> { - let right = binary_like_expression.right()?; + let right = JsAnyBinaryLikeLeftExpression::JsAnyExpression(binary_like_expression.right()?); let has_comments = right.syntax().has_comments_direct(); - let mut buffer = VecBuffer::new(f.state_mut()); - write!(buffer, [right.format()])?; - let right_formatted = buffer.into_element(); - - let (formatted_node, _) = format_with_or_without_parenthesis( - binary_like_expression.operator()?, - right.syntax(), - right_formatted, - f, - )?; - - let flatten_item = - FlattenItem::regular(formatted_node, parent_operator, has_comments.into()); + let flatten_item = FlattenItem::new( + FlattenedBinaryExpressionPart::Right { + parent: binary_like_expression, + }, + parent_operator, + has_comments.into(), + ); self.items.push(flatten_item); Ok(()) } /// The left hand-side expression and the current operator cannot be flattened. - /// Format the left hand side by its own and potentially wrap it in parentheses before formatting + /// Format the left hand side on its own and potentially wrap it in parentheses before formatting /// the right-hand side of the current expression. - fn format_new_binary_like_group( + fn flatten_new_binary_like_group( &mut self, binary_like_expression: JsAnyBinaryLikeExpression, parent_operator: Option, - f: &mut JsFormatter, ) -> FormatResult<()> { if let Some(last) = self.items.last_mut() { // Remove any line breaks and the trailing operator so that the operator/trailing aren't part @@ -361,13 +377,15 @@ impl FlattenItems { let operator = binary_like_expression.operator()?; let operator_token = binary_like_expression.operator_token()?; - let left_formatted = self.take_format_element(left.syntax(), f)?; - let (left_formatted, _) = - format_with_or_without_parenthesis(operator, left.syntax(), left_formatted, f)?; - let operator_has_trailing_comments = operator_token.has_trailing_comments(); - let mut left_item = FlattenItem::regular( - left_formatted, + let left_parenthesized = needs_parens(operator, &left)?; + let mut left_item = FlattenItem::new( + FlattenedBinaryExpressionPart::Group { + current: left, + expressions_start: self.current_group_start, + expressions_end: self.items.len(), + parenthesized: left_parenthesized, + }, Some(operator_token), operator_has_trailing_comments.into(), ); @@ -376,28 +394,23 @@ impl FlattenItems { left_item = left_item.with_terminator(TrailingTerminator::HardLineBreak); } + self.current_group_start = self.len(); self.items.push(left_item); - let right = binary_like_expression.right()?; - - let mut buffer = VecBuffer::new(f.state_mut()); - write!(buffer, [right.format()])?; - let formatted_node = buffer.into_element(); - - // Format the right node - let (formatted_right, parenthesized) = - format_with_or_without_parenthesis(operator, right.syntax(), formatted_node, f)?; + let right = JsAnyBinaryLikeLeftExpression::JsAnyExpression(binary_like_expression.right()?); + // Flatten the right node let parent_operator_has_comments = parent_operator .as_ref() .map(|operator| operator.has_leading_comments()); - let mut right_item = - if !parenthesized && matches!(right, JsAnyExpression::JsLogicalExpression(_)) { - FlattenItem::group(formatted_right, parent_operator, Comments::NoComments) - } else { - FlattenItem::regular(formatted_right, parent_operator, Comments::NoComments) - }; + let mut right_item = FlattenItem::new( + FlattenedBinaryExpressionPart::Right { + parent: binary_like_expression, + }, + parent_operator, + Comments::NoComments, + ); // Format the parent operator if let Some(parent_operator_has_comments) = parent_operator_has_comments { @@ -419,91 +432,6 @@ impl FlattenItems { Ok(()) } - - fn take_format_element( - &mut self, - current_node: &JsSyntaxNode, - f: &mut JsFormatter, - ) -> FormatResult { - let mut buffer = VecBuffer::new(f.state_mut()); - - write!( - buffer, - [format_once(|f| { - let can_hard_group = can_hard_group(&self.items); - - let mut groups = self.items.drain(..).map(|group| { - format_once(move |f| { - // groups not like ["something &&", "something &&" ] - // we want to add a space between them in case they don't break - - f.write_element(group.formatted)?; - - if let Some(operator) = group.operator { - write!(f, [space_token(), operator.format()])?; - } - - match group.terminator { - TrailingTerminator::None => (), - TrailingTerminator::HardLineBreak => write!(f, [hard_line_break()])?, - }; - - Ok(()) - }) - }); - - if can_hard_group { - // we bail early if group doesn't need to be broken. We don't need to do further checks - f.join_with(&space_token()).entries(groups).finish() - } else if is_inside_parenthesis(current_node) { - f.join_with(&soft_line_break_or_space()) - .entries(groups) - .finish() - } else if should_not_indent_if_parent_indents(current_node) { - write!( - f, - [group_elements(&format_once(|f| { - f.join_with(&soft_line_break_or_space()) - .entries(groups) - .finish() - }))] - ) - } else if should_indent_if_parent_inlines(current_node) { - // in order to correctly break, we need to check if the parent created a group - // that breaks or not. In order to do that , we need to create two conditional groups - // that behave differently depending on the situation - write!( - f, - [soft_line_indent_or_space(&group_elements(&format_once( - |f| { - f.join_with(&soft_line_break_or_space()) - .entries(groups) - .finish() - } - )))] - ) - } else { - // if none of the previous conditions is met, - // we take take out the first element from the rest of group, then we hard group the "head" - // and we indent the rest of the groups in a new line - write!(f, [groups.next().unwrap()])?; - - write!( - f, - [group_elements(&soft_line_indent_or_space(&format_once( - |f| { - f.join_with(&soft_line_break_or_space()) - .entries(groups) - .finish() - } - )))] - ) - } - })] - )?; - - Ok(buffer.into_element()) - } } #[derive(Debug)] @@ -529,10 +457,138 @@ impl From for Comments { } } +/// The left or right sub part of a binary expression. +#[derive(Debug)] +enum FlattenedBinaryExpressionPart { + /// The right hand side of a binary expression. Needs to format the parent operator and the right expression + Right { + /// The parent expression + parent: JsAnyBinaryLikeExpression, + }, + + /// The very first left hand side of a binary expression. Only formats the expression + Left { + /// The left hand side expression + expression: JsAnyBinaryLikeLeftExpression, + }, + + /// A group of expressions that can be grouped/printed together. + Group { + /// The binary expression that should be formatted now + current: JsAnyBinaryLikeLeftExpression, + + /// Start end/index into the flattened items array from where the left hand side expressions start + expressions_start: usize, + expressions_end: usize, + + /// Whether to parenthesize the expression + parenthesized: bool, + }, +} + +impl FlattenedBinaryExpressionPart { + fn write(&self, f: &mut JsFormatter, items: &[FlattenItem]) -> FormatResult<()> { + match self { + FlattenedBinaryExpressionPart::Right { parent } => { + let right = JsAnyBinaryLikeLeftExpression::JsAnyExpression(parent.right()?); + + write!(f, [format_sub_expression(parent.operator()?, &right)])?; + Ok(()) + } + FlattenedBinaryExpressionPart::Left { expression } => { + write!(f, [expression]) + } + FlattenedBinaryExpressionPart::Group { + current, + expressions_start, + expressions_end, + parenthesized, + } => { + let expressions = &items[*expressions_start..*expressions_end]; + let content = format_with(|f| { + let keep_on_same_line = keep_on_same_line(expressions); + + let mut groups = expressions.iter().map(|group| { + format_with(|f| { + group.expression.write(f, items)?; + + if let Some(operator) = &group.operator { + write!(f, [space_token(), operator.format()])?; + } + + match &group.terminator { + TrailingTerminator::None => (), + TrailingTerminator::HardLineBreak => { + write!(f, [hard_line_break()])? + } + }; + + Ok(()) + }) + }); + + if keep_on_same_line { + // we bail early if group doesn't need to be broken. We don't need to do further checks + f.join_with(space_token()).entries(groups).finish() + } else if is_inside_parenthesis(current.syntax()) { + f.join_with(soft_line_break_or_space()) + .entries(groups) + .finish() + } else if should_not_indent_if_parent_indents(current.syntax()) { + write!( + f, + [group_elements(&format_once(|f| { + f.join_with(soft_line_break_or_space()) + .entries(groups) + .finish() + }))] + ) + } else if should_indent_if_parent_inlines(current) { + write!( + f, + [soft_line_indent_or_space(&group_elements(&format_once( + |f| { + f.join_with(soft_line_break_or_space()) + .entries(groups) + .finish() + } + )))] + ) + } else { + // if none of the previous conditions is met, + // we take out the first element from the rest of the group + // and indent the rest of the groups in a new line + + // SAFETY: Safe because `keep_on_same_line` returns `true` if this is a single + // binary expression without any nested sub expressions. + write!(f, [groups.next().unwrap()])?; + + write!( + f, + [group_elements(&soft_line_indent_or_space(&format_once( + |f| { + f.join_with(soft_line_break_or_space()) + .entries(groups) + .finish() + } + )))] + ) + } + }); + + if *parenthesized { + write!(f, [format_parenthesized(content)]) + } else { + write!(f, [content]) + } + } + } + } +} + #[derive(Debug)] struct FlattenItem { - kind: FlattenItemKind, - formatted: FormatElement, + expression: FlattenedBinaryExpressionPart, operator: Option, terminator: TrailingTerminator, comments: Comments, @@ -545,38 +601,19 @@ enum TrailingTerminator { } impl FlattenItem { - fn regular( - formatted: FormatElement, + fn new( + expression: FlattenedBinaryExpressionPart, operator: Option, comments: Comments, ) -> Self { Self { - formatted, + expression, operator, - kind: FlattenItemKind::Regular, terminator: TrailingTerminator::None, comments, } } - fn group( - formatted: FormatElement, - operator: Option, - comments: Comments, - ) -> Self { - Self { - formatted, - comments, - operator, - terminator: TrailingTerminator::None, - kind: FlattenItemKind::Group, - } - } - - fn is_group(&self) -> bool { - matches!(self.kind, FlattenItemKind::Group) - } - fn has_comments(&self) -> bool { matches!(self.comments, Comments::WithComments) } @@ -592,14 +629,6 @@ impl FlattenItem { } } -#[derive(Debug)] -enum FlattenItemKind { - Regular, - /// Used when the right side of a binary/logical expression is another binary/logical. - /// When we have such cases we - Group, -} - /// The [PostorderIterator] visits every node twice. First on the way down to find the left most binary /// like expression, then on the way back up when it yields the binary like expressions. /// This enum encodes the information whatever the iterator is on its way down (`Enter`) or traversing @@ -756,6 +785,23 @@ impl JsAnyBinaryLikeExpression { JsAnyBinaryLikeExpression::JsInExpression(in_expression) => in_expression.object(), } } + + fn into_expression(self) -> JsAnyExpression { + match self { + JsAnyBinaryLikeExpression::JsLogicalExpression(logical) => { + JsAnyExpression::JsLogicalExpression(logical) + } + JsAnyBinaryLikeExpression::JsBinaryExpression(binary) => { + JsAnyExpression::JsBinaryExpression(binary) + } + JsAnyBinaryLikeExpression::JsInstanceofExpression(instanceof) => { + JsAnyExpression::JsInstanceofExpression(instanceof) + } + JsAnyBinaryLikeExpression::JsInExpression(in_expression) => { + JsAnyExpression::JsInExpression(in_expression) + } + } + } } impl AstNode for JsAnyBinaryLikeExpression {