From cfc3531e5d4710ee331390913bcb4fbf01a756c5 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 | 17 +- .../src/js/expressions/array_expression.rs | 4 + .../expressions/arrow_function_expression.rs | 14 +- .../js/expressions/assignment_expression.rs | 53 +-- .../src/js/expressions/await_expression.rs | 44 ++- .../src/js/expressions/binary_expression.rs | 69 +++- .../src/js/expressions/call_arguments.rs | 106 +++--- .../src/js/expressions/call_expression.rs | 6 +- .../expressions/computed_member_expression.rs | 1 + .../js/expressions/conditional_expression.rs | 6 +- .../js/expressions/import_call_expression.rs | 13 +- .../src/js/expressions/in_expression.rs | 115 +++++- .../js/expressions/instanceof_expression.rs | 61 ++- .../src/js/expressions/logical_expression.rs | 64 +++- .../src/js/expressions/new_expression.rs | 13 +- .../expressions/parenthesized_expression.rs | 136 +++---- .../js/expressions/post_update_expression.rs | 4 +- .../js/expressions/pre_update_expression.rs | 4 +- .../src/js/expressions/sequence_expression.rs | 34 +- .../expressions/static_member_expression.rs | 79 +++- .../src/js/expressions/unary_expression.rs | 4 +- .../src/js/module/import_meta.rs | 17 +- .../src/js/unknown/unknown_expression.rs | 20 +- .../src/jsx/expressions/tag_expression.rs | 23 -- crates/rome_js_formatter/src/lib.rs | 56 ++- crates/rome_js_formatter/src/parentheses.rs | 356 ++++++++++++++---- .../src/ts/expressions/as_expression.rs | 48 ++- .../expressions/type_assertion_expression.rs | 72 +++- .../src/utils/assignment_like.rs | 73 ++-- .../src/utils/binary_like_expression.rs | 229 +++++++---- .../src/utils/conditional.rs | 4 +- .../src/utils/member_chain/chain_member.rs | 19 +- .../src/utils/member_chain/groups.rs | 2 +- .../src/utils/member_chain/mod.rs | 6 +- crates/rome_js_formatter/src/utils/mod.rs | 7 +- .../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 +++ 54 files changed, 1513 insertions(+), 1092 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 71c91ebf688..7fb772fbf97 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::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::NewTarget; use rome_js_syntax::NewTargetFields; +use rome_js_syntax::{JsSyntaxNode, NewTarget}; #[derive(Debug, Clone, Default)] pub struct FormatNewTarget; @@ -24,4 +25,18 @@ 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 + } } 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 349254f990f..2bbecb2e065 100644 --- a/crates/rome_js_formatter/src/js/expressions/array_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/array_expression.rs @@ -28,6 +28,10 @@ impl FormatNodeRule for FormatJsArrayExpression { ] ) } + + fn needs_parentheses(&self, item: &JsArrayExpression) -> bool { + item.needs_parentheses() + } } impl NeedsParentheses for JsArrayExpression { 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 21a0d589a54..180baef0073 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,8 +2,8 @@ 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, + is_binary_like_left_or_right, is_conditional_test, + update_or_lower_expression_needs_parentheses, NeedsParentheses, }; use crate::utils::{ resolve_expression, resolve_left_most_expression, JsAnyBinaryLikeLeftExpression, @@ -105,14 +105,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, }, @@ -178,7 +170,7 @@ 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) } } 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 29b5634318f..3005383c981 100644 --- a/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs @@ -3,10 +3,12 @@ use crate::utils::{resolve_expression_syntax, JsAnyAssignmentLike}; use crate::parentheses::{is_first_in_statement, FirstInStatementMode, NeedsParentheses}; use rome_formatter::write; + use rome_js_syntax::{ JsAnyAssignmentPattern, JsAnyForInitializer, JsAnyFunctionBody, JsArrowFunctionExpression, - JsAssignmentExpression, JsForStatement, JsParenthesizedExpression, JsSyntaxKind, JsSyntaxNode, + JsAssignmentExpression, JsForStatement, JsSyntaxKind, JsSyntaxNode, }; +use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] pub struct FormatJsAssignmentExpression; @@ -23,11 +25,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]` @@ -70,23 +67,35 @@ 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, } @@ -110,6 +119,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 56e8a2b5273..7e350899062 100644 --- a/crates/rome_js_formatter/src/js/expressions/await_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/await_expression.rs @@ -2,8 +2,8 @@ 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, + resolve_expression_parent, update_or_lower_expression_needs_parentheses, NeedsParentheses, }; use rome_js_syntax::{JsAwaitExpression, JsSyntaxNode}; @@ -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 = resolve_expression_parent(node.syntax()); + + 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,12 +63,19 @@ 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) } } 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 7cb50929cc4..3cd07f2173e 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::NeedsParentheses; +use rome_js_syntax::{JsBinaryExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsBinaryExpression; @@ -18,3 +21,65 @@ 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) + } +} + +#[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/call_arguments.rs b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs index aa6a4becd42..ee68609a633 100644 --- a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs +++ b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs @@ -105,20 +105,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 +165,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 +205,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 +221,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 +261,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 +283,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 +297,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 @@ -381,14 +374,23 @@ 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(resolve_expression(expression.clone())) + } + }; + + 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 +411,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 @@ -531,7 +531,7 @@ fn is_framework_test_call(payload: IsTestFrameworkCallPayload) -> SyntaxResult resolve_call_argument_expression(&argument), + Ok(argument) => resolve_call_argument_expression(argument), _ => None, }); @@ -679,8 +679,10 @@ fn matches_test_call(callee: &JsAnyExpression) -> SyntaxResult { let value_token = identifier.name()?.value_token()?; @@ -699,6 +701,7 @@ fn matches_test_call(callee: &JsAnyExpression) -> SyntaxResult { + i -= 1; // Don't increment the depth parenthesized.expression()? } _ => break, @@ -771,6 +774,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 b17b07a8214..3d1d3560d68 100644 --- a/crates/rome_js_formatter/src/js/expressions/call_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/call_expression.rs @@ -21,11 +21,7 @@ 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, - - _ => false, - } + matches!(parent.kind(), JsSyntaxKind::JS_NEW_EXPRESSION) } } 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 d28c53441c7..efd3170b389 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 @@ -69,6 +69,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 fef5cbf04e3..9dfe44eae5e 100644 --- a/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs @@ -2,8 +2,8 @@ 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, NeedsParentheses, }; use rome_js_syntax::{JsConditionalExpression, JsSyntaxKind, JsSyntaxNode}; @@ -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) } 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 0aea1188262..a277f6eb54b 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::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::JsImportCallExpression; use rome_js_syntax::JsImportCallExpressionFields; +use rome_js_syntax::{JsImportCallExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsImportCallExpression; @@ -16,4 +17,14 @@ 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) + } } 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 8aa7575ad88..d78eaccd0c0 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,12 @@ 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::NeedsParentheses; + +use rome_js_syntax::{JsAnyStatement, JsForStatement, JsInExpression, JsSyntaxNode}; +use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] pub struct FormatJsInExpression; @@ -14,3 +19,109 @@ 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) + } +} + +/// 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 a9e6e2d4e32..076658070d0 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::NeedsParentheses; +use rome_js_syntax::{JsInstanceofExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsInstanceofExpression; @@ -18,3 +21,57 @@ 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) + } +} + +#[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 aec16bf932e..e1288b7dca1 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::NeedsParentheses; +use rome_js_syntax::{JsLogicalExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsLogicalExpression; @@ -18,3 +21,60 @@ 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) + } +} + +#[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 a4f323f3e04..cc95edbf40e 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::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::JsNewExpressionFields; use rome_js_syntax::{JsNewExpression, JsSyntaxKind}; +use rome_js_syntax::{JsNewExpressionFields, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsNewExpression; @@ -41,4 +42,14 @@ 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) + } } 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 e22370002a2..b8cfbf2e437 100644 --- a/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs @@ -7,8 +7,10 @@ use rome_formatter::write; use crate::utils::JsAnyBinaryLikeExpression; +use crate::parentheses::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(_) ) } } @@ -181,16 +138,6 @@ 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 parent_precedence = FormatPrecedence::with_precedence_for_parenthesis(parent.as_ref()); if parent_precedence > FormatPrecedence::Low { @@ -198,19 +145,22 @@ 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 { + fn needs_parentheses(&self) -> bool { + false + } + + fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { + false + } +} 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 f943a3caf3e..67a77ac50ea 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,7 +1,7 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{update_expression_needs_parentheses, NeedsParentheses}; +use crate::parentheses::{unary_like_expression_needs_parentheses, NeedsParentheses}; use rome_js_syntax::JsPostUpdateExpressionFields; use rome_js_syntax::{JsPostUpdateExpression, JsSyntaxNode}; @@ -25,7 +25,7 @@ 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) } } 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 77742b51eba..9e1909981b1 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,4 +1,4 @@ -use crate::parentheses::{update_expression_needs_parentheses, NeedsParentheses}; +use crate::parentheses::{unary_like_expression_needs_parentheses, NeedsParentheses}; use crate::prelude::*; use rome_formatter::write; @@ -38,7 +38,7 @@ 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), } } } 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 dc3167857a9..f1c46e32a11 100644 --- a/crates/rome_js_formatter/src/js/expressions/sequence_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/sequence_expression.rs @@ -76,16 +76,32 @@ 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 + ) + } +} + +#[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 a75c890a304..2d0fbec0295 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::{resolve_expression_parent, NeedsParentheses}; +use crate::utils::{resolve_expression, 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