From 552beabfdf5fd4175c7b9babf52828a7db56c472 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 12 Aug 2022 13:44:09 +0200 Subject: [PATCH] feat(rome_js_formatter): parenthesise TsTypeAssertionExpression --- .../src/js/auxiliary/new_target.rs | 30 +- .../src/js/expressions/array_expression.rs | 22 +- .../expressions/arrow_function_expression.rs | 34 +- .../js/expressions/assignment_expression.rs | 79 ++- .../src/js/expressions/await_expression.rs | 58 +- .../expressions/big_int_literal_expression.rs | 18 +- .../src/js/expressions/binary_expression.rs | 81 ++- .../expressions/boolean_literal_expression.rs | 18 +- .../src/js/expressions/call_arguments.rs | 147 ++--- .../src/js/expressions/call_expression.rs | 20 +- .../src/js/expressions/class_expression.rs | 16 +- .../expressions/computed_member_expression.rs | 17 +- .../js/expressions/conditional_expression.rs | 20 +- .../src/js/expressions/function_expression.rs | 17 +- .../js/expressions/identifier_expression.rs | 18 +- .../js/expressions/import_call_expression.rs | 27 +- .../src/js/expressions/in_expression.rs | 129 +++- .../js/expressions/instanceof_expression.rs | 73 ++- .../src/js/expressions/logical_expression.rs | 76 ++- .../src/js/expressions/new_expression.rs | 27 +- .../js/expressions/null_literal_expression.rs | 18 +- .../expressions/number_literal_expression.rs | 16 +- .../src/js/expressions/object_expression.rs | 18 +- .../expressions/parenthesized_expression.rs | 156 ++--- .../js/expressions/post_update_expression.rs | 20 +- .../js/expressions/pre_update_expression.rs | 21 +- .../expressions/regex_literal_expression.rs | 18 +- .../src/js/expressions/sequence_expression.rs | 50 +- .../expressions/static_member_expression.rs | 91 ++- .../expressions/string_literal_expression.rs | 16 +- .../src/js/expressions/super_expression.rs | 18 +- .../src/js/expressions/template.rs | 16 +- .../src/js/expressions/this_expression.rs | 18 +- .../src/js/expressions/unary_expression.rs | 20 +- .../src/js/expressions/yield_expression.rs | 16 +- .../src/js/module/import_meta.rs | 31 +- .../src/js/unknown/unknown_expression.rs | 34 +- .../src/jsx/expressions/tag_expression.rs | 46 +- crates/rome_js_formatter/src/lib.rs | 56 +- crates/rome_js_formatter/src/parentheses.rs | 552 +++++++++++++++--- .../src/ts/expressions/as_expression.rs | 61 +- .../non_null_assertion_expression.rs | 16 +- .../expressions/type_assertion_expression.rs | 86 ++- .../src/utils/assignment_like.rs | 79 +-- .../src/utils/binary_like_expression.rs | 232 +++++--- .../src/utils/conditional.rs | 66 +-- crates/rome_js_formatter/src/utils/jsx.rs | 4 +- .../src/utils/member_chain/chain_member.rs | 19 +- .../src/utils/member_chain/groups.rs | 2 +- .../src/utils/member_chain/mod.rs | 15 +- crates/rome_js_formatter/src/utils/mod.rs | 9 +- .../declarations/variable_declaration.js.snap | 135 +++-- .../prettier/js/assignment/chain.js.snap | 27 +- .../prettier/js/assignment/issue-4094.js.snap | 39 -- .../prettier/js/async/inline-await.js.snap | 49 -- .../specs/prettier/js/async/nested.js.snap | 50 -- .../prettier/js/export-default/body.js.snap | 29 + .../js/new-expression/new_expression.js.snap | 50 -- .../prettier/js/objects/expression.js.snap | 4 +- .../prettier/js/sequence-break/break.js.snap | 108 ---- .../prettier/js/template/parenthesis.js.snap | 19 +- .../js/unary-expression/comments.js.snap | 49 +- .../typescript/as/nested-await-and-as.ts.snap | 47 -- .../prettier/typescript/as/ternary.ts.snap | 23 +- .../typescript/assignment/lone-arg.ts.snap | 46 -- .../typescript/cast/generic-cast.ts.snap | 38 +- .../typescript/compiler/castOfAwait.ts.snap | 51 -- .../compiler/castParentheses.ts.snap | 18 +- .../tests/specs/ts/parenthesis.ts.snap | 2 +- crates/rome_js_syntax/src/expr_ext.rs | 42 ++ 70 files changed, 2333 insertions(+), 1240 deletions(-) delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/assignment/issue-4094.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/async/inline-await.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/async/nested.js.snap create mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/export-default/body.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/new-expression/new_expression.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/sequence-break/break.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/as/nested-await-and-as.ts.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/assignment/lone-arg.ts.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/compiler/castOfAwait.ts.snap 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 21a0d589a549..18777a2c4818 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, JsArrowFunctionExpression, JsArrowFunctionExpressionFields, JsSyntaxKind, JsSyntaxNode, @@ -87,7 +85,7 @@ 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(_) @@ -105,14 +103,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, }, @@ -123,7 +113,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!( @@ -178,13 +168,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 d28c53441c78..74c79f47e272 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,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::{format_args, write}; -use rome_js_syntax::{JsComputedMemberExpression, JsSyntaxNode}; +use rome_js_syntax::{JsAnyExpression, JsComputedMemberExpression, JsSyntaxNode}; use rome_js_syntax::{JsComputedMemberExpressionFields, JsSyntaxKind}; #[derive(Debug, Clone, Default)] @@ -53,6 +53,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 { @@ -69,6 +81,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 a75c890a3040..23630070ed71 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,16 +1,34 @@ use crate::prelude::*; -use crate::parentheses::NeedsParentheses; +use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::utils::MemberChainLabel; use rome_formatter::{format_args, write}; use rome_js_syntax::{ - JsAnyExpression, JsAssignmentExpression, JsStaticMemberExpression, - JsStaticMemberExpressionFields, JsSyntaxKind, JsSyntaxNode, JsVariableDeclarator, + JsAnyExpression, JsAssignmentExpression, JsInitializerClause, JsStaticMemberExpression, + JsStaticMemberExpressionFields, JsSyntaxKind, JsSyntaxNode, }; use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] pub struct FormatJsStaticMemberExpression; +struct MemberLabel; + +#[derive(Copy, Clone, Debug)] +enum Label { + Member, + MemberChain, +} + +impl Label { + fn label_id(&self) -> LabelId { + match self { + Label::Member => LabelId::of::(), + Label::MemberChain => LabelId::of::(), + } + } +} + impl FormatNodeRule for FormatJsStaticMemberExpression { fn fmt_fields(&self, node: &JsStaticMemberExpression, f: &mut JsFormatter) -> FormatResult<()> { let JsStaticMemberExpressionFields { @@ -19,11 +37,24 @@ impl FormatNodeRule for FormatJsStaticMemberExpression member, } = node.as_fields(); - write!(f, [object.format()])?; + let mut object_label: Option