From 72cad1a2c7bd5ec35173a5fc4cc028cf33ce3a9c Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 19 Aug 2022 18:28:48 +0200 Subject: [PATCH 01/29] feat(rome_js_formatter): Add parentheses pre-processing step --- crates/rome_formatter/src/comments.rs | 11 +- crates/rome_formatter/src/lib.rs | 103 +++--- crates/rome_formatter/src/token.rs | 4 + .../src/js/auxiliary/new_target.rs | 15 +- .../src/js/classes/extends_clause.rs | 4 +- .../src/js/expressions/array_expression.rs | 16 +- .../expressions/arrow_function_expression.rs | 23 +- .../js/expressions/assignment_expression.rs | 27 +- .../src/js/expressions/await_expression.rs | 18 +- .../expressions/big_int_literal_expression.rs | 16 +- .../src/js/expressions/binary_expression.rs | 16 +- .../expressions/boolean_literal_expression.rs | 16 +- .../src/js/expressions/call_arguments.rs | 32 +- .../src/js/expressions/call_expression.rs | 16 +- .../src/js/expressions/class_expression.rs | 16 +- .../expressions/computed_member_expression.rs | 14 +- .../js/expressions/conditional_expression.rs | 16 +- .../src/js/expressions/function_expression.rs | 17 +- .../js/expressions/identifier_expression.rs | 16 +- .../js/expressions/import_call_expression.rs | 16 +- .../src/js/expressions/in_expression.rs | 16 +- .../js/expressions/instanceof_expression.rs | 16 +- .../src/js/expressions/logical_expression.rs | 16 +- .../src/js/expressions/new_expression.rs | 16 +- .../js/expressions/null_literal_expression.rs | 16 +- .../expressions/number_literal_expression.rs | 16 +- .../src/js/expressions/object_expression.rs | 18 +- .../expressions/parenthesized_expression.rs | 22 +- .../js/expressions/post_update_expression.rs | 18 +- .../js/expressions/pre_update_expression.rs | 18 +- .../expressions/regex_literal_expression.rs | 16 +- .../src/js/expressions/sequence_expression.rs | 20 +- .../expressions/static_member_expression.rs | 24 +- .../expressions/string_literal_expression.rs | 16 +- .../src/js/expressions/super_expression.rs | 16 +- .../src/js/expressions/template.rs | 14 +- .../src/js/expressions/template_element.rs | 6 +- .../src/js/expressions/this_expression.rs | 16 +- .../src/js/expressions/unary_expression.rs | 18 +- .../src/js/expressions/yield_expression.rs | 16 +- .../src/js/lists/template_element_list.rs | 1 - .../src/js/module/import_meta.rs | 16 +- .../src/js/statements/return_statement.rs | 13 +- .../src/js/unknown/unknown_expression.rs | 25 +- .../src/jsx/expressions/tag_expression.rs | 23 +- crates/rome_js_formatter/src/lib.rs | 230 ++++++++++--- crates/rome_js_formatter/src/parentheses.rs | 234 +++---------- crates/rome_js_formatter/src/preprocessor.rs | 315 ++++++++++++++++++ .../src/ts/expressions/as_expression.rs | 18 +- .../non_null_assertion_expression.rs | 16 +- .../expressions/type_assertion_expression.rs | 18 +- .../src/utils/assignment_like.rs | 24 +- .../src/utils/binary_like_expression.rs | 153 +++------ .../src/utils/conditional.rs | 79 +---- crates/rome_js_formatter/src/utils/jsx.rs | 14 +- .../src/utils/member_chain/chain_member.rs | 143 +------- .../src/utils/member_chain/groups.rs | 76 ++--- .../src/utils/member_chain/mod.rs | 57 ++-- .../binary-expr.js.snap | 30 -- .../extra-spaces-and-asterisks.js.snap | 57 ---- .../superclass.js.snap | 29 -- .../ways-to-specify-type.js.snap | 75 ----- .../binary_and_template.js.snap | 29 -- .../js/export-default/class_instance.js.snap | 29 -- .../object-property-ignore/issue-5678.js.snap | 137 -------- .../typescript/as/export_default_as.ts.snap | 29 -- .../export-default/function_as.ts.snap | 29 -- 67 files changed, 869 insertions(+), 1776 deletions(-) create mode 100644 crates/rome_js_formatter/src/preprocessor.rs delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/binary-expr.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/extra-spaces-and-asterisks.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/superclass.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/ways-to-specify-type.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/export-default/binary_and_template.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/export-default/class_instance.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/object-property-ignore/issue-5678.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/as/export_default_as.ts.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/export-default/function_as.ts.snap diff --git a/crates/rome_formatter/src/comments.rs b/crates/rome_formatter/src/comments.rs index 0e37efb1b26..b675228a13a 100644 --- a/crates/rome_formatter/src/comments.rs +++ b/crates/rome_formatter/src/comments.rs @@ -1,4 +1,3 @@ -use crate::CstFormatContext; use rome_rowan::{ Direction, Language, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxTriviaPieceComments, WalkEvent, @@ -158,10 +157,10 @@ pub struct Comments { impl Comments { /// Extracts all the suppressions from `root` and its child nodes. - pub fn from_node(root: &SyntaxNode, context: &Context) -> Self - where - Context: CstFormatContext, - { + pub fn from_node>( + root: &SyntaxNode, + language: &FormatLanguage, + ) -> Self { let mut suppressed_nodes = HashSet::new(); let mut current_node = None; @@ -190,7 +189,7 @@ impl Comments { .pieces() .filter_map(|piece| piece.as_comments()) { - if context.comment_style().is_suppression(comment.text()) { + if language.comment_style().is_suppression(comment.text()) { suppressed_nodes.insert(current_node); break; } diff --git a/crates/rome_formatter/src/lib.rs b/crates/rome_formatter/src/lib.rs index 10e803f6041..2d11c007de8 100644 --- a/crates/rome_formatter/src/lib.rs +++ b/crates/rome_formatter/src/lib.rs @@ -225,6 +225,7 @@ pub trait CstFormatContext: FormatContext { type Style: CommentStyle; /// Customizes how comments are formatted + #[deprecated] fn comment_style(&self) -> Self::Style; /// Returns a ref counted [Comments]. @@ -793,32 +794,63 @@ where }) } +pub trait FormatLanguage { + type SyntaxLanguage: Language; + type Context: CstFormatContext; + type CommentStyle: CommentStyle; + type FormatRule: FormatRule, Context = Self::Context> + Default; + + /// Customizes how comments are formatted + fn comment_style(&self) -> Self::CommentStyle; + + fn transform( + &self, + root: &SyntaxNode, + ) -> SyntaxNode { + root.clone() + } + + /// Because this function is language-agnostic, it must be provided with a + /// `predicate` function that's used to select appropriate "root nodes" for the + /// range formatting process: for instance in JavaScript the predicate returns + /// true for statement and declaration nodes, to ensure the entire statement + /// gets formatted instead of the smallest sub-expression that fits the range + fn is_range_formatting_node(&self, _node: &SyntaxNode) -> bool { + true + } + + fn context(&self) -> &Self::Context; + + fn into_context(self, comments: Comments) -> Self::Context; +} + /// Formats a syntax node file based on its features. /// /// It returns a [Formatted] result, which the user can use to override a file. -pub fn format_node< - Context: CstFormatContext, - N: FormatWithRule>, ->( - context: Context, - root: &N, -) -> FormatResult> { +pub fn format_node( + root: &SyntaxNode, + language: L, +) -> FormatResult> { tracing::trace_span!("format_node").in_scope(move || { - let suppressions = Comments::from_node(root.item(), &context); - let context = context.with_comments(Rc::new(suppressions)); + let root = language.transform(root); + + let comments = Comments::from_node(&root, &language); + let format_node = FormatRefWithRule::new(&root, L::FormatRule::default()); + + let context = language.into_context(comments); let mut state = FormatState::new(context); let mut buffer = VecBuffer::new(&mut state); - write!(&mut buffer, [root])?; + write!(buffer, [format_node])?; let document = buffer.into_element(); - state.assert_formatted_all_tokens(root.item()); + state.assert_formatted_all_tokens(&root); state .context() .comments() - .assert_checked_all_suppressions(root.item()); + .assert_checked_all_suppressions(&root); Ok(Formatted::new(document, state.into_context())) }) @@ -864,12 +896,6 @@ where /// Formats a range within a file, supported by Rome /// -/// Because this function is language-agnostic, it must be provided with a -/// `predicate` function that's used to select appropriate "root nodes" for the -/// range formatting process: for instance in JavaScript the predicate returns -/// true for statement and declaration nodes, to ensure the entire statement -/// gets formatted instead of the smallest sub-expression that fits the range -/// /// This runs a simple heuristic to determine the initial indentation /// level of the node based on the provided [FormatContext], which /// must match currently the current initial of the file. Additionally, @@ -879,17 +905,11 @@ where /// /// It returns a [Formatted] result with a range corresponding to the /// range of the input that was effectively overwritten by the formatter -pub fn format_range( - context: Context, - root: &SyntaxNode, +pub fn format_range( + root: &SyntaxNode, mut range: TextRange, - mut predicate: P, -) -> FormatResult -where - Context: CstFormatContext, - R: FormatRule, Context = Context> + Default, - P: FnMut(&SyntaxNode) -> bool, -{ + language: Language, +) -> FormatResult { if range.is_empty() { return Ok(Printed::new( String::new(), @@ -980,11 +1000,11 @@ where // the language implementation) in the ancestors of the start and end tokens let start_node = start_token .ancestors() - .find(&mut predicate) + .find(|node| language.is_range_formatting_node(node)) .unwrap_or_else(|| root.clone()); let end_node = end_token .ancestors() - .find(predicate) + .find(|node| language.is_range_formatting_node(node)) .unwrap_or_else(|| root.clone()); let common_root = if start_node == end_node { @@ -1037,10 +1057,7 @@ where // Perform the actual formatting of the root node with // an appropriate indentation level - let mut printed = format_sub_tree( - context, - &FormatRefWithRule::<_, R>::new(common_root, R::default()), - )?; + let mut printed = format_sub_tree(common_root, language)?; // This finds the closest marker to the beginning of the source // starting before or at said starting point, and the closest @@ -1119,17 +1136,13 @@ where /// even if it's a mismatch from the rest of the block the selection is in /// /// It returns a [Formatted] result -pub fn format_sub_tree< - C: CstFormatContext, - N: FormatWithRule>, ->( - context: C, - root: &N, +pub fn format_sub_tree( + root: &SyntaxNode, + language: L, ) -> FormatResult { - let syntax = root.item(); // Determine the initial indentation level for the printer by inspecting the trivia pieces // of each token from the first token of the common root towards the start of the file - let mut tokens = std::iter::successors(syntax.first_token(), |token| token.prev_token()); + let mut tokens = std::iter::successors(root.first_token(), |token| token.prev_token()); // From the iterator of tokens, build an iterator of trivia pieces (once again the iterator is // reversed, starting from the last trailing trivia towards the first leading trivia). @@ -1165,7 +1178,7 @@ pub fn format_sub_tree< // of indentation type detection yet. Unfortunately this // may not actually match the current content of the file let length = trivia.text().len() as u16; - match context.indent_style() { + match language.context().indent_style() { IndentStyle::Tab => length, IndentStyle::Space(width) => length / u16::from(width), } @@ -1175,13 +1188,13 @@ pub fn format_sub_tree< None => 0, }; - let formatted = format_node(context, root)?; + let formatted = format_node(root, language)?; let mut printed = formatted.print_with_indent(initial_indent); let sourcemap = printed.take_sourcemap(); let verbatim_ranges = printed.take_verbatim_ranges(); Ok(Printed::new( printed.into_code(), - Some(syntax.text_range()), + Some(root.text_range()), sourcemap, verbatim_ranges, )) diff --git a/crates/rome_formatter/src/token.rs b/crates/rome_formatter/src/token.rs index a74bd806a90..c4be014663e 100644 --- a/crates/rome_formatter/src/token.rs +++ b/crates/rome_formatter/src/token.rs @@ -78,6 +78,7 @@ where // Insert a space if the previous token has any trailing comments and this is not a group // end token + #[allow(deprecated)] if is_last_content_inline_content && !f.context().comment_style().is_group_end_token(token_kind) { space().fmt(f)?; @@ -676,6 +677,7 @@ where TriviaPrintMode::Trim => 0, }); + #[allow(deprecated)] let comment_kind = f .context() .comment_style() @@ -789,6 +791,7 @@ where continue; } + #[allow(deprecated)] let kind = f .context() .comment_style() @@ -800,6 +803,7 @@ where if !is_single_line { match self.token_kind { // Don't write a space if this is a group start token and it isn't the first trailing comment + #[allow(deprecated)] Some(token) if f.context().comment_style().is_group_start_token(token) && index == 0 => {} 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 4305a61bf61..34e9f3d48c2 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,8 @@ use crate::prelude::*; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, NewTargetFields}; +use rome_js_syntax::{NewTargetFields}; use rome_js_syntax::{JsSyntaxNode, NewTarget}; #[derive(Debug, Clone, Default)] @@ -40,14 +40,3 @@ impl NeedsParentheses for NewTarget { 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/classes/extends_clause.rs b/crates/rome_js_formatter/src/js/classes/extends_clause.rs index 8d37c889ae6..64b00a294a9 100644 --- a/crates/rome_js_formatter/src/js/classes/extends_clause.rs +++ b/crates/rome_js_formatter/src/js/classes/extends_clause.rs @@ -1,6 +1,5 @@ use crate::prelude::*; -use crate::parentheses::resolve_parent; use rome_formatter::{format_args, write}; use rome_js_syntax::JsExtendsClauseFields; use rome_js_syntax::JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION; @@ -31,8 +30,7 @@ impl FormatNodeRule for FormatJsExtendsClause { if node .syntax() - .parent() - .and_then(|node| resolve_parent(&node)) + .grand_parent() .map_or(false, |p| p.kind() == JS_ASSIGNMENT_EXPRESSION) { if super_class.syntax().has_leading_comments() || has_trailing_comments { 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 880c310bcc3..d963034443e 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::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsArrayExpressionFields}; +use rome_js_syntax::{JsArrayExpressionFields}; use rome_js_syntax::{JsArrayExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -44,15 +44,3 @@ impl NeedsParentheses for JsArrayExpression { 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 d4a85e26b72..7fc6fbc101d 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 @@ -3,7 +3,7 @@ use rome_formatter::{format_args, write}; use crate::parentheses::{ is_binary_like_left_or_right, is_conditional_test, - update_or_lower_expression_needs_parentheses, ExpressionNode, NeedsParentheses, + update_or_lower_expression_needs_parentheses, NeedsParentheses, }; use crate::utils::{resolve_left_most_expression, JsAnyBinaryLikeLeftExpression}; use rome_js_syntax::{ @@ -86,13 +86,12 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi // going to get broken anyways. let body_has_soft_line_break = match &body { JsFunctionBody(_) => true, - JsAnyExpression(expr) => match expr.resolve() { + JsAnyExpression(expr) => match expr { JsArrowFunctionExpression(_) | JsArrayExpression(_) | JsObjectExpression(_) - | JsParenthesizedExpression(_) | JsxTagExpression(_) => true, - JsTemplate(template) => is_multiline_template_starting_on_same_line(&template), + JsTemplate(template) => is_multiline_template_starting_on_same_line(template), JsSequenceExpression(_) => { return write!( f, @@ -116,9 +115,7 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi // case and added by the object expression itself let should_add_parens = match &body { JsAnyExpression(expression) => { - let resolved = expression.resolve(); - - let is_conditional = matches!(resolved, JsConditionalExpression(_)); + let is_conditional = matches!(expression, JsConditionalExpression(_)); let are_parentheses_mandatory = matches!( resolve_left_most_expression(expression), JsAnyBinaryLikeLeftExpression::JsAnyExpression( @@ -178,18 +175,6 @@ impl NeedsParentheses for JsArrowFunctionExpression { } } -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 0a1c3ab9690..c65f12249b8 100644 --- a/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/assignment_expression.rs @@ -2,13 +2,12 @@ use crate::prelude::*; use crate::utils::JsAnyAssignmentLike; use crate::parentheses::{ - is_arrow_function_body, is_first_in_statement, ExpressionNode, FirstInStatementMode, - NeedsParentheses, + is_arrow_function_body, is_first_in_statement, FirstInStatementMode, NeedsParentheses, }; use rome_formatter::write; use rome_js_syntax::{ - JsAnyAssignmentPattern, JsAnyExpression, JsAnyForInitializer, JsAssignmentExpression, + JsAnyAssignmentPattern, JsAnyForInitializer, JsAssignmentExpression, JsForStatement, JsSyntaxKind, JsSyntaxNode, }; use rome_rowan::AstNode; @@ -40,15 +39,12 @@ impl NeedsParentheses for JsAssignmentExpression { let for_statement = JsForStatement::unwrap_cast(parent.clone()); let is_initializer = match for_statement.initializer() { Some(JsAnyForInitializer::JsAnyExpression(expression)) => { - &expression.resolve_syntax() == self.syntax() + expression.syntax() == self.syntax() } None | Some(_) => false, }; - let is_update = for_statement - .update() - .map(ExpressionNode::into_resolved_syntax) - .as_ref() + let is_update = for_statement.update().map(AstNode::into_syntax).as_ref() == Some(self.syntax()); !(is_initializer || is_update) @@ -68,8 +64,7 @@ impl NeedsParentheses for JsAssignmentExpression { for ancestor in parent.ancestors().skip(1) { match ancestor.kind() { - JsSyntaxKind::JS_SEQUENCE_EXPRESSION - | JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION => child = ancestor, + JsSyntaxKind::JS_SEQUENCE_EXPRESSION => child = ancestor, JsSyntaxKind::JS_FOR_STATEMENT => { let for_statement = JsForStatement::unwrap_cast(ancestor); @@ -98,18 +93,6 @@ impl NeedsParentheses for JsAssignmentExpression { } } -impl ExpressionNode for JsAssignmentExpression { - #[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/await_expression.rs b/crates/rome_js_formatter/src/js/expressions/await_expression.rs index 1a4d24ae050..466405e2534 100644 --- a/crates/rome_js_formatter/src/js/expressions/await_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/await_expression.rs @@ -3,10 +3,10 @@ use rome_formatter::write; use crate::parentheses::{ is_binary_like_left_or_right, is_callee, is_conditional_test, is_member_object, is_spread, - update_or_lower_expression_needs_parentheses, ExpressionNode, NeedsParentheses, + update_or_lower_expression_needs_parentheses, NeedsParentheses, }; -use rome_js_syntax::{JsAnyExpression, JsAwaitExpression, JsSyntaxNode}; +use rome_js_syntax::{JsAwaitExpression, JsSyntaxNode}; use rome_js_syntax::{JsAwaitExpressionFields, JsSyntaxKind}; #[derive(Debug, Clone, Default)] @@ -22,7 +22,7 @@ impl FormatNodeRule for FormatJsAwaitExpression { let format_inner = format_with(|f| write![f, [await_token.format(), space(), argument.format()]]); - let parent = node.resolve_parent(); + let parent = node.syntax().parent(); if let Some(parent) = parent { if is_callee(node.syntax(), &parent) || is_member_object(node.syntax(), &parent) { @@ -81,18 +81,6 @@ pub(super) fn await_or_yield_needs_parens(parent: &JsSyntaxNode, node: &JsSyntax } } -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 cc4a50c9a9d..72332f83148 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::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsBigIntLiteralExpressionFields}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsBigIntLiteralExpressionFields}; use rome_js_syntax::{JsBigIntLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -40,18 +40,6 @@ 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 { 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 3a8d952d2b1..777ecdd3a38 100644 --- a/crates/rome_js_formatter/src/js/expressions/binary_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/binary_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use crate::utils::{needs_binary_like_parentheses, JsAnyBinaryLikeExpression}; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsBinaryExpression, JsSyntaxNode}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsBinaryExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsBinaryExpression; @@ -27,18 +27,6 @@ impl NeedsParentheses for JsBinaryExpression { } } -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}; 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 883f1869170..7d21ed7b136 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::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsBooleanLiteralExpressionFields}; +use rome_js_syntax::{JsBooleanLiteralExpressionFields}; use rome_js_syntax::{JsBooleanLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -34,15 +34,3 @@ impl NeedsParentheses for JsBooleanLiteralExpression { 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 437c28adec1..dde224a48e9 100644 --- a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs +++ b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs @@ -1,5 +1,4 @@ use crate::builders::{format_close_delimiter, format_open_delimiter}; -use crate::parentheses::ExpressionNode; use crate::prelude::*; use crate::utils::{is_call_like_expression, write_arguments_multi_line}; use rome_formatter::{format_args, write}; @@ -252,7 +251,7 @@ fn should_group_first_argument(list: &JsCallArgumentList) -> SyntaxResult let has_comments = first.syntax().has_comments_direct(); - let is_function_like = match resolve_call_argument_expression(&first) { + let is_function_like = match first.as_js_any_expression() { Some(JsAnyExpression::JsFunctionExpression(_)) => true, Some(JsAnyExpression::JsArrowFunctionExpression(arrow)) => { matches!(arrow.body()?, JsAnyFunctionBody::JsFunctionBody(_)) @@ -260,7 +259,7 @@ fn should_group_first_argument(list: &JsCallArgumentList) -> SyntaxResult _ => false, }; - let (second_arg_is_function_like, can_group) = match resolve_call_argument_expression(&second) { + let (second_arg_is_function_like, can_group) = match second.as_js_any_expression() { Some(second_expression) => { let second_arg_is_function_like = matches!( &second_expression, @@ -322,7 +321,7 @@ fn could_group_expression_argument( argument: &JsAnyExpression, is_arrow_recursion: bool, ) -> SyntaxResult { - let result = match argument.resolve() { + let result = match argument { JsAnyExpression::JsObjectExpression(object_expression) => { object_expression.members().len() > 0 || object_expression @@ -375,7 +374,7 @@ fn could_group_expression_argument( let expression_body = match &body { JsAnyFunctionBody::JsFunctionBody(_) => None, - JsAnyFunctionBody::JsAnyExpression(expression) => Some(expression.resolve()), + JsAnyFunctionBody::JsAnyExpression(expression) => Some(expression), }; let body_is_delimited = matches!(body, JsAnyFunctionBody::JsFunctionBody(_)) @@ -406,7 +405,7 @@ fn could_group_expression_argument( && is_nested_arrow_function && can_group_type && (!is_arrow_recursion - && (is_call_like_expression(&any_expression) + && (is_call_like_expression(any_expression) || matches!( any_expression, JsAnyExpression::JsConditionalExpression(_) @@ -433,7 +432,7 @@ fn is_react_hook_with_deps_array( second_argument: &JsAnyCallArgument, ) -> SyntaxResult { let first_expression = match first_argument { - JsAnyCallArgument::JsAnyExpression(expression) => Some(expression.resolve()), + JsAnyCallArgument::JsAnyExpression(expression) => Some(expression), _ => None, }; @@ -520,13 +519,13 @@ fn is_framework_test_call(payload: IsTestFrameworkCallPayload) -> SyntaxResult resolve_call_argument_expression(argument), + Ok(argument) => argument.as_js_any_expression(), _ => None, }); @@ -578,15 +577,6 @@ fn is_framework_test_call(payload: IsTestFrameworkCallPayload) -> SyntaxResult Option { - match argument { - JsAnyCallArgument::JsAnyExpression(expression) => Some(expression.resolve()), - _ => None, - } -} - /// This function checks if a call expressions has one of the following members: /// - `it` /// - `it.only` @@ -693,10 +683,6 @@ fn matches_test_call(callee: &JsAnyExpression) -> SyntaxResult break, } } - JsAnyExpression::JsParenthesizedExpression(parenthesized) => { - i -= 1; // Don't increment the depth - parenthesized.expression()? - } _ => break, }; } 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 7566246f39d..3d1d3560d68 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::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use crate::utils::get_member_chain; -use rome_js_syntax::{JsAnyExpression, JsCallExpression, JsSyntaxKind, JsSyntaxNode}; +use rome_js_syntax::{JsCallExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsCallExpression; @@ -25,18 +25,6 @@ impl NeedsParentheses for JsCallExpression { } } -impl ExpressionNode for JsCallExpression { - #[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/class_expression.rs b/crates/rome_js_formatter/src/js/expressions/class_expression.rs index 167b372b120..45951b11812 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, ExpressionNode, FirstInStatementMode, NeedsParentheses, + is_callee, is_first_in_statement, FirstInStatementMode, NeedsParentheses, }; -use rome_js_syntax::{JsAnyExpression, JsClassExpression, JsSyntaxNode}; +use rome_js_syntax::{JsClassExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsClassExpression; @@ -29,18 +29,6 @@ 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 c9a84b5eb16..649f6ee37ba 100644 --- a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs @@ -1,7 +1,7 @@ use crate::prelude::*; use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::{format_args, write}; use rome_js_syntax::{ JsAnyExpression, JsAnyLiteralExpression, JsComputedMemberAssignment, @@ -121,18 +121,6 @@ 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 { 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 aecf825431c..9dfe44eae5e 100644 --- a/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/conditional_expression.rs @@ -3,9 +3,9 @@ use crate::utils::JsAnyConditional; use crate::parentheses::{ is_binary_like_left_or_right, is_conditional_test, is_spread, - update_or_lower_expression_needs_parentheses, ExpressionNode, NeedsParentheses, + update_or_lower_expression_needs_parentheses, NeedsParentheses, }; -use rome_js_syntax::{JsAnyExpression, JsConditionalExpression, JsSyntaxKind, JsSyntaxNode}; +use rome_js_syntax::{JsConditionalExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsConditionalExpression; @@ -42,18 +42,6 @@ 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 87c649ab839..34710226385 100644 --- a/crates/rome_js_formatter/src/js/expressions/function_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/function_expression.rs @@ -2,11 +2,10 @@ use crate::prelude::*; use crate::js::declarations::function_declaration::FormatFunction; use crate::parentheses::{ - is_callee, is_first_in_statement, is_tag, ExpressionNode, FirstInStatementMode, - NeedsParentheses, + is_callee, is_first_in_statement, is_tag, FirstInStatementMode, NeedsParentheses, }; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsFunctionExpression, JsSyntaxNode}; +use rome_js_syntax::{JsFunctionExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsFunctionExpression; @@ -32,18 +31,6 @@ 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 c5cd76d023e..63a832bda0b 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::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsIdentifierExpressionFields}; +use rome_js_syntax::{JsIdentifierExpressionFields}; use rome_js_syntax::{JsIdentifierExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -30,15 +30,3 @@ impl NeedsParentheses for JsIdentifierExpression { 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 239748810cb..68bcfac0f50 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,8 @@ use crate::prelude::*; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsImportCallExpressionFields}; +use rome_js_syntax::{JsImportCallExpressionFields}; use rome_js_syntax::{JsImportCallExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -28,15 +28,3 @@ impl NeedsParentheses for JsImportCallExpression { 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 2403ec5e9da..bd296f66052 100644 --- a/crates/rome_js_formatter/src/js/expressions/in_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/in_expression.rs @@ -1,10 +1,10 @@ use crate::prelude::*; use crate::utils::{needs_binary_like_parentheses, JsAnyBinaryLikeExpression}; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_js_syntax::{ - JsAnyExpression, JsAnyStatement, JsForStatement, JsInExpression, JsSyntaxNode, + JsAnyStatement, JsForStatement, JsInExpression, JsSyntaxNode, }; use rome_rowan::AstNode; @@ -31,18 +31,6 @@ impl NeedsParentheses for JsInExpression { } } -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(); 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 1d55c34968d..a7183e8a8c2 100644 --- a/crates/rome_js_formatter/src/js/expressions/instanceof_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/instanceof_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use crate::utils::{needs_binary_like_parentheses, JsAnyBinaryLikeExpression}; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsInstanceofExpression, JsSyntaxNode}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsInstanceofExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsInstanceofExpression; @@ -27,18 +27,6 @@ impl NeedsParentheses for JsInstanceofExpression { } } -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}; 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 2eaa000bd51..22c5561f5f0 100644 --- a/crates/rome_js_formatter/src/js/expressions/logical_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/logical_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use crate::utils::{needs_binary_like_parentheses, JsAnyBinaryLikeExpression}; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsLogicalExpression, JsSyntaxNode}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsLogicalExpression, JsSyntaxNode}; use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] @@ -39,18 +39,6 @@ impl NeedsParentheses for JsLogicalExpression { } } -impl ExpressionNode for JsLogicalExpression { - #[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/new_expression.rs b/crates/rome_js_formatter/src/js/expressions/new_expression.rs index 08fbc328234..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,8 @@ use crate::prelude::*; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsNewExpression, JsSyntaxKind}; +use rome_js_syntax::{JsNewExpression, JsSyntaxKind}; use rome_js_syntax::{JsNewExpressionFields, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -53,15 +53,3 @@ impl NeedsParentheses for JsNewExpression { 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 b98f3037aa6..a6af0d05c94 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::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsNullLiteralExpressionFields}; +use rome_js_syntax::{JsNullLiteralExpressionFields}; use rome_js_syntax::{JsNullLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -30,15 +30,3 @@ impl NeedsParentheses for JsNullLiteralExpression { 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 2138c228a94..8ae0f5d0db7 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, ExpressionNode, NeedsParentheses}; +use crate::parentheses::{is_member_object, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsNumberLiteralExpression}; +use rome_js_syntax::{JsNumberLiteralExpression}; use rome_js_syntax::{JsNumberLiteralExpressionFields, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -31,18 +31,6 @@ 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 5746064a641..0a58a3d8eb7 100644 --- a/crates/rome_js_formatter/src/js/expressions/object_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/object_expression.rs @@ -1,10 +1,8 @@ -use crate::parentheses::{ - is_first_in_statement, ExpressionNode, FirstInStatementMode, NeedsParentheses, -}; +use crate::parentheses::{is_first_in_statement, FirstInStatementMode, NeedsParentheses}; use crate::prelude::*; use crate::utils::JsObjectLike; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsObjectExpression, JsSyntaxKind, JsSyntaxNode}; +use rome_js_syntax::{JsObjectExpression, JsSyntaxKind, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsObjectExpression; @@ -29,18 +27,6 @@ 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 c894357fbfd..f31709c883f 100644 --- a/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs @@ -1,9 +1,9 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_js_syntax::{ - JsAnyExpression, JsParenthesizedExpression, JsParenthesizedExpressionFields, JsSyntaxNode, + JsParenthesizedExpression, JsParenthesizedExpressionFields, JsSyntaxNode, }; #[derive(Debug, Clone, Default)] @@ -26,9 +26,9 @@ impl FormatNodeRule for FormatJsParenthesizedExpressi write!( f, [ - format_removed(&l_paren_token?), + l_paren_token.format(), expression.format(), - format_removed(&r_paren_token?) + r_paren_token.format() ] ) } @@ -49,17 +49,3 @@ impl NeedsParentheses for JsParenthesizedExpression { 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 9a87865f119..d5e1342d717 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,10 +1,8 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{ - unary_like_expression_needs_parentheses, ExpressionNode, NeedsParentheses, -}; -use rome_js_syntax::{JsAnyExpression, JsPostUpdateExpressionFields}; +use crate::parentheses::{unary_like_expression_needs_parentheses, NeedsParentheses}; +use rome_js_syntax::{JsPostUpdateExpressionFields}; use rome_js_syntax::{JsPostUpdateExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -31,18 +29,6 @@ impl NeedsParentheses for JsPostUpdateExpression { } } -impl ExpressionNode for JsPostUpdateExpression { - #[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/pre_update_expression.rs b/crates/rome_js_formatter/src/js/expressions/pre_update_expression.rs index 34b2db5b703..7f7b0cf1dac 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,11 +1,9 @@ -use crate::parentheses::{ - unary_like_expression_needs_parentheses, ExpressionNode, NeedsParentheses, -}; +use crate::parentheses::{unary_like_expression_needs_parentheses, NeedsParentheses}; use crate::prelude::*; use rome_formatter::write; use rome_js_syntax::{ - JsAnyExpression, JsPreUpdateExpression, JsPreUpdateOperator, JsSyntaxNode, JsUnaryExpression, + JsPreUpdateExpression, JsPreUpdateOperator, JsSyntaxNode, JsUnaryExpression, JsUnaryOperator, }; use rome_js_syntax::{JsPreUpdateExpressionFields, JsSyntaxKind}; @@ -46,18 +44,6 @@ impl NeedsParentheses for JsPreUpdateExpression { } } -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 a1174c4bd61..6f3976b15d3 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::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsRegexLiteralExpressionFields}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsRegexLiteralExpressionFields}; use rome_js_syntax::{JsRegexLiteralExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -57,15 +57,3 @@ impl NeedsParentheses for JsRegexLiteralExpression { 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 aa0cf8dab80..20aa63fc01a 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::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::{format_args, write}; -use rome_js_syntax::JsSyntaxKind::{JS_PARENTHESIZED_EXPRESSION, JS_SEQUENCE_EXPRESSION}; +use rome_js_syntax::JsSyntaxKind::JS_SEQUENCE_EXPRESSION; use rome_js_syntax::{ - JsAnyExpression, JsSequenceExpression, JsSequenceExpressionFields, JsSyntaxKind, JsSyntaxNode, + JsSequenceExpression, JsSequenceExpressionFields, JsSyntaxKind, JsSyntaxNode, }; use rome_rowan::AstNode; @@ -26,7 +26,7 @@ impl FormatNodeRule for FormatJsSequenceExpression { for parent in node.syntax().ancestors().skip(1) { if parent.kind() == JS_SEQUENCE_EXPRESSION { is_nested = true; - } else if parent.kind() != JS_PARENTHESIZED_EXPRESSION { + } else { first_non_sequence_or_paren_parent = Some(parent); break; } @@ -89,18 +89,6 @@ impl NeedsParentheses for JsSequenceExpression { } } -impl ExpressionNode for JsSequenceExpression { - #[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/static_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs index 941beca7a67..fd6a202a3eb 100644 --- a/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs @@ -1,12 +1,12 @@ use crate::prelude::*; use crate::js::expressions::computed_member_expression::JsAnyComputedMemberLike; -use crate::parentheses::{resolve_parent, AssignmentNode, ExpressionNode, NeedsParentheses}; +use crate::parentheses::{AssignmentNode, NeedsParentheses}; use rome_formatter::{format_args, write}; use rome_js_syntax::{ JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyName, JsAssignmentExpression, - JsInitializerClause, JsParenthesizedAssignment, JsParenthesizedExpression, - JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, + JsInitializerClause, JsParenthesizedAssignment, JsStaticMemberAssignment, + JsStaticMemberExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, }; use rome_rowan::{declare_node_union, AstNode, SyntaxResult}; @@ -85,8 +85,8 @@ impl JsAnyStaticMemberLike { } fn layout(&self) -> SyntaxResult { - let parent = resolve_parent(self.syntax()); - let object = self.object()?.resolve(); + let parent = self.syntax().parent(); + let object = self.object()?; let is_nested = match &parent { Some(parent) => { @@ -126,7 +126,6 @@ impl JsAnyStaticMemberLike { let first_non_static_member_ancestor = self.syntax().ancestors().find(|parent| { !(JsAnyStaticMemberLike::can_cast(parent.kind()) || JsAnyComputedMemberLike::can_cast(parent.kind()) - || JsParenthesizedExpression::can_cast(parent.kind()) || JsParenthesizedAssignment::can_cast(parent.kind())) }); @@ -176,7 +175,6 @@ pub(crate) fn member_chain_callee_needs_parens( JsComputedMemberExpression(member) => member.object().ok(), JsTemplate(template) => template.tag(), TsNonNullAssertionExpression(assertion) => assertion.expression().ok(), - JsParenthesizedExpression(expression) => expression.expression().ok(), _ => None, }); @@ -186,18 +184,6 @@ pub(crate) fn member_chain_callee_needs_parens( } } -impl ExpressionNode for JsStaticMemberExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} - #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs b/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs index ffd6d6a8a5d..53db631a1f7 100644 --- a/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/string_literal_expression.rs @@ -2,8 +2,8 @@ use crate::prelude::*; use crate::utils::{FormatLiteralStringToken, StringLiteralParentKind}; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsStringLiteralExpressionFields}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsStringLiteralExpressionFields}; use rome_js_syntax::{JsExpressionStatement, JsSyntaxKind}; use rome_js_syntax::{JsStringLiteralExpression, JsSyntaxNode}; use rome_rowan::AstNode; @@ -49,18 +49,6 @@ impl NeedsParentheses for JsStringLiteralExpression { } } -impl ExpressionNode for JsStringLiteralExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(self)) - } -} - #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/super_expression.rs b/crates/rome_js_formatter/src/js/expressions/super_expression.rs index e03e6cd7907..304e9037624 100644 --- a/crates/rome_js_formatter/src/js/expressions/super_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/super_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsSuperExpressionFields}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsSuperExpressionFields}; use rome_js_syntax::{JsSuperExpression, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -30,15 +30,3 @@ impl NeedsParentheses for JsSuperExpression { false } } - -impl ExpressionNode for JsSuperExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} diff --git a/crates/rome_js_formatter/src/js/expressions/template.rs b/crates/rome_js_formatter/src/js/expressions/template.rs index 20c1f8cb9b9..73fd9223daa 100644 --- a/crates/rome_js_formatter/src/js/expressions/template.rs +++ b/crates/rome_js_formatter/src/js/expressions/template.rs @@ -2,7 +2,7 @@ use crate::prelude::*; use rome_formatter::write; use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_js_syntax::{JsAnyExpression, JsSyntaxNode, JsTemplate, TsTemplateLiteralType}; use rome_js_syntax::{JsSyntaxToken, TsTypeArguments}; use rome_rowan::{declare_node_union, SyntaxResult}; @@ -93,15 +93,3 @@ impl NeedsParentheses for JsTemplate { } } } - -impl ExpressionNode for JsTemplate { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} diff --git a/crates/rome_js_formatter/src/js/expressions/template_element.rs b/crates/rome_js_formatter/src/js/expressions/template_element.rs index 75c63c646b4..683ba0049d3 100644 --- a/crates/rome_js_formatter/src/js/expressions/template_element.rs +++ b/crates/rome_js_formatter/src/js/expressions/template_element.rs @@ -4,7 +4,6 @@ use rome_formatter::{format_args, write, FormatContext, FormatRuleWithOptions, V use crate::context::TabWidth; use crate::js::lists::template_element_list::{TemplateElementIndention, TemplateElementLayout}; -use crate::parentheses::ExpressionNode; use rome_js_syntax::{ JsAnyExpression, JsSyntaxNode, JsSyntaxToken, JsTemplateElement, TsTemplateElement, }; @@ -107,10 +106,7 @@ impl Format for FormatTemplateElement { TemplateElementLayout::Fit => { use JsAnyExpression::*; - let expression = self - .element - .expression() - .map(|expression| expression.resolve()); + let expression = self.element.expression(); // It's preferred to break after/before `${` and `}` rather than breaking in the // middle of some expressions. diff --git a/crates/rome_js_formatter/src/js/expressions/this_expression.rs b/crates/rome_js_formatter/src/js/expressions/this_expression.rs index ac6f92c59ca..14bfa88b846 100644 --- a/crates/rome_js_formatter/src/js/expressions/this_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/this_expression.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsThisExpressionFields}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsThisExpressionFields}; use rome_js_syntax::{JsSyntaxNode, JsThisExpression}; #[derive(Debug, Clone, Default)] @@ -29,15 +29,3 @@ impl NeedsParentheses for JsThisExpression { false } } - -impl ExpressionNode for JsThisExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} diff --git a/crates/rome_js_formatter/src/js/expressions/unary_expression.rs b/crates/rome_js_formatter/src/js/expressions/unary_expression.rs index d2673b5bbfa..f5ad35ba091 100644 --- a/crates/rome_js_formatter/src/js/expressions/unary_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/unary_expression.rs @@ -1,11 +1,9 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{ - unary_like_expression_needs_parentheses, ExpressionNode, NeedsParentheses, -}; +use crate::parentheses::{unary_like_expression_needs_parentheses, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsSyntaxNode}; +use rome_js_syntax::{JsSyntaxNode}; use rome_js_syntax::{JsSyntaxKind, JsUnaryExpression}; use rome_js_syntax::{JsUnaryExpressionFields, JsUnaryOperator}; @@ -58,18 +56,6 @@ impl NeedsParentheses for JsUnaryExpression { } } -impl ExpressionNode for JsUnaryExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} - #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/expressions/yield_expression.rs b/crates/rome_js_formatter/src/js/expressions/yield_expression.rs index ba12871d163..e1a8a010e0c 100644 --- a/crates/rome_js_formatter/src/js/expressions/yield_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/yield_expression.rs @@ -2,8 +2,8 @@ use crate::prelude::*; use rome_formatter::write; use crate::js::expressions::await_expression::await_or_yield_needs_parens; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyExpression, JsSyntaxKind, JsYieldExpressionFields}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsSyntaxKind, JsYieldExpressionFields}; use rome_js_syntax::{JsSyntaxNode, JsYieldExpression}; #[derive(Debug, Clone, Default)] @@ -31,18 +31,6 @@ impl NeedsParentheses for JsYieldExpression { } } -impl ExpressionNode for JsYieldExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} - #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/js/lists/template_element_list.rs b/crates/rome_js_formatter/src/js/lists/template_element_list.rs index 18033616fd9..fb8330106fe 100644 --- a/crates/rome_js_formatter/src/js/lists/template_element_list.rs +++ b/crates/rome_js_formatter/src/js/lists/template_element_list.rs @@ -165,7 +165,6 @@ fn is_simple_member_expression(expression: JsAnyExpression) -> SyntaxResult expression.expression()?, JsAnyExpression::JsIdentifierExpression(_) | JsAnyExpression::JsThisExpression(_) => { return Ok(true); } diff --git a/crates/rome_js_formatter/src/js/module/import_meta.rs b/crates/rome_js_formatter/src/js/module/import_meta.rs index eec52142bb6..8765d988595 100644 --- a/crates/rome_js_formatter/src/js/module/import_meta.rs +++ b/crates/rome_js_formatter/src/js/module/import_meta.rs @@ -1,9 +1,9 @@ use crate::prelude::*; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; use rome_js_syntax::{ImportMeta, JsSyntaxNode}; -use rome_js_syntax::{ImportMetaFields, JsAnyExpression}; +use rome_js_syntax::{ImportMetaFields}; #[derive(Debug, Clone, Default)] pub struct FormatImportMeta; @@ -43,15 +43,3 @@ impl NeedsParentheses for ImportMeta { false } } - -impl ExpressionNode for ImportMeta { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} diff --git a/crates/rome_js_formatter/src/js/statements/return_statement.rs b/crates/rome_js_formatter/src/js/statements/return_statement.rs index 1eed0b8e7d0..9f4bea10566 100644 --- a/crates/rome_js_formatter/src/js/statements/return_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/return_statement.rs @@ -64,7 +64,7 @@ impl Format for FormatReturnOrThrowArgument<'_> { last_token.as_ref() )] ) - } else if is_binary_or_sequence_argument(argument)? { + } else if is_binary_or_sequence_argument(argument) { write!( f, [group(&format_args![ @@ -108,14 +108,7 @@ fn has_argument_leading_comments(argument: &JsAnyExpression) -> SyntaxResult SyntaxResult { - if JsSequenceExpression::can_cast(argument.syntax().kind()) +fn is_binary_or_sequence_argument(argument: &JsAnyExpression) -> bool { + JsSequenceExpression::can_cast(argument.syntax().kind()) || JsAnyBinaryLikeExpression::can_cast(argument.syntax().kind()) - { - Ok(true) - } else if let JsAnyExpression::JsParenthesizedExpression(inner) = argument { - is_binary_or_sequence_argument(&inner.expression()?) - } else { - Ok(false) - } } diff --git a/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs b/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs index c00eaedf3bc..fa8b3386e7d 100644 --- a/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs +++ b/crates/rome_js_formatter/src/js/unknown/unknown_expression.rs @@ -1,9 +1,7 @@ use crate::prelude::*; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; -use rome_js_syntax::{ - JsAnyExpression, JsParenthesizedExpression, JsSyntaxNode, JsUnknownExpression, -}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsSyntaxNode, JsUnknownExpression}; use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] @@ -24,26 +22,13 @@ impl FormatNodeRule for FormatJsUnknownExpression { } impl NeedsParentheses for JsUnknownExpression { + #[inline] fn needs_parentheses(&self) -> bool { - // Keep parens if it is parenthesized. - self.syntax().parent().map_or(false, |parent| { - JsParenthesizedExpression::can_cast(parent.kind()) - }) + false } + #[inline] fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool { self.needs_parentheses() } } - -impl ExpressionNode for JsUnknownExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} diff --git a/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs b/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs index a281168cb01..647f7025e4c 100644 --- a/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs +++ b/crates/rome_js_formatter/src/jsx/expressions/tag_expression.rs @@ -1,11 +1,12 @@ -use crate::parentheses::{is_callee, is_tag, ExpressionNode, NeedsParentheses}; +use crate::parentheses::{is_callee, is_tag, NeedsParentheses}; use crate::prelude::*; use crate::utils::jsx::{get_wrap_state, WrapState}; use rome_formatter::{format_args, write}; use rome_js_syntax::{ - JsAnyExpression, JsBinaryExpression, JsBinaryOperator, JsSyntaxKind, JsSyntaxNode, + JsBinaryExpression, JsBinaryOperator, JsSyntaxKind, JsSyntaxNode, JsxTagExpression, }; +use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] pub struct FormatJsxTagExpression; @@ -36,11 +37,7 @@ impl NeedsParentheses for JsxTagExpression { JsSyntaxKind::JS_BINARY_EXPRESSION => { let binary = JsBinaryExpression::unwrap_cast(parent.clone()); - let is_left = binary - .left() - .map(ExpressionNode::into_resolved_syntax) - .as_ref() - == Ok(self.syntax()); + let is_left = binary.left().map(AstNode::into_syntax).as_ref() == Ok(self.syntax()); matches!(binary.operator(), Ok(JsBinaryOperator::LessThan)) && is_left } JsSyntaxKind::TS_AS_EXPRESSION @@ -58,15 +55,3 @@ impl NeedsParentheses for JsxTagExpression { } } } - -impl ExpressionNode for JsxTagExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} diff --git a/crates/rome_js_formatter/src/lib.rs b/crates/rome_js_formatter/src/lib.rs index 50df690fc74..f17d615135c 100644 --- a/crates/rome_js_formatter/src/lib.rs +++ b/crates/rome_js_formatter/src/lib.rs @@ -249,20 +249,22 @@ mod ts; pub mod utils; use rome_formatter::prelude::*; -use rome_formatter::{write, CstFormatContext}; +use rome_formatter::{write, Comments, CstFormatContext, Format, FormatLanguage}; use rome_formatter::{Buffer, FormatOwnedWithRule, FormatRefWithRule, Formatted, Printed}; use rome_js_syntax::{ JsAnyDeclaration, JsAnyStatement, JsLanguage, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, }; -use rome_rowan::AstNode; use rome_rowan::SyntaxResult; use rome_rowan::TextRange; +use rome_rowan::{AstNode, SyntaxNode}; use crate::builders::{format_parenthesize, format_suppressed_node}; -use crate::context::JsFormatContext; +use crate::context::{JsCommentStyle, JsFormatContext}; use crate::cst::FormatJsSyntaxNode; +use crate::preprocessor::preprocess; use std::iter::FusedIterator; use std::marker::PhantomData; +use std::rc::Rc; pub(crate) type JsFormatter<'buf> = Formatter<'buf, JsFormatContext>; @@ -473,6 +475,175 @@ impl IntoFormat for JsSyntaxToken { } } +struct JsFormatLanguage { + context: JsFormatContext, +} +impl JsFormatLanguage { + fn new(context: JsFormatContext) -> Self { + Self { context } + } +} + +impl FormatLanguage for JsFormatLanguage { + type SyntaxLanguage = JsLanguage; + type Context = JsFormatContext; + type CommentStyle = JsCommentStyle; + type FormatRule = FormatJsSyntaxNode; + + fn comment_style(&self) -> Self::CommentStyle { + JsCommentStyle + } + + fn transform( + &self, + root: &SyntaxNode, + ) -> SyntaxNode { + preprocess(root) + + // + // + // for node in root + // .descendants() + // .filter_map(JsParenthesizedExpression::cast) + // { + // match ( + // node.l_paren_token(), + // node.expression(), + // node.r_paren_token(), + // ) { + // (Ok(l_paren_token), Ok(inner), Ok(r_paren_token)) => { + // // Don't remove parentheses if the left or right parens has any skipped token trivia attached + // // or the inner node is an unknown node (Rome can't know if an unknown needs parentheses or not) + // if l_paren_token.leading_trivia().has_skipped() + // || r_paren_token.leading_trivia().has_skipped() + // || inner.syntax().kind().is_unknown() + // { + // continue; + // } + // + // match ( + // l_paren_token.prev_token(), + // inner.syntax().first_token(), + // inner.syntax().last_token(), + // r_paren_token.next_token(), + // ) { + // (prev_token, Some(first_token), Some(last_token), Some(next_token)) => { + // let l_leading_trivia = l_paren_token.leading_trivia(); + // let l_trailing_trivia = l_paren_token.trailing_trivia(); + // + // if !l_leading_trivia.is_empty() && !l_trailing_trivia.is_empty() { + // match prev_token { + // Some(prev_token) if l_leading_trivia.is_empty() => { + // let new_prev_token = prev_token.with_trailing_trivia( + // prev_token + // .trailing_trivia() + // .pieces() + // .chain(l_trailing_trivia.pieces()) + // .collect::>() + // .iter() + // .map(|piece| (piece.kind(), piece.text())), + // ); + // + // mutation.replace_element_discard_trivia( + // SyntaxElement::Token(prev_token), + // SyntaxElement::Token(new_prev_token), + // ); + // } + // _ => { + // first_token.with_leading_trivia( + // l_leading_trivia + // .pieces() + // .chain(l_trailing_trivia.pieces()) + // .chain(first_token.leading_trivia().pieces()) + // .collect::>() + // .iter() + // .map(|piece| (piece.kind(), piece.text())), + // ); + // } + // } + // } + // + // let r_leading_trivia = r_paren_token.leading_trivia(); + // let r_trailing_trivia = r_paren_token.trailing_trivia(); + // + // // how does this fuckery work: + // // if first == last token + // // it may change leading/trailing trivia + // // if first != last: Two individual tokens + // + // // Now, fuckery level 100: What if nested: + // // Keep a vec of all concatenated leading / trailing pieces (for l_paren, and r_paren separately) + // // Otherwise apply the same logic + // + // // Issue, how to avoid re-visiting the same parenthesized expressions multiple times? + // // Handle parens in exit event of non paren node? + // + // if !r_leading_trivia.is_empty() || !r_trailing_trivia.is_empty() { + // if r_leading_trivia.is_empty() { + // let new_last_token = last_token.with_trailing_trivia( + // last_token + // .trailing_trivia() + // .pieces() + // .chain(r_trailing_trivia.pieces()) + // .collect::>() + // .iter() + // .map(|piece| (piece.kind(), piece.text())), + // ); + // mutation.replace_element_discard_trivia( + // SyntaxElement::Token(last_token), + // SyntaxElement::Token(new_last_token), + // ); + // } else { + // next_token.with_leading_trivia( + // r_leading_trivia + // .pieces() + // .chain(r_trailing_trivia.pieces()) + // .chain(next_token.leading_trivia().pieces()) + // .collect::>() + // .iter() + // .map(|piece| (piece.kind(), piece.text())), + // ); + // } + // } + // + // mutation.replace_element_discard_trivia( + // SyntaxElement::Node(node.into_syntax()), + // SyntaxElement::Node(inner.into_syntax()), + // ); + // } + // _ => continue, + // } + // } + // _ => {} + // } + // } + } + + fn is_range_formatting_node(&self, node: &SyntaxNode) -> bool { + let kind = node.kind(); + + // Do not format variable declaration nodes, format the whole statement instead + if matches!(kind, JsSyntaxKind::JS_VARIABLE_DECLARATION) { + return false; + } + + JsAnyStatement::can_cast(kind) + || JsAnyDeclaration::can_cast(kind) + || matches!( + kind, + JsSyntaxKind::JS_DIRECTIVE | JsSyntaxKind::JS_EXPORT | JsSyntaxKind::JS_IMPORT + ) + } + + fn context(&self) -> &Self::Context { + &self.context + } + + fn into_context(self, comments: Comments) -> Self::Context { + self.context.with_comments(Rc::new(comments)) + } +} + /// Formats a range within a file, supported by Rome /// /// This runs a simple heuristic to determine the initial indentation @@ -489,28 +660,8 @@ pub fn format_range( root: &JsSyntaxNode, range: TextRange, ) -> FormatResult { - rome_formatter::format_range::<_, FormatJsSyntaxNode, _>( - context, - root, - range, - is_range_formatting_root, - ) -} - -fn is_range_formatting_root(node: &JsSyntaxNode) -> bool { - let kind = node.kind(); - - // Do not format variable declaration nodes, format the whole statement instead - if matches!(kind, JsSyntaxKind::JS_VARIABLE_DECLARATION) { - return false; - } - - JsAnyStatement::can_cast(kind) - || JsAnyDeclaration::can_cast(kind) - || matches!( - kind, - JsSyntaxKind::JS_DIRECTIVE | JsSyntaxKind::JS_EXPORT | JsSyntaxKind::JS_IMPORT - ) + let language = JsFormatLanguage::new(context); + rome_formatter::format_range(root, range, language) } /// Formats a JavaScript (and its super languages) file based on its features. @@ -520,7 +671,8 @@ pub fn format_node( context: JsFormatContext, root: &JsSyntaxNode, ) -> FormatResult> { - rome_formatter::format_node(context, &root.format()) + let language = JsFormatLanguage::new(context); + rome_formatter::format_node(root, language) } /// Formats a single node within a file, supported by Rome. @@ -534,7 +686,8 @@ pub fn format_node( /// /// It returns a [Formatted] result pub fn format_sub_tree(context: JsFormatContext, root: &JsSyntaxNode) -> FormatResult { - rome_formatter::format_sub_tree(context, &root.format()) + let language = JsFormatLanguage::new(context); + rome_formatter::format_sub_tree(root, language) } #[cfg(test)] @@ -701,6 +854,7 @@ mod generated; pub(crate) mod builders; pub mod context; mod parentheses; +mod preprocessor; pub(crate) mod separated; #[cfg(test)] @@ -710,27 +864,27 @@ mod test { use rome_js_parser::parse; use rome_js_syntax::{SourceType, TextRange, TextSize}; - #[ignore] #[test] // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" -test.expect(t => { - t.true(a); -}, false); +(( + a) + /* comment */ + ); "#; let syntax = SourceType::tsx(); let tree = parse(src, 0, syntax); let result = format_node(JsFormatContext::default(), &tree.syntax()) .unwrap() .print(); - check_reformat(CheckReformatParams { - root: &tree.syntax(), - text: result.as_code(), - source_type: syntax, - file_name: "quick_test", - format_context: JsFormatContext::default(), - }); + // check_reformat(CheckReformatParams { + // root: &tree.syntax(), + // text: result.as_code(), + // source_type: syntax, + // file_name: "quick_test", + // format_context: JsFormatContext::default(), + // }); assert_eq!( result.as_code(), "type B8 = /*1*/ (C);\ntype B9 = (/*1*/ C);\ntype B10 = /*1*/ /*2*/ C;\n" diff --git a/crates/rome_js_formatter/src/parentheses.rs b/crates/rome_js_formatter/src/parentheses.rs index 256b817375a..f672b5191e8 100644 --- a/crates/rome_js_formatter/src/parentheses.rs +++ b/crates/rome_js_formatter/src/parentheses.rs @@ -50,66 +50,25 @@ use rome_js_syntax::{ JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsArrowFunctionExpression, JsAssignmentExpression, JsBinaryExpression, JsBinaryOperator, JsComputedMemberAssignment, JsComputedMemberExpression, - JsConditionalExpression, JsLanguage, JsSequenceExpression, JsSyntaxKind, JsSyntaxNode, + JsConditionalExpression, JsLanguage, JsParenthesizedExpression, JsSequenceExpression, + JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, }; -use rome_rowan::AstNode; +use rome_rowan::{declare_node_union, AstNode, SyntaxResult}; /// Node that may be parenthesized to ensure it forms valid syntax or to improve readability pub trait NeedsParentheses: AstNode { fn needs_parentheses(&self) -> bool { - self.resolve_parent() + self.syntax() + .parent() .map_or(false, |parent| self.needs_parentheses_with_parent(&parent)) } - fn resolve_parent(&self) -> Option { - resolve_parent(self.syntax()) - } - /// Returns `true` if this node requires parentheses to form valid syntax or improve readability. /// /// Returns `false` if the parentheses can be omitted safely without changing semantics. fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool; } -/// Trait implemented by all JavaScript expressions. -pub trait ExpressionNode: NeedsParentheses { - /// Resolves an expression to the first non parenthesized expression. - fn resolve(&self) -> JsAnyExpression; - - /// Consumes `self` and returns the first expression that isn't a parenthesized expression. - fn into_resolved(self) -> JsAnyExpression; - - /// Resolves an expression to the first non parenthesized expression and returns its [JsSyntaxNode]. - fn resolve_syntax(&self) -> JsSyntaxNode { - self.resolve().into_syntax() - } - - /// Consumes `self` and returns the [JsSyntaxNode] of the first expression that isn't a parenthesized expression. - fn into_resolved_syntax(self) -> JsSyntaxNode { - self.into_resolved().into_syntax() - } -} - -/// Resolves to the first parent that isn't a parenthesized expression, assignment, or type. -pub(crate) fn resolve_parent(node: &JsSyntaxNode) -> Option { - let mut current = node.parent(); - - while let Some(parent) = current { - if matches!( - parent.kind(), - JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION - | JsSyntaxKind::JS_PARENTHESIZED_ASSIGNMENT - | JsSyntaxKind::TS_PARENTHESIZED_TYPE - ) { - current = parent.parent(); - } else { - return Some(parent); - } - } - - None -} - impl NeedsParentheses for JsAnyLiteralExpression { #[inline] fn needs_parentheses(&self) -> bool { @@ -156,38 +115,6 @@ impl NeedsParentheses for JsAnyLiteralExpression { } } -impl ExpressionNode for JsAnyLiteralExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - match self { - JsAnyLiteralExpression::JsBigIntLiteralExpression(big_int) => big_int.resolve(), - JsAnyLiteralExpression::JsBooleanLiteralExpression(boolean) => boolean.resolve(), - JsAnyLiteralExpression::JsNullLiteralExpression(null_literal) => null_literal.resolve(), - JsAnyLiteralExpression::JsNumberLiteralExpression(number_literal) => { - number_literal.resolve() - } - JsAnyLiteralExpression::JsRegexLiteralExpression(regex) => regex.resolve(), - JsAnyLiteralExpression::JsStringLiteralExpression(string) => string.resolve(), - } - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - match self { - JsAnyLiteralExpression::JsBigIntLiteralExpression(big_int) => big_int.into_resolved(), - JsAnyLiteralExpression::JsBooleanLiteralExpression(boolean) => boolean.into_resolved(), - JsAnyLiteralExpression::JsNullLiteralExpression(null_literal) => { - null_literal.into_resolved() - } - JsAnyLiteralExpression::JsNumberLiteralExpression(number_literal) => { - number_literal.into_resolved() - } - JsAnyLiteralExpression::JsRegexLiteralExpression(regex) => regex.into_resolved(), - JsAnyLiteralExpression::JsStringLiteralExpression(string) => string.into_resolved(), - } - } -} - impl NeedsParentheses for JsAnyExpression { fn needs_parentheses(&self) -> bool { match self { @@ -334,98 +261,6 @@ impl NeedsParentheses for JsAnyExpression { } } -impl ExpressionNode for JsAnyExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - match self { - JsAnyExpression::ImportMeta(meta) => meta.resolve(), - JsAnyExpression::JsAnyLiteralExpression(literal) => literal.resolve(), - JsAnyExpression::JsArrayExpression(array) => array.resolve(), - JsAnyExpression::JsArrowFunctionExpression(arrow) => arrow.resolve(), - JsAnyExpression::JsAssignmentExpression(assignment) => assignment.resolve(), - JsAnyExpression::JsAwaitExpression(await_expression) => await_expression.resolve(), - JsAnyExpression::JsBinaryExpression(binary) => binary.resolve(), - JsAnyExpression::JsCallExpression(call) => call.resolve(), - JsAnyExpression::JsClassExpression(class) => class.resolve(), - JsAnyExpression::JsComputedMemberExpression(member) => member.resolve(), - JsAnyExpression::JsConditionalExpression(conditional) => conditional.resolve(), - JsAnyExpression::JsFunctionExpression(function) => function.resolve(), - JsAnyExpression::JsIdentifierExpression(identifier) => identifier.resolve(), - JsAnyExpression::JsImportCallExpression(import_call) => import_call.resolve(), - JsAnyExpression::JsInExpression(in_expression) => in_expression.resolve(), - JsAnyExpression::JsInstanceofExpression(instanceof) => instanceof.resolve(), - JsAnyExpression::JsLogicalExpression(logical) => logical.resolve(), - JsAnyExpression::JsNewExpression(new) => new.resolve(), - JsAnyExpression::JsObjectExpression(object) => object.resolve(), - JsAnyExpression::JsParenthesizedExpression(parenthesized) => parenthesized.resolve(), - JsAnyExpression::JsPostUpdateExpression(update) => update.resolve(), - JsAnyExpression::JsPreUpdateExpression(update) => update.resolve(), - JsAnyExpression::JsSequenceExpression(sequence) => sequence.resolve(), - JsAnyExpression::JsStaticMemberExpression(member) => member.resolve(), - JsAnyExpression::JsSuperExpression(sup) => sup.resolve(), - JsAnyExpression::JsTemplate(template) => template.resolve(), - JsAnyExpression::JsThisExpression(this) => this.resolve(), - JsAnyExpression::JsUnaryExpression(unary) => unary.resolve(), - JsAnyExpression::JsUnknownExpression(unknown) => unknown.resolve(), - JsAnyExpression::JsYieldExpression(yield_expression) => yield_expression.resolve(), - JsAnyExpression::JsxTagExpression(jsx) => jsx.resolve(), - JsAnyExpression::NewTarget(target) => target.resolve(), - JsAnyExpression::TsAsExpression(as_expression) => as_expression.resolve(), - JsAnyExpression::TsNonNullAssertionExpression(non_null) => non_null.resolve(), - JsAnyExpression::TsTypeAssertionExpression(type_assertion) => type_assertion.resolve(), - } - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - match self { - JsAnyExpression::ImportMeta(meta) => meta.into_resolved(), - JsAnyExpression::JsAnyLiteralExpression(literal) => literal.into_resolved(), - JsAnyExpression::JsArrayExpression(array) => array.into_resolved(), - JsAnyExpression::JsArrowFunctionExpression(arrow) => arrow.into_resolved(), - JsAnyExpression::JsAssignmentExpression(assignment) => assignment.into_resolved(), - JsAnyExpression::JsAwaitExpression(await_expression) => { - await_expression.into_resolved() - } - JsAnyExpression::JsBinaryExpression(binary) => binary.into_resolved(), - JsAnyExpression::JsCallExpression(call) => call.into_resolved(), - JsAnyExpression::JsClassExpression(class) => class.into_resolved(), - JsAnyExpression::JsComputedMemberExpression(member) => member.into_resolved(), - JsAnyExpression::JsConditionalExpression(conditional) => conditional.into_resolved(), - JsAnyExpression::JsFunctionExpression(function) => function.into_resolved(), - JsAnyExpression::JsIdentifierExpression(identifier) => identifier.into_resolved(), - JsAnyExpression::JsImportCallExpression(import_call) => import_call.into_resolved(), - JsAnyExpression::JsInExpression(in_expression) => in_expression.into_resolved(), - JsAnyExpression::JsInstanceofExpression(instanceof) => instanceof.into_resolved(), - JsAnyExpression::JsLogicalExpression(logical) => logical.into_resolved(), - JsAnyExpression::JsNewExpression(new) => new.into_resolved(), - JsAnyExpression::JsObjectExpression(object) => object.into_resolved(), - JsAnyExpression::JsParenthesizedExpression(parenthesized) => { - parenthesized.into_resolved() - } - JsAnyExpression::JsPostUpdateExpression(update) => update.into_resolved(), - JsAnyExpression::JsPreUpdateExpression(update) => update.into_resolved(), - JsAnyExpression::JsSequenceExpression(sequence) => sequence.into_resolved(), - JsAnyExpression::JsStaticMemberExpression(member) => member.into_resolved(), - JsAnyExpression::JsSuperExpression(sup) => sup.into_resolved(), - JsAnyExpression::JsTemplate(template) => template.into_resolved(), - JsAnyExpression::JsThisExpression(this) => this.into_resolved(), - JsAnyExpression::JsUnaryExpression(unary) => unary.into_resolved(), - JsAnyExpression::JsUnknownExpression(unknown) => unknown.into_resolved(), - JsAnyExpression::JsYieldExpression(yield_expression) => { - yield_expression.into_resolved() - } - JsAnyExpression::JsxTagExpression(jsx) => jsx.into_resolved(), - JsAnyExpression::NewTarget(target) => target.into_resolved(), - JsAnyExpression::TsAsExpression(as_expression) => as_expression.into_resolved(), - JsAnyExpression::TsNonNullAssertionExpression(non_null) => non_null.into_resolved(), - JsAnyExpression::TsTypeAssertionExpression(type_assertion) => { - type_assertion.into_resolved() - } - } - } -} - /// Returns the left most expression of `expression`. /// /// For example, returns `a` for `(a ? b : c) + d` because it first resolves the @@ -458,7 +293,6 @@ pub(crate) fn get_expression_left_side( use JsAnyExpression::*; let left_expression = match expression { - JsParenthesizedExpression(parenthesized) => parenthesized.expression().ok(), JsSequenceExpression(sequence) => sequence.left().ok(), JsStaticMemberExpression(member) => member.object().ok(), JsComputedMemberExpression(member) => member.object().ok(), @@ -499,15 +333,13 @@ pub(crate) fn is_first_in_statement(node: JsSyntaxNode, mode: FirstInStatementMo return true; } - JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION - | JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION + JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION | JsSyntaxKind::JS_STATIC_MEMBER_ASSIGNMENT | JsSyntaxKind::JS_TEMPLATE | JsSyntaxKind::JS_CALL_EXPRESSION | JsSyntaxKind::JS_NEW_EXPRESSION | JsSyntaxKind::TS_AS_EXPRESSION - | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION - | JsSyntaxKind::JS_PARENTHESIZED_ASSIGNMENT => parent, + | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => parent, JsSyntaxKind::JS_SEQUENCE_EXPRESSION => { let sequence = JsSequenceExpression::unwrap_cast(parent); @@ -640,11 +472,7 @@ pub(crate) fn unary_like_expression_needs_parentheses( let binary = JsBinaryExpression::unwrap_cast(parent.clone()); matches!(binary.operator(), Ok(JsBinaryOperator::Exponent)) - && binary - .left() - .map(ExpressionNode::into_resolved_syntax) - .as_ref() - == Ok(expression) + && binary.left().map(AstNode::into_syntax).as_ref() == Ok(expression) } _ => update_or_lower_expression_needs_parentheses(expression, parent), } @@ -689,7 +517,7 @@ pub(crate) fn is_member_object(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bo member_expression .object() - .map(ExpressionNode::into_resolved_syntax) + .map(AstNode::into_syntax) .as_ref() == Ok(node) } @@ -699,7 +527,7 @@ pub(crate) fn is_member_object(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bo member_assignment .object() - .map(ExpressionNode::into_resolved_syntax) + .map(AstNode::into_syntax) .as_ref() == Ok(node) } @@ -733,11 +561,7 @@ pub(crate) fn is_conditional_test(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> JsSyntaxKind::JS_CONDITIONAL_EXPRESSION => { let conditional = JsConditionalExpression::unwrap_cast(parent.clone()); - conditional - .test() - .map(ExpressionNode::into_resolved_syntax) - .as_ref() - == Ok(node) + conditional.test().map(AstNode::into_syntax).as_ref() == Ok(node) } _ => false, } @@ -751,9 +575,7 @@ pub(crate) fn is_arrow_function_body(node: &JsSyntaxNode, parent: &JsSyntaxNode) let arrow = JsArrowFunctionExpression::unwrap_cast(parent.clone()); match arrow.body() { - Ok(JsAnyFunctionBody::JsAnyExpression(expression)) => { - &expression.resolve_syntax() == node - } + Ok(JsAnyFunctionBody::JsAnyExpression(expression)) => expression.syntax() == node, _ => false, } } @@ -782,6 +604,36 @@ pub(crate) fn is_spread(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { ) } +declare_node_union! { + pub(crate) JsAnyParenthesizedNode = JsParenthesizedExpression +} + +impl JsAnyParenthesizedNode { + pub(crate) fn l_paren_token(&self) -> SyntaxResult { + match self { + JsAnyParenthesizedNode::JsParenthesizedExpression(expression) => { + expression.l_paren_token() + } + } + } + + pub(crate) fn inner(&self) -> SyntaxResult { + match self { + JsAnyParenthesizedNode::JsParenthesizedExpression(expression) => { + expression.expression().map(AstNode::into_syntax) + } + } + } + + pub(crate) fn r_paren_token(&self) -> SyntaxResult { + match self { + JsAnyParenthesizedNode::JsParenthesizedExpression(expression) => { + expression.r_paren_token() + } + } + } +} + /// Returns `true` if `parent` is a [JsAnyBinaryLikeExpression] and `node` is the `left` or `right` of that expression. pub(crate) fn is_binary_like_left_or_right(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { debug_assert_is_expression(node); @@ -948,7 +800,7 @@ fn debug_assert_is_expression(node: &JsSyntaxNode) { fn debug_assert_is_parent(node: &JsSyntaxNode, parent: &JsSyntaxNode) { debug_assert!( - resolve_parent(node).as_ref() == Some(parent), + node.parent().as_ref() == Some(parent), "Node {node:#?} is not a child of ${parent:#?}" ) } diff --git a/crates/rome_js_formatter/src/preprocessor.rs b/crates/rome_js_formatter/src/preprocessor.rs new file mode 100644 index 00000000000..1e6ed198680 --- /dev/null +++ b/crates/rome_js_formatter/src/preprocessor.rs @@ -0,0 +1,315 @@ +use rome_js_syntax::{ + JsAnyExpression, JsLanguage, JsParenthesizedExpression, JsSyntaxNode, JsSyntaxToken, +}; +use rome_rowan::{AstNode, BatchMutation, SyntaxKind, SyntaxTriviaPiece}; +use std::collections::HashMap; +use std::iter::FusedIterator; + +pub(super) fn preprocess(root: &JsSyntaxNode) -> JsSyntaxNode { + let mut mutation = BatchMutation::new(root.clone()); + // tracks all changed tokens where the key is the token in the original tree and the value is the + // updated token. Necessary, because removing parentheses may require adding leading/trailing trivia + // to the same node: `(a) /* leading */ + (/* trailing 2*/ b)` Both comments should be merged with the `+` tokens trivia. + let mut tokens: HashMap = HashMap::new(); + + let mut prev_token_trailing = Vec::new(); + let mut first_inner_leading = Vec::new(); + let mut last_inner_trailing = Vec::new(); + let mut next_token_leading = Vec::new(); + + let mut parentheses: Option = None; + let mut prev_token: Option = None; + let mut next_token: Option = None; + + for node in root.descendants() { + match get_left_paren_inner_and_right_paren(node) { + Ok((l_paren, inner, r_paren, parenthesized)) => { + let l_paren_leading = l_paren.leading_trivia(); + let l_paren_trailing = l_paren.trailing_trivia(); + + if parentheses.is_none() { + debug_assert!(prev_token_trailing.is_empty()); + debug_assert!(first_inner_leading.is_empty()); + debug_assert!(last_inner_trailing.is_empty()); + debug_assert!(next_token_leading.is_empty()); + debug_assert!(prev_token.is_none()); + debug_assert!(next_token.is_none()); + + prev_token = l_paren.prev_token(); + next_token = r_paren.next_token(); + parentheses = Some(parenthesized); + } + + if first_inner_leading.is_empty() && l_paren_leading.is_empty() { + prev_token_trailing.extend(l_paren_trailing.pieces()); + } else { + first_inner_leading + .extend(l_paren_leading.pieces().chain(l_paren_trailing.pieces())); + } + + let r_paren_leading = r_paren.leading_trivia(); + let r_paren_trailing = r_paren.trailing_trivia(); + + if next_token_leading.is_empty() && r_paren_leading.is_empty() { + last_inner_trailing.extend(r_paren_trailing.pieces()); + } else { + next_token_leading + .extend(r_paren_leading.pieces().chain(r_paren_trailing.pieces())) + } + } + Err(node) => { + let parenthesized = parentheses.take(); + let prev_token = prev_token.take(); + let next_token = next_token.take(); + + if !prev_token_trailing.is_empty() { + debug_assert!(parenthesized.is_some()); + + match prev_token { + Some(prev_token) => { + let new_prev_token = tokens.get(&prev_token).unwrap_or(&prev_token); + + let new_prev_token = + new_prev_token.with_trailing_trivia_pieces(chain_pieces( + new_prev_token.trailing_trivia().pieces(), + prev_token_trailing.drain(..), + )); + + tokens.insert(prev_token, new_prev_token); + } + None => { + // No previous token, everything becomes the leading of the inner token. + // May happen if the parenthesized expression is at the start of the program. + first_inner_leading.append(&mut prev_token_trailing); + } + } + } + + let mut new_node = node; + + if !first_inner_leading.is_empty() { + debug_assert!(parenthesized.is_some()); + + match new_node.first_token() { + Some(first_token) => { + let new_first_token = + tokens.remove(&first_token).unwrap_or(first_token.clone()); + + let new_first_token = + new_first_token.with_leading_trivia_pieces(chain_pieces( + new_first_token.trailing_trivia().pieces(), + first_inner_leading.drain(..), + )); + + new_node = new_node + .replace_child(first_token.into(), new_first_token.into()) + .unwrap(); + } + None => { + // The only case this can happen is if we have `()` which isn't valid code. + // Make the trivia the leading trivia of the next token (doesn't change order because `prev_token` will also be None). + next_token_leading.append(&mut first_inner_leading); + } + } + } + + if !last_inner_trailing.is_empty() { + debug_assert!(parenthesized.is_some()); + + match new_node.last_token() { + Some(last_token) => { + let new_last_token = + tokens.remove(&last_token).unwrap_or(last_token.clone()); + + let new_last_token = + new_last_token.with_trailing_trivia_pieces(chain_pieces( + new_last_token.trailing_trivia().pieces(), + last_inner_trailing.drain(..), + )); + + new_node = new_node + .replace_child(last_token.into(), new_last_token.into()) + .unwrap(); + } + None => { + // This happens if the expression is `()` which isn't valid code. + // Make the trivia the leading trivia of the next token (which hopefully will exist). + next_token_leading.append(&mut last_inner_trailing); + } + } + } + + if !next_token_leading.is_empty() { + debug_assert!(parenthesized.is_some()); + + match next_token { + Some(next_token) => { + let new_next_token = tokens.get(&next_token).unwrap_or(&next_token); + let new_next_token = + new_next_token.with_leading_trivia_pieces(chain_pieces( + next_token_leading.drain(..), + new_next_token.leading_trivia().pieces(), + )); + + tokens.insert(next_token, new_next_token); + } + None => { + panic!("Missing `EOF` token."); + } + } + } + + if let Some(parenthesized) = parenthesized { + mutation.replace_element_discard_trivia( + parenthesized.into_syntax().into(), + new_node.into(), + ); + } + } + } + } + + for (old, new) in tokens.into_iter() { + mutation.replace_element_discard_trivia(old.into(), new.into()) + } + + mutation.commit() +} + +fn get_left_paren_inner_and_right_paren( + node: JsSyntaxNode, +) -> Result< + ( + JsSyntaxToken, + JsAnyExpression, + JsSyntaxToken, + JsParenthesizedExpression, + ), + JsSyntaxNode, +> { + match JsParenthesizedExpression::try_cast(node) { + Ok(parenthesized) => match ( + parenthesized.l_paren_token(), + parenthesized.expression(), + parenthesized.r_paren_token(), + ) { + (Ok(l_paren), Ok(expression), Ok(r_paren)) => { + // Keep parentheses around unknown expressions. Rome can't know the precedence. + if expression.syntax().kind().is_unknown() + // Don't remove parentheses if they have skipped trivia. We don't know for certain what the intended syntax is. + || l_paren.leading_trivia().has_skipped() + || r_paren.leading_trivia().has_skipped() + { + Err(parenthesized.into_syntax()) + } else if expression.syntax().first_token().is_none() { + // This should never happen but we need to be sure. + Err(parenthesized.into_syntax()) + } else { + Ok((l_paren, expression, r_paren, parenthesized)) + } + } + _ => { + // At least one missing child, handle as a regular node + Err(parenthesized.into_syntax()) + } + }, + Err(node) => Err(node), + } +} + +fn chain_pieces(first: F, second: S) -> ChainTriviaPiecesIterator +where + F: Iterator>, + S: Iterator>, +{ + ChainTriviaPiecesIterator::new(first, second) +} + +/// Chain iterator that chains two iterators over syntax trivia together. +/// +/// This is the same as Rust's [Chain] iterator but implements [ExactSizeIterator]. +/// Rust doesn't implement [ExactSizeIterator] because adding the sizes of both pieces may overflow. +/// +/// Implementing [ExactSizeIterator] in our case is safe because our ranges are limited to [u32], which in turn +/// guarantees that a tree can never have more than 2^32 tokens or pieces and adding a `u32 + u32` is safely in the range +/// of a `usize`. +struct ChainTriviaPiecesIterator { + first: Option, + second: S, +} + +impl ChainTriviaPiecesIterator { + fn new(first: F, second: S) -> Self { + Self { + first: Some(first), + second, + } + } +} + +impl Iterator for ChainTriviaPiecesIterator +where + F: Iterator>, + S: Iterator>, +{ + type Item = SyntaxTriviaPiece; + + fn next(&mut self) -> Option { + match &mut self.first { + Some(first) => match first.next() { + Some(next) => Some(next), + None => { + self.first.take(); + self.second.next() + } + }, + None => self.second.next(), + } + } + + fn size_hint(&self) -> (usize, Option) { + match &self.first { + Some(first) => { + let (first_lower, first_upper) = first.size_hint(); + let (second_lower, second_upper) = self.second.size_hint(); + + let lower = first_lower.saturating_add(second_lower); + + let upper = match (first_upper, second_upper) { + (Some(first), Some(second)) => first.checked_add(second), + _ => None, + }; + + (lower, upper) + } + None => self.second.size_hint(), + } + } +} + +impl FusedIterator for ChainTriviaPiecesIterator +where + F: Iterator>, + S: Iterator>, +{ +} + +impl ExactSizeIterator for ChainTriviaPiecesIterator +where + F: ExactSizeIterator>, + S: ExactSizeIterator>, +{ + fn len(&self) -> usize { + match &self.first { + Some(first) => { + let first_len = first.len(); + let second_len = self.second.len(); + + // SAFETY: Should be safe because a program can never contain more than u32 pieces + // because the text ranges are represented as u32 (and each piece must at least contain a single character). + first_len + second_len + } + None => self.second.len(), + } + } +} diff --git a/crates/rome_js_formatter/src/ts/expressions/as_expression.rs b/crates/rome_js_formatter/src/ts/expressions/as_expression.rs index ceea654aacf..63cbdac28e0 100644 --- a/crates/rome_js_formatter/src/ts/expressions/as_expression.rs +++ b/crates/rome_js_formatter/src/ts/expressions/as_expression.rs @@ -1,11 +1,11 @@ use crate::prelude::*; use crate::parentheses::{ - is_binary_like_left_or_right, is_callee, is_member_object, ExpressionNode, NeedsParentheses, + is_binary_like_left_or_right, is_callee, is_member_object, NeedsParentheses, }; use crate::ts::expressions::type_assertion_expression::type_cast_like_needs_parens; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, TsAsExpressionFields}; +use rome_js_syntax::{TsAsExpressionFields}; use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, TsAsExpression}; #[derive(Debug, Clone, Default)] @@ -32,7 +32,7 @@ impl FormatNodeRule for FormatTsAsExpression { ] }); - let parent = node.resolve_parent(); + let parent = node.syntax().parent(); let is_callee_or_object = parent.map_or(false, |parent| { is_callee(node.syntax(), &parent) || is_member_object(node.syntax(), &parent) @@ -63,18 +63,6 @@ impl NeedsParentheses for TsAsExpression { } } -impl ExpressionNode for TsAsExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} - #[cfg(test)] mod tests { diff --git a/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs b/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs index 5dc096bfc9c..7ada1b0bb9f 100644 --- a/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs +++ b/crates/rome_js_formatter/src/ts/expressions/non_null_assertion_expression.rs @@ -1,9 +1,9 @@ use crate::prelude::*; use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsSyntaxKind, TsNonNullAssertionExpressionFields}; +use rome_js_syntax::{JsSyntaxKind, TsNonNullAssertionExpressionFields}; use rome_js_syntax::{JsSyntaxNode, TsNonNullAssertionExpression}; #[derive(Debug, Clone, Default)] @@ -34,15 +34,3 @@ impl NeedsParentheses for TsNonNullAssertionExpression { || member_chain_callee_needs_parens(self.clone().into(), parent) } } - -impl ExpressionNode for TsNonNullAssertionExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} diff --git a/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs b/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs index f8a360cae2e..a66b45e92b0 100644 --- a/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs +++ b/crates/rome_js_formatter/src/ts/expressions/type_assertion_expression.rs @@ -1,10 +1,8 @@ use crate::prelude::*; -use crate::parentheses::{ - is_callee, is_member_object, is_spread, is_tag, ExpressionNode, NeedsParentheses, -}; +use crate::parentheses::{is_callee, is_member_object, is_spread, is_tag, NeedsParentheses}; use rome_formatter::write; -use rome_js_syntax::{JsAnyExpression, JsSyntaxNode}; +use rome_js_syntax::{JsSyntaxNode}; use rome_js_syntax::{JsSyntaxKind, TsTypeAssertionExpression, TsTypeAssertionExpressionFields}; #[derive(Debug, Clone, Default)] @@ -47,18 +45,6 @@ impl NeedsParentheses for TsTypeAssertionExpression { } } -impl ExpressionNode for TsTypeAssertionExpression { - #[inline] - fn resolve(&self) -> JsAnyExpression { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyExpression { - self.into() - } -} - pub(super) fn type_cast_like_needs_parens(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { debug_assert!(matches!( node.kind(), diff --git a/crates/rome_js_formatter/src/utils/assignment_like.rs b/crates/rome_js_formatter/src/utils/assignment_like.rs index 4e07d0caaba..117990efe62 100644 --- a/crates/rome_js_formatter/src/utils/assignment_like.rs +++ b/crates/rome_js_formatter/src/utils/assignment_like.rs @@ -1,6 +1,4 @@ -use crate::parentheses::{ - get_expression_left_side, resolve_parent, ExpressionNode, NeedsParentheses, -}; +use crate::parentheses::{get_expression_left_side}; use crate::prelude::*; use crate::utils::member_chain::is_member_call_chain; use crate::utils::object::write_member_name; @@ -588,7 +586,7 @@ impl JsAnyAssignmentLike { return Ok(AssignmentLikeLayout::SuppressedInitializer); } } - let right_expression = right.as_expression().map(ExpressionNode::into_resolved); + let right_expression = right.as_expression(); if let Some(layout) = self.chain_formatting_layout(right_expression.as_ref())? { return Ok(layout); @@ -621,9 +619,6 @@ impl JsAnyAssignmentLike { let right_expression = iter::successors(right_expression, |expression| match expression { JsAnyExpression::JsUnaryExpression(unary) => unary.argument().ok(), JsAnyExpression::TsNonNullAssertionExpression(assertion) => assertion.expression().ok(), - JsAnyExpression::JsParenthesizedExpression(parenthesized) => { - parenthesized.expression().ok() - } _ => None, }) .last(); @@ -693,14 +688,14 @@ impl JsAnyAssignmentLike { let upper_chain_is_eligible = // First, we check if the current node is an assignment expression if let JsAnyAssignmentLike::JsAssignmentExpression(assignment) = self { - assignment.resolve_parent().map_or(false, |parent| { + assignment.syntax().parent().map_or(false, |parent| { // Then we check if the parent is assignment expression or variable declarator if matches!( parent.kind(), JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION | JsSyntaxKind::JS_INITIALIZER_CLAUSE ) { - let great_parent_kind = resolve_parent(&parent).map(|n| n.kind()); + let great_parent_kind = parent.parent().map(|n| n.kind()); // Finally, we check the great parent. // The great parent triggers the eligibility when // - the current node that we were inspecting is not a "tail" @@ -731,7 +726,7 @@ impl JsAnyAssignmentLike { match this_body { JsAnyFunctionBody::JsAnyExpression(expression) => { if matches!( - expression.resolve(), + expression, JsAnyExpression::JsArrowFunctionExpression(_) ) { Some(AssignmentLikeLayout::ChainTailArrowFunction) @@ -835,13 +830,11 @@ pub(crate) fn should_break_after_operator(right: &JsAnyExpression) -> SyntaxResu break; } - let right = right.resolve(); - let result = match right { // head is a long chain, meaning that right -> right are both assignment expressions JsAnyExpression::JsAssignmentExpression(assignment) => { matches!( - assignment.right()?.resolve(), + assignment.right()?, JsAnyExpression::JsAssignmentExpression(_) ) } @@ -854,7 +847,7 @@ pub(crate) fn should_break_after_operator(right: &JsAnyExpression) -> SyntaxResu JsAnyExpression::JsSequenceExpression(_) => true, JsAnyExpression::JsConditionalExpression(conditional) => { - JsAnyBinaryLikeExpression::cast(conditional.test()?.into_resolved_syntax()) + JsAnyBinaryLikeExpression::cast(conditional.test()?.into_syntax()) .map_or(false, |expression| { !expression.should_inline_logical_expression() }) @@ -1064,7 +1057,6 @@ fn is_poorly_breakable_member_or_call_chain( is_chain = true; Some(node.object()?) } - JsAnyExpression::JsParenthesizedExpression(node) => Some(node.expression()?), JsAnyExpression::JsIdentifierExpression(_) | JsAnyExpression::JsThisExpression(_) => { is_chain_head_simple = true; break; @@ -1122,7 +1114,7 @@ fn is_short_argument(argument: JsAnyCallArgument, threshold: u16) -> SyntaxResul } if let JsAnyCallArgument::JsAnyExpression(expression) = argument { - let is_short_argument = match expression.resolve() { + let is_short_argument = match expression { JsAnyExpression::JsThisExpression(_) => true, JsAnyExpression::JsIdentifierExpression(identifier) => { identifier.name()?.value_token()?.text_trimmed().len() <= threshold as usize diff --git a/crates/rome_js_formatter/src/utils/binary_like_expression.rs b/crates/rome_js_formatter/src/utils/binary_like_expression.rs index f18cfff521f..c4a098c3ff2 100644 --- a/crates/rome_js_formatter/src/utils/binary_like_expression.rs +++ b/crates/rome_js_formatter/src/utils/binary_like_expression.rs @@ -58,13 +58,12 @@ use rome_formatter::{format_args, write, Buffer, CommentStyle, CstFormatContext} use rome_js_syntax::{ JsAnyExpression, JsAnyInProperty, JsBinaryExpression, JsBinaryOperator, JsDoWhileStatement, JsIfStatement, JsInExpression, JsInstanceofExpression, JsLogicalExpression, JsLogicalOperator, - JsParenthesizedExpression, JsPrivateName, JsSwitchStatement, JsSyntaxKind, JsSyntaxNode, - JsSyntaxToken, JsUnaryExpression, JsWhileStatement, OperatorPrecedence, + JsPrivateName, JsSwitchStatement, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, JsUnaryExpression, + JsWhileStatement, OperatorPrecedence, }; use crate::parentheses::{ - is_arrow_function_body, is_callee, is_member_object, is_spread, is_tag, resolve_parent, - ExpressionNode, NeedsParentheses, + is_arrow_function_body, is_callee, is_member_object, is_spread, is_tag, NeedsParentheses, }; use crate::context::JsCommentStyle; @@ -81,7 +80,7 @@ declare_node_union! { impl Format for JsAnyBinaryLikeExpression { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { - let parent = self.resolve_parent(); + let parent = self.syntax().parent(); let is_inside_condition = self.is_inside_condition(parent.as_ref()); let parts = split_into_left_and_right_sides(self, is_inside_condition)?; @@ -194,14 +193,10 @@ fn split_into_left_and_right_sides( // Calling skip_subtree prevents the exit event being triggered for this event. expressions.skip_subtree(); - items.push(BinaryLeftOrRightSide::Left { - is_root: binary.syntax() == root.syntax(), - parent: binary, - }); + items.push(BinaryLeftOrRightSide::Left { parent: binary }); } } VisitEvent::Exit(expression) => items.push(BinaryLeftOrRightSide::Right { - is_root: expression.syntax() == root.syntax(), parent: expression, inside_condition, }), @@ -249,14 +244,12 @@ fn should_indent_if_parent_inlines(parent: Option<&JsSyntaxNode>) -> bool { parent.map_or(false, |parent| match parent.kind() { JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION | JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER => true, - JsSyntaxKind::JS_INITIALIZER_CLAUSE => { - resolve_parent(parent).map_or(false, |grand_parent| { - matches!( - grand_parent.kind(), - JsSyntaxKind::JS_VARIABLE_DECLARATOR | JsSyntaxKind::JS_PROPERTY_CLASS_MEMBER - ) - }) - } + JsSyntaxKind::JS_INITIALIZER_CLAUSE => parent.parent().map_or(false, |grand_parent| { + matches!( + grand_parent.kind(), + JsSyntaxKind::JS_VARIABLE_DECLARATOR | JsSyntaxKind::JS_PROPERTY_CLASS_MEMBER + ) + }), _ => false, }) } @@ -267,10 +260,7 @@ enum BinaryLeftOrRightSide { /// A terminal left hand side of a binary expression. /// /// Formats the left hand side only. - Left { - parent: JsAnyBinaryLikeExpression, - is_root: bool, - }, + Left { parent: JsAnyBinaryLikeExpression }, /// The right hand side of a binary expression. /// Formats the operand together with the right hand side. @@ -278,7 +268,6 @@ enum BinaryLeftOrRightSide { parent: JsAnyBinaryLikeExpression, /// Is the parent the condition of a `if` / `while` / `do-while` / `for` statement? inside_condition: bool, - is_root: bool, }, } @@ -288,15 +277,12 @@ impl BinaryLeftOrRightSide { match self { BinaryLeftOrRightSide::Left { parent, .. } => match parent.left() { Ok(JsAnyBinaryLikeLeftExpression::JsAnyExpression(expression)) => { - matches!(expression.resolve(), JsAnyExpression::JsxTagExpression(_)) + matches!(expression, JsAnyExpression::JsxTagExpression(_)) } _ => false, }, BinaryLeftOrRightSide::Right { parent, .. } => { - matches!( - parent.right().map(JsAnyExpression::into_resolved), - Ok(JsAnyExpression::JsxTagExpression(_)) - ) + matches!(parent.right(), Ok(JsAnyExpression::JsxTagExpression(_))) } } } @@ -305,29 +291,12 @@ impl BinaryLeftOrRightSide { impl Format for BinaryLeftOrRightSide { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { match self { - BinaryLeftOrRightSide::Left { parent, is_root } => { - if !is_root { - for ancestor in parent.syntax().ancestors().skip(1) { - match ancestor.kind() { - JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION => { - f.context().comments().mark_suppression_checked(&ancestor); - - let parenthesized = - JsParenthesizedExpression::unwrap_cast(ancestor); - - write!(f, [format_removed(&parenthesized.l_paren_token()?)])?; - } - _ => break, - } - } - } - + BinaryLeftOrRightSide::Left { parent } => { write!(f, [group(&parent.left())]) } BinaryLeftOrRightSide::Right { parent: binary_like_expression, inside_condition: inside_parenthesis, - is_root, } => { // It's only possible to suppress the formatting of the whole binary expression formatting OR // the formatting of the right hand side value but not of a nested binary expression. @@ -352,71 +321,31 @@ impl Format for BinaryLeftOrRightSide { write!(f, [right.format()])?; - if !is_root { - for ancestor in binary_like_expression.syntax().ancestors().skip(1) { - match ancestor.kind() { - JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION => { - let parenthesized = - JsParenthesizedExpression::unwrap_cast(ancestor); - - write!(f, [format_removed(&parenthesized.r_paren_token()?)])?; - } - _ => break, - } - } - } - Ok(()) }); let syntax = binary_like_expression.syntax(); - let parent = resolve_parent(syntax); + let parent = syntax.parent(); // Doesn't match prettier that only distinguishes between logical and binary let parent_has_same_kind = parent.as_ref().map_or(false, |parent| { is_same_binary_expression_kind(binary_like_expression, parent) }); - let left_has_same_kind = - binary_like_expression - .left()? - .into_expression() - .map_or(false, |left| { - is_same_binary_expression_kind( - binary_like_expression, - &left.resolve_syntax(), - ) - }); + let left_has_same_kind = binary_like_expression + .left()? + .into_expression() + .map_or(false, |left| { + is_same_binary_expression_kind(binary_like_expression, left.syntax()) + }); let right_has_same_kind = - is_same_binary_expression_kind(binary_like_expression, &right.resolve_syntax()); + is_same_binary_expression_kind(binary_like_expression, right.syntax()); let should_break = { if has_token_trailing_line_comment(&operator_token) { true } else { - let mut current = Some(binary_like_expression.left()?.into_syntax()); - loop { - if let Some(left) = current { - if has_leading_own_line_comment(&left) { - break true; - } - - match left.kind() { - JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION => { - let parenthesized = - JsParenthesizedExpression::unwrap_cast(left); - - current = parenthesized - .expression() - .ok() - .map(AstNode::into_syntax); - } - _ => { - break false; - } - } - } - } + has_leading_own_line_comment(binary_like_expression.left()?.syntax()) } }; @@ -530,7 +459,7 @@ impl JsAnyBinaryLikeExpression { _ => return false, }; - test.map_or(false, |test| &test.resolve_syntax() == self.syntax()) + test.map_or(false, |test| test.syntax() == self.syntax()) }) } @@ -539,7 +468,7 @@ impl JsAnyBinaryLikeExpression { fn can_flatten(&self) -> SyntaxResult { let left = self.left()?.into_expression(); - let left_expression = left.as_ref().map(|expression| expression.resolve_syntax()); + let left_expression = left.map(|expression| expression.into_syntax()); if let Some(left_binary_like) = left_expression.and_then(JsAnyBinaryLikeExpression::cast) { let operator = self.operator()?; @@ -554,14 +483,12 @@ impl JsAnyBinaryLikeExpression { pub(crate) fn should_inline_logical_expression(&self) -> bool { match self { JsAnyBinaryLikeExpression::JsLogicalExpression(logical) => { - logical - .right() - .map_or(false, |right| match right.resolve() { - JsAnyExpression::JsObjectExpression(object) => !object.members().is_empty(), - JsAnyExpression::JsArrayExpression(array) => !array.elements().is_empty(), - JsAnyExpression::JsxTagExpression(_) => true, - _ => false, - }) + logical.right().map_or(false, |right| match right { + JsAnyExpression::JsObjectExpression(object) => !object.members().is_empty(), + JsAnyExpression::JsArrayExpression(array) => !array.elements().is_empty(), + JsAnyExpression::JsxTagExpression(_) => true, + _ => false, + }) } _ => false, } @@ -585,7 +512,7 @@ impl JsAnyBinaryLikeExpression { is_arrow_function_body(self.syntax(), parent) } JsSyntaxKind::JS_CONDITIONAL_EXPRESSION => { - let grand_parent = resolve_parent(parent); + let grand_parent = parent.parent(); grand_parent.map_or(false, |grand_parent| { !matches!( @@ -664,11 +591,8 @@ pub(crate) fn needs_binary_like_parentheses( return true; } - let is_right = parent - .right() - .map(ExpressionNode::into_resolved_syntax) - .as_ref() - == Ok(node.syntax()); + let is_right = + parent.right().map(AstNode::into_syntax).as_ref() == Ok(node.syntax()); // `a ** b ** c` if is_right && parent_precedence == precedence { @@ -900,7 +824,8 @@ impl BinaryLikePreorder { // SAFETY: Calling `unwrap` here is safe because the iterator only enters (traverses into) a node // if it is a valid binary like expression and it is guaranteed to have a parent. let expression = binary - .resolve_parent() + .syntax() + .parent() .and_then(JsAnyBinaryLikeExpression::cast) .unwrap(); @@ -929,7 +854,7 @@ impl Iterator for BinaryLikePreorder { .ok() .and_then(|left| left.into_expression()) .and_then(|expression| { - JsAnyBinaryLikeExpression::cast(expression.into_resolved_syntax()) + JsAnyBinaryLikeExpression::cast(expression.into_syntax()) }); if let Some(binary) = next { @@ -941,7 +866,7 @@ impl Iterator for BinaryLikePreorder { } VisitEvent::Exit(node) => { if node.syntax() != &self.start { - self.next = node.resolve_parent().map(|parent| { + self.next = node.syntax().parent().map(|parent| { // SAFETY: Calling `unwrap` here is safe because the iterator only enters (traverses into) a node // if it is a valid binary like expression. let expression = JsAnyBinaryLikeExpression::cast(parent).unwrap(); diff --git a/crates/rome_js_formatter/src/utils/conditional.rs b/crates/rome_js_formatter/src/utils/conditional.rs index 09805c9e74c..983a5813fd5 100644 --- a/crates/rome_js_formatter/src/utils/conditional.rs +++ b/crates/rome_js_formatter/src/utils/conditional.rs @@ -1,10 +1,10 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{ExpressionNode, NeedsParentheses}; + use rome_js_syntax::{ JsAnyExpression, JsAssignmentExpression, JsCallExpression, JsComputedMemberExpression, - JsConditionalExpression, JsInitializerClause, JsNewExpression, JsParenthesizedExpression, + JsConditionalExpression, JsInitializerClause, JsNewExpression, JsReturnStatement, JsStaticMemberExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, JsThrowStatement, JsUnaryExpression, JsYieldArgument, TsAsExpression, TsConditionalType, TsNonNullAssertionExpression, TsType, @@ -33,7 +33,7 @@ impl Format for JsAnyConditional { ] )?; - let is_consequent_nested = consequent.resolve().kind() == syntax.kind(); + let is_consequent_nested = consequent.syntax().kind() == syntax.kind(); if is_consequent_nested { // Add parentheses around the consequent if it is a conditional expression and fits on the same line @@ -60,7 +60,7 @@ impl Format for JsAnyConditional { ] )?; - let is_alternate_nested = alternate.resolve().kind() == syntax.kind(); + let is_alternate_nested = alternate.syntax().kind() == syntax.kind(); // Don't "indent" a nested alternate by two spaces so that the consequent and alternates nicely align // ``` @@ -183,16 +183,6 @@ declare_node_union! { ExpressionOrType = JsAnyExpression | TsType } -impl ExpressionOrType { - /// Resolves to the first non parenthesized expression. Returns self for types. - fn resolve(&self) -> JsSyntaxNode { - match self { - ExpressionOrType::JsAnyExpression(expression) => expression.resolve_syntax(), - ExpressionOrType::TsType(ty) => ty.syntax().clone(), - } - } -} - impl Format for ExpressionOrType { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { match self { @@ -286,7 +276,7 @@ impl ConditionalLayout { impl JsAnyConditional { fn layout(&self) -> ConditionalLayout { let resolved_parent = match self { - JsAnyConditional::JsConditionalExpression(conditional) => conditional.resolve_parent(), + JsAnyConditional::JsConditionalExpression(conditional) => conditional.syntax().parent(), JsAnyConditional::TsConditionalType(ty) => ty.syntax().parent(), }; @@ -324,7 +314,6 @@ impl JsAnyConditional { JsAnyConditional::JsConditionalExpression(conditional) => conditional .test() .ok() - .map(ExpressionNode::into_resolved) .map_or(false, |resolved| resolved.syntax() == node), JsAnyConditional::TsConditionalType(conditional) => { conditional.check_type().map(AstNode::into_syntax).as_ref() == Ok(node) @@ -378,10 +367,9 @@ impl JsAnyConditional { /// Returns `true` if the passed node is the `alternate` of this conditional expression. fn is_alternate(&self, node: &JsSyntaxNode) -> bool { let alternate = match self { - JsAnyConditional::JsConditionalExpression(conditional) => conditional - .alternate() - .map(ExpressionNode::into_resolved_syntax) - .ok(), + JsAnyConditional::JsConditionalExpression(conditional) => { + conditional.alternate().map(AstNode::into_syntax).ok() + } JsAnyConditional::TsConditionalType(ts_conditional) => { ts_conditional.false_type().ok().map(AstNode::into_syntax) } @@ -430,8 +418,7 @@ impl JsAnyConditional { let ancestors = layout .parent() .into_iter() - .flat_map(|parent| parent.ancestors()) - .filter(|parent| !JsParenthesizedExpression::can_cast(parent.kind())); + .flat_map(|parent| parent.ancestors()); let mut parent = None; let mut expression = JsAnyExpression::from(conditional.clone()); @@ -443,12 +430,7 @@ impl JsAnyConditional { JsSyntaxKind::JS_CALL_EXPRESSION => { let call_expression = JsCallExpression::unwrap_cast(ancestor); - if call_expression - .callee() - .map(ExpressionNode::into_resolved) - .as_ref() - == Ok(&expression) - { + if call_expression.callee().as_ref() == Ok(&expression) { Ancestor::MemberChain(call_expression.into()) } else { Ancestor::Root(call_expression.into_syntax()) @@ -458,12 +440,7 @@ impl JsAnyConditional { JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION => { let member_expression = JsStaticMemberExpression::unwrap_cast(ancestor); - if member_expression - .object() - .map(ExpressionNode::into_resolved) - .as_ref() - == Ok(&expression) - { + if member_expression.object().as_ref() == Ok(&expression) { Ancestor::MemberChain(member_expression.into()) } else { Ancestor::Root(member_expression.into_syntax()) @@ -472,12 +449,7 @@ impl JsAnyConditional { JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION => { let member_expression = JsComputedMemberExpression::unwrap_cast(ancestor); - if member_expression - .object() - .map(ExpressionNode::into_resolved) - .as_ref() - == Ok(&expression) - { + if member_expression.object().as_ref() == Ok(&expression) { Ancestor::MemberChain(member_expression.into()) } else { Ancestor::Root(member_expression.into_syntax()) @@ -486,12 +458,7 @@ impl JsAnyConditional { JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => { let non_null_assertion = TsNonNullAssertionExpression::unwrap_cast(ancestor); - if non_null_assertion - .expression() - .map(ExpressionNode::into_resolved) - .as_ref() - == Ok(&expression) - { + if non_null_assertion.expression().as_ref() == Ok(&expression) { Ancestor::MemberChain(non_null_assertion.into()) } else { Ancestor::Root(non_null_assertion.into_syntax()) @@ -501,13 +468,8 @@ impl JsAnyConditional { let new_expression = JsNewExpression::unwrap_cast(ancestor); // Skip over new expressions - if new_expression - .callee() - .map(ExpressionNode::into_resolved) - .as_ref() - == Ok(&expression) - { - parent = new_expression.resolve_parent(); + if new_expression.callee().as_ref() == Ok(&expression) { + parent = new_expression.syntax().parent(); expression = new_expression.into(); break; } @@ -517,13 +479,8 @@ impl JsAnyConditional { JsSyntaxKind::TS_AS_EXPRESSION => { let as_expression = TsAsExpression::unwrap_cast(ancestor.clone()); - if as_expression - .expression() - .map(ExpressionNode::into_resolved) - .as_ref() - == Ok(&expression) - { - parent = as_expression.resolve_parent(); + if as_expression.expression().as_ref() == Ok(&expression) { + parent = as_expression.syntax().parent(); expression = as_expression.into(); break; } @@ -585,7 +542,7 @@ impl JsAnyConditional { _ => None, }; - argument.map_or(false, |argument| argument.resolve() == expression) + argument.map_or(false, |argument| argument == expression) } } } diff --git a/crates/rome_js_formatter/src/utils/jsx.rs b/crates/rome_js_formatter/src/utils/jsx.rs index 1fa140fb255..64cdc9cd096 100644 --- a/crates/rome_js_formatter/src/utils/jsx.rs +++ b/crates/rome_js_formatter/src/utils/jsx.rs @@ -1,5 +1,5 @@ use crate::context::QuoteStyle; -use crate::parentheses::NeedsParentheses; + use crate::prelude::*; use rome_formatter::{format_args, write}; use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, JsxAnyChild, JsxChildList, JsxTagExpression}; @@ -77,7 +77,7 @@ pub enum WrapState { pub fn get_wrap_state(node: &JsxTagExpression) -> WrapState { // We skip the first item because the first item in ancestors is the node itself, i.e. // the JSX Element in this case. - let parent = node.resolve_parent(); + let parent = node.syntax().parent(); parent.map_or(WrapState::NoWrap, |parent| match parent.kind() { JsSyntaxKind::JS_ARRAY_EXPRESSION @@ -112,16 +112,6 @@ pub fn is_jsx_inside_arrow_function_inside_call_inside_expression_child( // the JSX Element in this case. let mut ancestors = node.ancestors().skip(2).peekable(); - // This matching should work with or without parentheses around the JSX element - // therefore we ignore parenthesized expressions. - if ancestors - .peek() - .map(|a| a.kind() == JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION) - .unwrap_or(false) - { - ancestors.next(); - } - let required_ancestors = [ JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION, JsSyntaxKind::JS_CALL_ARGUMENT_LIST, diff --git a/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs b/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs index 2efa1c00cca..2a0680df8bf 100644 --- a/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs +++ b/crates/rome_js_formatter/src/utils/member_chain/chain_member.rs @@ -4,139 +4,12 @@ use rome_formatter::write; use rome_js_syntax::{ JsAnyExpression, JsCallExpression, JsCallExpressionFields, JsComputedMemberExpression, JsComputedMemberExpressionFields, JsIdentifierExpression, JsImportCallExpression, - JsNewExpression, JsParenthesizedExpression, JsStaticMemberExpression, + JsNewExpression, JsStaticMemberExpression, JsStaticMemberExpressionFields, JsSyntaxNode, JsThisExpression, }; use rome_rowan::{AstNode, SyntaxResult}; use std::fmt::Debug; -/// One entry in a member chain. -#[derive(Clone, Debug)] -pub(crate) enum ChainEntry { - /// A member that is parenthesized in the source document - Parenthesized { - /// The chain member - member: ChainMember, - /// The top most ancestor of the chain member that is a parenthesized expression. - /// - /// ```text - /// (a.b).c() - /// ^^^ -> member - /// ^----^ -> top_most_parentheses - /// - /// ((a.b)).c() - /// ^^^ -> member - /// ^-----^ -> top most parentheses (skips inner parentheses node) - /// ``` - top_most_parentheses: JsParenthesizedExpression, - }, - Member(ChainMember), -} - -impl ChainEntry { - /// Returns the inner member - pub fn member(&self) -> &ChainMember { - match self { - ChainEntry::Parenthesized { member, .. } => member, - ChainEntry::Member(member) => member, - } - } - - /// Returns the top most parentheses node if any - pub fn top_most_parentheses(&self) -> Option<&JsParenthesizedExpression> { - match self { - ChainEntry::Parenthesized { - top_most_parentheses, - .. - } => Some(top_most_parentheses), - ChainEntry::Member(_) => None, - } - } - - pub fn into_member(self) -> ChainMember { - match self { - ChainEntry::Parenthesized { member, .. } => member, - ChainEntry::Member(member) => member, - } - } - - pub(crate) fn has_trailing_comments(&self) -> bool { - self.nodes().any(|node| node.has_trailing_comments()) - } - - /// Returns true if the member any of it's ancestor parentheses nodes has any leading comments. - pub(crate) fn has_leading_comments(&self) -> SyntaxResult { - let has_operator_comment = match self.member() { - ChainMember::StaticMember(node) => node.operator_token()?.has_leading_comments(), - _ => false, - }; - - Ok(self.nodes().any(|node| node.has_leading_comments()) || has_operator_comment) - } - - fn nodes(&self) -> impl Iterator { - let first = match self { - ChainEntry::Parenthesized { - top_most_parentheses, - .. - } => top_most_parentheses.syntax().clone(), - ChainEntry::Member(member) => member.syntax().clone(), - }; - - let is_parenthesized = matches!(self, ChainEntry::Parenthesized { .. }); - - std::iter::successors(Some(first), move |previous| { - if is_parenthesized { - JsParenthesizedExpression::cast(previous.clone()).and_then(|parenthesized| { - parenthesized.expression().map(AstNode::into_syntax).ok() - }) - } else { - None - } - }) - } -} - -impl Format for ChainEntry { - fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { - let parentheses = self.top_most_parentheses(); - - if let Some(parentheses) = parentheses { - let mut current = parentheses.clone(); - - loop { - write!(f, [format_removed(¤t.l_paren_token()?)])?; - - match current.expression()? { - JsAnyExpression::JsParenthesizedExpression(inner) => { - current = inner; - } - _ => break, - } - } - } - - write!(f, [self.member()])?; - - if let Some(parentheses) = parentheses { - let mut current = parentheses.clone(); - - loop { - write!(f, [format_removed(¤t.r_paren_token()?)])?; - - match current.expression()? { - JsAnyExpression::JsParenthesizedExpression(inner) => { - current = inner; - } - _ => break, - } - } - } - - Ok(()) - } -} - /// Data structure that holds the node with its formatted version #[derive(Clone, Debug)] pub(crate) enum ChainMember { @@ -152,6 +25,20 @@ pub(crate) enum ChainMember { } impl ChainMember { + pub(crate) fn has_trailing_comments(&self) -> bool { + self.syntax().has_trailing_comments() + } + + /// Returns true if the member any of it's ancestor parentheses nodes has any leading comments. + pub(crate) fn has_leading_comments(&self) -> SyntaxResult { + let has_operator_comment = match self { + ChainMember::StaticMember(node) => node.operator_token()?.has_leading_comments(), + _ => false, + }; + + Ok(self.syntax().has_leading_comments() || has_operator_comment) + } + /// checks if the current node is a [rome_js_syntax::JsCallExpression], [rome_js_syntax::JsImportExpression] or a [rome_js_syntax::JsNewExpression] pub fn is_loose_call_expression(&self) -> bool { match self { diff --git a/crates/rome_js_formatter/src/utils/member_chain/groups.rs b/crates/rome_js_formatter/src/utils/member_chain/groups.rs index dc936581a90..949fc2b1411 100644 --- a/crates/rome_js_formatter/src/utils/member_chain/groups.rs +++ b/crates/rome_js_formatter/src/utils/member_chain/groups.rs @@ -1,7 +1,7 @@ use crate::context::TabWidth; use crate::parentheses::NeedsParentheses; use crate::prelude::*; -use crate::utils::member_chain::chain_member::{ChainEntry, ChainMember}; +use crate::utils::member_chain::chain_member::ChainMember; use rome_formatter::write; use rome_js_syntax::JsCallExpression; use rome_rowan::SyntaxResult; @@ -32,14 +32,14 @@ impl MemberChainGroupsBuilder { } /// starts a new group - pub fn start_group(&mut self, flatten_item: ChainEntry) { - debug_assert!(self.current_group.entries.is_empty()); - self.current_group.entries.push(flatten_item); + pub fn start_group(&mut self, flatten_item: ChainMember) { + debug_assert!(self.current_group.members.is_empty()); + self.current_group.members.push(flatten_item); } /// continues of starts a new group - pub fn start_or_continue_group(&mut self, flatten_item: ChainEntry) { - if self.current_group.entries.is_empty() { + pub fn start_or_continue_group(&mut self, flatten_item: ChainMember) { + if self.current_group.members.is_empty() { self.start_group(flatten_item); } else { self.continue_group(flatten_item); @@ -47,14 +47,14 @@ impl MemberChainGroupsBuilder { } /// adds the passed element to the current group - pub fn continue_group(&mut self, flatten_item: ChainEntry) { - debug_assert!(!self.current_group.entries.is_empty()); - self.current_group.entries.push(flatten_item); + pub fn continue_group(&mut self, flatten_item: ChainMember) { + debug_assert!(!self.current_group.members.is_empty()); + self.current_group.members.push(flatten_item); } /// clears the current group, and adds a new group to the groups pub fn close_group(&mut self) { - if !self.current_group.entries.is_empty() { + if !self.current_group.members.is_empty() { let mut elements = MemberChainGroup::default(); std::mem::swap(&mut elements, &mut self.current_group); self.groups.push(elements); @@ -62,7 +62,7 @@ impl MemberChainGroupsBuilder { } pub(super) fn finish(self) -> MemberChainGroups { - debug_assert!(self.current_group.entries().is_empty()); + debug_assert!(self.current_group.members().is_empty()); MemberChainGroups { groups: self.groups, @@ -98,7 +98,7 @@ impl MemberChainGroups { Ok(!self.groups.len() >= 1 && self.should_not_wrap(head_group)? && !self.groups[0] - .entries + .members .first() .map_or(false, |item| item.has_trailing_comments())) } @@ -107,7 +107,7 @@ impl MemberChainGroups { pub fn has_comments(&self) -> SyntaxResult { let mut has_leading_comments = false; - let flat_groups = self.groups.iter().flat_map(|item| item.entries.iter()); + let flat_groups = self.groups.iter().flat_map(|item| item.members.iter()); for item in flat_groups { if item.has_leading_comments()? { has_leading_comments = true; @@ -118,13 +118,13 @@ impl MemberChainGroups { let has_trailing_comments = self .groups .iter() - .flat_map(|item| item.entries.iter()) + .flat_map(|item| item.members.iter()) .any(|item| item.has_trailing_comments()); let cutoff_has_leading_comments = if self.groups.len() >= self.cutoff as usize { let group = self.groups.get(self.cutoff as usize); if let Some(group) = group { - let first_item = group.entries.first(); + let first_item = group.members.first(); if let Some(first_item) = first_item { first_item.has_leading_comments()? } else { @@ -145,9 +145,9 @@ impl MemberChainGroups { pub fn get_call_expressions(&self) -> impl Iterator { self.groups .iter() - .flat_map(|group| group.entries.iter()) + .flat_map(|group| group.members.iter()) .filter_map(|item| { - if let ChainMember::CallExpression(call_expression, ..) = item.member() { + if let ChainMember::CallExpression(call_expression, ..) = item { Some(call_expression) } else { None @@ -163,16 +163,16 @@ impl MemberChainGroups { // SAFETY: guarded by the previous check let group = &self.groups[0]; group - .entries + .members .first() - .map_or(false, |item| item.member().is_computed_expression()) + .map_or(false, |item| item.is_computed_expression()) } else { false }; - if first_group.entries().len() == 1 { + if first_group.members().len() == 1 { // SAFETY: access is guarded by the previous check - let first_node = first_group.entries().first().unwrap().member(); + let first_node = first_group.members().first().unwrap(); return Ok(first_node.is_this_expression() || (first_node.is_identifier_expression() @@ -186,11 +186,9 @@ impl MemberChainGroups { let last_node_is_factory = self .groups .iter() - .flat_map(|group| group.entries.iter()) + .flat_map(|group| group.members.iter()) .last() - .map_or(false, |item| { - item.member().is_factory(false).unwrap_or(false) - }); + .map_or(false, |item| item.is_factory(false).unwrap_or(false)); Ok(last_node_is_factory || has_computed_property) } @@ -233,44 +231,44 @@ impl Format for MemberChainGroups { #[derive(Debug, Clone, Default)] pub(super) struct MemberChainGroup { - entries: Vec, + members: Vec, } impl MemberChainGroup { - pub(super) fn into_entries(self) -> Vec { - self.entries + pub(super) fn into_members(self) -> Vec { + self.members } - fn entries(&self) -> &[ChainEntry] { - &self.entries + fn members(&self) -> &[ChainMember] { + &self.members } - pub(super) fn expand_group(&mut self, group: impl IntoIterator) { - self.entries.extend(group) + pub(super) fn expand_group(&mut self, group: impl IntoIterator) { + self.members.extend(group) } pub(super) fn has_comments(&self) -> bool { - self.entries.iter().any(|item| item.has_trailing_comments()) + self.members.iter().any(|item| item.has_trailing_comments()) } } -impl From> for MemberChainGroup { - fn from(entries: Vec) -> Self { - Self { entries } +impl From> for MemberChainGroup { + fn from(entries: Vec) -> Self { + Self { members: entries } } } impl Format for MemberChainGroup { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { - let last = self.entries.last(); + let last = self.members.last(); - let needs_parens = last.map_or(false, |last| match last.member() { + let needs_parens = last.map_or(false, |last| match last { ChainMember::StaticMember(member) => member.needs_parentheses(), ChainMember::ComputedMember(member) => member.needs_parentheses(), _ => false, }); - let format_entries = format_with(|f| f.join().entries(self.entries.iter()).finish()); + let format_entries = format_with(|f| f.join().entries(self.members.iter()).finish()); if needs_parens { write!(f, [text("("), format_entries, text(")")]) diff --git a/crates/rome_js_formatter/src/utils/member_chain/mod.rs b/crates/rome_js_formatter/src/utils/member_chain/mod.rs index 5a24aeb2114..75e41ad5975 100644 --- a/crates/rome_js_formatter/src/utils/member_chain/mod.rs +++ b/crates/rome_js_formatter/src/utils/member_chain/mod.rs @@ -106,9 +106,9 @@ mod chain_member; mod groups; mod simple_argument; -use crate::parentheses::NeedsParentheses; + use crate::prelude::*; -use crate::utils::member_chain::chain_member::{ChainEntry, ChainMember}; +use crate::utils::member_chain::chain_member::ChainMember; use crate::utils::member_chain::groups::{ MemberChainGroup, MemberChainGroups, MemberChainGroupsBuilder, }; @@ -194,9 +194,10 @@ pub(crate) fn get_member_chain( f: &mut JsFormatter, ) -> SyntaxResult { let mut chain_members = vec![]; - let parent_is_expression_statement = call_expression.resolve_parent().map_or(false, |parent| { - JsExpressionStatement::can_cast(parent.kind()) - }); + let parent_is_expression_statement = + call_expression.syntax().parent().map_or(false, |parent| { + JsExpressionStatement::can_cast(parent.kind()) + }); let root = flatten_member_chain( &mut chain_members, @@ -210,7 +211,7 @@ pub(crate) fn get_member_chain( // will be used later to decide on how to format it let calls_count = chain_members .iter() - .filter(|item| item.member().is_loose_call_expression()) + .filter(|item| item.is_loose_call_expression()) .count(); // as explained before, the first group is particular, so we calculate it @@ -235,7 +236,7 @@ pub(crate) fn get_member_chain( if let Some(group_to_merge) = rest_of_groups.should_merge_with_first_group(&head_group) { let group_to_merge = group_to_merge .into_iter() - .flat_map(|group| group.into_entries()); + .flat_map(|group| group.into_members()); head_group.expand_group(group_to_merge); } @@ -249,7 +250,7 @@ pub(crate) fn get_member_chain( /// Retrieves the index where we want to calculate the first group. /// The first group gathers inside it all those nodes that are not a sequence of something like: /// `[ StaticMemberExpression -> AnyNode + JsCallExpression ]` -fn compute_first_group_index(flatten_items: &[ChainEntry]) -> usize { +fn compute_first_group_index(flatten_items: &[ChainMember]) -> usize { flatten_items .iter() .enumerate() @@ -257,7 +258,7 @@ fn compute_first_group_index(flatten_items: &[ChainEntry]) -> usize { .skip(1) // we now find the index, all items before this index will belong to the first group .find_map(|(index, item)| { - let should_skip = match item.member() { + let should_skip = match item { // This where we apply the first two points explained in the description of the main public function. // We want to keep iterating over the items until we have call expressions or computed expressions: // - `something()()()()` @@ -298,7 +299,7 @@ fn compute_first_group_index(flatten_items: &[ChainEntry]) -> usize { ChainMember::StaticMember(_) => { let next_flatten_item = &flatten_items[index + 1]; matches!( - next_flatten_item.member(), + next_flatten_item, ChainMember::StaticMember(_) | ChainMember::ComputedMember(_) ) } @@ -319,7 +320,7 @@ fn compute_first_group_index(flatten_items: &[ChainEntry]) -> usize { /// computes groups coming after the first group fn compute_groups( - flatten_items: impl Iterator, + flatten_items: impl Iterator, in_expression_statement: bool, f: &JsFormatter, ) -> MemberChainGroups { @@ -327,9 +328,9 @@ fn compute_groups( let mut groups_builder = MemberChainGroupsBuilder::new(in_expression_statement, f.context().tab_width()); for item in flatten_items { - let has_trailing_comments = item.member().syntax().has_trailing_comments(); + let has_trailing_comments = item.syntax().has_trailing_comments(); - match item.member() { + match item { ChainMember::StaticMember(_) => { // if we have seen a JsCallExpression, we want to close the group. // The resultant group will be something like: [ . , then, () ]; @@ -345,7 +346,7 @@ fn compute_groups( } } ChainMember::CallExpression(_) => { - let is_loose_call_expression = item.member().is_loose_call_expression(); + let is_loose_call_expression = item.is_loose_call_expression(); groups_builder.start_or_continue_group(item); if is_loose_call_expression { has_seen_call_expression = true; @@ -374,14 +375,14 @@ fn compute_groups( /// This function tries to flatten the AST. It stores nodes and its formatted version /// inside an vector of [FlattenItem]. The first element of the vector is the last one. fn flatten_member_chain( - queue: &mut Vec, + queue: &mut Vec, node: JsAnyExpression, comments: &Comments, -) -> SyntaxResult { +) -> SyntaxResult { use JsAnyExpression::*; if comments.is_suppressed(node.syntax()) { - return Ok(ChainEntry::Member(ChainMember::Node(node.into_syntax()))); + return Ok(ChainMember::Node(node.into_syntax())); } match node { @@ -390,16 +391,14 @@ fn flatten_member_chain( let left = flatten_member_chain(queue, callee, comments)?; queue.push(left); - Ok(ChainEntry::Member(ChainMember::CallExpression( - call_expression, - ))) + Ok(ChainMember::CallExpression(call_expression)) } JsStaticMemberExpression(static_member) => { let object = static_member.object()?; let left = flatten_member_chain(queue, object, comments)?; queue.push(left); - Ok(ChainEntry::Member(ChainMember::StaticMember(static_member))) + Ok(ChainMember::StaticMember(static_member)) } JsComputedMemberExpression(computed_expression) => { @@ -408,21 +407,9 @@ fn flatten_member_chain( let left = flatten_member_chain(queue, object, comments)?; queue.push(left); - Ok(ChainEntry::Member(ChainMember::ComputedMember( - computed_expression, - ))) - } - JsParenthesizedExpression(parenthesized) => { - let inner = flatten_member_chain(queue, parenthesized.expression()?, comments)?; - - Ok(ChainEntry::Parenthesized { - member: inner.into_member(), - top_most_parentheses: parenthesized, - }) + Ok(ChainMember::ComputedMember(computed_expression)) } - expression => Ok(ChainEntry::Member(ChainMember::Node( - expression.into_syntax(), - ))), + expression => Ok(ChainMember::Node(expression.into_syntax())), } } diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/binary-expr.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/binary-expr.js.snap deleted file mode 100644 index b14dc3f4a22..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/binary-expr.js.snap +++ /dev/null @@ -1,30 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -var a = b || /** @type {string} */ - (c); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1 +1 @@ --var a = b || /** @type {string} */ (c); -+var a = b || /** @type {string} */ c; -``` - -# Output - -```js -var a = b || /** @type {string} */ c; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/extra-spaces-and-asterisks.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/extra-spaces-and-asterisks.js.snap deleted file mode 100644 index 3b3d0662a35..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/extra-spaces-and-asterisks.js.snap +++ /dev/null @@ -1,57 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -const foo1 = /** @type {!Foo} */(bar); -const foo2 = /** @type {!Foo} **/(bar); -const foo3 = /** @type {!Foo} * */(bar); -const foo4 = /** @type {!Foo} ***/(bar); -const foo5 = /** @type {!Foo} * * */(bar); -const foo6 = /** @type {!Foo} *****/(bar); -const foo7 = /** @type {!Foo} * * * * */(bar); -const foo8 = /** @type {!Foo} ** * * */(bar); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,8 +1,8 @@ --const foo1 = /** @type {!Foo} */ (bar); --const foo2 = /** @type {!Foo} **/ (bar); --const foo3 = /** @type {!Foo} * */ (bar); --const foo4 = /** @type {!Foo} ***/ (bar); --const foo5 = /** @type {!Foo} * * */ (bar); --const foo6 = /** @type {!Foo} *****/ (bar); --const foo7 = /** @type {!Foo} * * * * */ (bar); --const foo8 = /** @type {!Foo} ** * * */ (bar); -+const foo1 = /** @type {!Foo} */ bar; -+const foo2 = /** @type {!Foo} **/ bar; -+const foo3 = /** @type {!Foo} * */ bar; -+const foo4 = /** @type {!Foo} ***/ bar; -+const foo5 = /** @type {!Foo} * * */ bar; -+const foo6 = /** @type {!Foo} *****/ bar; -+const foo7 = /** @type {!Foo} * * * * */ bar; -+const foo8 = /** @type {!Foo} ** * * */ bar; -``` - -# Output - -```js -const foo1 = /** @type {!Foo} */ bar; -const foo2 = /** @type {!Foo} **/ bar; -const foo3 = /** @type {!Foo} * */ bar; -const foo4 = /** @type {!Foo} ***/ bar; -const foo5 = /** @type {!Foo} * * */ bar; -const foo6 = /** @type {!Foo} *****/ bar; -const foo7 = /** @type {!Foo} * * * * */ bar; -const foo8 = /** @type {!Foo} ** * * */ bar; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/superclass.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/superclass.js.snap deleted file mode 100644 index db1c0a6b5a5..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/superclass.js.snap +++ /dev/null @@ -1,29 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -class Foo extends /** @type {string} */ (Bar) {} -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1 +1 @@ --class Foo extends /** @type {string} */ (Bar) {} -+class Foo extends /** @type {string} */ Bar {} -``` - -# Output - -```js -class Foo extends /** @type {string} */ Bar {} -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/ways-to-specify-type.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/ways-to-specify-type.js.snap deleted file mode 100644 index a348cb594f0..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/ways-to-specify-type.js.snap +++ /dev/null @@ -1,75 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -const curlyBraces = /** @type {string} */ (foo); -const curlyBraces2 = /**@type {string} */ (foo); -const noWhitespace = /** @type{string} */ (foo); -const noWhitespace2 = /**@type{string} */ (foo); -const noBraces = /** @type string */ (foo); -const parens = /** @type (string | number) */ (foo); - -// Prettier just searches for "@type" and doesn't check the syntax of types. -const v1 = /** @type {} */ (value); -const v2 = /** @type {}} */ (value); -const v3 = /** @type } */ (value); -const v4 = /** @type { */ (value); -const v5 = /** @type {{} */ (value); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,13 +1,13 @@ --const curlyBraces = /** @type {string} */ (foo); --const curlyBraces2 = /**@type {string} */ (foo); --const noWhitespace = /** @type{string} */ (foo); --const noWhitespace2 = /**@type{string} */ (foo); --const noBraces = /** @type string */ (foo); --const parens = /** @type (string | number) */ (foo); -+const curlyBraces = /** @type {string} */ foo; -+const curlyBraces2 = /**@type {string} */ foo; -+const noWhitespace = /** @type{string} */ foo; -+const noWhitespace2 = /**@type{string} */ foo; -+const noBraces = /** @type string */ foo; -+const parens = /** @type (string | number) */ foo; - - // Prettier just searches for "@type" and doesn't check the syntax of types. --const v1 = /** @type {} */ (value); --const v2 = /** @type {}} */ (value); --const v3 = /** @type } */ (value); --const v4 = /** @type { */ (value); --const v5 = /** @type {{} */ (value); -+const v1 = /** @type {} */ value; -+const v2 = /** @type {}} */ value; -+const v3 = /** @type } */ value; -+const v4 = /** @type { */ value; -+const v5 = /** @type {{} */ value; -``` - -# Output - -```js -const curlyBraces = /** @type {string} */ foo; -const curlyBraces2 = /**@type {string} */ foo; -const noWhitespace = /** @type{string} */ foo; -const noWhitespace2 = /**@type{string} */ foo; -const noBraces = /** @type string */ foo; -const parens = /** @type (string | number) */ foo; - -// Prettier just searches for "@type" and doesn't check the syntax of types. -const v1 = /** @type {} */ value; -const v2 = /** @type {}} */ value; -const v3 = /** @type } */ value; -const v4 = /** @type { */ value; -const v5 = /** @type {{} */ value; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/export-default/binary_and_template.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/export-default/binary_and_template.js.snap deleted file mode 100644 index 4322f88d229..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/export-default/binary_and_template.js.snap +++ /dev/null @@ -1,29 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -export default (function() {} + foo)``; -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1 +1 @@ --export default (function () {} + foo)``; -+export default ((function () {}) + foo)``; -``` - -# Output - -```js -export default ((function () {}) + foo)``; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/export-default/class_instance.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/export-default/class_instance.js.snap deleted file mode 100644 index 41e86369725..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/export-default/class_instance.js.snap +++ /dev/null @@ -1,29 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -export default (class {}.getInstance()); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1 +1 @@ --export default (class {}.getInstance()); -+export default (class {}).getInstance(); -``` - -# Output - -```js -export default (class {}).getInstance(); -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/object-property-ignore/issue-5678.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/object-property-ignore/issue-5678.js.snap deleted file mode 100644 index 3042c24c37d..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/object-property-ignore/issue-5678.js.snap +++ /dev/null @@ -1,137 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -// #5678 -const refreshTokenPayload = { - type: 'refreshToken', - sub: this._id, - role: this.role, - // prettier-ignore - exp: now + (60 * 60 * 24 * 90), // (90 days) - }; - -export default { - // prettier-ignore - protagonist: " 0\r\n" + - "0 00\r\n" + - "00000\r\n" + - "0 0\r\n" + - "0 0", - - // prettier-ignore - wall: "00000\r\n" + - "00000\r\n" + - "00000\r\n" + - "00000\r\n" + - "00000", - - // prettier-ignore - cheese: "0\r\n" + - " 0\r\n" + - "000\r\n" + - "00 0\r\n" + - "00000", - - // prettier-ignore - enemy: "0 0\r\n" + - "00 00\r\n" + - "00000\r\n" + - "0 0 0\r\n" + - "00000", - - // prettier-ignore - home: "00000\r\n" + - "0 0\r\n" + - "0 0\r\n" + - "0 0\r\n" + - "00000", - - // prettier-ignore - dog: "00 00\r\n" + - "00000\r\n" + - "0 0\r\n" + - "0 0 0\r\n" + - " 000 " -} -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -4,7 +4,7 @@ - sub: this._id, - role: this.role, - // prettier-ignore -- exp: now + (60 * 60 * 24 * 90), // (90 days) -+ exp: now + (60 * 60 * 24 * 90), // (90 days) - }; - - export default { -``` - -# Output - -```js -// #5678 -const refreshTokenPayload = { - type: "refreshToken", - sub: this._id, - role: this.role, - // prettier-ignore - exp: now + (60 * 60 * 24 * 90), // (90 days) -}; - -export default { - // prettier-ignore - protagonist: " 0\r\n" + - "0 00\r\n" + - "00000\r\n" + - "0 0\r\n" + - "0 0", - - // prettier-ignore - wall: "00000\r\n" + - "00000\r\n" + - "00000\r\n" + - "00000\r\n" + - "00000", - - // prettier-ignore - cheese: "0\r\n" + - " 0\r\n" + - "000\r\n" + - "00 0\r\n" + - "00000", - - // prettier-ignore - enemy: "0 0\r\n" + - "00 00\r\n" + - "00000\r\n" + - "0 0 0\r\n" + - "00000", - - // prettier-ignore - home: "00000\r\n" + - "0 0\r\n" + - "0 0\r\n" + - "0 0\r\n" + - "00000", - - // prettier-ignore - dog: "00 00\r\n" + - "00000\r\n" + - "0 0\r\n" + - "0 0 0\r\n" + - " 000 ", -}; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/as/export_default_as.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/as/export_default_as.ts.snap deleted file mode 100644 index 7fd5bfbdf72..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/as/export_default_as.ts.snap +++ /dev/null @@ -1,29 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -export default (function log() {} as typeof console.log) -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1 +1 @@ --export default (function log() {} as typeof console.log); -+export default (function log() {}) as typeof console.log; -``` - -# Output - -```js -export default (function log() {}) as typeof console.log; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/export-default/function_as.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/export-default/function_as.ts.snap deleted file mode 100644 index 8ea53add4b4..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/export-default/function_as.ts.snap +++ /dev/null @@ -1,29 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -export default (function log(){} as typeof console.log); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1 +1 @@ --export default (function log() {} as typeof console.log); -+export default (function log() {}) as typeof console.log; -``` - -# Output - -```js -export default (function log() {}) as typeof console.log; -``` - - - From b2c77feaec6729a8010d4cfe7e6d55fee8473090 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sun, 21 Aug 2022 11:17:45 +0200 Subject: [PATCH 02/29] introduce and use syntax rewriter --- crates/rome_js_formatter/Cargo.toml | 1 + .../assignments/array_assignment_pattern.rs | 16 +- .../assignments/computed_member_assignment.rs | 18 +- .../js/assignments/identifier_assignment.rs | 18 +- .../assignments/object_assignment_pattern.rs | 16 +- .../assignments/parenthesized_assignment.rs | 20 +- .../assignments/static_member_assignment.rs | 18 +- .../src/js/expressions/call_arguments.rs | 9 - .../src/js/expressions/logical_expression.rs | 13 +- .../expressions/parenthesized_expression.rs | 39 +- .../expressions/static_member_expression.rs | 4 +- .../src/js/expressions/unary_expression.rs | 17 +- .../src/js/statements/return_statement.rs | 39 +- .../src/js/unknown/unknown_assignment.rs | 18 +- crates/rome_js_formatter/src/lib.rs | 175 +-- crates/rome_js_formatter/src/parentheses.rs | 99 +- crates/rome_js_formatter/src/preprocessor.rs | 466 +++++--- .../src/ts/assignments/as_assignment.rs | 16 +- .../non_null_assertion_assignment.rs | 18 +- .../assignments/type_assertion_assignment.rs | 19 +- .../tests/specs/js/module/comments.js.snap | 4 +- .../tests/specs/js/module/suppression.js.snap | 6 +- .../closure-compiler-type-cast.js.snap | 111 +- .../comment-in-the-middle.js.snap | 9 +- .../comment-placement.js.snap | 28 +- .../js/comments-closure-typecast/iife.js.snap | 67 -- .../issue-4124.js.snap | 57 +- .../issue-8045.js.snap | 54 +- .../issue-9358.js.snap | 51 - .../comments-closure-typecast/nested.js.snap | 55 - .../object-with-comment.js.snap | 36 +- .../styled-components.js.snap | 5 +- .../js/comments/function-declaration.js.snap | 14 +- .../js/comments/return-statement.js.snap | 15 +- .../arrow-params.js.snap | 4 +- .../tests/specs/prettier/js/do/do.js.snap | 15 +- .../binary_and_template.js.snap | 29 + .../class_instance.js.snap} | 8 +- .../js/logical_expressions/issue-7024.js.snap | 39 - .../logical_expression_operators.js.snap | 128 -- .../nullish_coalesing_operator.js.snap | 77 -- .../object-property-ignore/issue-5678.js.snap | 137 +++ .../fsharp_style_pipeline_operator.js.snap | 38 +- .../minimal_pipeline_operator.js.snap | 53 +- .../js/unary-expression/comments.js.snap | 597 ++++------ .../typescript/as/export_default_as.ts.snap | 29 + .../export-default/function_as.ts.snap | 29 + website/playground/package.json | 34 +- website/playground/pnpm-lock.yaml | 1034 ++++++++--------- 49 files changed, 1555 insertions(+), 2247 deletions(-) delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/iife.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-9358.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/nested.js.snap create mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/export-default/binary_and_template.js.snap rename crates/rome_js_formatter/tests/specs/prettier/js/{comments-closure-typecast/member.js.snap => export-default/class_instance.js.snap} (52%) delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/logical_expressions/issue-7024.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/logical_expressions/logical_expression_operators.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/nullish-coalescing/nullish_coalesing_operator.js.snap create mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/object-property-ignore/issue-5678.js.snap create mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/as/export_default_as.ts.snap create mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/export-default/function_as.ts.snap diff --git a/crates/rome_js_formatter/Cargo.toml b/crates/rome_js_formatter/Cargo.toml index 9a04cf2d11b..cf174a7ac47 100644 --- a/crates/rome_js_formatter/Cargo.toml +++ b/crates/rome_js_formatter/Cargo.toml @@ -11,6 +11,7 @@ license = "MIT" [dependencies] cfg-if = "1.0.0" rome_js_syntax = { path = "../rome_js_syntax" } +rome_js_factory = { path = "../rome_js_factory" } rome_formatter = { path = "../rome_formatter" } rome_rowan = { path = "../rome_rowan" } tracing = { version = "0.1.31", default-features = false, features = ["std"] } diff --git a/crates/rome_js_formatter/src/js/assignments/array_assignment_pattern.rs b/crates/rome_js_formatter/src/js/assignments/array_assignment_pattern.rs index 3ae9ee52af6..a27dcda70ab 100644 --- a/crates/rome_js_formatter/src/js/assignments/array_assignment_pattern.rs +++ b/crates/rome_js_formatter/src/js/assignments/array_assignment_pattern.rs @@ -1,7 +1,7 @@ -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use crate::prelude::*; use rome_formatter::write; -use rome_js_syntax::{JsAnyAssignmentPattern, JsArrayAssignmentPattern}; +use rome_js_syntax::JsArrayAssignmentPattern; use rome_js_syntax::{JsArrayAssignmentPatternFields, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -29,18 +29,6 @@ impl FormatNodeRule for FormatJsArrayAssignmentPattern } } -impl AssignmentNode for JsArrayAssignmentPattern { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - self.into() - } -} - impl NeedsParentheses for JsArrayAssignmentPattern { #[inline] fn needs_parentheses(&self) -> bool { diff --git a/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs b/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs index 47d3fd189a1..b008aeae6b0 100644 --- a/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs +++ b/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs @@ -1,10 +1,8 @@ use crate::prelude::*; use crate::js::expressions::computed_member_expression::JsAnyComputedMemberLike; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; -use rome_js_syntax::{ - JsAnyAssignment, JsAnyAssignmentPattern, JsComputedMemberAssignment, JsSyntaxNode, -}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsComputedMemberAssignment, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsComputedMemberAssignment; @@ -23,18 +21,6 @@ impl FormatNodeRule for FormatJsComputedMemberAssign } } -impl AssignmentNode for JsComputedMemberAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self)) - } -} - impl NeedsParentheses for JsComputedMemberAssignment { #[inline] fn needs_parentheses(&self) -> bool { diff --git a/crates/rome_js_formatter/src/js/assignments/identifier_assignment.rs b/crates/rome_js_formatter/src/js/assignments/identifier_assignment.rs index 3573cf5c46c..08524331d1b 100644 --- a/crates/rome_js_formatter/src/js/assignments/identifier_assignment.rs +++ b/crates/rome_js_formatter/src/js/assignments/identifier_assignment.rs @@ -1,9 +1,9 @@ use crate::prelude::*; use rome_formatter::write; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; -use rome_js_syntax::{JsAnyAssignment, JsForOfStatement, JsIdentifierAssignmentFields}; -use rome_js_syntax::{JsAnyAssignmentPattern, JsIdentifierAssignment, JsSyntaxNode}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsForOfStatement, JsIdentifierAssignmentFields}; +use rome_js_syntax::{JsIdentifierAssignment, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsIdentifierAssignment; @@ -20,18 +20,6 @@ impl FormatNodeRule for FormatJsIdentifierAssignment { } } -impl AssignmentNode for JsIdentifierAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self)) - } -} - impl NeedsParentheses for JsIdentifierAssignment { #[inline] fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { diff --git a/crates/rome_js_formatter/src/js/assignments/object_assignment_pattern.rs b/crates/rome_js_formatter/src/js/assignments/object_assignment_pattern.rs index 831621c63ef..f9b39bafa77 100644 --- a/crates/rome_js_formatter/src/js/assignments/object_assignment_pattern.rs +++ b/crates/rome_js_formatter/src/js/assignments/object_assignment_pattern.rs @@ -1,8 +1,8 @@ -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use crate::prelude::*; use crate::utils::JsObjectPatternLike; use rome_formatter::write; -use rome_js_syntax::{JsAnyAssignmentPattern, JsObjectAssignmentPattern, JsSyntaxNode}; +use rome_js_syntax::{JsObjectAssignmentPattern, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsObjectAssignmentPattern; @@ -21,18 +21,6 @@ impl FormatNodeRule for FormatJsObjectAssignmentPatte } } -impl AssignmentNode for JsObjectAssignmentPattern { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - self.clone().into() - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - self.into() - } -} - impl NeedsParentheses for JsObjectAssignmentPattern { #[inline] fn needs_parentheses(&self) -> bool { diff --git a/crates/rome_js_formatter/src/js/assignments/parenthesized_assignment.rs b/crates/rome_js_formatter/src/js/assignments/parenthesized_assignment.rs index 658e61a36f3..82add83c8a5 100644 --- a/crates/rome_js_formatter/src/js/assignments/parenthesized_assignment.rs +++ b/crates/rome_js_formatter/src/js/assignments/parenthesized_assignment.rs @@ -1,7 +1,7 @@ -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use crate::prelude::*; use rome_formatter::write; -use rome_js_syntax::{JsAnyAssignmentPattern, JsParenthesizedAssignment}; +use rome_js_syntax::JsParenthesizedAssignment; use rome_js_syntax::{JsParenthesizedAssignmentFields, JsSyntaxNode}; #[derive(Debug, Clone, Default)] @@ -34,22 +34,6 @@ impl FormatNodeRule for FormatJsParenthesizedAssignme } } -impl AssignmentNode for JsParenthesizedAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - let assignment = self.assignment().unwrap_or_else(|_| self.clone().into()); - - JsAnyAssignmentPattern::JsAnyAssignment(assignment) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - let assignment = self.assignment().unwrap_or_else(|_| self.into()); - - JsAnyAssignmentPattern::JsAnyAssignment(assignment) - } -} - impl NeedsParentheses for JsParenthesizedAssignment { #[inline] fn needs_parentheses(&self) -> bool { diff --git a/crates/rome_js_formatter/src/js/assignments/static_member_assignment.rs b/crates/rome_js_formatter/src/js/assignments/static_member_assignment.rs index c55bdab1201..899744853c5 100644 --- a/crates/rome_js_formatter/src/js/assignments/static_member_assignment.rs +++ b/crates/rome_js_formatter/src/js/assignments/static_member_assignment.rs @@ -1,9 +1,7 @@ use crate::js::expressions::static_member_expression::JsAnyStaticMemberLike; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use crate::prelude::*; -use rome_js_syntax::{ - JsAnyAssignment, JsAnyAssignmentPattern, JsStaticMemberAssignment, JsSyntaxNode, -}; +use rome_js_syntax::{JsStaticMemberAssignment, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub struct FormatJsStaticMemberAssignment; @@ -18,18 +16,6 @@ impl FormatNodeRule for FormatJsStaticMemberAssignment } } -impl AssignmentNode for JsStaticMemberAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self)) - } -} - impl NeedsParentheses for JsStaticMemberAssignment { #[inline] fn needs_parentheses(&self) -> bool { 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 dde224a48e9..6fa80892512 100644 --- a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs +++ b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs @@ -753,15 +753,6 @@ 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/logical_expression.rs b/crates/rome_js_formatter/src/js/expressions/logical_expression.rs index 22c5561f5f0..3e74ad1cfbf 100644 --- a/crates/rome_js_formatter/src/js/expressions/logical_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/logical_expression.rs @@ -25,17 +25,10 @@ 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 if parent.operator() != self.operator() { - true - } else { - // TODO: Parentheses should never be needed for the same operators BUT this is causing a re-formatting - // issue if a logical expression has an in-balanced tree. See issue-7024.js for a test case.. - // The way prettier solves this is by re-balancing the tree before formatting, something, Rome' doesn't yet support. - Ok(self.syntax()) != parent.left().map(AstNode::into_syntax).as_ref() - }; + parent.operator() != self.operator() + } else { + needs_binary_like_parentheses(&JsAnyBinaryLikeExpression::from(self.clone()), parent) } - - needs_binary_like_parentheses(&JsAnyBinaryLikeExpression::from(self.clone()), parent) } } 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 f31709c883f..6b773b44a84 100644 --- a/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/parenthesized_expression.rs @@ -1,9 +1,9 @@ use crate::prelude::*; -use rome_formatter::write; +use rome_formatter::{format_args, write}; use crate::parentheses::NeedsParentheses; use rome_js_syntax::{ - JsParenthesizedExpression, JsParenthesizedExpressionFields, JsSyntaxNode, + JsAnyExpression, JsParenthesizedExpression, JsParenthesizedExpressionFields, JsSyntaxNode, }; #[derive(Debug, Clone, Default)] @@ -21,16 +21,35 @@ impl FormatNodeRule for FormatJsParenthesizedExpressi r_paren_token, } = node.as_fields(); + let l_paren_token = l_paren_token?; let expression = expression?; - write!( - f, - [ - l_paren_token.format(), - expression.format(), - r_paren_token.format() - ] - ) + let should_hug = !(expression.syntax().has_comments_direct() + || l_paren_token.has_trailing_comments()) + && (matches!( + expression, + JsAnyExpression::JsObjectExpression(_) | JsAnyExpression::JsArrayExpression(_) + )); + + if should_hug { + write!( + f, + [ + l_paren_token.format(), + expression.format(), + r_paren_token.format() + ] + ) + } else { + write!( + f, + [group(&format_args![ + l_paren_token.format(), + soft_block_indent(&expression.format()), + r_paren_token.format() + ])] + ) + } } fn needs_parentheses(&self, item: &JsParenthesizedExpression) -> bool { 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 fd6a202a3eb..4819667eecb 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,7 +1,7 @@ use crate::prelude::*; use crate::js::expressions::computed_member_expression::JsAnyComputedMemberLike; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::{format_args, write}; use rome_js_syntax::{ JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyName, JsAssignmentExpression, @@ -133,7 +133,7 @@ impl JsAnyStaticMemberLike { Some(JsAnyExpression::JsNewExpression(_)) => StaticMemberLikeLayout::NoBreak, Some(JsAnyExpression::JsAssignmentExpression(assignment)) => { if matches!( - assignment.left()?.resolve(), + assignment.left()?, JsAnyAssignmentPattern::JsAnyAssignment( JsAnyAssignment::JsIdentifierAssignment(_) ) diff --git a/crates/rome_js_formatter/src/js/expressions/unary_expression.rs b/crates/rome_js_formatter/src/js/expressions/unary_expression.rs index f5ad35ba091..98967fd2e79 100644 --- a/crates/rome_js_formatter/src/js/expressions/unary_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/unary_expression.rs @@ -1,9 +1,9 @@ use crate::prelude::*; -use rome_formatter::write; +use rome_formatter::{format_args, write}; use crate::parentheses::{unary_like_expression_needs_parentheses, NeedsParentheses}; -use rome_js_syntax::{JsSyntaxNode}; +use rome_js_syntax::JsSyntaxNode; use rome_js_syntax::{JsSyntaxKind, JsUnaryExpression}; use rome_js_syntax::{JsUnaryExpressionFields, JsUnaryOperator}; @@ -32,7 +32,18 @@ impl FormatNodeRule for FormatJsUnaryExpression { write!(f, [space()])?; } - write![f, [argument.format(),]] + if argument.syntax().has_leading_comments() { + write!( + f, + [group(&format_args![ + format_inserted(JsSyntaxKind::L_PAREN), + soft_block_indent(&argument.format()), + format_inserted(JsSyntaxKind::R_PAREN) + ])] + ) + } else { + write![f, [argument.format()]] + } } fn needs_parentheses(&self, item: &JsUnaryExpression) -> bool { diff --git a/crates/rome_js_formatter/src/js/statements/return_statement.rs b/crates/rome_js_formatter/src/js/statements/return_statement.rs index 9f4bea10566..1493da0fb9e 100644 --- a/crates/rome_js_formatter/src/js/statements/return_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/return_statement.rs @@ -1,13 +1,11 @@ use crate::prelude::*; -use crate::utils::{FormatWithSemicolon, JsAnyBinaryLikeExpression, JsAnyBinaryLikeLeftExpression}; +use crate::utils::{FormatWithSemicolon, JsAnyBinaryLikeExpression}; use rome_formatter::{format_args, write}; -use crate::parentheses::get_expression_left_side; use rome_js_syntax::{ - JsAnyExpression, JsReturnStatement, JsReturnStatementFields, JsSequenceExpression, + JsAnyExpression, JsReturnStatement, JsReturnStatementFields, JsSequenceExpression, JsSyntaxKind, }; -use rome_rowan::SyntaxResult; #[derive(Debug, Clone, Default)] pub struct FormatJsReturnStatement; @@ -52,17 +50,14 @@ impl Format for FormatReturnOrThrowArgument<'_> { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { let argument = self.0; - if has_argument_leading_comments(argument)? { - let syntax = argument.syntax(); - let first_token = syntax.first_token(); - let last_token = syntax.last_token(); + if has_argument_leading_comments(argument) { write!( f, - [format_parenthesize( - first_token.as_ref(), + [ + format_inserted(JsSyntaxKind::L_PAREN), &block_indent(&argument.format()), - last_token.as_ref() - )] + format_inserted(JsSyntaxKind::R_PAREN) + ] ) } else if is_binary_or_sequence_argument(argument) { write!( @@ -85,27 +80,13 @@ impl Format for FormatReturnOrThrowArgument<'_> { /// /// Traversing the left nodes is necessary in case the first node is parenthesized because /// parentheses will be removed (and be re-added by the return statement, but only if the argument breaks) -fn has_argument_leading_comments(argument: &JsAnyExpression) -> SyntaxResult { +fn has_argument_leading_comments(argument: &JsAnyExpression) -> bool { if matches!(argument, JsAnyExpression::JsxTagExpression(_)) { // JSX formatting takes care of adding parens - return Ok(false); + return false; } - if argument.syntax().has_leading_comments() { - return Ok(true); - } - - let result = match get_expression_left_side(argument) { - Some(JsAnyBinaryLikeLeftExpression::JsAnyExpression(expression)) => { - has_argument_leading_comments(&expression)? - } - Some(JsAnyBinaryLikeLeftExpression::JsPrivateName(name)) => { - name.syntax().has_leading_comments() - } - None => false, - }; - - Ok(result) + argument.syntax().has_leading_comments() } fn is_binary_or_sequence_argument(argument: &JsAnyExpression) -> bool { diff --git a/crates/rome_js_formatter/src/js/unknown/unknown_assignment.rs b/crates/rome_js_formatter/src/js/unknown/unknown_assignment.rs index 204f9f5cdf9..df04763ceff 100644 --- a/crates/rome_js_formatter/src/js/unknown/unknown_assignment.rs +++ b/crates/rome_js_formatter/src/js/unknown/unknown_assignment.rs @@ -1,9 +1,7 @@ use crate::prelude::*; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; -use rome_js_syntax::{ - JsAnyAssignment, JsAnyAssignmentPattern, JsSyntaxKind, JsSyntaxNode, JsUnknownAssignment, -}; +use crate::parentheses::NeedsParentheses; +use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, JsUnknownAssignment}; use rome_rowan::AstNode; #[derive(Debug, Clone, Default)] @@ -23,18 +21,6 @@ impl FormatNodeRule for FormatJsUnknownAssignment { } } -impl AssignmentNode for JsUnknownAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self)) - } -} - impl NeedsParentheses for JsUnknownAssignment { fn needs_parentheses_with_parent(&self, _: &JsSyntaxNode) -> bool { self.syntax().parent().map_or(false, |parent| { diff --git a/crates/rome_js_formatter/src/lib.rs b/crates/rome_js_formatter/src/lib.rs index f17d615135c..7106bec3089 100644 --- a/crates/rome_js_formatter/src/lib.rs +++ b/crates/rome_js_formatter/src/lib.rs @@ -248,6 +248,16 @@ pub mod prelude; mod ts; pub mod utils; +#[cfg(test)] +mod check_reformat; +#[rustfmt::skip] +mod generated; +pub(crate) mod builders; +pub mod context; +mod parentheses; +mod preprocessor; +pub(crate) mod separated; + use rome_formatter::prelude::*; use rome_formatter::{write, Comments, CstFormatContext, Format, FormatLanguage}; use rome_formatter::{Buffer, FormatOwnedWithRule, FormatRefWithRule, Formatted, Printed}; @@ -499,124 +509,6 @@ impl FormatLanguage for JsFormatLanguage { root: &SyntaxNode, ) -> SyntaxNode { preprocess(root) - - // - // - // for node in root - // .descendants() - // .filter_map(JsParenthesizedExpression::cast) - // { - // match ( - // node.l_paren_token(), - // node.expression(), - // node.r_paren_token(), - // ) { - // (Ok(l_paren_token), Ok(inner), Ok(r_paren_token)) => { - // // Don't remove parentheses if the left or right parens has any skipped token trivia attached - // // or the inner node is an unknown node (Rome can't know if an unknown needs parentheses or not) - // if l_paren_token.leading_trivia().has_skipped() - // || r_paren_token.leading_trivia().has_skipped() - // || inner.syntax().kind().is_unknown() - // { - // continue; - // } - // - // match ( - // l_paren_token.prev_token(), - // inner.syntax().first_token(), - // inner.syntax().last_token(), - // r_paren_token.next_token(), - // ) { - // (prev_token, Some(first_token), Some(last_token), Some(next_token)) => { - // let l_leading_trivia = l_paren_token.leading_trivia(); - // let l_trailing_trivia = l_paren_token.trailing_trivia(); - // - // if !l_leading_trivia.is_empty() && !l_trailing_trivia.is_empty() { - // match prev_token { - // Some(prev_token) if l_leading_trivia.is_empty() => { - // let new_prev_token = prev_token.with_trailing_trivia( - // prev_token - // .trailing_trivia() - // .pieces() - // .chain(l_trailing_trivia.pieces()) - // .collect::>() - // .iter() - // .map(|piece| (piece.kind(), piece.text())), - // ); - // - // mutation.replace_element_discard_trivia( - // SyntaxElement::Token(prev_token), - // SyntaxElement::Token(new_prev_token), - // ); - // } - // _ => { - // first_token.with_leading_trivia( - // l_leading_trivia - // .pieces() - // .chain(l_trailing_trivia.pieces()) - // .chain(first_token.leading_trivia().pieces()) - // .collect::>() - // .iter() - // .map(|piece| (piece.kind(), piece.text())), - // ); - // } - // } - // } - // - // let r_leading_trivia = r_paren_token.leading_trivia(); - // let r_trailing_trivia = r_paren_token.trailing_trivia(); - // - // // how does this fuckery work: - // // if first == last token - // // it may change leading/trailing trivia - // // if first != last: Two individual tokens - // - // // Now, fuckery level 100: What if nested: - // // Keep a vec of all concatenated leading / trailing pieces (for l_paren, and r_paren separately) - // // Otherwise apply the same logic - // - // // Issue, how to avoid re-visiting the same parenthesized expressions multiple times? - // // Handle parens in exit event of non paren node? - // - // if !r_leading_trivia.is_empty() || !r_trailing_trivia.is_empty() { - // if r_leading_trivia.is_empty() { - // let new_last_token = last_token.with_trailing_trivia( - // last_token - // .trailing_trivia() - // .pieces() - // .chain(r_trailing_trivia.pieces()) - // .collect::>() - // .iter() - // .map(|piece| (piece.kind(), piece.text())), - // ); - // mutation.replace_element_discard_trivia( - // SyntaxElement::Token(last_token), - // SyntaxElement::Token(new_last_token), - // ); - // } else { - // next_token.with_leading_trivia( - // r_leading_trivia - // .pieces() - // .chain(r_trailing_trivia.pieces()) - // .chain(next_token.leading_trivia().pieces()) - // .collect::>() - // .iter() - // .map(|piece| (piece.kind(), piece.text())), - // ); - // } - // } - // - // mutation.replace_element_discard_trivia( - // SyntaxElement::Node(node.into_syntax()), - // SyntaxElement::Node(inner.into_syntax()), - // ); - // } - // _ => continue, - // } - // } - // _ => {} - // } - // } } fn is_range_formatting_node(&self, node: &SyntaxNode) -> bool { @@ -693,13 +585,16 @@ pub fn format_sub_tree(context: JsFormatContext, root: &JsSyntaxNode) -> FormatR #[cfg(test)] mod tests { - use super::format_range; + use super::{format_node, format_range}; use crate::context::JsFormatContext; use rome_formatter::IndentStyle; - use rome_js_parser::parse_script; + use rome_js_parser::{parse, parse_script}; + use rome_js_syntax::SourceType; use rome_rowan::{TextRange, TextSize}; + use crate::check_reformat::{check_reformat, CheckReformatParams}; + #[test] fn test_range_formatting() { let input = " @@ -845,46 +740,28 @@ function() { assert_eq!(result.as_code(), ""); assert_eq!(result.range(), Some(TextRange::new(range_start, range_end))); } -} - -#[cfg(test)] -mod check_reformat; -#[rustfmt::skip] -mod generated; -pub(crate) mod builders; -pub mod context; -mod parentheses; -mod preprocessor; -pub(crate) mod separated; - -#[cfg(test)] -mod test { - use crate::check_reformat::{check_reformat, CheckReformatParams}; - use crate::{format_node, format_range, JsFormatContext}; - use rome_js_parser::parse; - use rome_js_syntax::{SourceType, TextRange, TextSize}; + #[ignore] #[test] // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" -(( - a) - /* comment */ - ); +test.expect(t => { + t.true(a); +}, false); "#; let syntax = SourceType::tsx(); let tree = parse(src, 0, syntax); let result = format_node(JsFormatContext::default(), &tree.syntax()) .unwrap() .print(); - // check_reformat(CheckReformatParams { - // root: &tree.syntax(), - // text: result.as_code(), - // source_type: syntax, - // file_name: "quick_test", - // format_context: JsFormatContext::default(), - // }); + check_reformat(CheckReformatParams { + root: &tree.syntax(), + text: result.as_code(), + source_type: syntax, + file_name: "quick_test", + format_context: JsFormatContext::default(), + }); assert_eq!( result.as_code(), "type B8 = /*1*/ (C);\ntype B9 = (/*1*/ C);\ntype B10 = /*1*/ /*2*/ C;\n" diff --git a/crates/rome_js_formatter/src/parentheses.rs b/crates/rome_js_formatter/src/parentheses.rs index f672b5191e8..68d26338476 100644 --- a/crates/rome_js_formatter/src/parentheses.rs +++ b/crates/rome_js_formatter/src/parentheses.rs @@ -50,8 +50,8 @@ use rome_js_syntax::{ JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsArrowFunctionExpression, JsAssignmentExpression, JsBinaryExpression, JsBinaryOperator, JsComputedMemberAssignment, JsComputedMemberExpression, - JsConditionalExpression, JsLanguage, JsParenthesizedExpression, JsSequenceExpression, - JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, + JsConditionalExpression, JsLanguage, JsParenthesizedAssignment, JsParenthesizedExpression, + JsSequenceExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, }; use rome_rowan::{declare_node_union, AstNode, SyntaxResult}; @@ -605,31 +605,32 @@ pub(crate) fn is_spread(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool { } declare_node_union! { - pub(crate) JsAnyParenthesizedNode = JsParenthesizedExpression + pub(crate) JsAnyParenthesized = JsParenthesizedExpression | JsParenthesizedAssignment } -impl JsAnyParenthesizedNode { +impl JsAnyParenthesized { pub(crate) fn l_paren_token(&self) -> SyntaxResult { match self { - JsAnyParenthesizedNode::JsParenthesizedExpression(expression) => { - expression.l_paren_token() - } + JsAnyParenthesized::JsParenthesizedExpression(expression) => expression.l_paren_token(), + JsAnyParenthesized::JsParenthesizedAssignment(assignment) => assignment.l_paren_token(), } } pub(crate) fn inner(&self) -> SyntaxResult { match self { - JsAnyParenthesizedNode::JsParenthesizedExpression(expression) => { + JsAnyParenthesized::JsParenthesizedExpression(expression) => { expression.expression().map(AstNode::into_syntax) } + JsAnyParenthesized::JsParenthesizedAssignment(assignment) => { + assignment.assignment().map(AstNode::into_syntax) + } } } pub(crate) fn r_paren_token(&self) -> SyntaxResult { match self { - JsAnyParenthesizedNode::JsParenthesizedExpression(expression) => { - expression.r_paren_token() - } + JsAnyParenthesized::JsParenthesizedExpression(expression) => expression.r_paren_token(), + JsAnyParenthesized::JsParenthesizedAssignment(assignment) => assignment.r_paren_token(), } } } @@ -642,53 +643,6 @@ pub(crate) fn is_binary_like_left_or_right(node: &JsSyntaxNode, parent: &JsSynta JsAnyBinaryLikeExpression::can_cast(parent.kind()) } -/// Trait implemented by all JavaScript assignments. -pub trait AssignmentNode: NeedsParentheses { - /// Resolves an assignment to the first non parenthesized assignment. - fn resolve(&self) -> JsAnyAssignmentPattern; - - /// Consumes `self` and returns the first assignment that isn't a parenthesized assignment. - fn into_resolved(self) -> JsAnyAssignmentPattern; - - /// Resolves an assignment to the first non parenthesized assignment and returns its [JsSyntaxNode]. - fn resolve_syntax(&self) -> JsSyntaxNode { - self.resolve().into_syntax() - } - - /// Consumes `self` and returns the [JsSyntaxNode] of the first assignment that isn't a assignment expression. - fn into_resolved_syntax(self) -> JsSyntaxNode { - self.into_resolved().into_syntax() - } -} - -impl AssignmentNode for JsAnyAssignment { - fn resolve(&self) -> JsAnyAssignmentPattern { - match self { - JsAnyAssignment::JsComputedMemberAssignment(assignment) => assignment.resolve(), - JsAnyAssignment::JsIdentifierAssignment(assignment) => assignment.resolve(), - JsAnyAssignment::JsParenthesizedAssignment(assignment) => assignment.resolve(), - JsAnyAssignment::JsStaticMemberAssignment(assignment) => assignment.resolve(), - JsAnyAssignment::JsUnknownAssignment(assignment) => assignment.resolve(), - JsAnyAssignment::TsAsAssignment(assignment) => assignment.resolve(), - JsAnyAssignment::TsNonNullAssertionAssignment(assignment) => assignment.resolve(), - JsAnyAssignment::TsTypeAssertionAssignment(assignment) => assignment.resolve(), - } - } - - fn into_resolved(self) -> JsAnyAssignmentPattern { - match self { - JsAnyAssignment::JsComputedMemberAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignment::JsIdentifierAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignment::JsParenthesizedAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignment::JsStaticMemberAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignment::JsUnknownAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignment::TsAsAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignment::TsNonNullAssertionAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignment::TsTypeAssertionAssignment(assignment) => assignment.into_resolved(), - } - } -} - impl NeedsParentheses for JsAnyAssignment { fn needs_parentheses(&self) -> bool { match self { @@ -741,28 +695,6 @@ impl NeedsParentheses for JsAnyAssignment { } } -impl AssignmentNode for JsAnyAssignmentPattern { - fn resolve(&self) -> JsAnyAssignmentPattern { - match self { - JsAnyAssignmentPattern::JsAnyAssignment(assignment) => assignment.resolve(), - JsAnyAssignmentPattern::JsArrayAssignmentPattern(assignment) => assignment.resolve(), - JsAnyAssignmentPattern::JsObjectAssignmentPattern(assignment) => assignment.resolve(), - } - } - - fn into_resolved(self) -> JsAnyAssignmentPattern { - match self { - JsAnyAssignmentPattern::JsAnyAssignment(assignment) => assignment.into_resolved(), - JsAnyAssignmentPattern::JsArrayAssignmentPattern(assignment) => { - assignment.into_resolved() - } - JsAnyAssignmentPattern::JsObjectAssignmentPattern(assignment) => { - assignment.into_resolved() - } - } - } -} - impl NeedsParentheses for JsAnyAssignmentPattern { fn needs_parentheses(&self) -> bool { match self { @@ -808,6 +740,7 @@ fn debug_assert_is_parent(node: &JsSyntaxNode, parent: &JsSyntaxNode) { #[cfg(test)] pub(crate) mod tests { use super::NeedsParentheses; + use crate::preprocess; use rome_js_syntax::{JsLanguage, SourceType}; use rome_rowan::AstNode; @@ -827,7 +760,8 @@ pub(crate) mod tests { ); let root = parse.syntax(); - let matching_nodes: Vec<_> = root.descendants().filter_map(T::cast).collect(); + let transformed = preprocess(&root); + let matching_nodes: Vec<_> = transformed.descendants().filter_map(T::cast).collect(); let node = if let Some(index) = index { matching_nodes.get(index).unwrap_or_else(|| { @@ -867,7 +801,8 @@ pub(crate) mod tests { ); let root = parse.syntax(); - let matching_nodes: Vec<_> = root.descendants().filter_map(T::cast).collect(); + let transformed = preprocess(&root); + let matching_nodes: Vec<_> = transformed.descendants().filter_map(T::cast).collect(); let node = if let Some(index) = index { matching_nodes.get(index).unwrap_or_else(|| { diff --git a/crates/rome_js_formatter/src/preprocessor.rs b/crates/rome_js_formatter/src/preprocessor.rs index 1e6ed198680..ca861bbbcdf 100644 --- a/crates/rome_js_formatter/src/preprocessor.rs +++ b/crates/rome_js_formatter/src/preprocessor.rs @@ -1,220 +1,256 @@ +use crate::parentheses::JsAnyParenthesized; use rome_js_syntax::{ - JsAnyExpression, JsLanguage, JsParenthesizedExpression, JsSyntaxNode, JsSyntaxToken, + JsAnyAssignment, JsAnyExpression, JsLanguage, JsLogicalExpression, JsSyntaxKind, JsSyntaxNode, }; -use rome_rowan::{AstNode, BatchMutation, SyntaxKind, SyntaxTriviaPiece}; -use std::collections::HashMap; -use std::iter::FusedIterator; +use rome_rowan::syntax::SyntaxTrivia; +use rome_rowan::{ + AstNode, Language, SyntaxKind, SyntaxNode, SyntaxSlot, SyntaxToken, SyntaxTriviaPiece, + SyntaxTriviaPieceComments, +}; +use std::iter::{once, FusedIterator}; pub(super) fn preprocess(root: &JsSyntaxNode) -> JsSyntaxNode { - let mut mutation = BatchMutation::new(root.clone()); - // tracks all changed tokens where the key is the token in the original tree and the value is the - // updated token. Necessary, because removing parentheses may require adding leading/trailing trivia - // to the same node: `(a) /* leading */ + (/* trailing 2*/ b)` Both comments should be merged with the `+` tokens trivia. - let mut tokens: HashMap = HashMap::new(); - - let mut prev_token_trailing = Vec::new(); - let mut first_inner_leading = Vec::new(); - let mut last_inner_trailing = Vec::new(); - let mut next_token_leading = Vec::new(); - - let mut parentheses: Option = None; - let mut prev_token: Option = None; - let mut next_token: Option = None; - - for node in root.descendants() { - match get_left_paren_inner_and_right_paren(node) { - Ok((l_paren, inner, r_paren, parenthesized)) => { - let l_paren_leading = l_paren.leading_trivia(); - let l_paren_trailing = l_paren.trailing_trivia(); - - if parentheses.is_none() { - debug_assert!(prev_token_trailing.is_empty()); - debug_assert!(first_inner_leading.is_empty()); - debug_assert!(last_inner_trailing.is_empty()); - debug_assert!(next_token_leading.is_empty()); - debug_assert!(prev_token.is_none()); - debug_assert!(next_token.is_none()); - - prev_token = l_paren.prev_token(); - next_token = r_paren.next_token(); - parentheses = Some(parenthesized); - } - - if first_inner_leading.is_empty() && l_paren_leading.is_empty() { - prev_token_trailing.extend(l_paren_trailing.pieces()); - } else { - first_inner_leading - .extend(l_paren_leading.pieces().chain(l_paren_trailing.pieces())); - } + rewrite(root.clone(), &mut JsFormatSyntaxRewriter::default()) +} - let r_paren_leading = r_paren.leading_trivia(); - let r_paren_trailing = r_paren.trailing_trivia(); +#[derive(Default)] +struct JsFormatSyntaxRewriter; + +impl JsFormatSyntaxRewriter { + /// Replaces parenthesized expression that: + /// * have no syntax error: has no missing required child or no skipped token trivia attached to the left or right paren + /// * inner expression isn't an unknown node + /// * no closure or type cast type cast comment + /// + /// with the inner expression. + fn visit_parenthesized( + &mut self, + parenthesized: JsAnyParenthesized, + ) -> VisitNodeSignal { + let (l_paren, inner, r_paren) = match ( + parenthesized.l_paren_token(), + parenthesized.inner(), + parenthesized.r_paren_token(), + ) { + (Ok(l_paren), Ok(inner), Ok(r_paren)) => { + let prev_token = l_paren.prev_token(); - if next_token_leading.is_empty() && r_paren_leading.is_empty() { - last_inner_trailing.extend(r_paren_trailing.pieces()); + // Keep parentheses around unknown expressions. Rome can't know the precedence. + if inner.kind().is_unknown() + // Don't remove parentheses if they have skipped trivia. We don't know for certain what the intended syntax is. + // Nor if there's a leading type cast comment + || has_type_cast_comment_or_skipped(&l_paren.leading_trivia()) + || prev_token.map_or(false, |prev_token| has_type_cast_comment_or_skipped(&prev_token.trailing_trivia())) + || r_paren.leading_trivia().has_skipped() + { + return VisitNodeSignal::Traverse(parenthesized.into_syntax()); } else { - next_token_leading - .extend(r_paren_leading.pieces().chain(r_paren_trailing.pieces())) + (l_paren, inner, r_paren) } } - Err(node) => { - let parenthesized = parentheses.take(); - let prev_token = prev_token.take(); - let next_token = next_token.take(); - - if !prev_token_trailing.is_empty() { - debug_assert!(parenthesized.is_some()); + _ => { + // At least one missing child, handle as a regular node + return VisitNodeSignal::Traverse(parenthesized.into_syntax()); + } + }; - match prev_token { - Some(prev_token) => { - let new_prev_token = tokens.get(&prev_token).unwrap_or(&prev_token); + let inner = rewrite(inner, self); - let new_prev_token = - new_prev_token.with_trailing_trivia_pieces(chain_pieces( - new_prev_token.trailing_trivia().pieces(), - prev_token_trailing.drain(..), - )); + match inner.first_token() { + None => { + // This can only happen if we have `()` which is highly unlikely to ever be the case. + // Return the parenthesized expression as is. This will be formatted as verbatim - tokens.insert(prev_token, new_prev_token); - } - None => { - // No previous token, everything becomes the leading of the inner token. - // May happen if the parenthesized expression is at the start of the program. - first_inner_leading.append(&mut prev_token_trailing); - } + let updated = match parenthesized { + JsAnyParenthesized::JsParenthesizedExpression(expression) => { + // SAFETY: Safe because the rewriter never rewrites an expression to a non expression. + expression + .with_expression(JsAnyExpression::unwrap_cast(inner)) + .into_syntax() } - } - - let mut new_node = node; - - if !first_inner_leading.is_empty() { - debug_assert!(parenthesized.is_some()); + JsAnyParenthesized::JsParenthesizedAssignment(assignment) => { + // SAFETY: Safe because the rewriter never rewrites an assignment to a non assignment. + assignment + .with_assignment(JsAnyAssignment::unwrap_cast(inner)) + .into_syntax() + } + }; - match new_node.first_token() { - Some(first_token) => { - let new_first_token = - tokens.remove(&first_token).unwrap_or(first_token.clone()); + VisitNodeSignal::Replace(updated) + } - let new_first_token = - new_first_token.with_leading_trivia_pieces(chain_pieces( - new_first_token.trailing_trivia().pieces(), - first_inner_leading.drain(..), - )); + Some(first_token) => { + let l_paren_trivia = chain_pieces( + l_paren.leading_trivia().pieces(), + l_paren.trailing_trivia().pieces(), + ); + + let new_leading = chain_pieces( + l_paren_trivia, + first_token + .leading_trivia() + .pieces() + // TODO: This requires source map support + .skip_while(|piece| piece.is_newline() || piece.is_whitespace()) + .collect::>() + .into_iter(), + ); + + let new_first = first_token.with_leading_trivia_pieces(new_leading); + + // SAFETY: Calling `unwrap` is safe because we know that `inner_first` is part of the `inner` subtree. + let updated = inner + .replace_child(first_token.into(), new_first.into()) + .unwrap(); + + let r_paren_trivia = chain_pieces( + r_paren.leading_trivia().pieces(), + r_paren.trailing_trivia().pieces(), + ); + + // SAFETY: Calling `unwrap` is safe because `last_token` only returns `None` if a node's subtree + // doesn't contain ANY token, but we know that the subtree contains at least the first token. + let last_token = updated.last_token().unwrap(); + let new_last = last_token.with_trailing_trivia_pieces(chain_pieces( + last_token.trailing_trivia().pieces(), + r_paren_trivia, + )); + + // SAFETY: Calling `unwrap` is safe because we know that `last_token` is part of the `updated` subtree. + VisitNodeSignal::Replace( + updated + .replace_child(last_token.into(), new_last.into()) + .unwrap(), + ) + } + } + } - new_node = new_node - .replace_child(first_token.into(), new_first_token.into()) - .unwrap(); - } - None => { - // The only case this can happen is if we have `()` which isn't valid code. - // Make the trivia the leading trivia of the next token (doesn't change order because `prev_token` will also be None). - next_token_leading.append(&mut first_inner_leading); + /// Re-balances right-recursive logical expressions with the same operator to be left recursive (relies on the parentheses removal) + /// + /// ```javascript + /// a && (b && c) + /// ``` + /// + /// has the tree (parentheses omitted) + /// + /// ```text + /// && + /// a && + /// b c + /// ``` + /// + /// This transform re-balances the tree so that it becomes left-recursive + /// + /// ```text + /// && + /// && c + /// a b + /// ``` + /// + /// This is required so that the binary like expression formatting only has to resolve left recursive expressions. + fn visit_logical_expression( + &mut self, + logical: JsLogicalExpression, + ) -> VisitNodeSignal { + match (logical.left(), logical.operator_token(), logical.right()) { + (Ok(left), Ok(operator), Ok(right)) => { + // SAFETY: Safe because the rewriter never rewrites an expression to a non expression. + let left = JsAnyExpression::unwrap_cast(rewrite(left.into_syntax(), self)); + let operator = self.visit_token(operator); + // SAFETY: Safe because the rewriter never rewrites an expression to a non expression. + let right = JsAnyExpression::unwrap_cast(rewrite(right.into_syntax(), self)); + + let updated = match right { + JsAnyExpression::JsLogicalExpression(right_logical) => { + match ( + right_logical.left(), + right_logical.operator_token(), + right_logical.right(), + ) { + (Ok(right_left), Ok(right_operator), Ok(right_right)) + if right_operator.kind() == operator.kind() => + { + logical + .with_left( + rome_js_factory::make::js_logical_expression( + left, operator, right_left, + ) + .into(), + ) + .with_operator_token_token(right_operator) + .with_right(right_right) + } + + // Don't re-balance a logical expression that has syntax errors + _ => logical + .with_left(left) + .with_operator_token_token(operator) + .with_right(right_logical.into()), } } - } - - if !last_inner_trailing.is_empty() { - debug_assert!(parenthesized.is_some()); - match new_node.last_token() { - Some(last_token) => { - let new_last_token = - tokens.remove(&last_token).unwrap_or(last_token.clone()); - - let new_last_token = - new_last_token.with_trailing_trivia_pieces(chain_pieces( - new_last_token.trailing_trivia().pieces(), - last_inner_trailing.drain(..), - )); + // Don't re-balance logical expressions with different operators + right => logical + .with_left(left) + .with_operator_token_token(operator) + .with_right(right), + }; - new_node = new_node - .replace_child(last_token.into(), new_last_token.into()) - .unwrap(); - } - None => { - // This happens if the expression is `()` which isn't valid code. - // Make the trivia the leading trivia of the next token (which hopefully will exist). - next_token_leading.append(&mut last_inner_trailing); - } - } - } + VisitNodeSignal::Replace(updated.into_syntax()) + } + _ => VisitNodeSignal::Traverse(logical.into_syntax()), + } + } +} - if !next_token_leading.is_empty() { - debug_assert!(parenthesized.is_some()); +impl SyntaxRewriter for JsFormatSyntaxRewriter { + type Language = JsLanguage; - match next_token { - Some(next_token) => { - let new_next_token = tokens.get(&next_token).unwrap_or(&next_token); - let new_next_token = - new_next_token.with_leading_trivia_pieces(chain_pieces( - next_token_leading.drain(..), - new_next_token.leading_trivia().pieces(), - )); + fn visit_node(&mut self, node: JsSyntaxNode) -> VisitNodeSignal { + match node.kind() { + kind if JsAnyParenthesized::can_cast(kind) => { + let parenthesized = JsAnyParenthesized::unwrap_cast(node); - tokens.insert(next_token, new_next_token); - } - None => { - panic!("Missing `EOF` token."); - } - } - } + self.visit_parenthesized(parenthesized) + } + JsSyntaxKind::JS_LOGICAL_EXPRESSION => { + let logical = JsLogicalExpression::unwrap_cast(node); - if let Some(parenthesized) = parenthesized { - mutation.replace_element_discard_trivia( - parenthesized.into_syntax().into(), - new_node.into(), - ); - } + self.visit_logical_expression(logical) } + _ => VisitNodeSignal::Traverse(node), } } +} - for (old, new) in tokens.into_iter() { - mutation.replace_element_discard_trivia(old.into(), new.into()) - } - - mutation.commit() +fn has_type_cast_comment_or_skipped(trivia: &SyntaxTrivia) -> bool { + trivia.pieces().any(|piece| { + if let Some(comment) = piece.as_comments() { + is_type_comment(&comment) + } else { + piece.is_skipped() + } + }) } -fn get_left_paren_inner_and_right_paren( - node: JsSyntaxNode, -) -> Result< - ( - JsSyntaxToken, - JsAnyExpression, - JsSyntaxToken, - JsParenthesizedExpression, - ), - JsSyntaxNode, -> { - match JsParenthesizedExpression::try_cast(node) { - Ok(parenthesized) => match ( - parenthesized.l_paren_token(), - parenthesized.expression(), - parenthesized.r_paren_token(), - ) { - (Ok(l_paren), Ok(expression), Ok(r_paren)) => { - // Keep parentheses around unknown expressions. Rome can't know the precedence. - if expression.syntax().kind().is_unknown() - // Don't remove parentheses if they have skipped trivia. We don't know for certain what the intended syntax is. - || l_paren.leading_trivia().has_skipped() - || r_paren.leading_trivia().has_skipped() - { - Err(parenthesized.into_syntax()) - } else if expression.syntax().first_token().is_none() { - // This should never happen but we need to be sure. - Err(parenthesized.into_syntax()) - } else { - Ok((l_paren, expression, r_paren, parenthesized)) - } - } - _ => { - // At least one missing child, handle as a regular node - Err(parenthesized.into_syntax()) - } - }, - Err(node) => Err(node), +/// Returns `true` if `comment` is a [Closure type comment](https://github.com/google/closure-compiler/wiki/Types-in-the-Closure-Type-System) +/// or [TypeScript type comment](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#type) +fn is_type_comment(comment: &SyntaxTriviaPieceComments) -> bool { + let text = comment.text(); + + // Must be a `/**` comment + if !text.starts_with("/**") { + return false; } + + text.trim_start_matches("/**") + .trim_end_matches("*/") + .split_whitespace() + .any(|word| match word.strip_prefix("@type") { + Some(after) => after.is_empty() || after.starts_with('{'), + None => false, + }) } fn chain_pieces(first: F, second: S) -> ChainTriviaPiecesIterator @@ -313,3 +349,61 @@ where } } } + +pub fn rewrite(root: SyntaxNode, rewriter: &mut R) -> SyntaxNode +where + R: SyntaxRewriter, +{ + match rewriter.visit_node(root) { + VisitNodeSignal::Replace(updated) => updated, + VisitNodeSignal::Traverse(mut parent) => { + for slot in parent.slots() { + match slot { + SyntaxSlot::Node(node) => { + let updated = rewrite(node.clone(), rewriter); + + if updated != node { + parent = parent.splice_slots( + node.index()..=node.index(), + once(Some(updated.into())), + ); + } + } + SyntaxSlot::Token(token) => { + let updated = rewriter.visit_token(token.clone()); + + if updated != token { + parent = parent.splice_slots( + token.index()..=token.index(), + once(Some(updated.into())), + ); + } + } + SyntaxSlot::Empty => { + // Nothing to visit + } + } + } + + parent + } + } +} + +pub trait SyntaxRewriter { + type Language: Language; + + fn visit_node(&mut self, node: SyntaxNode) -> VisitNodeSignal { + VisitNodeSignal::Traverse(node) + } + + fn visit_token(&mut self, token: SyntaxToken) -> SyntaxToken { + token + } +} + +#[derive(Debug, Clone)] +pub enum VisitNodeSignal { + Replace(SyntaxNode), + Traverse(SyntaxNode), +} diff --git a/crates/rome_js_formatter/src/ts/assignments/as_assignment.rs b/crates/rome_js_formatter/src/ts/assignments/as_assignment.rs index 8df765b4722..3dad08bb83f 100644 --- a/crates/rome_js_formatter/src/ts/assignments/as_assignment.rs +++ b/crates/rome_js_formatter/src/ts/assignments/as_assignment.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; -use rome_js_syntax::{JsAnyAssignment, JsAnyAssignmentPattern, TsAsAssignment}; +use rome_js_syntax::TsAsAssignment; use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, TsAsAssignmentFields}; #[derive(Debug, Clone, Default)] @@ -33,18 +33,6 @@ impl FormatNodeRule for FormatTsAsAssignment { } } -impl AssignmentNode for TsAsAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self)) - } -} - impl NeedsParentheses for TsAsAssignment { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { matches!( diff --git a/crates/rome_js_formatter/src/ts/assignments/non_null_assertion_assignment.rs b/crates/rome_js_formatter/src/ts/assignments/non_null_assertion_assignment.rs index da025d23cc0..8988da914d1 100644 --- a/crates/rome_js_formatter/src/ts/assignments/non_null_assertion_assignment.rs +++ b/crates/rome_js_formatter/src/ts/assignments/non_null_assertion_assignment.rs @@ -1,11 +1,9 @@ use crate::prelude::*; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_formatter::write; use rome_js_syntax::TsNonNullAssertionAssignmentFields; -use rome_js_syntax::{ - JsAnyAssignment, JsAnyAssignmentPattern, JsSyntaxNode, TsNonNullAssertionAssignment, -}; +use rome_js_syntax::{JsSyntaxNode, TsNonNullAssertionAssignment}; #[derive(Debug, Clone, Default)] pub struct FormatTsNonNullAssertionAssignment; @@ -28,18 +26,6 @@ impl FormatNodeRule for FormatTsNonNullAssertionAs } } -impl AssignmentNode for TsNonNullAssertionAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self)) - } -} - impl NeedsParentheses for TsNonNullAssertionAssignment { #[inline] fn needs_parentheses(&self) -> bool { diff --git a/crates/rome_js_formatter/src/ts/assignments/type_assertion_assignment.rs b/crates/rome_js_formatter/src/ts/assignments/type_assertion_assignment.rs index 4b64afe4904..4187c061d4b 100644 --- a/crates/rome_js_formatter/src/ts/assignments/type_assertion_assignment.rs +++ b/crates/rome_js_formatter/src/ts/assignments/type_assertion_assignment.rs @@ -1,11 +1,8 @@ use crate::prelude::*; use rome_formatter::write; -use rome_js_syntax::{ - JsAnyAssignment, JsAnyAssignmentPattern, JsSyntaxKind, JsSyntaxNode, - TsTypeAssertionAssignmentFields, -}; +use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, TsTypeAssertionAssignmentFields}; -use crate::parentheses::{AssignmentNode, NeedsParentheses}; +use crate::parentheses::NeedsParentheses; use rome_js_syntax::TsTypeAssertionAssignment; #[derive(Debug, Clone, Default)] @@ -39,18 +36,6 @@ impl FormatNodeRule for FormatTsTypeAssertionAssignme } } -impl AssignmentNode for TsTypeAssertionAssignment { - #[inline] - fn resolve(&self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self.clone())) - } - - #[inline] - fn into_resolved(self) -> JsAnyAssignmentPattern { - JsAnyAssignmentPattern::JsAnyAssignment(JsAnyAssignment::from(self)) - } -} - impl NeedsParentheses for TsTypeAssertionAssignment { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { matches!( diff --git a/crates/rome_js_formatter/tests/specs/js/module/comments.js.snap b/crates/rome_js_formatter/tests/specs/js/module/comments.js.snap index e86b472ab5d..3a280bed2b0 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/comments.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/comments.js.snap @@ -156,8 +156,8 @@ function name( 4 + /* plus trailing */ 3 * 2 /* 2 trailing */; -/* leading of opening */ /* trailing of opening */ 4 + 3 -/* leading of closing */ /* trailing of closing */; +/* leading of opening */ /* trailing of opening */ 4 + + 3 /* leading of closing */ /* trailing of closing */; [3 /* trailing num */ /* trailing sep */]; diff --git a/crates/rome_js_formatter/tests/specs/js/module/suppression.js.snap b/crates/rome_js_formatter/tests/specs/js/module/suppression.js.snap index b4f92e16543..e5699f50d98 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/suppression.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/suppression.js.snap @@ -55,9 +55,9 @@ if (true) statement(); const expr = // rome-ignore format: the array should not be formatted [ - (2*n)/(r-l), 0, (r+l)/(r-l), 0, - 0, (2*n)/(t-b), (t+b)/(t-b), 0, - 0, 0, -(f+n)/(f-n), -(2*f*n)/(f-n), + 2*n/r-l, 0, r+l/r-l, 0, + 0, 2*n/t-b, t+b/t-b, 0, + 0, 0, -f+n/f-n, -2*f*n/f-n, 0, 0, -1, 0, ]; diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/closure-compiler-type-cast.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/closure-compiler-type-cast.js.snap index bfeea10da12..53686ecb0e7 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/closure-compiler-type-cast.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/closure-compiler-type-cast.js.snap @@ -73,79 +73,14 @@ const style2 =/** ```diff --- Prettier +++ Rome -@@ -9,35 +9,35 @@ - }; - - // preserve parens only for type casts --let assignment = /** @type {string} */ (getValue()); --let value = /** @type {string} */ (this.members[0]).functionCall(); -+let assignment = /** @type {string} */ getValue(); -+let value = /** @type {string} */ this.members[0].functionCall(); - --functionCall(1 + /** @type {string} */ (value), /** @type {!Foo} */ ({})); -+functionCall(1 + /** @type {string} */ value, /** @type {!Foo} */ {}); - - function returnValue() { -- return /** @type {!Array.} */ (["hello", "you"]); -+ return /** @type {!Array.} */ ["hello", "you"]; - } - - // Only numberOrString is typecast --var newArray = /** @type {array} */ (numberOrString).map((x) => x); --var newArray = /** @type {array} */ (numberOrString).map((x) => x); --var newArray = test(/** @type {array} */ (numberOrString).map((x) => x)); --var newArray = test(/** @type {array} */ (numberOrString).map((x) => x)); -+var newArray = /** @type {array} */ numberOrString.map((x) => x); -+var newArray = /** @type {array} */ numberOrString.map((x) => x); -+var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); -+var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); - - // The numberOrString.map CallExpression is typecast --var newArray = /** @type {array} */ (numberOrString.map((x) => x)); --var newArray = /** @type {array} */ (numberOrString.map((x) => x)); --var newArray = test(/** @type {array} */ (numberOrString.map((x) => x))); --var newArray = test(/** @type {array} */ (numberOrString.map((x) => x))); -+var newArray = /** @type {array} */ numberOrString.map((x) => x); -+var newArray = /** @type {array} */ numberOrString.map((x) => x); -+var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); -+var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); - --test(/** @type {number} */ (num) + 1); --test(/** @type {!Array} */ (arrOrString).length + 1); --test(/** @type {!Array} */ (arrOrString).length + 1); -+test(/** @type {number} */ num + 1); -+test(/** @type {!Array} */ arrOrString.length + 1); -+test(/** @type {!Array} */ arrOrString.length + 1); - - const data = functionCall( - arg1, - arg2, -- /** @type {{height: number, width: number}} */ (arg3), -+ /** @type {{height: number, width: number}} */ arg3, - ); - - const style = /** @type {{ -@@ -47,16 +47,16 @@ - marginLeft: number, - marginRight: number, - marginBottom: number, --}} */ ({ -+}} */ { - width, - height, - ...margins, --}); -+}; - - const style2 = /** +@@ -57,6 +57,6 @@ * @type {{ * width: number, * }} - */ ({ -+*/ { ++*/ ({ width, --}); -+}; + }); ``` # Output @@ -162,35 +97,35 @@ let object = { }; // preserve parens only for type casts -let assignment = /** @type {string} */ getValue(); -let value = /** @type {string} */ this.members[0].functionCall(); +let assignment = /** @type {string} */ (getValue()); +let value = /** @type {string} */ (this.members[0]).functionCall(); -functionCall(1 + /** @type {string} */ value, /** @type {!Foo} */ {}); +functionCall(1 + /** @type {string} */ (value), /** @type {!Foo} */ ({})); function returnValue() { - return /** @type {!Array.} */ ["hello", "you"]; + return /** @type {!Array.} */ (["hello", "you"]); } // Only numberOrString is typecast -var newArray = /** @type {array} */ numberOrString.map((x) => x); -var newArray = /** @type {array} */ numberOrString.map((x) => x); -var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); -var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); +var newArray = /** @type {array} */ (numberOrString).map((x) => x); +var newArray = /** @type {array} */ (numberOrString).map((x) => x); +var newArray = test(/** @type {array} */ (numberOrString).map((x) => x)); +var newArray = test(/** @type {array} */ (numberOrString).map((x) => x)); // The numberOrString.map CallExpression is typecast -var newArray = /** @type {array} */ numberOrString.map((x) => x); -var newArray = /** @type {array} */ numberOrString.map((x) => x); -var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); -var newArray = test(/** @type {array} */ numberOrString.map((x) => x)); +var newArray = /** @type {array} */ (numberOrString.map((x) => x)); +var newArray = /** @type {array} */ (numberOrString.map((x) => x)); +var newArray = test(/** @type {array} */ (numberOrString.map((x) => x))); +var newArray = test(/** @type {array} */ (numberOrString.map((x) => x))); -test(/** @type {number} */ num + 1); -test(/** @type {!Array} */ arrOrString.length + 1); -test(/** @type {!Array} */ arrOrString.length + 1); +test(/** @type {number} */ (num) + 1); +test(/** @type {!Array} */ (arrOrString).length + 1); +test(/** @type {!Array} */ (arrOrString).length + 1); const data = functionCall( arg1, arg2, - /** @type {{height: number, width: number}} */ arg3, + /** @type {{height: number, width: number}} */ (arg3), ); const style = /** @type {{ @@ -200,19 +135,19 @@ const style = /** @type {{ marginLeft: number, marginRight: number, marginBottom: number, -}} */ { +}} */ ({ width, height, ...margins, -}; +}); const style2 = /** * @type {{ * width: number, * }} -*/ { +*/ ({ width, -}; +}); ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-in-the-middle.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-in-the-middle.js.snap index d61bb653ac9..9d5e8c2aae1 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-in-the-middle.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-in-the-middle.js.snap @@ -24,9 +24,8 @@ console.log(a.foo()); ```diff --- Prettier +++ Rome -@@ -1,11 +1,12 @@ +@@ -1,11 +1,11 @@ var a = -+ window /** - * bla bla bla - * @type {string | @@ -41,8 +40,7 @@ console.log(a.foo()); +* bla bla bla + */ //2 -- (window["s"]).toString(); -+ ["s"].toString(); + (window["s"]).toString(); console.log(a.foo()); ``` @@ -50,7 +48,6 @@ console.log(a.foo()); ```js var a = - window /** * bla bla bla * @type {string | @@ -59,7 +56,7 @@ var a = * bla bla bla */ //2 - ["s"].toString(); + (window["s"]).toString(); console.log(a.foo()); ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-placement.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-placement.js.snap index d33f9a5c481..053c9cf6526 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-placement.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/comment-placement.js.snap @@ -33,47 +33,37 @@ const foo5 = ```diff --- Prettier +++ Rome -@@ -1,13 +1,15 @@ --const foo1 = /** @type {string} */ (value); -+const foo1 = /** @type {string} */ value; - - const foo2 = - /** @type {string} */ -- (value); -+ value; - - const foo3 = +@@ -8,6 +8,8 @@ /** @type {string} */ -- (value); -+ value; + (value); -const foo4 = /** @type {string} */ (value); +const foo4 = -+ /** @type {string} */ value; ++ /** @type {string} */ (value); -const foo5 = /** @type {string} */ (value); +const foo5 = -+ /** @type {string} */ value; ++ /** @type {string} */ (value); ``` # Output ```js -const foo1 = /** @type {string} */ value; +const foo1 = /** @type {string} */ (value); const foo2 = /** @type {string} */ - value; + (value); const foo3 = /** @type {string} */ - value; + (value); const foo4 = - /** @type {string} */ value; + /** @type {string} */ (value); const foo5 = - /** @type {string} */ value; + /** @type {string} */ (value); ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/iife.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/iife.js.snap deleted file mode 100644 index 728349da835..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/iife.js.snap +++ /dev/null @@ -1,67 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -const helpers1 = /** @type {Helpers} */ (( - (helpers = {}) => helpers -)()); - -const helpers2 = /** @type {Helpers} */ (( - function() { return something } -)()); - -// TODO: @param is misplaced https://github.com/prettier/prettier/issues/5850 -const helpers = /** @type {Helpers} */ (( - /** @param {Partial} helpers */ - (helpers = {}) => helpers -)()); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,13 +1,10 @@ --const helpers1 = /** @type {Helpers} */ (((helpers = {}) => helpers)()); -+const helpers1 = /** @type {Helpers} */ ((helpers = {}) => helpers)(); - --const helpers2 = /** @type {Helpers} */ ( -- (function () { -- return something; -- })() --); -+const helpers2 = /** @type {Helpers} */ (function () { -+ return something; -+})(); - - // TODO: @param is misplaced https://github.com/prettier/prettier/issues/5850 --const helpers = /** @type {Helpers} */ ( -+const helpers = /** @type {Helpers} */ - /** @param {Partial} helpers */ -- ((helpers = {}) => helpers)() --); -+ ((helpers = {}) => helpers)(); -``` - -# Output - -```js -const helpers1 = /** @type {Helpers} */ ((helpers = {}) => helpers)(); - -const helpers2 = /** @type {Helpers} */ (function () { - return something; -})(); - -// TODO: @param is misplaced https://github.com/prettier/prettier/issues/5850 -const helpers = /** @type {Helpers} */ - /** @param {Partial} helpers */ - ((helpers = {}) => helpers)(); -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-4124.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-4124.js.snap index c561aea9791..96b30f08c0c 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-4124.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-4124.js.snap @@ -29,67 +29,42 @@ const test = /** @type (function (*): ?|undefined) */ (foo); ```diff --- Prettier +++ Rome -@@ -1,18 +1,22 @@ --/** @type {Object} */ (myObject.property).someProp = true; --/** @type {Object} */ (myObject.property).someProp = true; -+/** @type {Object} */ myObject.property.someProp = true; -+/** @type {Object} */ myObject.property.someProp = true; +@@ -3,9 +3,9 @@ --const prop = /** @type {Object} */ (myObject.property).someProp; -+const prop = /** @type {Object} */ myObject.property.someProp; + const prop = /** @type {Object} */ (myObject.property).someProp; -const test = - /** @type (function (*): ?|undefined) */ - (goog.partial(NewThing.onTemplateChange, rationaleField, typeField)); -+const test = /** @type (function (*): ?|undefined) */ goog.partial( -+ NewThing.onTemplateChange, -+ rationaleField, -+ typeField, ++const test = /** @type (function (*): ?|undefined) */ ( ++ goog.partial(NewThing.onTemplateChange, rationaleField, typeField) +); --const test = /** @type (function (*): ?|undefined) */ ( -- goog.partial(NewThing.onTemplateChange, rationaleField, typeField) -+const test = /** @type (function (*): ?|undefined) */ goog.partial( -+ NewThing.onTemplateChange, -+ rationaleField, -+ typeField, - ); - --const model = /** @type {?{getIndex: Function}} */ (model); -+const model = /** @type {?{getIndex: Function}} */ model; - --const foo = /** @type {string} */ (bar); -+const foo = /** @type {string} */ bar; - --const test = /** @type (function (*): ?|undefined) */ (foo); -+const test = /** @type (function (*): ?|undefined) */ foo; + const test = /** @type (function (*): ?|undefined) */ ( + goog.partial(NewThing.onTemplateChange, rationaleField, typeField) ``` # Output ```js -/** @type {Object} */ myObject.property.someProp = true; -/** @type {Object} */ myObject.property.someProp = true; +/** @type {Object} */ (myObject.property).someProp = true; +/** @type {Object} */ (myObject.property).someProp = true; -const prop = /** @type {Object} */ myObject.property.someProp; +const prop = /** @type {Object} */ (myObject.property).someProp; -const test = /** @type (function (*): ?|undefined) */ goog.partial( - NewThing.onTemplateChange, - rationaleField, - typeField, +const test = /** @type (function (*): ?|undefined) */ ( + goog.partial(NewThing.onTemplateChange, rationaleField, typeField) ); -const test = /** @type (function (*): ?|undefined) */ goog.partial( - NewThing.onTemplateChange, - rationaleField, - typeField, +const test = /** @type (function (*): ?|undefined) */ ( + goog.partial(NewThing.onTemplateChange, rationaleField, typeField) ); -const model = /** @type {?{getIndex: Function}} */ model; +const model = /** @type {?{getIndex: Function}} */ (model); -const foo = /** @type {string} */ bar; +const foo = /** @type {string} */ (bar); -const test = /** @type (function (*): ?|undefined) */ foo; +const test = /** @type (function (*): ?|undefined) */ (foo); ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-8045.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-8045.js.snap index 3b9bf50f337..94bf9a77f64 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-8045.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-8045.js.snap @@ -34,45 +34,42 @@ function jsdocCastInReturn() { ```diff --- Prettier +++ Rome -@@ -1,30 +1,20 @@ +@@ -1,5 +1,5 @@ -const myLongVariableName = - /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ (fooBarBaz); +const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ -+ fooBarBaz; ++ (fooBarBaz); function jsdocCastInReturn() { -- return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ ( -- fooBarBaz -- ); -+ return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; + return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ ( +@@ -7,24 +7,20 @@ + ); } -const myLongVariableName = - /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ -- (fooBarBaz); +const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ -+ fooBarBaz; + (fooBarBaz); function jsdocCastInReturn() { -- return ( + return ( - /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - (fooBarBaz) -- ); -+ return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; ++ /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ (fooBarBaz) + ); } -const myLongVariableName = - /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ -- (fooBarBaz); +const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ -+ fooBarBaz; + (fooBarBaz); function jsdocCastInReturn() { -- return ( + return ( - /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - (fooBarBaz) -- ); -+ return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; ++ /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ (fooBarBaz) + ); } ``` @@ -80,24 +77,30 @@ function jsdocCastInReturn() { ```js const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - fooBarBaz; + (fooBarBaz); function jsdocCastInReturn() { - return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; + return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ ( + fooBarBaz + ); } const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - fooBarBaz; + (fooBarBaz); function jsdocCastInReturn() { - return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; + return ( + /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ (fooBarBaz) + ); } const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - fooBarBaz; + (fooBarBaz); function jsdocCastInReturn() { - return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; + return ( + /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ (fooBarBaz) + ); } ``` @@ -105,10 +108,7 @@ function jsdocCastInReturn() { # Lines exceeding max width of 80 characters ``` 1: const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - 5: return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; - 8: const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - 12: return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; - 15: const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ - 19: return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ fooBarBaz; + 10: const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ + 19: const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-9358.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-9358.js.snap deleted file mode 100644 index 36b5e1881cd..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/issue-9358.js.snap +++ /dev/null @@ -1,51 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -const fooooba1 = /** @type {Array.} */ (fooobaarbazzItems || foo); -const fooooba2 = /** @type {Array.} */ (fooobaarbazzItems + foo); -const fooooba3 = /** @type {Array.} */ (fooobaarbazzItems || foo) ? foo : bar; -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,11 +1,6 @@ --const fooooba1 = /** @type {Array.} */ ( -- fooobaarbazzItems || foo --); --const fooooba2 = /** @type {Array.} */ ( -- fooobaarbazzItems + foo --); --const fooooba3 = /** @type {Array.} */ ( -- fooobaarbazzItems || foo --) -- ? foo -- : bar; -+const fooooba1 = /** @type {Array.} */ -+ fooobaarbazzItems || foo; -+const fooooba2 = /** @type {Array.} */ -+ fooobaarbazzItems + foo; -+const fooooba3 = /** @type {Array.} */ -+ fooobaarbazzItems || foo ? foo : bar; -``` - -# Output - -```js -const fooooba1 = /** @type {Array.} */ - fooobaarbazzItems || foo; -const fooooba2 = /** @type {Array.} */ - fooobaarbazzItems + foo; -const fooooba3 = /** @type {Array.} */ - fooobaarbazzItems || foo ? foo : bar; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/nested.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/nested.js.snap deleted file mode 100644 index 4ec2cee6b97..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/nested.js.snap +++ /dev/null @@ -1,55 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -foo = /** @type {!Foo} */ (/** @type {!Baz} */ (baz).bar ); - -const BarImpl = /** @type {BarConstructor} */ ( - /** @type {unknown} */ - (function Bar() { - throw new Error("Internal error: Illegal constructor"); - }) -); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,10 +1,7 @@ --foo = /** @type {!Foo} */ (/** @type {!Baz} */ (baz).bar); -+foo = /** @type {!Foo} */ /** @type {!Baz} */ baz.bar; - --const BarImpl = /** @type {BarConstructor} */ ( -+const BarImpl = /** @type {BarConstructor} */ - /** @type {unknown} */ -- ( -- function Bar() { -- throw new Error("Internal error: Illegal constructor"); -- } -- ) --); -+ function Bar() { -+ throw new Error("Internal error: Illegal constructor"); -+ }; -``` - -# Output - -```js -foo = /** @type {!Foo} */ /** @type {!Baz} */ baz.bar; - -const BarImpl = /** @type {BarConstructor} */ - /** @type {unknown} */ - function Bar() { - throw new Error("Internal error: Illegal constructor"); - }; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/object-with-comment.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/object-with-comment.js.snap index 15bafcfd80f..40b34891364 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/object-with-comment.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/object-with-comment.js.snap @@ -24,38 +24,34 @@ const objectWithComment2 = /** @type MyType */ ( /* comment */ { ```diff --- Prettier +++ Rome -@@ -1,12 +1,9 @@ --const objectWithComment = /** @type MyType */ ( -+const objectWithComment = /** @type MyType */ - /* comment */ - { - foo: bar, -- } --); -+ }; +@@ -5,8 +5,8 @@ + } + ); -const objectWithComment2 = /** @type MyType */ ( - /* comment */ { -- foo: bar, -- } --); -+const objectWithComment2 = /** @type MyType */ /* comment */ { -+ foo: bar, -+}; ++const objectWithComment2 = /** @type MyType */ (/* comment */ ++ { + foo: bar, + } + ); ``` # Output ```js -const objectWithComment = /** @type MyType */ +const objectWithComment = /** @type MyType */ ( /* comment */ { foo: bar, - }; + } +); -const objectWithComment2 = /** @type MyType */ /* comment */ { - foo: bar, -}; +const objectWithComment2 = /** @type MyType */ (/* comment */ + { + foo: bar, + } +); ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/styled-components.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/styled-components.js.snap index 93a6b1a6a44..95bab75730e 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/styled-components.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments-closure-typecast/styled-components.js.snap @@ -26,9 +26,8 @@ top: ${p => p.overlap === 'next' && 0}; @@ -1,10 +1,10 @@ const OverlapWrapper = /** @type {import('styled-components').ThemedStyledFunction<'div',null,{overlap: boolean}>} */ -- (styled.div)` + (styled.div)` - position: relative; -+ styled.div` +position:relative; > { - position: absolute; @@ -48,7 +47,7 @@ top: ${p => p.overlap === 'next' && 0}; ```js const OverlapWrapper = /** @type {import('styled-components').ThemedStyledFunction<'div',null,{overlap: boolean}>} */ - styled.div` + (styled.div)` position:relative; > { position: absolute; diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments/function-declaration.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments/function-declaration.js.snap index 4cc3745827c..6e2a946131b 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments/function-declaration.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments/function-declaration.js.snap @@ -74,14 +74,17 @@ function foo4() { ```diff --- Prettier +++ Rome -@@ -3,34 +3,32 @@ +@@ -3,34 +3,35 @@ function c(/* comment */ argA, argB, argC) {} // comment call((/*object*/ row) => {}); KEYPAD_NUMBERS.map( - ( - num, // Buttons 0-9 - ) =>
, -+ (num) =>
, // Buttons 0-9 ++ (num) => ( ++ // Buttons 0-9 ++
++ ), ); -function f1 /* f */() {} @@ -120,7 +123,7 @@ function foo4() { } class C2 { f(/* args */) {} -@@ -39,11 +37,12 @@ +@@ -39,11 +40,12 @@ f() /* returns */ {} } class C4 { @@ -146,7 +149,10 @@ function b() {} // comment function c(/* comment */ argA, argB, argC) {} // comment call((/*object*/ row) => {}); KEYPAD_NUMBERS.map( - (num) =>
, // Buttons 0-9 + (num) => ( + // Buttons 0-9 +
+ ), ); function f1 /* f */ () {} diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments/return-statement.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments/return-statement.js.snap index 107525ce77e..2c9d24ea4c5 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments/return-statement.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments/return-statement.js.snap @@ -151,17 +151,6 @@ function inlineComment() { } function excessiveEverything() { -@@ -103,8 +105,8 @@ - - function sequenceExpressionInside() { - return ( -- // Reason for a -- a, b -+ a, // Reason for a -+ b - ); - } - @@ -116,5 +118,7 @@ } @@ -283,8 +272,8 @@ function excessiveEverything() { function sequenceExpressionInside() { return ( - a, // Reason for a - b + // Reason for a + a, b ); } diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/destructuring-private-fields/arrow-params.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/destructuring-private-fields/arrow-params.js.snap index f703d420df2..af545138aa4 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/destructuring-private-fields/arrow-params.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/destructuring-private-fields/arrow-params.js.snap @@ -21,7 +21,7 @@ class C { class C { #x = 1; - #p = ({ #x: x }) => {}; -+ #p = ({ #x: x }) ++ #p = { #x: x } + => { +} } @@ -32,7 +32,7 @@ class C { ```js class C { #x = 1; - #p = ({ #x: x }) + #p = { #x: x } => { } } diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/do/do.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/do/do.js.snap index 06254dd5009..a06e183ae5b 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/do/do.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/do/do.js.snap @@ -67,7 +67,7 @@ function foo() { ```diff --- Prettier +++ Rome -@@ -1,55 +1,61 @@ +@@ -1,55 +1,60 @@ const envSpecific = { - domain: do { - if (env === "production") "https://abc.mno.com/"; @@ -106,8 +106,9 @@ function foo() { }; function foo() { - return ( -