diff --git a/crates/rome_js_formatter/src/js/auxiliary/new_target.rs b/crates/rome_js_formatter/src/js/auxiliary/new_target.rs index 71c91ebf6888..4305a61bf61a 100644 --- a/crates/rome_js_formatter/src/js/auxiliary/new_target.rs +++ b/crates/rome_js_formatter/src/js/auxiliary/new_target.rs @@ -1,8 +1,9 @@ use crate::prelude::*; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::NewTarget; -use rome_js_syntax::NewTargetFields; +use rome_js_syntax::{JsAnyExpression, NewTargetFields}; +use rome_js_syntax::{JsSyntaxNode, NewTarget}; #[derive(Debug, Clone, Default)] pub struct FormatNewTarget; @@ -24,4 +25,29 @@ impl FormatNodeRule for FormatNewTarget { ] ] } + + fn needs_parentheses(&self, item: &NewTarget) -> bool { + item.needs_parentheses() + } +} + +impl NeedsParentheses for NewTarget { + fn needs_parentheses(&self) -> bool { + false + } + + fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { + false + } +} + +impl ExpressionNode for NewTarget { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + fn into_resolved(self) -> JsAnyExpression { + self.into() + } } diff --git a/crates/rome_js_formatter/src/js/expressions/array_expression.rs b/crates/rome_js_formatter/src/js/expressions/array_expression.rs index 349254f990f2..880c310bcc32 100644 --- a/crates/rome_js_formatter/src/js/expressions/array_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/array_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::JsArrayExpressionFields; +use rome_js_syntax::{JsAnyExpression, JsArrayExpressionFields}; use rome_js_syntax::{JsArrayExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -28,13 +28,31 @@ impl FormatNodeRule for FormatJsArrayExpression { ] ) } + + fn needs_parentheses(&self, item: &JsArrayExpression) -> bool { + item.needs_parentheses() + } } impl NeedsParentheses for JsArrayExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } } + +impl ExpressionNode for JsArrayExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs b/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs index b2c909f46788..d4a85e26b725 100644 --- a/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs @@ -2,12 +2,10 @@ use crate::prelude::*; use rome_formatter::{format_args, write}; use crate::parentheses::{ - is_binary_like_left_or_right, is_conditional_test, is_in_left_hand_side_position, - NeedsParentheses, -}; -use crate::utils::{ - resolve_expression, resolve_left_most_expression, JsAnyBinaryLikeLeftExpression, + is_binary_like_left_or_right, is_conditional_test, + update_or_lower_expression_needs_parentheses, ExpressionNode, NeedsParentheses, }; +use crate::utils::{resolve_left_most_expression, JsAnyBinaryLikeLeftExpression}; use rome_js_syntax::{ JsAnyArrowFunctionParameters, JsAnyExpression, JsAnyFunctionBody, JsAnyTemplateElement, JsArrowFunctionExpression, JsArrowFunctionExpressionFields, JsSyntaxKind, JsSyntaxNode, @@ -88,13 +86,13 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi // going to get broken anyways. let body_has_soft_line_break = match &body { JsFunctionBody(_) => true, - JsAnyExpression(expr) => match resolve_expression(expr.clone()) { + JsAnyExpression(expr) => match expr.resolve() { JsArrowFunctionExpression(_) | JsArrayExpression(_) | JsObjectExpression(_) | JsParenthesizedExpression(_) - | JsTemplate(_) | JsxTagExpression(_) => true, + JsTemplate(template) => is_multiline_template_starting_on_same_line(&template), JsSequenceExpression(_) => { return write!( f, @@ -108,14 +106,6 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi ]) ])] ); - // // We handle sequence expressions as the body of arrows specially, - // // so that the required parentheses end up on their own lines. - // if (node.body.type === "SequenceExpression") { - // return group([ - // ...parts, - // group([" (", indent([softline, body]), softline, ")"]), - // ]); - // } } _ => false, }, @@ -126,7 +116,7 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi // case and added by the object expression itself let should_add_parens = match &body { JsAnyExpression(expression) => { - let resolved = resolve_expression(expression.clone()); + let resolved = expression.resolve(); let is_conditional = matches!(resolved, JsConditionalExpression(_)); let are_parentheses_mandatory = matches!( @@ -181,13 +171,25 @@ impl NeedsParentheses for JsArrowFunctionExpression { _ => { is_conditional_test(self.syntax(), parent) - || is_in_left_hand_side_position(self.syntax(), parent) + || update_or_lower_expression_needs_parentheses(self.syntax(), parent) || is_binary_like_left_or_right(self.syntax(), parent) } } } } +impl ExpressionNode for JsArrowFunctionExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs b/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs index 29b5634318f7..ddcb0d008f9c 100644 --- a/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs @@ -1,12 +1,16 @@ use crate::prelude::*; -use crate::utils::{resolve_expression_syntax, JsAnyAssignmentLike}; +use crate::utils::JsAnyAssignmentLike; -use crate::parentheses::{is_first_in_statement, FirstInStatementMode, NeedsParentheses}; +use crate::parentheses::{ + is_first_in_statement, ExpressionNode, FirstInStatementMode, NeedsParentheses, +}; use rome_formatter::write; + use rome_js_syntax::{ - JsAnyAssignmentPattern, JsAnyForInitializer, JsAnyFunctionBody, JsArrowFunctionExpression, - JsAssignmentExpression, JsForStatement, JsParenthesizedExpression, JsSyntaxKind, JsSyntaxNode, + JsAnyAssignmentPattern, JsAnyExpression, JsAnyForInitializer, JsAnyFunctionBody, + JsArrowFunctionExpression, JsAssignmentExpression, JsForStatement, JsSyntaxKind, JsSyntaxNode, }; +use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] pub struct FormatJsAssignmentExpression; @@ -23,11 +27,6 @@ impl FormatNodeRule for FormatJsAssignmentExpression { impl NeedsParentheses for JsAssignmentExpression { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { - let grand_parent = parent - .ancestors() - .skip(1) - .find(|parent| !JsParenthesizedExpression::can_cast(parent.kind())); - match parent.kind() { JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION => false, // `[a = b]` @@ -38,7 +37,7 @@ impl NeedsParentheses for JsAssignmentExpression { match arrow.body() { Ok(JsAnyFunctionBody::JsAnyExpression(expression)) => { - &resolve_expression_syntax(expression) == self.syntax() + &expression.resolve_syntax() == self.syntax() } _ => false, } @@ -47,14 +46,14 @@ impl NeedsParentheses for JsAssignmentExpression { let for_statement = JsForStatement::unwrap_cast(parent.clone()); let is_initializer = match for_statement.initializer() { Some(JsAnyForInitializer::JsAnyExpression(expression)) => { - &resolve_expression_syntax(expression) == self.syntax() + &expression.resolve_syntax() == self.syntax() } None | Some(_) => false, }; let is_update = for_statement .update() - .map(resolve_expression_syntax) + .map(ExpressionNode::into_resolved_syntax) .as_ref() == Some(self.syntax()); @@ -70,29 +69,53 @@ impl NeedsParentheses for JsAssignmentExpression { Ok(JsAnyAssignmentPattern::JsObjectAssignmentPattern(_)) ) } - JsSyntaxKind::JS_SEQUENCE_EXPRESSION => grand_parent - .and_then(JsForStatement::cast) - .map_or(true, |for_statement| { - let is_initializer = match for_statement.initializer() { - Some(JsAnyForInitializer::JsAnyExpression(expression)) => { - &resolve_expression_syntax(expression) == parent + JsSyntaxKind::JS_SEQUENCE_EXPRESSION => { + let mut child = parent.clone(); + + for ancestor in parent.ancestors().skip(1) { + match ancestor.kind() { + JsSyntaxKind::JS_SEQUENCE_EXPRESSION + | JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION => child = ancestor, + JsSyntaxKind::JS_FOR_STATEMENT => { + let for_statement = JsForStatement::unwrap_cast(ancestor); + + let is_initializer = match for_statement.initializer() { + Some(JsAnyForInitializer::JsAnyExpression(expression)) => { + expression.syntax() == &child + } + None | Some(_) => false, + }; + + let is_update = + for_statement.update().map(AstNode::into_syntax).as_ref() + == Some(&child); + + return !(is_initializer || is_update); } - None | Some(_) => false, - }; - let is_update = for_statement - .update() - .map(resolve_expression_syntax) - .as_ref() - == Some(parent); + _ => break, + } + } - !(is_initializer || is_update) - }), + true + } _ => true, } } } +impl ExpressionNode for JsAssignmentExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { @@ -110,6 +133,8 @@ mod tests { assert_not_needs_parentheses!("a => { a = 3 }", JsAssignmentExpression); assert_not_needs_parentheses!("for(a = 3;;) {}", JsAssignmentExpression); + assert_not_needs_parentheses!("for(a = 3, b = 2;;) {}", JsAssignmentExpression[1]); + assert_not_needs_parentheses!("for(a = 3, b = 2, c= 3;;) {}", JsAssignmentExpression[2]); assert_needs_parentheses!("for(; a = 3; ) {}", JsAssignmentExpression); assert_not_needs_parentheses!("for(;;a = 3) {}", JsAssignmentExpression); diff --git a/crates/rome_js_formatter/src/js/expressions/await_expression.rs b/crates/rome_js_formatter/src/js/expressions/await_expression.rs index 56e8a2b52730..1a4d24ae0505 100644 --- a/crates/rome_js_formatter/src/js/expressions/await_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/await_expression.rs @@ -2,11 +2,11 @@ use crate::prelude::*; use rome_formatter::write; use crate::parentheses::{ - is_binary_like_left_or_right, is_conditional_test, unary_expression_needs_parentheses, - NeedsParentheses, + is_binary_like_left_or_right, is_callee, is_conditional_test, is_member_object, is_spread, + update_or_lower_expression_needs_parentheses, ExpressionNode, NeedsParentheses, }; -use rome_js_syntax::{JsAwaitExpression, JsSyntaxNode}; +use rome_js_syntax::{JsAnyExpression, JsAwaitExpression, JsSyntaxNode}; use rome_js_syntax::{JsAwaitExpressionFields, JsSyntaxKind}; #[derive(Debug, Clone, Default)] @@ -19,7 +19,36 @@ impl FormatNodeRule for FormatJsAwaitExpression { argument, } = node.as_fields(); - write![f, [await_token.format(), space(), argument.format(),]] + let format_inner = + format_with(|f| write![f, [await_token.format(), space(), argument.format()]]); + + let parent = node.resolve_parent(); + + if let Some(parent) = parent { + if is_callee(node.syntax(), &parent) || is_member_object(node.syntax(), &parent) { + let ancestor_await_or_block = parent.ancestors().skip(1).find(|ancestor| { + matches!( + ancestor.kind(), + JsSyntaxKind::JS_AWAIT_EXPRESSION + // Stop at statement boundaries. + | JsSyntaxKind::JS_STATEMENT_LIST + | JsSyntaxKind::JS_MODULE_ITEM_LIST + ) + }); + + let indented = format_with(|f| write!(f, [soft_block_indent(&format_inner)])); + + return if ancestor_await_or_block.map_or(false, |ancestor| { + JsAwaitExpression::can_cast(ancestor.kind()) + }) { + write!(f, [indented]) + } else { + write!(f, [group(&indented)]) + }; + } + } + + write!(f, [format_inner]) } fn needs_parentheses(&self, item: &JsAwaitExpression) -> bool { @@ -34,17 +63,36 @@ impl NeedsParentheses for JsAwaitExpression { } pub(super) fn await_or_yield_needs_parens(parent: &JsSyntaxNode, node: &JsSyntaxNode) -> bool { + debug_assert!(matches!( + node.kind(), + JsSyntaxKind::JS_AWAIT_EXPRESSION | JsSyntaxKind::JS_YIELD_EXPRESSION + )); + match parent.kind() { JsSyntaxKind::JS_UNARY_EXPRESSION | JsSyntaxKind::TS_AS_EXPRESSION => true, _ => { + let expression = node; is_conditional_test(node, parent) - || unary_expression_needs_parentheses(node, parent) + || update_or_lower_expression_needs_parentheses(expression, parent) + || is_spread(expression, parent) || is_binary_like_left_or_right(node, parent) } } } +impl ExpressionNode for JsAwaitExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/big_int_literal_expression.rs b/crates/rome_js_formatter/src/js/expressions/big_int_literal_expression.rs index d37106f21957..cc4a50c9a9de 100644 --- a/crates/rome_js_formatter/src/js/expressions/big_int_literal_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/big_int_literal_expression.rs @@ -4,8 +4,8 @@ use std::borrow::Cow; use crate::prelude::*; use crate::utils::string_utils::ToAsciiLowercaseCow; -use crate::parentheses::NeedsParentheses; -use rome_js_syntax::JsBigIntLiteralExpressionFields; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsBigIntLiteralExpressionFields}; use rome_js_syntax::{JsBigIntLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -40,11 +40,25 @@ impl FormatNodeRule for FormatJsBigIntLiteralExpressi } } +impl ExpressionNode for JsBigIntLiteralExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self.clone())) + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self)) + } +} + impl NeedsParentheses for JsBigIntLiteralExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } diff --git a/crates/rome_js_formatter/src/js/expressions/binary_expression.rs b/crates/rome_js_formatter/src/js/expressions/binary_expression.rs index 7cb50929cc48..56f3e79d2333 100644 --- a/crates/rome_js_formatter/src/js/expressions/binary_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/binary_expression.rs @@ -1,7 +1,10 @@ use crate::prelude::*; -use crate::utils::{format_binary_like_expression, JsAnyBinaryLikeExpression}; +use crate::utils::{ + format_binary_like_expression, needs_binary_like_parentheses, JsAnyBinaryLikeExpression, +}; -use rome_js_syntax::JsBinaryExpression; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsBinaryExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsBinaryExpression; @@ -18,3 +21,77 @@ impl FormatNodeRule for FormatJsBinaryExpression { ) } } + +impl NeedsParentheses for JsBinaryExpression { + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + needs_binary_like_parentheses(&JsAnyBinaryLikeExpression::from(self.clone()), parent) + } +} + +impl ExpressionNode for JsBinaryExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + +#[cfg(test)] +mod tests { + use crate::{assert_needs_parentheses, assert_not_needs_parentheses}; + use rome_js_syntax::{JsBinaryExpression, SourceType}; + + #[test] + fn needs_parentheses() { + assert_needs_parentheses!("class X extends (4 + 4) {}", JsBinaryExpression); + + assert_needs_parentheses!("(4 + 4) as number", JsBinaryExpression); + assert_needs_parentheses!("(4 + 4)", JsBinaryExpression); + assert_needs_parentheses!("!(4 + 4)", JsBinaryExpression); + assert_needs_parentheses!("await (4 + 4)", JsBinaryExpression); + assert_needs_parentheses!("(4 + 4)!", JsBinaryExpression); + + assert_needs_parentheses!("(4 + 4)()", JsBinaryExpression); + assert_needs_parentheses!("(4 + 4)?.()", JsBinaryExpression); + assert_needs_parentheses!("new (4 + 4)()", JsBinaryExpression); + assert_needs_parentheses!("(4 + 4)`template`", JsBinaryExpression); + assert_needs_parentheses!("[...(4 + 4)]", JsBinaryExpression); + assert_needs_parentheses!("({...(4 + 4)})", JsBinaryExpression); + assert_needs_parentheses!( + "", + JsBinaryExpression, + SourceType::tsx() + ); + assert_needs_parentheses!( + "{...(4 + 4)}", + JsBinaryExpression, + SourceType::tsx() + ); + + assert_needs_parentheses!("(4 + 4).member", JsBinaryExpression); + assert_needs_parentheses!("(4 + 4)[member]", JsBinaryExpression); + assert_not_needs_parentheses!("object[4 + 4]", JsBinaryExpression); + + assert_needs_parentheses!("(4 + 4) * 3", JsBinaryExpression[1]); + assert_not_needs_parentheses!("(4 + 4) * 3", JsBinaryExpression[0]); + + assert_needs_parentheses!("a ** b ** c", JsBinaryExpression[1]); + assert_not_needs_parentheses!("a ** b ** c", JsBinaryExpression[0]); + + assert_needs_parentheses!("a * r >> 5", JsBinaryExpression[1]); + assert_not_needs_parentheses!("a * r >> 5", JsBinaryExpression[0]); + + assert_needs_parentheses!("a * r | 4", JsBinaryExpression[1]); + assert_not_needs_parentheses!("a * r | 5", JsBinaryExpression[0]); + + assert_needs_parentheses!("a % 4 + 4", JsBinaryExpression[1]); + assert_not_needs_parentheses!("a % 4 + 4", JsBinaryExpression[0]); + + assert_needs_parentheses!("a == b == c", JsBinaryExpression[1]); + assert_not_needs_parentheses!("a == b == c", JsBinaryExpression[0]); + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/boolean_literal_expression.rs b/crates/rome_js_formatter/src/js/expressions/boolean_literal_expression.rs index 221516cbc647..883f18691705 100644 --- a/crates/rome_js_formatter/src/js/expressions/boolean_literal_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/boolean_literal_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::JsBooleanLiteralExpressionFields; +use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsBooleanLiteralExpressionFields}; use rome_js_syntax::{JsBooleanLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -25,10 +25,24 @@ impl FormatNodeRule for FormatJsBooleanLiteralExpres } impl NeedsParentheses for JsBooleanLiteralExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } } + +impl ExpressionNode for JsBooleanLiteralExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self.clone())) + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self)) + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs index aa6a4becd427..437c28adec1a 100644 --- a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs +++ b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs @@ -1,7 +1,7 @@ use crate::builders::{format_close_delimiter, format_open_delimiter}; -use crate::parentheses::resolve_expression_parent; +use crate::parentheses::ExpressionNode; use crate::prelude::*; -use crate::utils::{is_call_like_expression, resolve_expression, write_arguments_multi_line}; +use crate::utils::{is_call_like_expression, write_arguments_multi_line}; use rome_formatter::{format_args, write}; use rome_js_syntax::{ JsAnyCallArgument, JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsAnyName, @@ -44,21 +44,20 @@ impl FormatNodeRule for FormatJsCallArguments { let first_argument = first_argument?; let second_argument = second_argument?; - let is_framework_test_call = if let Some(call_expression) = - resolve_expression_parent(node.syntax()).and_then(JsCallExpression::cast) - { - let callee = call_expression.callee()?; - - is_framework_test_call(IsTestFrameworkCallPayload { - first_argument: &first_argument, - second_argument: &second_argument, - third_argument: &third_argument, - arguments_len, - callee: &callee, - })? - } else { - false - }; + let is_framework_test_call = + if let Some(call_expression) = node.parent::() { + let callee = call_expression.callee()?; + + is_framework_test_call(IsTestFrameworkCallPayload { + first_argument: &first_argument, + second_argument: &second_argument, + third_argument: &third_argument, + arguments_len, + callee: &callee, + })? + } else { + false + }; let is_react_hook_with_deps_array = is_react_hook_with_deps_array(&first_argument, &second_argument)? @@ -105,20 +104,16 @@ impl FormatNodeRule for FormatJsCallArguments { .map(|e| e.memoized()) .collect(); - let mut any_breaks = false; let an_argument_breaks = separated .iter_mut() .enumerate() .any(|(index, element)| match element.inspect(f) { Ok(element) => { - if element.will_break() { - any_breaks = true; - should_group_first_argument && index > 0 - || (should_group_last_argument && index < args.len() - 1) - } else { - false - } + let in_relevant_range = should_group_first_argument && index > 0 + || (should_group_last_argument && index < args.len() - 1); + + in_relevant_range && element.will_break() } Err(_) => false, }); @@ -169,10 +164,6 @@ impl FormatNodeRule for FormatJsCallArguments { return write!(f, [all_arguments_expanded]); } - if any_breaks { - write!(f, [expand_parent()])?; - } - let edge_arguments_do_not_break = format_with(|f| { // `should_group_first_argument` and `should_group_last_argument` are mutually exclusive // which means that if one is `false`, then the other is `true`. @@ -213,9 +204,9 @@ impl FormatNodeRule for FormatJsCallArguments { l_leading_trivia, l_paren, l_trailing_trivia, - group(&format_args![format_with(|f| { + group(&format_with(|f| { write_arguments_multi_line(separated.iter(), f) - })]), + })), r_leading_trivia, r_paren, r_trailing_trivia @@ -229,19 +220,19 @@ impl FormatNodeRule for FormatJsCallArguments { f, [ l_leading_trivia, - &group(&format_args![ + group(&format_args![ l_paren, l_trailing_trivia, - &soft_block_indent(&format_with(|f| { + soft_block_indent(&format_with(|f| { let separated = args .format_separated(JsSyntaxKind::COMMA) .with_trailing_separator(TrailingSeparator::Omit) .nodes_grouped(); write_arguments_multi_line(separated, f) - }),), + })), r_leading_trivia, r_paren, - ],), + ]), r_trailing_trivia ] ) @@ -269,20 +260,20 @@ fn should_group_first_argument(list: &JsCallArgumentList) -> SyntaxResult _ => false, }; - let second_arg_is_function_like = matches!( - resolve_call_argument_expression(&second), - Some( - JsAnyExpression::JsFunctionExpression(_) - | JsAnyExpression::JsArrowFunctionExpression(_) - | JsAnyExpression::JsConditionalExpression(_) - ) - ); - - let can_group = match &second { - JsAnyCallArgument::JsAnyExpression(expression) => { - could_group_expression_argument(expression, false)? + let (second_arg_is_function_like, can_group) = match resolve_call_argument_expression(&second) { + Some(second_expression) => { + let second_arg_is_function_like = matches!( + &second_expression, + JsAnyExpression::JsFunctionExpression(_) + | JsAnyExpression::JsArrowFunctionExpression(_) + | JsAnyExpression::JsConditionalExpression(_) + ); + ( + second_arg_is_function_like, + could_group_expression_argument(&second_expression, false)?, + ) } - _ => false, + None => (false, false), }; Ok(!has_comments && is_function_like && !second_arg_is_function_like && !can_group) @@ -291,9 +282,9 @@ fn should_group_first_argument(list: &JsCallArgumentList) -> SyntaxResult /// Checks if the last group requires grouping fn should_group_last_argument(list: &JsCallArgumentList) -> SyntaxResult { let list_len = list.len(); - let mut iter = list.iter().rev(); - let last = iter.next(); - let penultimate = iter.next(); + let mut iter = list.iter(); + let last = iter.next_back(); + let penultimate = iter.next_back(); if let Some(last) = last { let last = last?; @@ -305,6 +296,7 @@ fn should_group_last_argument(list: &JsCallArgumentList) -> SyntaxResult { || !JsArrayExpression::can_cast(penultimate.syntax().kind()) || !JsArrowFunctionExpression::can_cast(last.syntax().kind()); + // TODO implement no poor printed array let _no_poor_printed_array = !list_len > 1 && JsArrayExpression::can_cast(last.syntax().kind()); different_kind && no_array_and_arrow_function @@ -330,7 +322,7 @@ fn could_group_expression_argument( argument: &JsAnyExpression, is_arrow_recursion: bool, ) -> SyntaxResult { - let result = match resolve_expression(argument.clone()) { + let result = match argument.resolve() { JsAnyExpression::JsObjectExpression(object_expression) => { object_expression.members().len() > 0 || object_expression @@ -381,14 +373,21 @@ fn could_group_expression_argument( } }); - let body_is_delimited = matches!( - body, - JsAnyFunctionBody::JsFunctionBody(_) - | JsAnyFunctionBody::JsAnyExpression(JsAnyExpression::JsObjectExpression(_)) - | JsAnyFunctionBody::JsAnyExpression(JsAnyExpression::JsArrayExpression(_)) - ); + let expression_body = match &body { + JsAnyFunctionBody::JsFunctionBody(_) => None, + JsAnyFunctionBody::JsAnyExpression(expression) => Some(expression.resolve()), + }; + + let body_is_delimited = matches!(body, JsAnyFunctionBody::JsFunctionBody(_)) + || matches!( + expression_body, + Some( + JsAnyExpression::JsObjectExpression(_) + | JsAnyExpression::JsArrayExpression(_) + ) + ); - if let JsAnyFunctionBody::JsAnyExpression(any_expression) = body.clone() { + if let Some(any_expression) = expression_body { let is_nested_arrow_function = if let JsAnyExpression::JsArrowFunctionExpression(arrow_function_expression) = &any_expression @@ -409,10 +408,8 @@ fn could_group_expression_argument( && (!is_arrow_recursion && (is_call_like_expression(&any_expression) || matches!( - body, - JsAnyFunctionBody::JsAnyExpression( - JsAnyExpression::JsConditionalExpression(_) - ) + any_expression, + JsAnyExpression::JsConditionalExpression(_) ))) } else { body_is_delimited && can_group_type @@ -436,9 +433,7 @@ fn is_react_hook_with_deps_array( second_argument: &JsAnyCallArgument, ) -> SyntaxResult { let first_expression = match first_argument { - JsAnyCallArgument::JsAnyExpression(expression) => { - Some(resolve_expression(expression.clone())) - } + JsAnyCallArgument::JsAnyExpression(expression) => Some(expression.resolve()), _ => None, }; @@ -531,7 +526,7 @@ fn is_framework_test_call(payload: IsTestFrameworkCallPayload) -> SyntaxResult resolve_call_argument_expression(&argument), + Ok(argument) => resolve_call_argument_expression(argument), _ => None, }); @@ -587,9 +582,7 @@ pub(super) fn resolve_call_argument_expression( argument: &JsAnyCallArgument, ) -> Option { match argument { - JsAnyCallArgument::JsAnyExpression(expression) => { - Some(resolve_expression(expression.clone())) - } + JsAnyCallArgument::JsAnyExpression(expression) => Some(expression.resolve()), _ => None, } } @@ -679,8 +672,10 @@ fn matches_test_call(callee: &JsAnyExpression) -> SyntaxResult { let value_token = identifier.name()?.value_token()?; @@ -699,6 +694,7 @@ fn matches_test_call(callee: &JsAnyExpression) -> SyntaxResult { + i -= 1; // Don't increment the depth parenthesized.expression()? } _ => break, @@ -771,6 +767,15 @@ mod test { ); } + #[test] + fn matches_parentheses() { + let call_expression = extract_call_expression("(test.describe.parallel).only();"); + assert_eq!( + contains_a_test_pattern(&call_expression.callee().unwrap()), + Ok(true) + ); + } + #[test] fn doesnt_static_member_expression_deep() { let call_expression = extract_call_expression("test.describe.parallel.only.AHAHA();"); diff --git a/crates/rome_js_formatter/src/js/expressions/call_expression.rs b/crates/rome_js_formatter/src/js/expressions/call_expression.rs index b17b07a8214d..7566246f39de 100644 --- a/crates/rome_js_formatter/src/js/expressions/call_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/call_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use crate::utils::get_member_chain; -use rome_js_syntax::{JsCallExpression, JsSyntaxKind, JsSyntaxNode}; +use rome_js_syntax::{JsAnyExpression, JsCallExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsCallExpression; @@ -21,11 +21,19 @@ impl FormatNodeRule for FormatJsCallExpression { impl NeedsParentheses for JsCallExpression { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { - match parent.kind() { - JsSyntaxKind::JS_NEW_EXPRESSION => true, + matches!(parent.kind(), JsSyntaxKind::JS_NEW_EXPRESSION) + } +} + +impl ExpressionNode for JsCallExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } - _ => false, - } + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() } } diff --git a/crates/rome_js_formatter/src/js/expressions/class_expression.rs b/crates/rome_js_formatter/src/js/expressions/class_expression.rs index 45951b11812a..167b372b1205 100644 --- a/crates/rome_js_formatter/src/js/expressions/class_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/class_expression.rs @@ -2,9 +2,9 @@ use crate::prelude::*; use crate::utils::format_class::FormatClass; use crate::parentheses::{ - is_callee, is_first_in_statement, FirstInStatementMode, NeedsParentheses, + is_callee, is_first_in_statement, ExpressionNode, FirstInStatementMode, NeedsParentheses, }; -use rome_js_syntax::{JsClassExpression, JsSyntaxNode}; +use rome_js_syntax::{JsAnyExpression, JsClassExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsClassExpression; @@ -29,6 +29,18 @@ impl NeedsParentheses for JsClassExpression { } } +impl ExpressionNode for JsClassExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs index f88bdfdac436..c9a84b5eb163 100644 --- a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs @@ -1,7 +1,7 @@ use crate::prelude::*; use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::{format_args, write}; use rome_js_syntax::{ JsAnyExpression, JsAnyLiteralExpression, JsComputedMemberAssignment, @@ -20,6 +20,10 @@ impl FormatNodeRule for FormatJsComputedMemberExpres ) -> FormatResult<()> { JsAnyComputedMemberLike::from(node.clone()).fmt(f) } + + fn needs_parentheses(&self, item: &JsComputedMemberExpression) -> bool { + item.needs_parentheses() + } } declare_node_union! { @@ -105,10 +109,6 @@ impl JsAnyComputedMemberLike { } } } - - fn needs_parentheses(&self, item: &JsComputedMemberExpression) -> bool { - item.needs_parentheses() - } } impl NeedsParentheses for JsComputedMemberExpression { @@ -121,6 +121,18 @@ impl NeedsParentheses for JsComputedMemberExpression { } } +impl ExpressionNode for JsComputedMemberExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { @@ -137,6 +149,7 @@ mod tests { ); assert_needs_parentheses!("new (test()![member])()", JsComputedMemberExpression); + assert_needs_parentheses!("new (a?.b[c])()", JsComputedMemberExpression); assert_not_needs_parentheses!("new (test[a])()", JsComputedMemberExpression); } } diff --git a/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs b/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs index fef5cbf04e3a..aecf825431ce 100644 --- a/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs @@ -2,10 +2,10 @@ use crate::prelude::*; use crate::utils::JsAnyConditional; use crate::parentheses::{ - is_binary_like_left_or_right, is_conditional_test, is_in_left_hand_side_position, is_spread, - NeedsParentheses, + is_binary_like_left_or_right, is_conditional_test, is_spread, + update_or_lower_expression_needs_parentheses, ExpressionNode, NeedsParentheses, }; -use rome_js_syntax::{JsConditionalExpression, JsSyntaxKind, JsSyntaxNode}; +use rome_js_syntax::{JsAnyExpression, JsConditionalExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsConditionalExpression; @@ -34,7 +34,7 @@ impl NeedsParentheses for JsConditionalExpression { _ => { is_conditional_test(self.syntax(), parent) - || is_in_left_hand_side_position(self.syntax(), parent) + || update_or_lower_expression_needs_parentheses(self.syntax(), parent) || is_binary_like_left_or_right(self.syntax(), parent) || is_spread(self.syntax(), parent) } @@ -42,6 +42,18 @@ impl NeedsParentheses for JsConditionalExpression { } } +impl ExpressionNode for JsConditionalExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/function_expression.rs b/crates/rome_js_formatter/src/js/expressions/function_expression.rs index 347102263855..87c649ab8397 100644 --- a/crates/rome_js_formatter/src/js/expressions/function_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/function_expression.rs @@ -2,10 +2,11 @@ use crate::prelude::*; use crate::js::declarations::function_declaration::FormatFunction; use crate::parentheses::{ - is_callee, is_first_in_statement, is_tag, FirstInStatementMode, NeedsParentheses, + is_callee, is_first_in_statement, is_tag, ExpressionNode, FirstInStatementMode, + NeedsParentheses, }; use rome_formatter::write; -use rome_js_syntax::{JsFunctionExpression, JsSyntaxNode}; +use rome_js_syntax::{JsAnyExpression, JsFunctionExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsFunctionExpression; @@ -31,6 +32,18 @@ impl NeedsParentheses for JsFunctionExpression { } } +impl ExpressionNode for JsFunctionExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/identifier_expression.rs b/crates/rome_js_formatter/src/js/expressions/identifier_expression.rs index 75e584bac977..c5cd76d023eb 100644 --- a/crates/rome_js_formatter/src/js/expressions/identifier_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/identifier_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::JsIdentifierExpressionFields; +use rome_js_syntax::{JsAnyExpression, JsIdentifierExpressionFields}; use rome_js_syntax::{JsIdentifierExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -21,10 +21,24 @@ impl FormatNodeRule for FormatJsIdentifierExpression { } impl NeedsParentheses for JsIdentifierExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } } + +impl ExpressionNode for JsIdentifierExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs b/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs index 0aea11882629..239748810cb8 100644 --- a/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs @@ -1,8 +1,9 @@ use crate::prelude::*; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::JsImportCallExpression; -use rome_js_syntax::JsImportCallExpressionFields; +use rome_js_syntax::{JsAnyExpression, JsImportCallExpressionFields}; +use rome_js_syntax::{JsImportCallExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsImportCallExpression; @@ -16,4 +17,26 @@ impl FormatNodeRule for FormatJsImportCallExpression { write![f, [import_token.format(), arguments.format(),]] } + + fn needs_parentheses(&self, item: &JsImportCallExpression) -> bool { + item.needs_parentheses() + } +} + +impl NeedsParentheses for JsImportCallExpression { + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + matches!(parent.kind(), JsSyntaxKind::JS_NEW_EXPRESSION) + } +} + +impl ExpressionNode for JsImportCallExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } } diff --git a/crates/rome_js_formatter/src/js/expressions/in_expression.rs b/crates/rome_js_formatter/src/js/expressions/in_expression.rs index 8aa7575ad882..381443ad447f 100644 --- a/crates/rome_js_formatter/src/js/expressions/in_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/in_expression.rs @@ -1,7 +1,14 @@ use crate::prelude::*; -use crate::utils::{format_binary_like_expression, JsAnyBinaryLikeExpression}; +use crate::utils::{ + format_binary_like_expression, needs_binary_like_parentheses, JsAnyBinaryLikeExpression, +}; -use rome_js_syntax::JsInExpression; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; + +use rome_js_syntax::{ + JsAnyExpression, JsAnyStatement, JsForStatement, JsInExpression, JsSyntaxNode, +}; +use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] pub struct FormatJsInExpression; @@ -14,3 +21,121 @@ impl FormatNodeRule for FormatJsInExpression { ) } } + +impl NeedsParentheses for JsInExpression { + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + if is_in_for_initializer(self) { + return true; + } + + needs_binary_like_parentheses(&JsAnyBinaryLikeExpression::from(self.clone()), parent) + } +} + +impl ExpressionNode for JsInExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + +/// Add parentheses if the `in` is inside of a `for` initializer (see tests). +fn is_in_for_initializer(expression: &JsInExpression) -> bool { + let mut current = expression.clone().into_syntax(); + + while let Some(parent) = current.parent() { + current = match JsForStatement::try_cast(parent) { + Ok(for_statement) => { + return for_statement + .initializer() + .map(AstNode::into_syntax) + .as_ref() + == Some(¤t); + } + Err(parent) => { + if JsAnyStatement::can_cast(parent.kind()) { + // Don't cross statement boundaries + break; + } + + parent + } + } + } + + false +} + +#[cfg(test)] +mod tests { + use crate::{assert_needs_parentheses, assert_not_needs_parentheses}; + use rome_js_syntax::{JsInExpression, SourceType}; + + #[test] + fn needs_parentheses() { + assert_needs_parentheses!("class X extends (a in b) {}", JsInExpression); + + assert_needs_parentheses!("(a in b) as number", JsInExpression); + assert_needs_parentheses!("(a in b)", JsInExpression); + assert_needs_parentheses!("!(a in b)", JsInExpression); + assert_needs_parentheses!("await (a in b)", JsInExpression); + assert_needs_parentheses!("(a in b)!", JsInExpression); + + assert_needs_parentheses!("(a in b)()", JsInExpression); + assert_needs_parentheses!("(a in b)?.()", JsInExpression); + assert_needs_parentheses!("new (a in b)()", JsInExpression); + assert_needs_parentheses!("(a in b)`template`", JsInExpression); + assert_needs_parentheses!("[...(a in b)]", JsInExpression); + assert_needs_parentheses!("({...(a in b)})", JsInExpression); + assert_needs_parentheses!("", JsInExpression, SourceType::tsx()); + assert_needs_parentheses!( + "{...(a in b)}", + JsInExpression, + SourceType::tsx() + ); + + assert_needs_parentheses!("(a in b).member", JsInExpression); + assert_needs_parentheses!("(a in b)[member]", JsInExpression); + assert_not_needs_parentheses!("object[a in b]", JsInExpression); + + assert_needs_parentheses!("(a in b) + c", JsInExpression); + + assert_not_needs_parentheses!("a in b > c", JsInExpression); + assert_not_needs_parentheses!("a in b instanceof C", JsInExpression); + assert_not_needs_parentheses!("a in b in c", JsInExpression[0]); + assert_not_needs_parentheses!("a in b in c", JsInExpression[1]); + } + + #[test] + fn for_in_needs_parentheses() { + assert_needs_parentheses!("for (let a = (b in c);;);", JsInExpression); + assert_needs_parentheses!("for (a && (b in c);;);", JsInExpression); + assert_needs_parentheses!("for (a => (b in c);;);", JsInExpression); + assert_needs_parentheses!( + "function* g() { + for (yield (a in b);;); +}", + JsInExpression + ); + assert_needs_parentheses!( + "async function f() { + for (await (a in b);;); +}", + JsInExpression + ); + + assert_not_needs_parentheses!("for (;a in b;);", JsInExpression); + assert_not_needs_parentheses!("for (;;a in b);", JsInExpression); + assert_not_needs_parentheses!( + r#" + for (function () { a in b }();;); + "#, + JsInExpression + ); + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/instanceof_expression.rs b/crates/rome_js_formatter/src/js/expressions/instanceof_expression.rs index a9e6e2d4e322..76dfdf1de585 100644 --- a/crates/rome_js_formatter/src/js/expressions/instanceof_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/instanceof_expression.rs @@ -1,7 +1,10 @@ use crate::prelude::*; -use crate::utils::{format_binary_like_expression, JsAnyBinaryLikeExpression}; +use crate::utils::{ + format_binary_like_expression, needs_binary_like_parentheses, JsAnyBinaryLikeExpression, +}; -use rome_js_syntax::JsInstanceofExpression; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsInstanceofExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsInstanceofExpression; @@ -18,3 +21,69 @@ impl FormatNodeRule for FormatJsInstanceofExpression { ) } } + +impl NeedsParentheses for JsInstanceofExpression { + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + needs_binary_like_parentheses(&JsAnyBinaryLikeExpression::from(self.clone()), parent) + } +} + +impl ExpressionNode for JsInstanceofExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + +#[cfg(test)] +mod tests { + use crate::{assert_needs_parentheses, assert_not_needs_parentheses}; + use rome_js_syntax::{JsInstanceofExpression, SourceType}; + + #[test] + fn needs_parentheses() { + assert_needs_parentheses!( + "class X extends (a instanceof b) {}", + JsInstanceofExpression + ); + + assert_needs_parentheses!("(a instanceof B) as number", JsInstanceofExpression); + assert_needs_parentheses!("(a instanceof B)", JsInstanceofExpression); + assert_needs_parentheses!("!(a instanceof B)", JsInstanceofExpression); + assert_needs_parentheses!("await (a instanceof B)", JsInstanceofExpression); + assert_needs_parentheses!("(a instanceof B)!", JsInstanceofExpression); + + assert_needs_parentheses!("(a instanceof B)()", JsInstanceofExpression); + assert_needs_parentheses!("(a instanceof B)?.()", JsInstanceofExpression); + assert_needs_parentheses!("new (a instanceof B)()", JsInstanceofExpression); + assert_needs_parentheses!("(a instanceof B)`template`", JsInstanceofExpression); + assert_needs_parentheses!("[...(a instanceof B)]", JsInstanceofExpression); + assert_needs_parentheses!("({...(a instanceof B)})", JsInstanceofExpression); + assert_needs_parentheses!( + "", + JsInstanceofExpression, + SourceType::tsx() + ); + assert_needs_parentheses!( + "{...(a instanceof B)}", + JsInstanceofExpression, + SourceType::tsx() + ); + + assert_needs_parentheses!("(a instanceof B).member", JsInstanceofExpression); + assert_needs_parentheses!("(a instanceof B)[member]", JsInstanceofExpression); + assert_not_needs_parentheses!("object[a instanceof B]", JsInstanceofExpression); + + assert_needs_parentheses!("(a instanceof B) + c", JsInstanceofExpression); + + assert_not_needs_parentheses!("a instanceof B > c", JsInstanceofExpression); + assert_not_needs_parentheses!("a instanceof B in c", JsInstanceofExpression); + assert_not_needs_parentheses!("a instanceof B instanceof c", JsInstanceofExpression[0]); + assert_not_needs_parentheses!("a instanceof B instanceof c", JsInstanceofExpression[1]); + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/logical_expression.rs b/crates/rome_js_formatter/src/js/expressions/logical_expression.rs index aec16bf932e3..a3184d006537 100644 --- a/crates/rome_js_formatter/src/js/expressions/logical_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/logical_expression.rs @@ -1,7 +1,10 @@ use crate::prelude::*; -use crate::utils::{format_binary_like_expression, JsAnyBinaryLikeExpression}; +use crate::utils::{ + format_binary_like_expression, needs_binary_like_parentheses, JsAnyBinaryLikeExpression, +}; -use rome_js_syntax::JsLogicalExpression; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsLogicalExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsLogicalExpression; @@ -18,3 +21,72 @@ impl FormatNodeRule for FormatJsLogicalExpression { ) } } + +impl NeedsParentheses for JsLogicalExpression { + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + if let Some(parent) = JsLogicalExpression::cast(parent.clone()) { + return parent.operator() != self.operator(); + } + + needs_binary_like_parentheses(&JsAnyBinaryLikeExpression::from(self.clone()), parent) + } +} + +impl ExpressionNode for JsLogicalExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + +#[cfg(test)] +mod tests { + + use crate::{assert_needs_parentheses, assert_not_needs_parentheses}; + use rome_js_syntax::{JsLogicalExpression, SourceType}; + + #[test] + fn needs_parentheses() { + assert_needs_parentheses!("class X extends (a && b) {}", JsLogicalExpression); + + assert_needs_parentheses!("(a && b) as number", JsLogicalExpression); + assert_needs_parentheses!("(a && b)", JsLogicalExpression); + assert_needs_parentheses!("!(a && b)", JsLogicalExpression); + assert_needs_parentheses!("await (a && b)", JsLogicalExpression); + assert_needs_parentheses!("(a && b)!", JsLogicalExpression); + + assert_needs_parentheses!("(a && b)()", JsLogicalExpression); + assert_needs_parentheses!("(a && b)?.()", JsLogicalExpression); + assert_needs_parentheses!("new (a && b)()", JsLogicalExpression); + assert_needs_parentheses!("(a && b)`template`", JsLogicalExpression); + assert_needs_parentheses!("[...(a && b)]", JsLogicalExpression); + assert_needs_parentheses!("({...(a && b)})", JsLogicalExpression); + assert_needs_parentheses!( + "", + JsLogicalExpression, + SourceType::tsx() + ); + assert_needs_parentheses!( + "{...(a && b)}", + JsLogicalExpression, + SourceType::tsx() + ); + + assert_needs_parentheses!("(a && b).member", JsLogicalExpression); + assert_needs_parentheses!("(a && b)[member]", JsLogicalExpression); + assert_not_needs_parentheses!("object[a && b]", JsLogicalExpression); + + assert_needs_parentheses!("(a && b) || c", JsLogicalExpression[1]); + assert_needs_parentheses!("(a && b) in c", JsLogicalExpression); + assert_needs_parentheses!("(a && b) instanceof c", JsLogicalExpression); + assert_needs_parentheses!("(a && b) + c", JsLogicalExpression); + + assert_not_needs_parentheses!("a && b && c", JsLogicalExpression[0]); + assert_not_needs_parentheses!("a && b && c", JsLogicalExpression[1]); + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/new_expression.rs b/crates/rome_js_formatter/src/js/expressions/new_expression.rs index a4f323f3e047..08fbc328234a 100644 --- a/crates/rome_js_formatter/src/js/expressions/new_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/new_expression.rs @@ -1,8 +1,9 @@ use crate::prelude::*; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::JsNewExpressionFields; -use rome_js_syntax::{JsNewExpression, JsSyntaxKind}; +use rome_js_syntax::{JsAnyExpression, JsNewExpression, JsSyntaxKind}; +use rome_js_syntax::{JsNewExpressionFields, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsNewExpression; @@ -41,4 +42,26 @@ impl FormatNodeRule for FormatJsNewExpression { } } } + + fn needs_parentheses(&self, item: &JsNewExpression) -> bool { + item.needs_parentheses() + } +} + +impl NeedsParentheses for JsNewExpression { + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + matches!(parent.kind(), JsSyntaxKind::JS_EXTENDS_CLAUSE) + } +} + +impl ExpressionNode for JsNewExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } } diff --git a/crates/rome_js_formatter/src/js/expressions/null_literal_expression.rs b/crates/rome_js_formatter/src/js/expressions/null_literal_expression.rs index fe6b9c90c10e..b98f3037aa61 100644 --- a/crates/rome_js_formatter/src/js/expressions/null_literal_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/null_literal_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::JsNullLiteralExpressionFields; +use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsNullLiteralExpressionFields}; use rome_js_syntax::{JsNullLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -21,10 +21,24 @@ impl FormatNodeRule for FormatJsNullLiteralExpression { } impl NeedsParentheses for JsNullLiteralExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } } + +impl ExpressionNode for JsNullLiteralExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self.clone())) + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self)) + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/number_literal_expression.rs b/crates/rome_js_formatter/src/js/expressions/number_literal_expression.rs index c970fd08667b..2138c228a949 100644 --- a/crates/rome_js_formatter/src/js/expressions/number_literal_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/number_literal_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::parentheses::{is_member_object, NeedsParentheses}; +use crate::parentheses::{is_member_object, ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::JsNumberLiteralExpression; +use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsNumberLiteralExpression}; use rome_js_syntax::{JsNumberLiteralExpressionFields, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -31,6 +31,18 @@ impl NeedsParentheses for JsNumberLiteralExpression { } } +impl ExpressionNode for JsNumberLiteralExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self.clone())) + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self)) + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/object_expression.rs b/crates/rome_js_formatter/src/js/expressions/object_expression.rs index 0a58a3d8eb71..5746064a6416 100644 --- a/crates/rome_js_formatter/src/js/expressions/object_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/object_expression.rs @@ -1,8 +1,10 @@ -use crate::parentheses::{is_first_in_statement, FirstInStatementMode, NeedsParentheses}; +use crate::parentheses::{ + is_first_in_statement, ExpressionNode, FirstInStatementMode, NeedsParentheses, +}; use crate::prelude::*; use crate::utils::JsObjectLike; use rome_formatter::write; -use rome_js_syntax::{JsObjectExpression, JsSyntaxKind, JsSyntaxNode}; +use rome_js_syntax::{JsAnyExpression, JsObjectExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsObjectExpression; @@ -27,6 +29,18 @@ impl NeedsParentheses for JsObjectExpression { } } +impl ExpressionNode for JsObjectExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { use crate::assert_needs_parentheses; diff --git a/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs b/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs index e22370002a2b..ca0bf10e70c4 100644 --- a/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs @@ -1,14 +1,16 @@ use crate::prelude::*; use crate::utils::{ - binary_argument_needs_parens, is_simple_expression, resolve_expression, FormatPrecedence, + binary_argument_needs_parens, is_simple_expression, FormatPrecedence, JsAnyBinaryLikeLeftExpression, }; use rome_formatter::write; use crate::utils::JsAnyBinaryLikeExpression; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_js_syntax::{ JsAnyExpression, JsParenthesizedExpression, JsParenthesizedExpressionFields, JsSyntaxKind, + JsSyntaxNode, }; use rome_rowan::{AstNode, SyntaxResult}; @@ -42,22 +44,7 @@ impl FormatNodeRule for FormatJsParenthesizedExpressi let parenthesis_can_be_omitted = parenthesis_can_be_omitted(node, &expression)?; - if is_simple_parenthesized_expression(node)? { - if parenthesis_can_be_omitted { - write!(f, [format_removed(&l_paren_token?)])?; - } else { - write![f, [l_paren_token.format()]]?; - }; - - write![f, [expression.format()]]?; - - if parenthesis_can_be_omitted { - write!(f, [format_removed(&r_paren_token?)])?; - } else { - write![f, [r_paren_token.format()]]?; - } - } else if parenthesis_can_be_omitted { - // we mimic the format delimited utility function + if parenthesis_can_be_omitted { write![ f, [ @@ -65,40 +52,29 @@ impl FormatNodeRule for FormatJsParenthesizedExpressi expression.format(), format_removed(&r_paren_token?), ] - ]?; + ] + } else if is_simple_parenthesized_expression(node)? { + write![ + f, + [ + l_paren_token.format(), + expression.format(), + r_paren_token.format() + ] + ] } else { - match expression { - JsAnyExpression::JsObjectExpression(_) | JsAnyExpression::JsCallExpression(_) => { - write![ - f, - [ - l_paren_token.format(), - expression.format(), - r_paren_token.format(), - ] - ] - } - JsAnyExpression::JsxTagExpression(expression) => { - write![ - f, - [ - format_removed(&l_paren_token?), - expression.format(), - format_removed(&r_paren_token?), - ] - ] - } - _ => write![ - f, - [ - format_delimited(&l_paren_token?, &expression.format(), &r_paren_token?,) - .soft_block_indent() - ] - ], - }?; + write![ + f, + [ + format_delimited(&l_paren_token?, &expression.format(), &r_paren_token?,) + .soft_block_indent() + ] + ] } + } - Ok(()) + fn needs_parentheses(&self, item: &JsParenthesizedExpression) -> bool { + item.needs_parentheses() } } @@ -120,7 +96,7 @@ fn is_simple_parenthesized_expression(node: &JsParenthesizedExpression) -> Synta Ok(true) } -// Allow list of nodes that manually handle inserting parens if needed +// Allow list of nodes that use the new `need_parens` formatting to determine if parentheses are necessary or not. pub(crate) fn is_expression_handling_parens(expression: &JsAnyExpression) -> bool { use JsAnyExpression::*; @@ -131,31 +107,12 @@ pub(crate) fn is_expression_handling_parens(expression: &JsAnyExpression) -> boo false } } else { - matches!( + !matches!( expression, - JsConditionalExpression(_) - | JsArrayExpression(_) - | JsUnaryExpression(_) - | JsPreUpdateExpression(_) - | JsPostUpdateExpression(_) - | JsObjectExpression(_) - | JsFunctionExpression(_) - | JsClassExpression(_) - | JsAwaitExpression(_) - | JsYieldExpression(_) - | JsIdentifierExpression(_) - | JsThisExpression(_) - | JsAnyLiteralExpression(_) - | JsSequenceExpression(_) - | JsSuperExpression(_) - | JsAssignmentExpression(_) - | JsArrowFunctionExpression(_) - | JsCallExpression(_) - | JsStaticMemberExpression(_) - | JsComputedMemberExpression(_) - | TsNonNullAssertionExpression(_) - | JsxTagExpression(_) - | TsAsExpression(_) + JsInstanceofExpression(_) + | JsBinaryExpression(_) + | JsInExpression(_) + | JsLogicalExpression(_) ) } } @@ -179,17 +136,7 @@ fn parenthesis_can_be_omitted( } } - let expression = resolve_expression(expression.clone()); - - match expression { - JsAnyExpression::JsConditionalExpression(_) => { - panic!("Reached conditional expression when it should have not, parent is:\n{parent:#?}\nexpression:\n{expression:#?}") - } - - _ => { - // fall through - } - } + let expression = expression.resolve(); let parent_precedence = FormatPrecedence::with_precedence_for_parenthesis(parent.as_ref()); @@ -198,19 +145,38 @@ fn parenthesis_can_be_omitted( } if let Some(parent) = parent { - if let Some(binary_like) = JsAnyBinaryLikeExpression::cast(parent) { - let operator = binary_like.operator()?; - let is_right = expression.syntax() == binary_like.right()?.syntax(); - - if !binary_argument_needs_parens( - operator, - &JsAnyBinaryLikeLeftExpression::from(expression.clone()), - is_right, - )? { - return Ok(true); - } + if JsAnyBinaryLikeExpression::can_cast(parent.kind()) + && !binary_argument_needs_parens(&JsAnyBinaryLikeLeftExpression::from(expression)) + { + return Ok(true); } } Ok(false) } + +impl NeedsParentheses for JsParenthesizedExpression { + #[inline(always)] + fn needs_parentheses(&self) -> bool { + false + } + + #[inline(always)] + fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { + false + } +} + +impl ExpressionNode for JsParenthesizedExpression { + fn resolve(&self) -> JsAnyExpression { + let inner = self.expression(); + + inner.unwrap_or_else(|_| self.clone().into()) + } + + fn into_resolved(self) -> JsAnyExpression { + let inner = self.expression(); + + inner.unwrap_or_else(|_| self.into()) + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/post_update_expression.rs b/crates/rome_js_formatter/src/js/expressions/post_update_expression.rs index f943a3caf3e6..9a87865f119d 100644 --- a/crates/rome_js_formatter/src/js/expressions/post_update_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/post_update_expression.rs @@ -1,8 +1,10 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{update_expression_needs_parentheses, NeedsParentheses}; -use rome_js_syntax::JsPostUpdateExpressionFields; +use crate::parentheses::{ + unary_like_expression_needs_parentheses, ExpressionNode, NeedsParentheses, +}; +use rome_js_syntax::{JsAnyExpression, JsPostUpdateExpressionFields}; use rome_js_syntax::{JsPostUpdateExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -25,7 +27,19 @@ impl FormatNodeRule for FormatJsPostUpdateExpression { impl NeedsParentheses for JsPostUpdateExpression { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { - update_expression_needs_parentheses(parent, self.syntax()) + unary_like_expression_needs_parentheses(self.syntax(), parent) + } +} + +impl ExpressionNode for JsPostUpdateExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() } } diff --git a/crates/rome_js_formatter/src/js/expressions/pre_update_expression.rs b/crates/rome_js_formatter/src/js/expressions/pre_update_expression.rs index 77742b51eba5..34b2db5b7031 100644 --- a/crates/rome_js_formatter/src/js/expressions/pre_update_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/pre_update_expression.rs @@ -1,9 +1,12 @@ -use crate::parentheses::{update_expression_needs_parentheses, NeedsParentheses}; +use crate::parentheses::{ + unary_like_expression_needs_parentheses, ExpressionNode, NeedsParentheses, +}; use crate::prelude::*; use rome_formatter::write; use rome_js_syntax::{ - JsPreUpdateExpression, JsPreUpdateOperator, JsSyntaxNode, JsUnaryExpression, JsUnaryOperator, + JsAnyExpression, JsPreUpdateExpression, JsPreUpdateOperator, JsSyntaxNode, JsUnaryExpression, + JsUnaryOperator, }; use rome_js_syntax::{JsPreUpdateExpressionFields, JsSyntaxKind}; @@ -38,11 +41,23 @@ impl NeedsParentheses for JsPreUpdateExpression { || (parent_operator == Ok(JsUnaryOperator::Minus) && operator == Ok(JsPreUpdateOperator::Decrement)) } - _ => update_expression_needs_parentheses(parent, self.syntax()), + _ => unary_like_expression_needs_parentheses(self.syntax(), parent), } } } +impl ExpressionNode for JsPreUpdateExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/regex_literal_expression.rs b/crates/rome_js_formatter/src/js/expressions/regex_literal_expression.rs index 99954b41a666..a1174c4bd61b 100644 --- a/crates/rome_js_formatter/src/js/expressions/regex_literal_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/regex_literal_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::NeedsParentheses; -use rome_js_syntax::JsRegexLiteralExpressionFields; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsRegexLiteralExpressionFields}; use rome_js_syntax::{JsRegexLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -48,10 +48,24 @@ impl FormatNodeRule for FormatJsRegexLiteralExpression } impl NeedsParentheses for JsRegexLiteralExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } } + +impl ExpressionNode for JsRegexLiteralExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self.clone())) + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self)) + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/sequence_expression.rs b/crates/rome_js_formatter/src/js/expressions/sequence_expression.rs index dc3167857a96..aa0cf8dab806 100644 --- a/crates/rome_js_formatter/src/js/expressions/sequence_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/sequence_expression.rs @@ -1,10 +1,10 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::{format_args, write}; use rome_js_syntax::JsSyntaxKind::{JS_PARENTHESIZED_EXPRESSION, JS_SEQUENCE_EXPRESSION}; use rome_js_syntax::{ - JsSequenceExpression, JsSequenceExpressionFields, JsSyntaxKind, JsSyntaxNode, + JsAnyExpression, JsSequenceExpression, JsSequenceExpressionFields, JsSyntaxKind, JsSyntaxNode, }; use rome_rowan::AstNode; @@ -76,16 +76,44 @@ impl FormatNodeRule for FormatJsSequenceExpression { impl NeedsParentheses for JsSequenceExpression { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { - match parent.kind() { - JsSyntaxKind::JS_RETURN_STATEMENT => false, + !matches!( + parent.kind(), + JsSyntaxKind::JS_RETURN_STATEMENT | // There's a precedence for writing `x++, y++` - JsSyntaxKind::JS_FOR_STATEMENT => false, - JsSyntaxKind::JS_EXPRESSION_STATEMENT => false, - JsSyntaxKind::JS_SEQUENCE_EXPRESSION => false, + JsSyntaxKind::JS_FOR_STATEMENT | + JsSyntaxKind::JS_EXPRESSION_STATEMENT | + JsSyntaxKind::JS_SEQUENCE_EXPRESSION | // Handled as part of the arrow function formatting - JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION => false, - // Be on the safer side - _ => true, - } + JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION + ) + } +} + +impl ExpressionNode for JsSequenceExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + +#[cfg(test)] +mod tests { + + use crate::assert_not_needs_parentheses; + use rome_js_syntax::JsSequenceExpression; + + #[test] + fn needs_parentheses() { + assert_not_needs_parentheses!("function test() { return a, b }", JsSequenceExpression); + assert_not_needs_parentheses!("for (let i, x; i++, x++;) {}", JsSequenceExpression); + assert_not_needs_parentheses!("a, b;", JsSequenceExpression); + assert_not_needs_parentheses!("a, b, c", JsSequenceExpression[0]); + assert_not_needs_parentheses!("a, b, c", JsSequenceExpression[1]); + assert_not_needs_parentheses!("a => a, b", JsSequenceExpression); } } diff --git a/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs index 5e542abae458..1268a07dd561 100644 --- a/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs @@ -1,12 +1,12 @@ use crate::prelude::*; use crate::js::expressions::computed_member_expression::JsAnyComputedMemberLike; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{resolve_parent, ExpressionNode, NeedsParentheses}; use rome_formatter::{format_args, write}; use rome_js_syntax::{ JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyName, JsAssignmentExpression, - JsInitializerClause, JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxKind, - JsSyntaxNode, JsSyntaxToken, + JsInitializerClause, JsParenthesizedExpression, JsStaticMemberAssignment, + JsStaticMemberExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, }; use rome_rowan::{declare_node_union, AstNode, SyntaxResult}; @@ -85,8 +85,8 @@ impl JsAnyStaticMemberLike { } fn layout(&self) -> SyntaxResult { - let parent = self.syntax().parent(); - let object = self.object()?; + let parent = resolve_parent(self.syntax()); + let object = self.object()?.resolve(); let is_nested = match &parent { Some(parent) => { @@ -124,8 +124,9 @@ impl JsAnyStaticMemberLike { } let first_non_static_member_ancestor = self.syntax().ancestors().find(|parent| { - !JsAnyStaticMemberLike::can_cast(parent.kind()) + !(JsAnyStaticMemberLike::can_cast(parent.kind()) || JsAnyComputedMemberLike::can_cast(parent.kind()) + || JsParenthesizedExpression::can_cast(parent.kind())) }); let layout = match first_non_static_member_ancestor.and_then(JsAnyExpression::cast) { @@ -174,6 +175,7 @@ pub(crate) fn member_chain_callee_needs_parens( JsComputedMemberExpression(member) => member.object().ok(), JsTemplate(template) => template.tag(), TsNonNullAssertionExpression(assertion) => assertion.expression().ok(), + JsParenthesizedExpression(expression) => expression.expression().ok(), _ => None, }); @@ -183,6 +185,18 @@ pub(crate) fn member_chain_callee_needs_parens( } } +impl ExpressionNode for JsStaticMemberExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs b/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs index 183244dc3044..ffd6d6a8a5d3 100644 --- a/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs @@ -2,8 +2,8 @@ use crate::prelude::*; use crate::utils::{FormatLiteralStringToken, StringLiteralParentKind}; -use crate::parentheses::NeedsParentheses; -use rome_js_syntax::JsStringLiteralExpressionFields; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsStringLiteralExpressionFields}; use rome_js_syntax::{JsExpressionStatement, JsSyntaxKind}; use rome_js_syntax::{JsStringLiteralExpression, JsSyntaxNode}; use rome_rowan::AstNode; @@ -49,6 +49,18 @@ impl NeedsParentheses for JsStringLiteralExpression { } } +impl ExpressionNode for JsStringLiteralExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self.clone())) + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self)) + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/super_expression.rs b/crates/rome_js_formatter/src/js/expressions/super_expression.rs index b08ce1843ff7..e03e6cd79073 100644 --- a/crates/rome_js_formatter/src/js/expressions/super_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/super_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::NeedsParentheses; -use rome_js_syntax::JsSuperExpressionFields; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsSuperExpressionFields}; use rome_js_syntax::{JsSuperExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -21,10 +21,24 @@ impl FormatNodeRule for FormatJsSuperExpression { } impl NeedsParentheses for JsSuperExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } } + +impl ExpressionNode for JsSuperExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/template.rs b/crates/rome_js_formatter/src/js/expressions/template.rs index c03142b989d5..20c1f8cb9b94 100644 --- a/crates/rome_js_formatter/src/js/expressions/template.rs +++ b/crates/rome_js_formatter/src/js/expressions/template.rs @@ -2,11 +2,9 @@ use crate::prelude::*; use rome_formatter::write; use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens; -use crate::parentheses::NeedsParentheses; -use rome_js_syntax::{ - JsAnyExpression, JsSyntaxNode, JsSyntaxToken, JsTemplate, TsTemplateLiteralType, - TsTypeArguments, -}; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsSyntaxNode, JsTemplate, TsTemplateLiteralType}; +use rome_js_syntax::{JsSyntaxToken, TsTypeArguments}; use rome_rowan::{declare_node_union, SyntaxResult}; #[derive(Debug, Clone, Default)] @@ -95,3 +93,15 @@ impl NeedsParentheses for JsTemplate { } } } + +impl ExpressionNode for JsTemplate { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/this_expression.rs b/crates/rome_js_formatter/src/js/expressions/this_expression.rs index 25dd41147261..ac6f92c59ca0 100644 --- a/crates/rome_js_formatter/src/js/expressions/this_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/this_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::NeedsParentheses; -use rome_js_syntax::JsThisExpressionFields; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsThisExpressionFields}; use rome_js_syntax::{JsSyntaxNode, JsThisExpression}; #[derive(Debug, Clone, Default)] @@ -20,10 +20,24 @@ impl FormatNodeRule for FormatJsThisExpression { } } impl NeedsParentheses for JsThisExpression { + #[inline(always)] fn needs_parentheses(&self) -> bool { false } + #[inline(always)] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { false } } + +impl ExpressionNode for JsThisExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} diff --git a/crates/rome_js_formatter/src/js/expressions/unary_expression.rs b/crates/rome_js_formatter/src/js/expressions/unary_expression.rs index cf9be8592007..d2673b5bbfa9 100644 --- a/crates/rome_js_formatter/src/js/expressions/unary_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/unary_expression.rs @@ -1,9 +1,11 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{update_expression_needs_parentheses, NeedsParentheses}; +use crate::parentheses::{ + unary_like_expression_needs_parentheses, ExpressionNode, NeedsParentheses, +}; -use rome_js_syntax::JsSyntaxNode; +use rome_js_syntax::{JsAnyExpression, JsSyntaxNode}; use rome_js_syntax::{JsSyntaxKind, JsUnaryExpression}; use rome_js_syntax::{JsUnaryExpressionFields, JsUnaryOperator}; @@ -51,11 +53,23 @@ impl NeedsParentheses for JsUnaryExpression { matches!(operator, Ok(JsUnaryOperator::Plus | JsUnaryOperator::Minus)) && parent_operator == operator } - _ => update_expression_needs_parentheses(parent, self.syntax()), + _ => unary_like_expression_needs_parentheses(self.syntax(), parent), } } } +impl ExpressionNode for JsUnaryExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/yield_expression.rs b/crates/rome_js_formatter/src/js/expressions/yield_expression.rs index e1a8a010e0c8..ba12871d1639 100644 --- a/crates/rome_js_formatter/src/js/expressions/yield_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/yield_expression.rs @@ -2,8 +2,8 @@ use crate::prelude::*; use rome_formatter::write; use crate::js::expressions::await_expression::await_or_yield_needs_parens; -use crate::parentheses::NeedsParentheses; -use rome_js_syntax::{JsSyntaxKind, JsYieldExpressionFields}; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{JsAnyExpression, JsSyntaxKind, JsYieldExpressionFields}; use rome_js_syntax::{JsSyntaxNode, JsYieldExpression}; #[derive(Debug, Clone, Default)] @@ -31,6 +31,18 @@ impl NeedsParentheses for JsYieldExpression { } } +impl ExpressionNode for JsYieldExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/module/import_meta.rs b/crates/rome_js_formatter/src/js/module/import_meta.rs index d40ad8521f2b..eec52142bb66 100644 --- a/crates/rome_js_formatter/src/js/module/import_meta.rs +++ b/crates/rome_js_formatter/src/js/module/import_meta.rs @@ -1,8 +1,9 @@ use crate::prelude::*; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::ImportMeta; -use rome_js_syntax::ImportMetaFields; +use rome_js_syntax::{ImportMeta, JsSyntaxNode}; +use rome_js_syntax::{ImportMetaFields, JsAnyExpression}; #[derive(Debug, Clone, Default)] pub struct FormatImportMeta; @@ -27,4 +28,30 @@ impl FormatNodeRule for FormatImportMeta { ] ] } + + fn needs_parentheses(&self, item: &ImportMeta) -> bool { + item.needs_parentheses() + } +} + +impl NeedsParentheses for ImportMeta { + fn needs_parentheses(&self) -> bool { + false + } + + fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { + false + } +} + +impl ExpressionNode for ImportMeta { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } } diff --git a/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs b/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs index 9fa15eb181ba..c00eaedf3bc6 100644 --- a/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs +++ b/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs @@ -1,6 +1,9 @@ use crate::prelude::*; -use rome_js_syntax::JsUnknownExpression; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use rome_js_syntax::{ + JsAnyExpression, JsParenthesizedExpression, JsSyntaxNode, JsUnknownExpression, +}; use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] @@ -14,4 +17,33 @@ impl FormatNodeRule for FormatJsUnknownExpression { ) -> FormatResult<()> { format_unknown_node(node.syntax()).fmt(formatter) } + + fn needs_parentheses(&self, item: &JsUnknownExpression) -> bool { + item.needs_parentheses() + } +} + +impl NeedsParentheses for JsUnknownExpression { + fn needs_parentheses(&self) -> bool { + // Keep parens if it is parenthesized. + self.syntax().parent().map_or(false, |parent| { + JsParenthesizedExpression::can_cast(parent.kind()) + }) + } + + fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { + self.needs_parentheses() + } +} + +impl ExpressionNode for JsUnknownExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } } diff --git a/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs b/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs index 0b428eff53ed..a281168cb017 100644 --- a/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs +++ b/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs @@ -1,10 +1,10 @@ -use crate::parentheses::{is_callee, is_tag, NeedsParentheses}; +use crate::parentheses::{is_callee, is_tag, ExpressionNode, NeedsParentheses}; use crate::prelude::*; use crate::utils::jsx::{get_wrap_state, WrapState}; -use crate::utils::resolve_expression_syntax; use rome_formatter::{format_args, write}; use rome_js_syntax::{ - JsBinaryExpression, JsBinaryOperator, JsSyntaxKind, JsSyntaxNode, JsxTagExpression, + JsAnyExpression, JsBinaryExpression, JsBinaryOperator, JsSyntaxKind, JsSyntaxNode, + JsxTagExpression, }; #[derive(Debug, Clone, Default)] @@ -36,8 +36,11 @@ impl NeedsParentheses for JsxTagExpression { JsSyntaxKind::JS_BINARY_EXPRESSION => { let binary = JsBinaryExpression::unwrap_cast(parent.clone()); - let is_left = - binary.left().map(resolve_expression_syntax).as_ref() == Ok(self.syntax()); + let is_left = binary + .left() + .map(ExpressionNode::into_resolved_syntax) + .as_ref() + == Ok(self.syntax()); matches!(binary.operator(), Ok(JsBinaryOperator::LessThan)) && is_left } JsSyntaxKind::TS_AS_EXPRESSION @@ -53,28 +56,17 @@ impl NeedsParentheses for JsxTagExpression { | JsSyntaxKind::JSX_SPREAD_CHILD => true, _ => is_callee(self.syntax(), parent) || is_tag(self.syntax(), parent), } + } +} + +impl ExpressionNode for JsxTagExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } - // (parent.type !== "ArrayExpression" && - // parent.type !== "ArrowFunctionExpression" && - // parent.type !== "AssignmentExpression" && - // parent.type !== "AssignmentPattern" && - // parent.type !== "BinaryExpression" && - // parent.type !== "NewExpression" && - // parent.type !== "ConditionalExpression" && - // parent.type !== "ExpressionStatement" && - // parent.type !== "JsExpressionRoot" && - // parent.type !== "JSXAttribute" && - // parent.type !== "JSXElement" && - // parent.type !== "JSXExpressionContainer" && - // parent.type !== "JSXFragment" && - // parent.type !== "LogicalExpression" && - // !isCallExpression(parent) && - // !isObjectProperty(parent) && - // parent.type !== "ReturnStatement" && - // parent.type !== "ThrowStatement" && - // parent.type !== "TypeCastExpression" && - // parent.type !== "VariableDeclarator" && - // parent.type !== "YieldExpression") - // ); + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() } } diff --git a/crates/rome_js_formatter/src/lib.rs b/crates/rome_js_formatter/src/lib.rs index 4d2a05548f8b..d6fc26c1986c 100644 --- a/crates/rome_js_formatter/src/lib.rs +++ b/crates/rome_js_formatter/src/lib.rs @@ -415,19 +415,17 @@ where if f.context().comments().is_suppressed(syntax) { write!(f, [format_suppressed_node(syntax)]) + } else if self.needs_parentheses(node) { + write!( + f, + [format_parenthesize( + node.syntax().first_token().as_ref(), + &format_once(|f| self.fmt_fields(node, f)), + node.syntax().last_token().as_ref(), + )] + ) } else { - if self.needs_parentheses(node) { - write!( - f, - [format_parenthesize( - node.syntax().first_token().as_ref(), - &format_once(|f| self.fmt_fields(node, f)), - node.syntax().last_token().as_ref(), - )] - ) - } else { - self.fmt_fields(node, f) - } + self.fmt_fields(node, f) } } @@ -714,33 +712,31 @@ mod test { use rome_js_parser::parse; use rome_js_syntax::{SourceType, TextRange, TextSize}; + #[ignore] #[test] // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" -it(`handles - some - newlines - does something really long and complicated so I have to write a very long name for the test`, () => { - console.log("hello!"); -}, 2500); - "#; +test.expect(t => { + t.true(a); +}, false); +"#; let syntax = SourceType::tsx(); let tree = parse(src, 0, syntax); let result = format_node(JsFormatContext::default(), &tree.syntax()) .unwrap() .print(); - // check_reformat(CheckReformatParams { - // root: &tree.syntax(), - // text: result.as_code(), - // source_type: syntax, - // file_name: "quick_test", - // format_context: JsFormatContext::default(), - // }); - // assert_eq!( - // result.as_code(), - // "type B8 = /*1*/ (C);\ntype B9 = (/*1*/ C);\ntype B10 = /*1*/ /*2*/ C;\n" - // ); + check_reformat(CheckReformatParams { + root: &tree.syntax(), + text: result.as_code(), + source_type: syntax, + file_name: "quick_test", + format_context: JsFormatContext::default(), + }); + assert_eq!( + result.as_code(), + "type B8 = /*1*/ (C);\ntype B9 = (/*1*/ C);\ntype B10 = /*1*/ /*2*/ C;\n" + ); } #[test] diff --git a/crates/rome_js_formatter/src/parentheses.rs b/crates/rome_js_formatter/src/parentheses.rs index 86310b533b16..7e83f348d092 100644 --- a/crates/rome_js_formatter/src/parentheses.rs +++ b/crates/rome_js_formatter/src/parentheses.rs @@ -1,11 +1,10 @@ -use crate::utils::{ - resolve_expression_syntax, JsAnyBinaryLikeExpression, JsAnyBinaryLikeLeftExpression, -}; +use crate::utils::{JsAnyBinaryLikeExpression, JsAnyBinaryLikeLeftExpression}; + use rome_js_syntax::{ JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsArrowFunctionExpression, - JsBinaryExpression, JsBinaryOperator, JsComputedMemberExpression, JsConditionalExpression, - JsLanguage, JsParenthesizedExpression, JsSequenceExpression, JsSyntaxKind, JsSyntaxNode, - JsTemplate, + JsAssignmentExpression, JsBinaryExpression, JsBinaryOperator, JsComputedMemberAssignment, + JsComputedMemberExpression, JsConditionalExpression, JsLanguage, JsSequenceExpression, + JsSyntaxKind, JsSyntaxNode, }; use rome_rowan::AstNode; @@ -17,14 +16,7 @@ pub trait NeedsParentheses: AstNode { } fn resolve_parent(&self) -> Option { - self.syntax().ancestors().skip(1).find(|parent| { - !matches!( - parent.kind(), - JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION - | JsSyntaxKind::JS_PARENTHESIZED_ASSIGNMENT - | JsSyntaxKind::TS_PARENTHESIZED_TYPE - ) - }) + resolve_parent(self.syntax()) } /// Returns `true` if this node requires parentheses to form valid syntax or improve readability. @@ -33,14 +25,46 @@ pub trait NeedsParentheses: AstNode { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool; } -/// Resolves the parent of the node that is not a parenthesized expression -pub(crate) fn resolve_expression_parent(node: &JsSyntaxNode) -> Option { - node.ancestors() - .skip(1) - .find(|parent| !JsParenthesizedExpression::can_cast(parent.kind())) +pub trait ExpressionNode: NeedsParentheses { + /// Resolves an expression to the first non parenthesized expression. + fn resolve(&self) -> JsAnyExpression; + + /// Consumes `self` and returns the first expression that isn't a parenthesized expression. + fn into_resolved(self) -> JsAnyExpression; + + /// Resolves an expression to the first non parenthesized expression and returns its [JsSyntaxNode]. + fn resolve_syntax(&self) -> JsSyntaxNode { + self.resolve().into_syntax() + } + + /// Consumes `self` and returns the [JsSyntaxNode] of the first expression that isn't a parenthesized expression. + fn into_resolved_syntax(self) -> JsSyntaxNode { + self.into_resolved().into_syntax() + } +} + +/// Resolves to the first parent that isn't a parenthesized expression, assignment, or type. +pub(crate) fn resolve_parent(node: &JsSyntaxNode) -> Option { + let mut current = node.parent(); + + while let Some(parent) = current { + if matches!( + parent.kind(), + JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION + | JsSyntaxKind::JS_PARENTHESIZED_ASSIGNMENT + | JsSyntaxKind::TS_PARENTHESIZED_TYPE + ) { + current = parent.parent(); + } else { + return Some(parent); + } + } + + None } impl NeedsParentheses for JsAnyLiteralExpression { + #[inline] fn needs_parentheses(&self) -> bool { match self { JsAnyLiteralExpression::JsBigIntLiteralExpression(big_int) => { @@ -60,6 +84,7 @@ impl NeedsParentheses for JsAnyLiteralExpression { } } + #[inline] fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { match self { JsAnyLiteralExpression::JsBigIntLiteralExpression(big_int) => { @@ -84,6 +109,276 @@ impl NeedsParentheses for JsAnyLiteralExpression { } } +impl ExpressionNode for JsAnyLiteralExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + match self { + JsAnyLiteralExpression::JsBigIntLiteralExpression(big_int) => big_int.resolve(), + JsAnyLiteralExpression::JsBooleanLiteralExpression(boolean) => boolean.resolve(), + JsAnyLiteralExpression::JsNullLiteralExpression(null_literal) => null_literal.resolve(), + JsAnyLiteralExpression::JsNumberLiteralExpression(number_literal) => { + number_literal.resolve() + } + JsAnyLiteralExpression::JsRegexLiteralExpression(regex) => regex.resolve(), + JsAnyLiteralExpression::JsStringLiteralExpression(string) => string.resolve(), + } + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + match self { + JsAnyLiteralExpression::JsBigIntLiteralExpression(big_int) => big_int.into_resolved(), + JsAnyLiteralExpression::JsBooleanLiteralExpression(boolean) => boolean.into_resolved(), + JsAnyLiteralExpression::JsNullLiteralExpression(null_literal) => { + null_literal.into_resolved() + } + JsAnyLiteralExpression::JsNumberLiteralExpression(number_literal) => { + number_literal.into_resolved() + } + JsAnyLiteralExpression::JsRegexLiteralExpression(regex) => regex.into_resolved(), + JsAnyLiteralExpression::JsStringLiteralExpression(string) => string.into_resolved(), + } + } +} + +impl NeedsParentheses for JsAnyExpression { + fn needs_parentheses(&self) -> bool { + match self { + JsAnyExpression::ImportMeta(meta) => meta.needs_parentheses(), + JsAnyExpression::JsAnyLiteralExpression(literal) => literal.needs_parentheses(), + JsAnyExpression::JsArrayExpression(array) => array.needs_parentheses(), + JsAnyExpression::JsArrowFunctionExpression(arrow) => arrow.needs_parentheses(), + JsAnyExpression::JsAssignmentExpression(assignment) => assignment.needs_parentheses(), + JsAnyExpression::JsAwaitExpression(await_expression) => { + await_expression.needs_parentheses() + } + JsAnyExpression::JsBinaryExpression(binary) => binary.needs_parentheses(), + JsAnyExpression::JsCallExpression(call) => call.needs_parentheses(), + JsAnyExpression::JsClassExpression(class) => class.needs_parentheses(), + JsAnyExpression::JsComputedMemberExpression(member) => member.needs_parentheses(), + JsAnyExpression::JsConditionalExpression(conditional) => { + conditional.needs_parentheses() + } + JsAnyExpression::JsFunctionExpression(function) => function.needs_parentheses(), + JsAnyExpression::JsIdentifierExpression(identifier) => identifier.needs_parentheses(), + JsAnyExpression::JsImportCallExpression(import_call) => import_call.needs_parentheses(), + JsAnyExpression::JsInExpression(in_expression) => in_expression.needs_parentheses(), + JsAnyExpression::JsInstanceofExpression(instanceof) => instanceof.needs_parentheses(), + JsAnyExpression::JsLogicalExpression(logical) => logical.needs_parentheses(), + JsAnyExpression::JsNewExpression(new) => new.needs_parentheses(), + JsAnyExpression::JsObjectExpression(object) => object.needs_parentheses(), + JsAnyExpression::JsParenthesizedExpression(parenthesized) => { + parenthesized.needs_parentheses() + } + JsAnyExpression::JsPostUpdateExpression(update) => update.needs_parentheses(), + JsAnyExpression::JsPreUpdateExpression(update) => update.needs_parentheses(), + JsAnyExpression::JsSequenceExpression(sequence) => sequence.needs_parentheses(), + JsAnyExpression::JsStaticMemberExpression(member) => member.needs_parentheses(), + JsAnyExpression::JsSuperExpression(sup) => sup.needs_parentheses(), + JsAnyExpression::JsTemplate(template) => template.needs_parentheses(), + JsAnyExpression::JsThisExpression(this) => this.needs_parentheses(), + JsAnyExpression::JsUnaryExpression(unary) => unary.needs_parentheses(), + JsAnyExpression::JsUnknownExpression(unknown) => unknown.needs_parentheses(), + JsAnyExpression::JsYieldExpression(yield_expression) => { + yield_expression.needs_parentheses() + } + JsAnyExpression::JsxTagExpression(jsx) => jsx.needs_parentheses(), + JsAnyExpression::NewTarget(target) => target.needs_parentheses(), + JsAnyExpression::TsAsExpression(as_expression) => as_expression.needs_parentheses(), + JsAnyExpression::TsNonNullAssertionExpression(non_null) => non_null.needs_parentheses(), + JsAnyExpression::TsTypeAssertionExpression(type_assertion) => { + type_assertion.needs_parentheses() + } + } + } + + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + match self { + JsAnyExpression::ImportMeta(meta) => meta.needs_parentheses_with_parent(parent), + JsAnyExpression::JsAnyLiteralExpression(literal) => { + literal.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsArrayExpression(array) => { + array.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsArrowFunctionExpression(arrow) => { + arrow.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsAssignmentExpression(assignment) => { + assignment.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsAwaitExpression(await_expression) => { + await_expression.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsBinaryExpression(binary) => { + binary.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsCallExpression(call) => call.needs_parentheses_with_parent(parent), + JsAnyExpression::JsClassExpression(class) => { + class.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsComputedMemberExpression(member) => { + member.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsConditionalExpression(conditional) => { + conditional.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsFunctionExpression(function) => { + function.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsIdentifierExpression(identifier) => { + identifier.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsImportCallExpression(import_call) => { + import_call.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsInExpression(in_expression) => { + in_expression.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsInstanceofExpression(instanceof) => { + instanceof.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsLogicalExpression(logical) => { + logical.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsNewExpression(new) => new.needs_parentheses_with_parent(parent), + JsAnyExpression::JsObjectExpression(object) => { + object.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsParenthesizedExpression(parenthesized) => { + parenthesized.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsPostUpdateExpression(update) => { + update.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsPreUpdateExpression(update) => { + update.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsSequenceExpression(sequence) => { + sequence.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsStaticMemberExpression(member) => { + member.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsSuperExpression(sup) => sup.needs_parentheses_with_parent(parent), + JsAnyExpression::JsTemplate(template) => template.needs_parentheses_with_parent(parent), + JsAnyExpression::JsThisExpression(this) => this.needs_parentheses_with_parent(parent), + JsAnyExpression::JsUnaryExpression(unary) => { + unary.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsUnknownExpression(unknown) => { + unknown.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsYieldExpression(yield_expression) => { + yield_expression.needs_parentheses_with_parent(parent) + } + JsAnyExpression::JsxTagExpression(jsx) => jsx.needs_parentheses_with_parent(parent), + JsAnyExpression::NewTarget(target) => target.needs_parentheses_with_parent(parent), + JsAnyExpression::TsAsExpression(as_expression) => { + as_expression.needs_parentheses_with_parent(parent) + } + JsAnyExpression::TsNonNullAssertionExpression(non_null) => { + non_null.needs_parentheses_with_parent(parent) + } + JsAnyExpression::TsTypeAssertionExpression(type_assertion) => { + type_assertion.needs_parentheses_with_parent(parent) + } + } + } +} + +impl ExpressionNode for JsAnyExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + match self { + JsAnyExpression::ImportMeta(meta) => meta.resolve(), + JsAnyExpression::JsAnyLiteralExpression(literal) => literal.resolve(), + JsAnyExpression::JsArrayExpression(array) => array.resolve(), + JsAnyExpression::JsArrowFunctionExpression(arrow) => arrow.resolve(), + JsAnyExpression::JsAssignmentExpression(assignment) => assignment.resolve(), + JsAnyExpression::JsAwaitExpression(await_expression) => await_expression.resolve(), + JsAnyExpression::JsBinaryExpression(binary) => binary.resolve(), + JsAnyExpression::JsCallExpression(call) => call.resolve(), + JsAnyExpression::JsClassExpression(class) => class.resolve(), + JsAnyExpression::JsComputedMemberExpression(member) => member.resolve(), + JsAnyExpression::JsConditionalExpression(conditional) => conditional.resolve(), + JsAnyExpression::JsFunctionExpression(function) => function.resolve(), + JsAnyExpression::JsIdentifierExpression(identifier) => identifier.resolve(), + JsAnyExpression::JsImportCallExpression(import_call) => import_call.resolve(), + JsAnyExpression::JsInExpression(in_expression) => in_expression.resolve(), + JsAnyExpression::JsInstanceofExpression(instanceof) => instanceof.resolve(), + JsAnyExpression::JsLogicalExpression(logical) => logical.resolve(), + JsAnyExpression::JsNewExpression(new) => new.resolve(), + JsAnyExpression::JsObjectExpression(object) => object.resolve(), + JsAnyExpression::JsParenthesizedExpression(parenthesized) => parenthesized.resolve(), + JsAnyExpression::JsPostUpdateExpression(update) => update.resolve(), + JsAnyExpression::JsPreUpdateExpression(update) => update.resolve(), + JsAnyExpression::JsSequenceExpression(sequence) => sequence.resolve(), + JsAnyExpression::JsStaticMemberExpression(member) => member.resolve(), + JsAnyExpression::JsSuperExpression(sup) => sup.resolve(), + JsAnyExpression::JsTemplate(template) => template.resolve(), + JsAnyExpression::JsThisExpression(this) => this.resolve(), + JsAnyExpression::JsUnaryExpression(unary) => unary.resolve(), + JsAnyExpression::JsUnknownExpression(unknown) => unknown.resolve(), + JsAnyExpression::JsYieldExpression(yield_expression) => yield_expression.resolve(), + JsAnyExpression::JsxTagExpression(jsx) => jsx.resolve(), + JsAnyExpression::NewTarget(target) => target.resolve(), + JsAnyExpression::TsAsExpression(as_expression) => as_expression.resolve(), + JsAnyExpression::TsNonNullAssertionExpression(non_null) => non_null.resolve(), + JsAnyExpression::TsTypeAssertionExpression(type_assertion) => type_assertion.resolve(), + } + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + match self { + JsAnyExpression::ImportMeta(meta) => meta.into_resolved(), + JsAnyExpression::JsAnyLiteralExpression(literal) => literal.into_resolved(), + JsAnyExpression::JsArrayExpression(array) => array.into_resolved(), + JsAnyExpression::JsArrowFunctionExpression(arrow) => arrow.into_resolved(), + JsAnyExpression::JsAssignmentExpression(assignment) => assignment.into_resolved(), + JsAnyExpression::JsAwaitExpression(await_expression) => { + await_expression.into_resolved() + } + JsAnyExpression::JsBinaryExpression(binary) => binary.into_resolved(), + JsAnyExpression::JsCallExpression(call) => call.into_resolved(), + JsAnyExpression::JsClassExpression(class) => class.into_resolved(), + JsAnyExpression::JsComputedMemberExpression(member) => member.into_resolved(), + JsAnyExpression::JsConditionalExpression(conditional) => conditional.into_resolved(), + JsAnyExpression::JsFunctionExpression(function) => function.into_resolved(), + JsAnyExpression::JsIdentifierExpression(identifier) => identifier.into_resolved(), + JsAnyExpression::JsImportCallExpression(import_call) => import_call.into_resolved(), + JsAnyExpression::JsInExpression(in_expression) => in_expression.into_resolved(), + JsAnyExpression::JsInstanceofExpression(instanceof) => instanceof.into_resolved(), + JsAnyExpression::JsLogicalExpression(logical) => logical.into_resolved(), + JsAnyExpression::JsNewExpression(new) => new.into_resolved(), + JsAnyExpression::JsObjectExpression(object) => object.into_resolved(), + JsAnyExpression::JsParenthesizedExpression(parenthesized) => { + parenthesized.into_resolved() + } + JsAnyExpression::JsPostUpdateExpression(update) => update.into_resolved(), + JsAnyExpression::JsPreUpdateExpression(update) => update.into_resolved(), + JsAnyExpression::JsSequenceExpression(sequence) => sequence.into_resolved(), + JsAnyExpression::JsStaticMemberExpression(member) => member.into_resolved(), + JsAnyExpression::JsSuperExpression(sup) => sup.into_resolved(), + JsAnyExpression::JsTemplate(template) => template.into_resolved(), + JsAnyExpression::JsThisExpression(this) => this.into_resolved(), + JsAnyExpression::JsUnaryExpression(unary) => unary.into_resolved(), + JsAnyExpression::JsUnknownExpression(unknown) => unknown.into_resolved(), + JsAnyExpression::JsYieldExpression(yield_expression) => { + yield_expression.into_resolved() + } + JsAnyExpression::JsxTagExpression(jsx) => jsx.into_resolved(), + JsAnyExpression::NewTarget(target) => target.into_resolved(), + JsAnyExpression::TsAsExpression(as_expression) => as_expression.into_resolved(), + JsAnyExpression::TsNonNullAssertionExpression(non_null) => non_null.into_resolved(), + JsAnyExpression::TsTypeAssertionExpression(type_assertion) => { + type_assertion.into_resolved() + } + } + } +} + /// Returns the left most expression of `expression`. /// /// For example, returns `a` for `(a ? b : c) + d` because it first resolves the @@ -120,7 +415,7 @@ pub(crate) fn get_expression_left_side( JsSequenceExpression(sequence) => sequence.left().ok(), JsStaticMemberExpression(member) => member.object().ok(), JsComputedMemberExpression(member) => member.object().ok(), - JsTemplate(template) => template.tag().map(|tag| tag), + JsTemplate(template) => template.tag(), JsNewExpression(new) => new.callee().ok(), JsCallExpression(call) => call.callee().ok(), JsConditionalExpression(conditional) => conditional.test().ok(), @@ -148,10 +443,10 @@ pub(crate) enum FirstInStatementMode { /// /// Traverses upwards the tree for as long as the `node` is the left most expression until the node isn't /// the left most node or reached a statement. -pub(crate) fn is_first_in_statement(node: JsAnyExpression, mode: FirstInStatementMode) -> bool { +pub(crate) fn is_first_in_statement(node: JsSyntaxNode, mode: FirstInStatementMode) -> bool { let mut current = node; - while let Some(parent) = current.syntax().parent() { + while let Some(parent) = current.parent() { let parent = match parent.kind() { JsSyntaxKind::JS_EXPRESSION_STATEMENT => { return true; @@ -159,20 +454,20 @@ pub(crate) fn is_first_in_statement(node: JsAnyExpression, mode: FirstInStatemen JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION | JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION + | JsSyntaxKind::JS_STATIC_MEMBER_ASSIGNMENT | JsSyntaxKind::JS_TEMPLATE | JsSyntaxKind::JS_CALL_EXPRESSION | JsSyntaxKind::JS_NEW_EXPRESSION | JsSyntaxKind::TS_AS_EXPRESSION - | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => { - JsAnyExpression::unwrap_cast(parent) - } + | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION + | JsSyntaxKind::JS_PARENTHESIZED_ASSIGNMENT => parent, JsSyntaxKind::JS_SEQUENCE_EXPRESSION => { let sequence = JsSequenceExpression::unwrap_cast(parent); - let is_left = sequence.left().as_ref() == Ok(¤t); + let is_left = sequence.left().map(AstNode::into_syntax).as_ref() == Ok(¤t); if is_left { - sequence.into() + sequence.into_syntax() } else { break; } @@ -181,10 +476,39 @@ pub(crate) fn is_first_in_statement(node: JsAnyExpression, mode: FirstInStatemen JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION => { let member_expression = JsComputedMemberExpression::unwrap_cast(parent); - let is_object = member_expression.object().as_ref() == Ok(¤t); + let is_object = member_expression + .object() + .map(AstNode::into_syntax) + .as_ref() + == Ok(¤t); + + if is_object { + member_expression.into_syntax() + } else { + break; + } + } + + JsSyntaxKind::JS_COMPUTED_MEMBER_ASSIGNMENT => { + let assignment = JsComputedMemberAssignment::unwrap_cast(parent); + + let is_object = + assignment.object().map(AstNode::into_syntax).as_ref() == Ok(¤t); if is_object { - member_expression.into() + assignment.into_syntax() + } else { + break; + } + } + + JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION => { + let assignment = JsAssignmentExpression::unwrap_cast(parent); + + let is_left = assignment.left().map(AstNode::into_syntax).as_ref() == Ok(¤t); + + if is_left { + assignment.into_syntax() } else { break; } @@ -193,8 +517,8 @@ pub(crate) fn is_first_in_statement(node: JsAnyExpression, mode: FirstInStatemen JsSyntaxKind::JS_CONDITIONAL_EXPRESSION => { let conditional = JsConditionalExpression::unwrap_cast(parent); - if conditional.test().as_ref() == Ok(¤t) { - conditional.into() + if conditional.test().map(AstNode::into_syntax).as_ref() == Ok(¤t) { + conditional.into_syntax() } else { break; } @@ -206,7 +530,9 @@ pub(crate) fn is_first_in_statement(node: JsAnyExpression, mode: FirstInStatemen let arrow = JsArrowFunctionExpression::unwrap_cast(parent); let is_body = arrow.body().map_or(false, |body| match body { - JsAnyFunctionBody::JsAnyExpression(expression) => &expression == ¤t, + JsAnyFunctionBody::JsAnyExpression(expression) => { + expression.syntax() == ¤t + } _ => false, }); @@ -228,13 +554,13 @@ pub(crate) fn is_first_in_statement(node: JsAnyExpression, mode: FirstInStatemen let is_left = binary_like.left().map_or(false, |left| match left { JsAnyBinaryLikeLeftExpression::JsAnyExpression(expression) => { - &expression == ¤t + expression.syntax() == ¤t } _ => false, }); if is_left { - binary_like.into() + binary_like.into_syntax() } else { break; } @@ -248,27 +574,11 @@ pub(crate) fn is_first_in_statement(node: JsAnyExpression, mode: FirstInStatemen false } -/// Returns `true` if the `expression` is in a position where only [`MemberExpression`s](https://tc39.es/ecma262/#prod-MemberExpression) are allowed. -pub(crate) fn is_in_member_expression_position( +/// Implements the shared logic for when parentheses are necessary for [JsPreUpdateExpression], [JsPostUpdateExpression], or [JsUnaryExpression] expressions. +/// Each expression may implement node specific rules, which is why calling `needs_parens` on the node is preferred. +pub(crate) fn unary_like_expression_needs_parentheses( expression: &JsSyntaxNode, parent: &JsSyntaxNode, -) -> bool { - debug_assert_is_expression(expression); - - match parent.kind() { - JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => true, - - _ => { - is_callee(expression, parent) - || is_member_object(expression, parent) - || is_tag(expression, parent) - } - } -} - -pub(crate) fn update_expression_needs_parentheses( - parent: &JsSyntaxNode, - expression: &JsSyntaxNode, ) -> bool { debug_assert!(matches!( expression.kind(), @@ -276,52 +586,75 @@ pub(crate) fn update_expression_needs_parentheses( | JsSyntaxKind::JS_POST_UPDATE_EXPRESSION | JsSyntaxKind::JS_UNARY_EXPRESSION )); + debug_assert_is_parent(expression, parent); match parent.kind() { JsSyntaxKind::JS_BINARY_EXPRESSION => { let binary = JsBinaryExpression::unwrap_cast(parent.clone()); matches!(binary.operator(), Ok(JsBinaryOperator::Exponent)) - && binary.left().map(resolve_expression_syntax).as_ref() == Ok(expression) + && binary + .left() + .map(ExpressionNode::into_resolved_syntax) + .as_ref() + == Ok(expression) } - _ => is_in_left_hand_side_position(expression, parent), + _ => update_or_lower_expression_needs_parentheses(expression, parent), } } -/// Returns `true` if the expression is in a position where only [`LeftHandSideExpression`s](https://tc39.es/ecma262/#prod-LeftHandSideExpression) are allowed. -pub(crate) fn is_in_left_hand_side_position( +/// Returns `true` if an expression with lower precedence than an update expression needs parentheses. +/// +/// This is generally the case if the expression is used in a left hand side, or primary expression context. +pub(crate) fn update_or_lower_expression_needs_parentheses( expression: &JsSyntaxNode, parent: &JsSyntaxNode, ) -> bool { + debug_assert_is_expression(expression); + debug_assert_is_parent(expression, parent); + match parent.kind() { JsSyntaxKind::JS_EXTENDS_CLAUSE => true, - _ => is_in_member_expression_position(expression, parent), - } -} + _ => match parent.kind() { + JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => true, -pub(crate) fn unary_expression_needs_parentheses( - expression: &JsSyntaxNode, - parent: &JsSyntaxNode, -) -> bool { - is_in_left_hand_side_position(expression, parent) || is_spread(expression, parent) + _ => { + is_callee(expression, parent) + || is_member_object(expression, parent) + || is_tag(expression, parent) + } + }, + } } /// Returns `true` if `node< is the `object` of a [JsStaticMemberExpression] or [JsComputedMemberExpression] pub(crate) fn is_member_object(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { debug_assert_is_expression(node); + debug_assert_is_parent(node, parent); match parent.kind() { - // Only allows expression in the true, + JsSyntaxKind::JS_STATIC_MEMBER_ASSIGNMENT => true, + JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION => { let member_expression = JsComputedMemberExpression::unwrap_cast(parent.clone()); - member_expression.optional_chain_token().is_none() - && member_expression - .object() - .map(resolve_expression_syntax) - .as_ref() - == Ok(node) + member_expression + .object() + .map(ExpressionNode::into_resolved_syntax) + .as_ref() + == Ok(node) + } + + JsSyntaxKind::JS_COMPUTED_MEMBER_ASSIGNMENT => { + let member_assignment = JsComputedMemberAssignment::unwrap_cast(parent.clone()); + + member_assignment + .object() + .map(ExpressionNode::into_resolved_syntax) + .as_ref() + == Ok(node) } _ => false, } @@ -330,6 +663,7 @@ pub(crate) fn is_member_object(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bo /// Returns `true` if `node` is the `callee` of a [JsNewExpression] or [JsCallExpression]. pub(crate) fn is_callee(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { debug_assert_is_expression(node); + debug_assert_is_parent(node, parent); // It isn't necessary to test if the node is the `callee` because the nodes only // allow expressions in the `callee` position; @@ -352,7 +686,11 @@ pub(crate) fn is_conditional_test(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> JsSyntaxKind::JS_CONDITIONAL_EXPRESSION => { let conditional = JsConditionalExpression::unwrap_cast(parent.clone()); - conditional.test().map(resolve_expression_syntax).as_ref() == Ok(node) + conditional + .test() + .map(ExpressionNode::into_resolved_syntax) + .as_ref() + == Ok(node) } _ => false, } @@ -361,20 +699,15 @@ pub(crate) fn is_conditional_test(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> /// Returns `true` if `node` is the `tag` of a [JsTemplate] expression pub(crate) fn is_tag(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { debug_assert_is_expression(node); + debug_assert_is_parent(node, parent); - match parent.kind() { - JsSyntaxKind::JS_TEMPLATE => { - let template = JsTemplate::unwrap_cast(parent.clone()); - - template.tag().map(resolve_expression_syntax).as_ref() == Some(node) - } - _ => false, - } + matches!(parent.kind(), JsSyntaxKind::JS_TEMPLATE) } /// Returns `true` if `node` is a spread `...node` pub(crate) fn is_spread(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { debug_assert_is_expression(node); + debug_assert_is_parent(node, parent); matches!( parent.kind(), @@ -384,8 +717,10 @@ pub(crate) fn is_spread(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { ) } +/// Returns `true` if `parent` is a [JsAnyBinaryLikeExpression] and `node` is the `left` or `right` of that expression. pub(crate) fn is_binary_like_left_or_right(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { debug_assert_is_expression(node); + debug_assert_is_parent(node, parent); JsAnyBinaryLikeExpression::can_cast(parent.kind()) } @@ -397,6 +732,13 @@ fn debug_assert_is_expression(node: &JsSyntaxNode) { ) } +fn debug_assert_is_parent(node: &JsSyntaxNode, parent: &JsSyntaxNode) { + debug_assert!( + resolve_parent(node).as_ref() == Some(parent), + "Node {node:#?} is not a child of ${parent:#?}" + ) +} + #[cfg(test)] pub(crate) mod tests { use super::NeedsParentheses; @@ -433,7 +775,7 @@ pub(crate) mod tests { core::any::type_name::(), ) } - 1 => matching_nodes.iter().next().unwrap(), + 1 => matching_nodes.get(0).unwrap(), _ => { panic!("Expected to find a single node matching '{}' in '{input}' but found multiple ones:\n {matching_nodes:#?}", core::any::type_name::()); } @@ -473,7 +815,7 @@ pub(crate) mod tests { core::any::type_name::(), ) } - 1 => matching_nodes.iter().next().unwrap(), + 1 => matching_nodes.get(0).unwrap(), _ => { panic!("Expected to find a single node matching '{}' in '{input}' but found multiple ones:\n {matching_nodes:#?}", core::any::type_name::()); } @@ -483,6 +825,28 @@ pub(crate) mod tests { assert!(!node.needs_parentheses()); } + /// Helper macro to test the [NeedsParentheses] implementation of a node. + /// + /// # Example + /// + /// + /// ``` + /// # use rome_js_formatter::assert_needs_parentheses; + /// use rome_js_syntax::JsStaticMemberExpression; + /// + /// assert_needs_parentheses!("new (test().a)()", JsStaticMemberExpression); + /// ``` + /// + /// Asserts that [NeedsParentheses.needs_parentheses()] returns true for the only [JsStaticMemberExpression] in the program. + /// + /// ``` + /// # use rome_js_syntax::JsStaticMemberExpression; + /// use rome_js_formatter::assert_needs_parentheses; + /// + /// assert_needs_parentheses!("new (test().a).b)()", JsStaticMemberExpression[1]); + /// ``` + /// + /// Asserts that [NeedsParentheses.needs_parentheses()] returns true for the second (in pre-order) [JsStaticMemberExpression] in the program. #[macro_export] macro_rules! assert_needs_parentheses { ($input:expr, $Node:ident) => {{ @@ -514,6 +878,28 @@ pub(crate) mod tests { }}; } + /// Helper macro to test the [NeedsParentheses] implementation of a node. + /// + /// # Example + /// + /// + /// ``` + /// # use rome_js_syntax::JsStaticMemberExpression; + /// use rome_js_formatter::assert_not_needs_parentheses; + /// + /// assert_not_needs_parentheses!("a.b", JsStaticMemberExpression); + /// ``` + /// + /// Asserts that [NeedsParentheses.needs_parentheses()] returns true for the only [JsStaticMemberExpression] in the program. + /// + /// ``` + /// # use rome_js_syntax::JsStaticMemberExpression; + /// use rome_js_formatter::assert_not_needs_parentheses; + /// + /// assert_not_needs_parentheses!("a.b.c", JsStaticMemberExpression[0]); + /// ``` + /// + /// Asserts that [NeedsParentheses.needs_parentheses()] returns true for the first (in pre-order) [JsStaticMemberExpression] in the program. #[macro_export] macro_rules! assert_not_needs_parentheses { ($input:expr, $Node:ident) => {{ diff --git a/crates/rome_js_formatter/src/ts/expressions/as_expression.rs b/crates/rome_js_formatter/src/ts/expressions/as_expression.rs index 8ee5a7f3bea1..ceea654aacf5 100644 --- a/crates/rome_js_formatter/src/ts/expressions/as_expression.rs +++ b/crates/rome_js_formatter/src/ts/expressions/as_expression.rs @@ -1,10 +1,11 @@ use crate::prelude::*; use crate::parentheses::{ - is_binary_like_left_or_right, is_callee, is_member_object, is_spread, is_tag, NeedsParentheses, + is_binary_like_left_or_right, is_callee, is_member_object, ExpressionNode, NeedsParentheses, }; +use crate::ts::expressions::type_assertion_expression::type_cast_like_needs_parens; use rome_formatter::write; -use rome_js_syntax::TsAsExpressionFields; +use rome_js_syntax::{JsAnyExpression, TsAsExpressionFields}; use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, TsAsExpression}; #[derive(Debug, Clone, Default)] @@ -18,16 +19,30 @@ impl FormatNodeRule for FormatTsAsExpression { expression, } = node.as_fields(); - write![ - f, - [ - expression.format(), - space(), - as_token.format(), - space(), - ty.format(), + let format_inner = format_with(|f| { + write![ + f, + [ + expression.format(), + space(), + as_token.format(), + space(), + ty.format(), + ] ] - ] + }); + + let parent = node.resolve_parent(); + + let is_callee_or_object = parent.map_or(false, |parent| { + is_callee(node.syntax(), &parent) || is_member_object(node.syntax(), &parent) + }); + + if is_callee_or_object { + write!(f, [group(&soft_block_indent(&format_inner))]) + } else { + write!(f, [format_inner]) + } } fn needs_parentheses(&self, item: &TsAsExpression) -> bool { @@ -38,24 +53,28 @@ impl FormatNodeRule for FormatTsAsExpression { impl NeedsParentheses for TsAsExpression { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { match parent.kind() { - JsSyntaxKind::JS_CONDITIONAL_EXPRESSION - | JsSyntaxKind::JS_EXTENDS_CLAUSE - | JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION - | JsSyntaxKind::JS_UNARY_EXPRESSION - | JsSyntaxKind::JS_AWAIT_EXPRESSION - | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => true, + JsSyntaxKind::JS_CONDITIONAL_EXPRESSION => true, _ => { - is_callee(self.syntax(), parent) - || is_tag(self.syntax(), parent) - || is_spread(self.syntax(), parent) - || is_member_object(self.syntax(), parent) + type_cast_like_needs_parens(self.syntax(), parent) || is_binary_like_left_or_right(self.syntax(), parent) } } } } +impl ExpressionNode for TsAsExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs b/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs index 7ada1b0bb9f2..5dc096bfc9cc 100644 --- a/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs +++ b/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs @@ -1,9 +1,9 @@ use crate::prelude::*; use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::{JsSyntaxKind, TsNonNullAssertionExpressionFields}; +use rome_js_syntax::{JsAnyExpression, JsSyntaxKind, TsNonNullAssertionExpressionFields}; use rome_js_syntax::{JsSyntaxNode, TsNonNullAssertionExpression}; #[derive(Debug, Clone, Default)] @@ -34,3 +34,15 @@ impl NeedsParentheses for TsNonNullAssertionExpression { || member_chain_callee_needs_parens(self.clone().into(), parent) } } + +impl ExpressionNode for TsNonNullAssertionExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} diff --git a/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs b/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs index e229596012a1..f8a360cae2ee 100644 --- a/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs +++ b/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs @@ -1,9 +1,11 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ + is_callee, is_member_object, is_spread, is_tag, ExpressionNode, NeedsParentheses, +}; use rome_formatter::write; -use rome_js_syntax::{JsSyntaxKind, TsAsExpression, TsTypeAssertionExpressionFields}; -use rome_js_syntax::{JsSyntaxNode, TsTypeAssertionExpression}; +use rome_js_syntax::{JsAnyExpression, JsSyntaxNode}; +use rome_js_syntax::{JsSyntaxKind, TsTypeAssertionExpression, TsTypeAssertionExpressionFields}; #[derive(Debug, Clone, Default)] pub struct FormatTsTypeAssertionExpression; @@ -30,4 +32,82 @@ impl FormatNodeRule for FormatTsTypeAssertionExpressi ] ] } + + fn needs_parentheses(&self, item: &TsTypeAssertionExpression) -> bool { + item.needs_parentheses() + } +} + +impl NeedsParentheses for TsTypeAssertionExpression { + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + match parent.kind() { + JsSyntaxKind::TS_AS_EXPRESSION => true, + _ => type_cast_like_needs_parens(self.syntax(), parent), + } + } +} + +impl ExpressionNode for TsTypeAssertionExpression { + #[inline] + fn resolve(&self) -> JsAnyExpression { + self.clone().into() + } + + #[inline] + fn into_resolved(self) -> JsAnyExpression { + self.into() + } +} + +pub(super) fn type_cast_like_needs_parens(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { + debug_assert!(matches!( + node.kind(), + JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION | JsSyntaxKind::TS_AS_EXPRESSION + )); + + match parent.kind() { + JsSyntaxKind::JS_EXTENDS_CLAUSE + | JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION + | JsSyntaxKind::JS_UNARY_EXPRESSION + | JsSyntaxKind::JS_AWAIT_EXPRESSION + | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => true, + + _ => { + is_callee(node, parent) + || is_tag(node, parent) + || is_spread(node, parent) + || is_member_object(node, parent) + } + } +} + +#[cfg(test)] +mod tests { + use crate::{assert_needs_parentheses, assert_not_needs_parentheses}; + use rome_js_syntax::TsTypeAssertionExpression; + + #[test] + fn needs_parentheses() { + assert_needs_parentheses!("( x) as any", TsTypeAssertionExpression); + + assert_needs_parentheses!("class X extends (B) {}", TsTypeAssertionExpression); + + assert_needs_parentheses!("(x)()", TsTypeAssertionExpression); + assert_needs_parentheses!("(x)?.()", TsTypeAssertionExpression); + assert_needs_parentheses!("new (x)()", TsTypeAssertionExpression); + + assert_needs_parentheses!("(x)", TsTypeAssertionExpression[1]); + assert_needs_parentheses!("(x)", TsTypeAssertionExpression[1]); + assert_needs_parentheses!("(x)`template`", TsTypeAssertionExpression); + assert_needs_parentheses!("!(x)", TsTypeAssertionExpression); + assert_needs_parentheses!("[...(x)]", TsTypeAssertionExpression); + assert_needs_parentheses!("({...(x)})", TsTypeAssertionExpression); + + assert_needs_parentheses!("await (x)", TsTypeAssertionExpression); + assert_needs_parentheses!("(x)!", TsTypeAssertionExpression); + + assert_needs_parentheses!("(x).member", TsTypeAssertionExpression); + assert_needs_parentheses!("(x)[member]", TsTypeAssertionExpression); + assert_not_needs_parentheses!("object[x]", TsTypeAssertionExpression); + } } diff --git a/crates/rome_js_formatter/src/utils/assignment_like.rs b/crates/rome_js_formatter/src/utils/assignment_like.rs index c90f178b692c..b58989a4121c 100644 --- a/crates/rome_js_formatter/src/utils/assignment_like.rs +++ b/crates/rome_js_formatter/src/utils/assignment_like.rs @@ -1,8 +1,10 @@ -use crate::parentheses::get_expression_left_side; +use crate::parentheses::{ + get_expression_left_side, resolve_parent, ExpressionNode, NeedsParentheses, +}; use crate::prelude::*; use crate::utils::member_chain::is_member_call_chain; use crate::utils::object::write_member_name; -use crate::utils::{resolve_expression, JsAnyBinaryLikeExpression, JsAnyBinaryLikeLeftExpression}; +use crate::utils::{JsAnyBinaryLikeExpression, JsAnyBinaryLikeLeftExpression}; use rome_formatter::{format_args, write, CstFormatContext, VecBuffer}; use rome_js_syntax::{ JsAnyAssignmentPattern, JsAnyBindingPattern, JsAnyCallArgument, JsAnyClassMemberName, @@ -586,13 +588,12 @@ impl JsAnyAssignmentLike { return Ok(AssignmentLikeLayout::SuppressedInitializer); } } + let right_expression = right.as_expression().map(ExpressionNode::into_resolved); - if let Some(layout) = self.chain_formatting_layout(&right)? { + if let Some(layout) = self.chain_formatting_layout(right_expression.as_ref())? { return Ok(layout); } - let right_expression = right.as_expression().map(resolve_expression); - if let Some(JsAnyExpression::JsCallExpression(call_expression)) = &right_expression { if call_expression.callee()?.syntax().text() == "require" { return Ok(AssignmentLikeLayout::NeverBreakAfterOperator); @@ -680,11 +681,11 @@ impl JsAnyAssignmentLike { /// and if so, it return the layout type fn chain_formatting_layout( &self, - right: &RightAssignmentLike, + right_expression: Option<&JsAnyExpression>, ) -> SyntaxResult> { let right_is_tail = !matches!( - right, - RightAssignmentLike::JsAnyExpression(JsAnyExpression::JsAssignmentExpression(_)) + right_expression, + Some(JsAnyExpression::JsAssignmentExpression(_)) ); // The chain goes up two levels, by checking up to the great parent if all the conditions @@ -692,14 +693,14 @@ impl JsAnyAssignmentLike { let upper_chain_is_eligible = // First, we check if the current node is an assignment expression if let JsAnyAssignmentLike::JsAssignmentExpression(assignment) = self { - assignment.syntax().parent().map_or(false, |parent| { + assignment.resolve_parent().map_or(false, |parent| { // Then we check if the parent is assignment expression or variable declarator if matches!( parent.kind(), JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION - | JsSyntaxKind::JS_VARIABLE_DECLARATOR + | JsSyntaxKind::JS_INITIALIZER_CLAUSE ) { - let great_parent_kind = parent.parent().map(|n| n.kind()); + let great_parent_kind = resolve_parent(&parent).map(|n| n.kind()); // Finally, we check the great parent. // The great parent triggers the eligibility when // - the current node that we were inspecting is not a "tail" @@ -724,20 +725,21 @@ impl JsAnyAssignmentLike { if !right_is_tail { Some(AssignmentLikeLayout::Chain) } else { - match right { - RightAssignmentLike::JsAnyExpression( - JsAnyExpression::JsArrowFunctionExpression(arrow), - ) => { + match right_expression { + Some(JsAnyExpression::JsArrowFunctionExpression(arrow)) => { let this_body = arrow.body()?; - if matches!( - this_body, - JsAnyFunctionBody::JsAnyExpression( - JsAnyExpression::JsArrowFunctionExpression(_) - ) - ) { - Some(AssignmentLikeLayout::ChainTailArrowFunction) - } else { - Some(AssignmentLikeLayout::ChainTail) + match this_body { + JsAnyFunctionBody::JsAnyExpression(expression) => { + if matches!( + expression.resolve(), + JsAnyExpression::JsArrowFunctionExpression(_) + ) { + Some(AssignmentLikeLayout::ChainTailArrowFunction) + } else { + Some(AssignmentLikeLayout::ChainTail) + } + } + _ => Some(AssignmentLikeLayout::ChainTail), } } @@ -807,7 +809,7 @@ impl JsAnyAssignmentLike { let result = if let Some(expression) = right.as_expression() { should_break_after_operator(&expression)? } else { - has_new_line_before_comment(right.syntax()) + has_leading_own_line_comment(right.syntax()) }; Ok(result) @@ -820,7 +822,7 @@ pub(crate) fn should_break_after_operator(right: &JsAnyExpression) -> SyntaxResu // that causes a line break. let mut current: JsAnyBinaryLikeLeftExpression = right.clone().into(); loop { - if has_new_line_before_comment(current.syntax()) { + if has_leading_own_line_comment(current.syntax()) { return Ok(true); } @@ -833,13 +835,13 @@ pub(crate) fn should_break_after_operator(right: &JsAnyExpression) -> SyntaxResu break; } - let right = resolve_expression(right.clone()); + let right = right.resolve(); let result = match right { // head is a long chain, meaning that right -> right are both assignment expressions JsAnyExpression::JsAssignmentExpression(assignment) => { matches!( - assignment.right()?, + assignment.right()?.resolve(), JsAnyExpression::JsAssignmentExpression(_) ) } @@ -861,8 +863,8 @@ pub(crate) fn should_break_after_operator(right: &JsAnyExpression) -> SyntaxResu Ok(result) } -/// If checks if among leading trivias, if there's a sequence of [Newline, Comment] -pub(crate) fn has_new_line_before_comment(node: &JsSyntaxNode) -> bool { +/// Tests if the node has any leading comment that will be placed on its own line. +pub(crate) fn has_leading_own_line_comment(node: &JsSyntaxNode) -> bool { if let Some(leading_trivia) = node.first_leading_trivia() { let mut first_comment = true; let mut after_comment = false; @@ -1045,24 +1047,29 @@ fn is_poorly_breakable_member_or_call_chain( let mut expression = Some(expression.clone()); while let Some(node) = expression.take() { - match node { + expression = match node { JsAnyExpression::JsCallExpression(call_expression) => { is_chain = true; - expression = Some(call_expression.callee()?); + let callee = call_expression.callee()?; call_expressions.push(call_expression); + Some(callee) } JsAnyExpression::JsStaticMemberExpression(node) => { is_chain = true; - expression = Some(node.object()?); + Some(node.object()?) } JsAnyExpression::JsComputedMemberExpression(node) => { is_chain = true; - expression = Some(node.object()?); + Some(node.object()?) } + JsAnyExpression::JsParenthesizedExpression(node) => Some(node.expression()?), JsAnyExpression::JsIdentifierExpression(_) | JsAnyExpression::JsThisExpression(_) => { is_chain_head_simple = true; + break; + } + _ => { + break; } - _ => {} } } @@ -1113,7 +1120,7 @@ fn is_short_argument(argument: JsAnyCallArgument, threshold: u16) -> SyntaxResul } if let JsAnyCallArgument::JsAnyExpression(expression) = argument { - let is_short_argument = match expression { + let is_short_argument = match expression.resolve() { JsAnyExpression::JsThisExpression(_) => true, JsAnyExpression::JsIdentifierExpression(identifier) => { identifier.name()?.value_token()?.text_trimmed().len() <= threshold as usize 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 d663e790d6fe..ddb855b377dd 100644 --- a/crates/rome_js_formatter/src/utils/binary_like_expression.rs +++ b/crates/rome_js_formatter/src/utils/binary_like_expression.rs @@ -6,6 +6,9 @@ use rome_js_syntax::{ JsSyntaxNode, JsSyntaxToken, OperatorPrecedence, }; +use crate::parentheses::{ + is_callee, is_member_object, is_spread, is_tag, ExpressionNode, NeedsParentheses, +}; use crate::utils::should_break_after_operator; use rome_rowan::{declare_node_union, AstNode, SyntaxResult}; use std::fmt::Debug; @@ -167,10 +170,6 @@ impl BinaryLikeOperator { } } - pub const fn is_logical(&self) -> bool { - matches!(self, BinaryLikeOperator::Logical(_)) - } - pub const fn is_remainder(&self) -> bool { matches!( self, @@ -191,64 +190,13 @@ impl From for BinaryLikeOperator { } } -/// This function returns `true` when the binary expression should be wrapped in parentheses to either -/// * a) correctly encode precedence -/// * b) Improve readability by adding parentheses around expressions where the precedence may not be obvious to many readers. -pub(crate) fn binary_argument_needs_parens( - parent_operator: BinaryLikeOperator, - node: &JsAnyBinaryLikeLeftExpression, - is_right: bool, -) -> SyntaxResult { - let current_operator = - if let Some(binary_like) = JsAnyBinaryLikeExpression::cast(node.syntax().clone()) { - binary_like.operator()? - } else { - return Ok(false); - }; - - // For logical nodes, add parentheses if the operators aren't the same - if parent_operator.is_logical() && current_operator.is_logical() { - return Ok(parent_operator != current_operator); - } - - let current_precedence = current_operator.precedence(); - let parent_precedence = parent_operator.precedence(); - - #[allow(clippy::if_same_then_else, clippy::needless_bool)] - // If the parent has a higher precedence than parentheses are necessary to not change the semantic meaning - // when re-parsing. - let result = if parent_precedence > current_precedence { - true - } else if is_right && parent_precedence == current_precedence { - true - } - // Add parentheses around bitwise and bit shift operators - // `a * 3 >> 5` -> `(a * 3) >> 5` - else if parent_precedence.is_bitwise() || parent_precedence.is_shift() { - true - } - // `a % 4 + 4` -> `a % (4 + 4)` - else if parent_precedence < current_precedence && current_operator.is_remainder() { - parent_precedence.is_additive() - } else if parent_precedence == current_precedence - && !should_flatten(parent_operator, current_operator) - { - true - } else { - false - }; - - 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 binary_argument_needs_parens(parent_operator, sub_expression, true)? { + if binary_argument_needs_parens(sub_expression) { format_parenthesize( sub_expression.syntax().first_token().as_ref(), &sub_expression, @@ -408,11 +356,10 @@ impl FlattenItems { } let left = binary_like_expression.left()?; - let operator = binary_like_expression.operator()?; let operator_token = binary_like_expression.operator_token()?; let operator_has_trailing_comments = operator_token.has_trailing_comments(); - let left_parenthesized = binary_argument_needs_parens(operator, &left, false)?; + let left_parenthesized = binary_argument_needs_parens(&left); let mut left_item = FlattenItem::new( FlattenedBinaryExpressionPart::Group { current: left, @@ -526,7 +473,7 @@ impl FlattenedBinaryExpressionPart { FlattenedBinaryExpressionPart::Right { parent } => { let right = JsAnyBinaryLikeLeftExpression::JsAnyExpression(parent.right()?); - write!(f, [format_sub_expression(parent.operator()?, &right)]) + write!(f, [format_sub_expression(&right)]) } FlattenedBinaryExpressionPart::Left { expression } => { write!(f, [expression]) @@ -878,6 +825,122 @@ impl From for JsAnyExpression { } } +/// This function returns `true` when the binary expression should be wrapped in parentheses to either +/// * a) correctly encode precedence +/// * b) Improve readability by adding parentheses around expressions where the precedence may not be obvious to many readers. +pub(crate) fn binary_argument_needs_parens(node: &JsAnyBinaryLikeLeftExpression) -> bool { + if let Some(binary_like) = JsAnyBinaryLikeExpression::cast(node.syntax().clone()) { + binary_like.needs_parentheses() + } else { + false + } +} + +pub(crate) fn needs_binary_like_parentheses( + node: &JsAnyBinaryLikeExpression, + parent: &JsSyntaxNode, +) -> bool { + match parent.kind() { + JsSyntaxKind::JS_EXTENDS_CLAUSE + | JsSyntaxKind::TS_AS_EXPRESSION + | JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION + | JsSyntaxKind::JS_UNARY_EXPRESSION + | JsSyntaxKind::JS_AWAIT_EXPRESSION + | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => true, + + kind if JsAnyBinaryLikeExpression::can_cast(kind) => { + let parent = JsAnyBinaryLikeExpression::unwrap_cast(parent.clone()); + + let operator = node.operator(); + let parent_operator = parent.operator(); + + match (operator, parent_operator) { + (Ok(operator), Ok(parent_operator)) => { + let precedence = operator.precedence(); + let parent_precedence = parent_operator.precedence(); + + #[allow(clippy::if_same_then_else, clippy::needless_bool)] + // If the parent has a higher precedence than parentheses are necessary to not change the semantic meaning + // when re-parsing. + if parent_precedence > precedence { + return true; + } + + let is_right = parent + .right() + .map(ExpressionNode::into_resolved_syntax) + .as_ref() + == Ok(node.syntax()); + + // `a ** b ** c` + if is_right && parent_precedence == precedence { + return true; + } + + // Add parentheses around bitwise and bit shift operators + // `a * 3 >> 5` -> `(a * 3) >> 5` + if parent_precedence.is_bitwise() || parent_precedence.is_shift() { + return true; + } + + // `a % 4 + 4` -> `(a % 4) + 4)` + if parent_precedence < precedence && operator.is_remainder() { + return parent_precedence.is_additive(); + } + + if parent_precedence == precedence && !should_flatten(parent_operator, operator) + { + return true; + } + + false + } + // Just to be sure + _ => true, + } + } + + _ => { + is_callee(node.syntax(), parent) + || is_tag(node.syntax(), parent) + || is_spread(node.syntax(), parent) + || is_member_object(node.syntax(), parent) + } + } +} + +impl NeedsParentheses for JsAnyBinaryLikeExpression { + fn needs_parentheses(&self) -> bool { + match self { + JsAnyBinaryLikeExpression::JsLogicalExpression(logical) => logical.needs_parentheses(), + JsAnyBinaryLikeExpression::JsBinaryExpression(binary) => binary.needs_parentheses(), + JsAnyBinaryLikeExpression::JsInstanceofExpression(instanceof) => { + instanceof.needs_parentheses() + } + JsAnyBinaryLikeExpression::JsInExpression(in_expression) => { + in_expression.needs_parentheses() + } + } + } + + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + match self { + JsAnyBinaryLikeExpression::JsLogicalExpression(logical) => { + logical.needs_parentheses_with_parent(parent) + } + JsAnyBinaryLikeExpression::JsBinaryExpression(binary) => { + binary.needs_parentheses_with_parent(parent) + } + JsAnyBinaryLikeExpression::JsInstanceofExpression(instanceof) => { + instanceof.needs_parentheses_with_parent(parent) + } + JsAnyBinaryLikeExpression::JsInExpression(in_expression) => { + in_expression.needs_parentheses_with_parent(parent) + } + } + } +} + /// Returns `true` if a binary expression nested into another binary expression should be flattened together. /// /// This is generally the case if both operator have the same precedence. See the inline comments for the exceptions @@ -886,38 +949,41 @@ fn should_flatten(parent_operator: BinaryLikeOperator, child_operator: BinaryLik let parent_precedence = parent_operator.precedence(); let child_precedence = child_operator.precedence(); - #[allow(clippy::if_same_then_else, clippy::needless_bool)] if parent_precedence != child_precedence { - false + return false; } + // `**` is right associative. - else if parent_precedence.is_exponential() { - false + if parent_precedence.is_exponential() { + return false; } // avoid `a == b == c` -> `(a == b) == c` - else if parent_precedence.is_equality() && child_precedence.is_equality() { - false + + if parent_precedence.is_equality() && child_precedence.is_equality() { + return false; } + // `a % b * c` -> `(a % b) * c` // `a * b % c` -> `(a * b) % c` - else if (child_operator.is_remainder() && parent_precedence.is_multiplicative()) + if (child_operator.is_remainder() && parent_precedence.is_multiplicative()) || (parent_operator.is_remainder() && child_precedence.is_multiplicative()) { - false + return false; } + // `a * b / c` -> `(a * b) / c` - else if child_operator != parent_operator + if child_operator != parent_operator && child_precedence.is_multiplicative() && parent_precedence.is_multiplicative() { - false + return false; } // `a >> b >> c` -> `(a >> b) >> c` - else if parent_precedence.is_shift() && child_precedence.is_shift() { - false - } else { - true + if parent_precedence.is_shift() && child_precedence.is_shift() { + return false; } + + true } declare_node_union! { @@ -933,6 +999,26 @@ impl JsAnyBinaryLikeLeftExpression { } } +impl NeedsParentheses for JsAnyBinaryLikeLeftExpression { + fn needs_parentheses(&self) -> bool { + match self { + JsAnyBinaryLikeLeftExpression::JsAnyExpression(expression) => { + expression.needs_parentheses() + } + JsAnyBinaryLikeLeftExpression::JsPrivateName(_) => false, + } + } + + fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { + match self { + JsAnyBinaryLikeLeftExpression::JsAnyExpression(expression) => { + expression.needs_parentheses_with_parent(parent) + } + JsAnyBinaryLikeLeftExpression::JsPrivateName(_) => false, + } + } +} + impl Format for JsAnyBinaryLikeLeftExpression { fn fmt(&self, f: &mut JsFormatter) -> FormatResult<()> { match self { diff --git a/crates/rome_js_formatter/src/utils/conditional.rs b/crates/rome_js_formatter/src/utils/conditional.rs index 3a4201233a5e..09805c9e74cf 100644 --- a/crates/rome_js_formatter/src/utils/conditional.rs +++ b/crates/rome_js_formatter/src/utils/conditional.rs @@ -1,7 +1,7 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::resolve_expression_parent; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; use rome_js_syntax::{ JsAnyExpression, JsAssignmentExpression, JsCallExpression, JsComputedMemberExpression, JsConditionalExpression, JsInitializerClause, JsNewExpression, JsParenthesizedExpression, @@ -187,9 +187,7 @@ impl ExpressionOrType { /// Resolves to the first non parenthesized expression. Returns self for types. fn resolve(&self) -> JsSyntaxNode { match self { - ExpressionOrType::JsAnyExpression(expression) => { - resolve_expression_syntax(expression.clone()) - } + ExpressionOrType::JsAnyExpression(expression) => expression.resolve_syntax(), ExpressionOrType::TsType(ty) => ty.syntax().clone(), } } @@ -287,7 +285,12 @@ impl ConditionalLayout { impl JsAnyConditional { fn layout(&self) -> ConditionalLayout { - let parent = match resolve_expression_parent(self.syntax()) { + let resolved_parent = match self { + JsAnyConditional::JsConditionalExpression(conditional) => conditional.resolve_parent(), + JsAnyConditional::TsConditionalType(ty) => ty.syntax().parent(), + }; + + let parent = match resolved_parent { None => return ConditionalLayout::Root { parent: None }, Some(parent) => parent, }; @@ -321,7 +324,7 @@ impl JsAnyConditional { JsAnyConditional::JsConditionalExpression(conditional) => conditional .test() .ok() - .map(resolve_expression) + .map(ExpressionNode::into_resolved) .map_or(false, |resolved| resolved.syntax() == node), JsAnyConditional::TsConditionalType(conditional) => { conditional.check_type().map(AstNode::into_syntax).as_ref() == Ok(node) @@ -377,9 +380,8 @@ impl JsAnyConditional { let alternate = match self { JsAnyConditional::JsConditionalExpression(conditional) => conditional .alternate() - .map(resolve_expression) - .ok() - .map(AstNode::into_syntax), + .map(ExpressionNode::into_resolved_syntax) + .ok(), JsAnyConditional::TsConditionalType(ts_conditional) => { ts_conditional.false_type().ok().map(AstNode::into_syntax) } @@ -441,7 +443,11 @@ impl JsAnyConditional { JsSyntaxKind::JS_CALL_EXPRESSION => { let call_expression = JsCallExpression::unwrap_cast(ancestor); - if call_expression.callee().map(resolve_expression).as_ref() == Ok(&expression) + if call_expression + .callee() + .map(ExpressionNode::into_resolved) + .as_ref() + == Ok(&expression) { Ancestor::MemberChain(call_expression.into()) } else { @@ -452,7 +458,10 @@ impl JsAnyConditional { JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION => { let member_expression = JsStaticMemberExpression::unwrap_cast(ancestor); - if member_expression.object().map(resolve_expression).as_ref() + if member_expression + .object() + .map(ExpressionNode::into_resolved) + .as_ref() == Ok(&expression) { Ancestor::MemberChain(member_expression.into()) @@ -463,7 +472,10 @@ impl JsAnyConditional { JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION => { let member_expression = JsComputedMemberExpression::unwrap_cast(ancestor); - if member_expression.object().map(resolve_expression).as_ref() + if member_expression + .object() + .map(ExpressionNode::into_resolved) + .as_ref() == Ok(&expression) { Ancestor::MemberChain(member_expression.into()) @@ -476,7 +488,7 @@ impl JsAnyConditional { if non_null_assertion .expression() - .map(resolve_expression) + .map(ExpressionNode::into_resolved) .as_ref() == Ok(&expression) { @@ -489,8 +501,13 @@ impl JsAnyConditional { let new_expression = JsNewExpression::unwrap_cast(ancestor); // Skip over new expressions - if new_expression.callee().map(resolve_expression).as_ref() == Ok(&expression) { - parent = new_expression.syntax().parent(); + if new_expression + .callee() + .map(ExpressionNode::into_resolved) + .as_ref() + == Ok(&expression) + { + parent = new_expression.resolve_parent(); expression = new_expression.into(); break; } @@ -500,10 +517,13 @@ impl JsAnyConditional { JsSyntaxKind::TS_AS_EXPRESSION => { let as_expression = TsAsExpression::unwrap_cast(ancestor.clone()); - if as_expression.expression().map(resolve_expression).as_ref() + if as_expression + .expression() + .map(ExpressionNode::into_resolved) + .as_ref() == Ok(&expression) { - parent = as_expression.syntax().parent(); + parent = as_expression.resolve_parent(); expression = as_expression.into(); break; } @@ -565,7 +585,7 @@ impl JsAnyConditional { _ => None, }; - argument.map_or(false, |argument| resolve_expression(argument) == expression) + argument.map_or(false, |argument| argument.resolve() == expression) } } } @@ -584,18 +604,3 @@ impl JsAnyConditional { } } } - -/// Resolves an expression to the first non parenthesized expression. -pub(crate) fn resolve_expression(expression: JsAnyExpression) -> JsAnyExpression { - match expression { - JsAnyExpression::JsParenthesizedExpression(expression) => expression - .expression() - .unwrap_or(JsAnyExpression::JsParenthesizedExpression(expression)), - _ => expression, - } -} - -/// Resolves an expression to the first non parenthesized expression and returns its [JsSyntaxNode]. -pub(crate) fn resolve_expression_syntax(expression: JsAnyExpression) -> JsSyntaxNode { - resolve_expression(expression).into_syntax() -} diff --git a/crates/rome_js_formatter/src/utils/jsx.rs b/crates/rome_js_formatter/src/utils/jsx.rs index 435d73378c52..1fa140fb255f 100644 --- a/crates/rome_js_formatter/src/utils/jsx.rs +++ b/crates/rome_js_formatter/src/utils/jsx.rs @@ -1,5 +1,5 @@ use crate::context::QuoteStyle; -use crate::parentheses::resolve_expression_parent; +use crate::parentheses::NeedsParentheses; use crate::prelude::*; use rome_formatter::{format_args, write}; use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, JsxAnyChild, JsxChildList, JsxTagExpression}; @@ -77,7 +77,7 @@ pub enum WrapState { pub fn get_wrap_state(node: &JsxTagExpression) -> WrapState { // We skip the first item because the first item in ancestors is the node itself, i.e. // the JSX Element in this case. - let parent = resolve_expression_parent(node.syntax()); + let parent = node.resolve_parent(); parent.map_or(WrapState::NoWrap, |parent| match parent.kind() { JsSyntaxKind::JS_ARRAY_EXPRESSION diff --git a/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs b/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs index 5020c9fc7354..4b648c4acc38 100644 --- a/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs +++ b/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs @@ -11,16 +11,31 @@ use rome_js_syntax::{ use rome_rowan::{AstNode, SyntaxResult}; use std::fmt::Debug; +/// One entry in a member chain. #[derive(Clone, Debug)] pub(crate) enum ChainEntry { + /// A member that is parenthesized in the source document Parenthesized { + /// The chain member member: ChainMember, + /// The top most ancestor of the chain member that is a parenthesized expression. + /// + /// ```text + /// (a.b).c() + /// ^^^ -> member + /// ^----^ -> top_most_parentheses + /// + /// ((a.b)).c() + /// ^^^ -> member + /// ^-----^ -> top most parentheses (skips inner parentheses node) + /// ``` top_most_parentheses: JsParenthesizedExpression, }, Member(ChainMember), } impl ChainEntry { + /// Returns the inner member pub fn member(&self) -> &ChainMember { match self { ChainEntry::Parenthesized { member, .. } => member, @@ -28,6 +43,7 @@ impl ChainEntry { } } + /// Returns the top most parentheses node if any pub fn top_most_parentheses(&self) -> Option<&JsParenthesizedExpression> { match self { ChainEntry::Parenthesized { @@ -49,6 +65,7 @@ impl ChainEntry { self.nodes().any(|node| node.has_trailing_comments()) } + /// Returns true if the member any of it's ancestor parentheses nodes has any leading comments. pub(crate) fn has_leading_comments(&self) -> SyntaxResult { let has_operator_comment = match self.member() { ChainMember::StaticMember(node) => node.operator_token()?.has_leading_comments(), @@ -134,8 +151,8 @@ impl Format for ChainEntry { } } -#[derive(Clone, Debug)] /// Data structure that holds the node with its formatted version +#[derive(Clone, Debug)] pub(crate) enum ChainMember { /// Holds onto a [rome_js_syntax::JsStaticMemberExpression] StaticMember(JsStaticMemberExpression), diff --git a/crates/rome_js_formatter/src/utils/member_chain/groups.rs b/crates/rome_js_formatter/src/utils/member_chain/groups.rs index c86fc243fbb7..dc936581a90d 100644 --- a/crates/rome_js_formatter/src/utils/member_chain/groups.rs +++ b/crates/rome_js_formatter/src/utils/member_chain/groups.rs @@ -49,7 +49,7 @@ impl MemberChainGroupsBuilder { /// adds the passed element to the current group pub fn continue_group(&mut self, flatten_item: ChainEntry) { debug_assert!(!self.current_group.entries.is_empty()); - self.current_group.entries.push(flatten_item.into()); + self.current_group.entries.push(flatten_item); } /// clears the current group, and adds a new group to the groups diff --git a/crates/rome_js_formatter/src/utils/member_chain/mod.rs b/crates/rome_js_formatter/src/utils/member_chain/mod.rs index 474dd3c6b27d..bddfa3ad34a4 100644 --- a/crates/rome_js_formatter/src/utils/member_chain/mod.rs +++ b/crates/rome_js_formatter/src/utils/member_chain/mod.rs @@ -16,7 +16,7 @@ ///! .catch() ///! ``` ///! -///! In order to achieve that we use the same heuristic that [Prettier applies]. +///! In order to achieve that we use the same heuristic that [Prettier applies](https://github.com/prettier/prettier/blob/main/src/language-js/print/member-chain.js). ///! ///! The process is the following: ///! @@ -102,13 +102,11 @@ ///! [StaticMemberExpression], ///! ] ///! ``` -///! -///! [Prettier applies]: https://github.com/prettier/prettier/blob/main/src/language-js/print/member-chain.js mod chain_member; mod groups; mod simple_argument; -use crate::parentheses::resolve_expression_parent; +use crate::parentheses::NeedsParentheses; use crate::prelude::*; use crate::utils::member_chain::chain_member::{ChainEntry, ChainMember}; use crate::utils::member_chain::groups::{ @@ -196,10 +194,9 @@ pub(crate) fn get_member_chain( f: &mut JsFormatter, ) -> SyntaxResult { let mut chain_members = vec![]; - let parent_is_expression_statement = resolve_expression_parent(call_expression.syntax()) - .map_or(false, |parent| { - JsExpressionStatement::can_cast(parent.kind()) - }); + let parent_is_expression_statement = call_expression.resolve_parent().map_or(false, |parent| { + JsExpressionStatement::can_cast(parent.kind()) + }); let root = flatten_member_chain( &mut chain_members, diff --git a/crates/rome_js_formatter/src/utils/mod.rs b/crates/rome_js_formatter/src/utils/mod.rs index 64696c88d34a..fe39d468a723 100644 --- a/crates/rome_js_formatter/src/utils/mod.rs +++ b/crates/rome_js_formatter/src/utils/mod.rs @@ -19,22 +19,18 @@ pub(crate) use crate::parentheses::resolve_left_most_expression; use crate::prelude::*; pub(crate) use assignment_like::{should_break_after_operator, JsAnyAssignmentLike}; pub(crate) use binary_like_expression::{ - binary_argument_needs_parens, format_binary_like_expression, JsAnyBinaryLikeExpression, - JsAnyBinaryLikeLeftExpression, + binary_argument_needs_parens, format_binary_like_expression, needs_binary_like_parentheses, + JsAnyBinaryLikeExpression, JsAnyBinaryLikeLeftExpression, }; -pub(crate) use conditional::{resolve_expression, resolve_expression_syntax, JsAnyConditional}; +pub(crate) use conditional::JsAnyConditional; pub(crate) use member_chain::get_member_chain; pub(crate) use object_like::JsObjectLike; pub(crate) use object_pattern_like::JsObjectPatternLike; -use rome_formatter::{format_args, normalize_newlines, write, Buffer, VecBuffer}; -use rome_js_syntax::{ - JsAnyExpression, JsAnyFunction, JsAnyStatement, JsInitializerClause, JsLanguage, - JsTemplateElement, Modifiers, TsTemplateElement, TsType, -}; +use rome_formatter::{format_args, write, Buffer, VecBuffer}; +use rome_js_syntax::{JsAnyExpression, JsAnyStatement, JsInitializerClause, JsLanguage, Modifiers}; use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, JsSyntaxToken}; use rome_rowan::{AstNode, AstNodeList, Direction}; pub(crate) use simple::*; -use std::fmt::Debug; pub(crate) use string_utils::*; pub(crate) use typescript::should_hug_type; diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/assignment/chain.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/assignment/chain.js.snap index 06d71977c5c2..09676c40128b 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/assignment/chain.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/assignment/chain.js.snap @@ -40,22 +40,7 @@ a=b=c; ```diff --- Prettier +++ Rome -@@ -1,21 +1,17 @@ - let bifornCringerMoshedPerplexSawder = - (askTrovenaBeenaDependsRowans = -- glimseGlyphsHazardNoopsTieTie = -- averredBathersBoxroomBuggyNurl = -- anodyneCondosMalateOverateRetinol = -- annularCooeedSplicesWalksWayWay = -- kochabCooieGameOnOboleUnweave); -+ glimseGlyphsHazardNoopsTieTie = -+ averredBathersBoxroomBuggyNurl = -+ anodyneCondosMalateOverateRetinol = -+ annularCooeedSplicesWalksWayWay = -+ kochabCooieGameOnOboleUnweave); - - bifornCringerMoshedPerplexSawder = - askTrovenaBeenaDependsRowans = +@@ -11,11 +11,7 @@ glimseGlyphsHazardNoopsTieTie = x = averredBathersBoxroomBuggyNurl = @@ -88,11 +73,11 @@ a=b=c; ```js let bifornCringerMoshedPerplexSawder = (askTrovenaBeenaDependsRowans = - glimseGlyphsHazardNoopsTieTie = - averredBathersBoxroomBuggyNurl = - anodyneCondosMalateOverateRetinol = - annularCooeedSplicesWalksWayWay = - kochabCooieGameOnOboleUnweave); + glimseGlyphsHazardNoopsTieTie = + averredBathersBoxroomBuggyNurl = + anodyneCondosMalateOverateRetinol = + annularCooeedSplicesWalksWayWay = + kochabCooieGameOnOboleUnweave); bifornCringerMoshedPerplexSawder = askTrovenaBeenaDependsRowans = diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/async/inline-await.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/async/inline-await.js.snap deleted file mode 100644 index 60e4a08cbf13..000000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/async/inline-await.js.snap +++ /dev/null @@ -1,49 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -async function f() { - const admins = (await(db.select('*').from('admins').leftJoin('bla').where('id', 'in', [1,2,3,4]))).map(({id, name})=>({id, name})) -} -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,9 +1,7 @@ - async function f() { -- const admins = ( -- await db -- .select("*") -- .from("admins") -- .leftJoin("bla") -- .where("id", "in", [1, 2, 3, 4]) -- ).map(({ id, name }) => ({ id, name })); -+ const admins = (await db -+ .select("*") -+ .from("admins") -+ .leftJoin("bla") -+ .where("id", "in", [1, 2, 3, 4])).map(({ id, name }) => ({ id, name })); - } -``` - -# Output - -```js -async function f() { - const admins = (await db - .select("*") - .from("admins") - .leftJoin("bla") - .where("id", "in", [1, 2, 3, 4])).map(({ id, name }) => ({ id, name })); -} -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/async/nested.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/async/nested.js.snap deleted file mode 100644 index d6805f125e7e..000000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/async/nested.js.snap +++ /dev/null @@ -1,50 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs -assertion_line: 271 -info: - test_file: js/async/nested.js ---- - -# Input - -```js -const getAccountCount = async () => - (await - (await ( - await focusOnSection(BOOKMARKED_PROJECTS_SECTION_NAME) - ).findItem("My bookmarks") - ).getChildren() - ).length -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,8 +1,4 @@ - const getAccountCount = async () => -- ( -- await ( -- await ( -- await focusOnSection(BOOKMARKED_PROJECTS_SECTION_NAME) -- ).findItem("My bookmarks") -- ).getChildren() -- ).length; -+ (await (await (await focusOnSection( -+ BOOKMARKED_PROJECTS_SECTION_NAME, -+ )).findItem("My bookmarks")).getChildren()).length; -``` - -# Output - -```js -const getAccountCount = async () => - (await (await (await focusOnSection( - BOOKMARKED_PROJECTS_SECTION_NAME, - )).findItem("My bookmarks")).getChildren()).length; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/export-default/body.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/export-default/body.js.snap new file mode 100644 index 000000000000..36114ba0da42 --- /dev/null +++ b/crates/rome_js_formatter/tests/specs/prettier/js/export-default/body.js.snap @@ -0,0 +1,29 @@ +--- +source: crates/rome_js_formatter/tests/prettier_tests.rs +--- + +# Input + +```js +export default (class {}[1] = 1); +``` + + +# Prettier differences + +```diff +--- Prettier ++++ Rome +@@ -1 +1 @@ +-export default (class {}[1] = 1); ++export default ((class {})[1] = 1); +``` + +# Output + +```js +export default ((class {})[1] = 1); +``` + + + diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/new-expression/new_expression.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/new-expression/new_expression.js.snap deleted file mode 100644 index 181be34c49b1..000000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/new-expression/new_expression.js.snap +++ /dev/null @@ -1,50 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -new (memoize.Cache || MapCache) -new (typeof this == "function" ? this : Dict()) -new (createObj()).prop(a()); -new (x()``.y)(); -new e[f().x].y(); -new e[f()].y(); -new (a().b)(); -new (a().b().c)(); -new (a``()); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,6 +1,6 @@ - new (memoize.Cache || MapCache)(); - new (typeof this == "function" ? this : Dict())(); --new (createObj().prop)(a()); -+new createObj().prop(a()); - new (x()``.y)(); - new e[f().x].y(); - new e[f()].y(); -``` - -# Output - -```js -new (memoize.Cache || MapCache)(); -new (typeof this == "function" ? this : Dict())(); -new createObj().prop(a()); -new (x()``.y)(); -new e[f().x].y(); -new e[f()].y(); -new (a().b)(); -new (a().b().c)(); -new (a``())(); -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/objects/expression.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/objects/expression.js.snap index b0e5d87e7a52..a702da396d36 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/objects/expression.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/objects/expression.js.snap @@ -45,7 +45,7 @@ const a3 = { -({}::b, 0); -({}::b()``[""].c++ && 0 ? 0 : 0, 0); -({}(), 0); -+() => (({})``); ++() => ({})``; +({})``; +a = () => ({}).x; +({}) && a, b; @@ -62,7 +62,7 @@ const a3 = { # Output ```js -() => (({})``); +() => ({})``; ({})``; a = () => ({}).x; ({}) && a, b; diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/sequence-break/break.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/sequence-break/break.js.snap deleted file mode 100644 index 6fce3da0ee03..000000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/sequence-break/break.js.snap +++ /dev/null @@ -1,108 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -const f = (argument1, argument2, argument3) => - (doSomethingWithArgument(argument1), doSomethingWithArgument(argument2),argument1); -(function(){ - return aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName; -}); -aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName; -a.then(() => (aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName)); -for (aLongIdentifierName = 0, aLongIdentifierName = 0, aLongIdentifierName = 0, aLongIdentifierName = 0; test; update) {} -(a = b ? c : function() { return 0; }), - (a = b ? c : function() { return 0; }), - (a = b ? c : function() { return 0; }), - (a = b ? c : function() { return 0; }), - (a = b ? c : function() { return 0; }); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -24,9 +24,9 @@ - ), - ); - for ( -- aLongIdentifierName = 0, -- aLongIdentifierName = 0, -- aLongIdentifierName = 0, -+ (aLongIdentifierName = 0), -+ (aLongIdentifierName = 0), -+ (aLongIdentifierName = 0), - aLongIdentifierName = 0; - test; - update -``` - -# Output - -```js -const f = (argument1, argument2, argument3) => ( - doSomethingWithArgument(argument1), - doSomethingWithArgument(argument2), - argument1 -); -(function () { - return ( - aLongIdentifierName, - aLongIdentifierName, - aLongIdentifierName, - aLongIdentifierName - ); -}); -aLongIdentifierName, - aLongIdentifierName, - aLongIdentifierName, - aLongIdentifierName; -a.then( - () => ( - aLongIdentifierName, - aLongIdentifierName, - aLongIdentifierName, - aLongIdentifierName - ), -); -for ( - (aLongIdentifierName = 0), - (aLongIdentifierName = 0), - (aLongIdentifierName = 0), - aLongIdentifierName = 0; - test; - update -) {} -(a = b - ? c - : function () { - return 0; - }), - (a = b - ? c - : function () { - return 0; - }), - (a = b - ? c - : function () { - return 0; - }), - (a = b - ? c - : function () { - return 0; - }), - (a = b - ? c - : function () { - return 0; - }); -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/template/parenthesis.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/template/parenthesis.js.snap index 285363bbf2ae..16e50fa7e373 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/template/parenthesis.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/template/parenthesis.js.snap @@ -76,12 +76,8 @@ function* d() { // "ConditionalExpression" (b ? c : d)``; -@@ -31,16 +31,16 @@ - b.c``; - - // "NewExpression" --new B()``; -+(new B())``; +@@ -34,7 +34,7 @@ + new B()``; // "ObjectExpression" -({}``); @@ -89,13 +85,6 @@ function* d() { // "SequenceExpression" (b, c)``; - - // "TaggedTemplateExpression" --````; -+(``)``; - - // "UnaryExpression" - (void b)``; ``` # Output @@ -134,7 +123,7 @@ b()``; b.c``; // "NewExpression" -(new B())``; +new B()``; // "ObjectExpression" ({})``; @@ -143,7 +132,7 @@ b.c``; (b, c)``; // "TaggedTemplateExpression" -(``)``; +````; // "UnaryExpression" (void b)``; diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/unary-expression/comments.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/unary-expression/comments.js.snap index 6873b92b3997..87d1fc703186 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/unary-expression/comments.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/unary-expression/comments.js.snap @@ -298,7 +298,7 @@ async function bar2() { ```diff --- Prettier +++ Rome -@@ -1,207 +1,162 @@ +@@ -1,283 +1,218 @@ !x; -!(x /* foo */); -!(/* foo */ x); @@ -624,9 +624,28 @@ async function bar2() { +!x(); // foo !new x(); - !(new x() /* foo */); -@@ -219,65 +174,49 @@ - ); +-!(new x() /* foo */); +-!(/* foo */ new x()); +-!( +- /* foo */ +- new x() +-); +-!( +- new x() +- /* foo */ +-); +-!( +- new x() // foo +-); ++!new x() /* foo */; ++! /* foo */ new x(); ++! ++/* foo */ ++new x(); ++!new x() ++/* foo */ ++; ++!new x(); // foo !(x, y); -!((x, y) /* foo */); @@ -893,19 +912,15 @@ x(); !x(); // foo !new x(); -!(new x() /* foo */); -!(/* foo */ new x()); -!( - /* foo */ - new x() -); -!( - new x() - /* foo */ -); -!( - new x() // foo -); +!new x() /* foo */; +! /* foo */ new x(); +! +/* foo */ +new x(); +!new x() +/* foo */ +; +!new x(); // foo !(x, y); !(x, y) /* foo */; diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/as/nested-await-and-as.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/as/nested-await-and-as.ts.snap deleted file mode 100644 index 0663061b3e9c..000000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/as/nested-await-and-as.ts.snap +++ /dev/null @@ -1,47 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -const getAccountCount = async () => - (await - ((await ( - await focusOnSection(BOOKMARKED_PROJECTS_SECTION_NAME) - ).findItem("My bookmarks")) as TreeItem - ).getChildren() - ).length -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,8 +1,4 @@ - const getAccountCount = async () => -- ( -- await ( -- (await ( -- await focusOnSection(BOOKMARKED_PROJECTS_SECTION_NAME) -- ).findItem("My bookmarks")) as TreeItem -- ).getChildren() -- ).length; -+ (await ((await (await focusOnSection( -+ BOOKMARKED_PROJECTS_SECTION_NAME, -+ )).findItem("My bookmarks")) as TreeItem).getChildren()).length; -``` - -# Output - -```js -const getAccountCount = async () => - (await ((await (await focusOnSection( - BOOKMARKED_PROJECTS_SECTION_NAME, - )).findItem("My bookmarks")) as TreeItem).getChildren()).length; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/as/ternary.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/as/ternary.ts.snap index da26a739dab2..190657cd4591 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/as/ternary.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/as/ternary.ts.snap @@ -53,18 +53,7 @@ bifornCringerMoshedPerplexSawder = ```diff --- Prettier +++ Rome -@@ -29,22 +29,17 @@ - } - - function foo() { -- void (( -- coooooooooooooooooooooooooooooooooooooooooooooooooooond -- ? baaaaaaaaaaaaaaaaaaaaar -- : baaaaaaaaaaaaaaaaaaaaaz -- ) as Fooooooooooo); -+ void ((coooooooooooooooooooooooooooooooooooooooooooooooooooond -+ ? baaaaaaaaaaaaaaaaaaaaar -+ : baaaaaaaaaaaaaaaaaaaaaz) as Fooooooooooo); +@@ -37,14 +37,11 @@ } bifornCringerMoshedPerplexSawder = @@ -117,9 +106,11 @@ function foo() { } function foo() { - void ((coooooooooooooooooooooooooooooooooooooooooooooooooooond - ? baaaaaaaaaaaaaaaaaaaaar - : baaaaaaaaaaaaaaaaaaaaaz) as Fooooooooooo); + void (( + coooooooooooooooooooooooooooooooooooooooooooooooooooond + ? baaaaaaaaaaaaaaaaaaaaar + : baaaaaaaaaaaaaaaaaaaaaz + ) as Fooooooooooo); } bifornCringerMoshedPerplexSawder = @@ -136,6 +127,6 @@ bifornCringerMoshedPerplexSawder = # Lines exceeding max width of 80 characters ``` - 43: askTrovenaBeenaDependsRowans + ((glimseGlyphsHazardNoopsTieTie === 0 && kochabCooieGameOnOboleUnweave === Math.PI + 45: askTrovenaBeenaDependsRowans + ((glimseGlyphsHazardNoopsTieTie === 0 && kochabCooieGameOnOboleUnweave === Math.PI ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap index 17f969157697..851a77fc7c14 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap @@ -151,31 +151,37 @@ function x() { ]; const testArrayLiteral2 = < -@@ -112,8 +108,8 @@ +@@ -111,9 +107,9 @@ + const fitsObjLiteral = | undefined>{ a: "test" }; const fitsArrayLiteral = | undefined>["t1", "t2"]; - const breakAfterCast = | undefined>( +- const breakAfterCast = | undefined>( - (permissions)[receiverType] - ); -+ permissions -+ )[receiverType]; ++ const breakAfterCast = | undefined>(< ++ any ++ >permissions)[receiverType]; const stillTooLong = < PermissionsChecker | undefined | number | string | boolean -@@ -130,10 +126,8 @@ +@@ -129,11 +125,11 @@ + | never >(permissions)[receiverType]; - const stillTooLong3 = | undefined>( +- const stillTooLong3 = | undefined>( - (permissions.someMethodWithLongName(parameter1, parameter2))[ - receiverTypeLongName - ] - ); -+ permissions.someMethodWithLongName(parameter1, parameter2) -+ )[receiverTypeLongName]; ++ const stillTooLong3 = | undefined>(< ++ any ++ >permissions.someMethodWithLongName(parameter1, parameter2))[ ++ receiverTypeLongName ++ ]; const stillTooLong4 = < | PermissionsChecker -@@ -163,9 +157,7 @@ +@@ -163,9 +159,7 @@ >{ prop1: "myPropVal" }; const testArrayLiteral = | undefined>[ @@ -300,9 +306,9 @@ function x() { const fitsObjLiteral = | undefined>{ a: "test" }; const fitsArrayLiteral = | undefined>["t1", "t2"]; - const breakAfterCast = | undefined>( - permissions - )[receiverType]; + const breakAfterCast = | undefined>(< + any + >permissions)[receiverType]; const stillTooLong = < PermissionsChecker | undefined | number | string | boolean @@ -318,9 +324,11 @@ function x() { | never >(permissions)[receiverType]; - const stillTooLong3 = | undefined>( - permissions.someMethodWithLongName(parameter1, parameter2) - )[receiverTypeLongName]; + const stillTooLong3 = | undefined>(< + any + >permissions.someMethodWithLongName(parameter1, parameter2))[ + receiverTypeLongName + ]; const stillTooLong4 = < | PermissionsChecker diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castOfAwait.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castOfAwait.ts.snap deleted file mode 100644 index 5424332c2555..000000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castOfAwait.ts.snap +++ /dev/null @@ -1,51 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs -assertion_line: 271 -info: - test_file: typescript/compiler/castOfAwait.ts ---- - -# Input - -```js -// @target: es6 -async function f() { - await 0; - typeof await 0; - void await 0; - await void typeof void await 0; - await await 0; -} -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -3,6 +3,6 @@ - await 0; - typeof (await 0); - void (await 0); -- await void (typeof (void (await 0))); -+ await void typeof void (await 0); - await await 0; - } -``` - -# Output - -```js -// @target: es6 -async function f() { - await 0; - typeof (await 0); - void (await 0); - await void typeof void (await 0); - await await 0; -} -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castParentheses.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castParentheses.ts.snap index adae8e026c9b..9278d0afc754 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castParentheses.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castParentheses.ts.snap @@ -24,22 +24,12 @@ var b = (new a).b ```diff --- Prettier +++ Rome -@@ -1,11 +1,11 @@ +@@ -1,4 +1,4 @@ -class a { +class a { static b: any; } --var b = a; -+var b = (a); - var b = (a).b; - var b = (a.b).c; - var b = (a.b()).c; --var b = new a(); --var b = new a.b(); -+var b = (new a()); -+var b = (new a.b()); - var b = (new a()).b; ``` # Output @@ -49,12 +39,12 @@ class a { static b: any; } -var b = (a); +var b = a; var b = (a).b; var b = (a.b).c; var b = (a.b()).c; -var b = (new a()); -var b = (new a.b()); +var b = new a(); +var b = new a.b(); var b = (new a()).b; ``` diff --git a/crates/rome_js_formatter/tests/specs/ts/parenthesis.ts.snap b/crates/rome_js_formatter/tests/specs/ts/parenthesis.ts.snap index 0f3c55647993..5b903110bdeb 100644 --- a/crates/rome_js_formatter/tests/specs/ts/parenthesis.ts.snap +++ b/crates/rome_js_formatter/tests/specs/ts/parenthesis.ts.snap @@ -15,6 +15,6 @@ Line width: 80 Quote style: Double Quotes ----- const a = (c && b) as boolean; -const a = (c && b) as boolean; +const a = ((c && b)) as boolean; const a = !(c && b) as boolean; diff --git a/crates/rome_js_syntax/src/expr_ext.rs b/crates/rome_js_syntax/src/expr_ext.rs index 4fd889f51bc4..ae813315a704 100644 --- a/crates/rome_js_syntax/src/expr_ext.rs +++ b/crates/rome_js_syntax/src/expr_ext.rs @@ -447,31 +447,73 @@ impl JsRegexLiteralExpression { } impl JsStaticMemberExpression { + /// Returns `true` if this is an optional member access + /// + /// ```javascript + /// a.b -> false, + /// a?.b -> true + /// a?.[b][c][d].e -> false + /// ``` pub fn is_optional(&self) -> bool { self.operator_token() .map_or(false, |token| token.kind() == JsSyntaxKind::QUESTIONDOT) } + /// Returns true if this member has an optional token or any member expression on the left side. + /// + /// ```javascript + /// a.b -> false + /// a?.b-> true + /// a?.[b][c][d].e -> true + /// ``` pub fn is_optional_chain(&self) -> bool { is_optional_chain(self.clone().into()) } } impl JsComputedMemberExpression { + /// Returns `true` if this is an optional member access + /// + /// ```javascript + /// a[b] -> false, + /// a?.[b] -> true + /// a?.b.c.d[e] -> false + /// ``` pub fn is_optional(&self) -> bool { self.optional_chain_token().is_some() } + /// Returns true if this member has an optional token or any member expression on the left side. + /// + /// ```javascript + /// a[b] -> false + /// a?.[b]-> true + /// a?.b.c.d[e] -> true + /// ``` pub fn is_optional_chain(&self) -> bool { is_optional_chain(self.clone().into()) } } impl JsCallExpression { + /// Returns `true` if this is an optional member access + /// + /// ```javascript + /// a() -> false, + /// a?.() -> true + /// a?.b() -> false + /// ``` pub fn is_optional(&self) -> bool { self.optional_chain_token().is_some() } + /// Returns true if this member has an optional token or any member expression on the left side. + /// + /// ```javascript + /// a() -> false + /// a?.()-> true + /// a?.b.c.d() -> true + /// ``` pub fn is_optional_chain(&self) -> bool { is_optional_chain(self.clone().into()) }