diff --git a/crates/rome_js_formatter/src/comments.rs b/crates/rome_js_formatter/src/comments.rs index 811c9a44f9b..e7b0c435700 100644 --- a/crates/rome_js_formatter/src/comments.rs +++ b/crates/rome_js_formatter/src/comments.rs @@ -1,9 +1,22 @@ use crate::prelude::*; -use rome_formatter::write; -use rome_formatter::{CommentKind, CommentStyle, SourceComment}; +use crate::utils::JsAnyConditional; +use rome_formatter::{ + comments::{ + CommentKind, CommentPlacement, CommentStyle, CommentTextPosition, Comments, + DecoratedComment, SourceComment, + }, + write, +}; use rome_js_syntax::suppression::{parse_suppression_comment, SuppressionCategory}; -use rome_js_syntax::{JsLanguage, JsSyntaxKind}; -use rome_rowan::{SyntaxTriviaPieceComments, TextLen}; +use rome_js_syntax::{ + JsAnyClass, JsAnyName, JsAnyRoot, JsAnyStatement, JsArrayHole, JsArrowFunctionExpression, + JsBlockStatement, JsCallArguments, JsCatchClause, JsEmptyStatement, JsFinallyClause, + JsFormalParameter, JsFunctionBody, JsIdentifierExpression, JsIfStatement, JsLanguage, + JsSyntaxKind, JsSyntaxNode, JsVariableDeclarator, JsWhileStatement, TsInterfaceDeclaration, +}; +use rome_rowan::{AstNode, SyntaxTriviaPieceComments, TextLen}; + +pub type JsComments = Comments; #[derive(Default)] pub struct FormatJsLeadingComment; @@ -47,7 +60,7 @@ impl FormatRule> for FormatJsLeadingComment { )] ) } else { - write!(f, [comment.piece()]) + write!(f, [comment.piece().as_piece()]) } } } @@ -117,14 +130,16 @@ pub fn is_doc_comment(comment: &SyntaxTriviaPieceComments) -> bool { #[derive(Eq, PartialEq, Copy, Clone, Debug, Default)] pub struct JsCommentStyle; -impl CommentStyle for JsCommentStyle { - fn is_suppression(&self, text: &str) -> bool { +impl CommentStyle for JsCommentStyle { + type Language = JsLanguage; + + fn is_suppression(text: &str) -> bool { parse_suppression_comment(text) .flat_map(|suppression| suppression.categories) .any(|(category, _)| category == SuppressionCategory::Format) } - fn get_comment_kind(&self, comment: &SyntaxTriviaPieceComments) -> CommentKind { + fn get_comment_kind(comment: &SyntaxTriviaPieceComments) -> CommentKind { if comment.text().starts_with("/*") { if comment.has_newline() { CommentKind::Block @@ -136,26 +151,987 @@ impl CommentStyle for JsCommentStyle { } } - fn is_group_start_token(&self, kind: JsSyntaxKind) -> bool { - matches!( - kind, - JsSyntaxKind::L_PAREN - | JsSyntaxKind::L_BRACK - | JsSyntaxKind::L_CURLY - | JsSyntaxKind::DOLLAR_CURLY - ) + fn place_comment( + &self, + comment: DecoratedComment, + ) -> CommentPlacement { + match comment.text_position() { + CommentTextPosition::EndOfLine => handle_typecast_comment(comment) + .or_else(handle_function_declaration_comment) + .or_else(handle_conditional_comment) + .or_else(handle_if_statement_comment) + .or_else(handle_while_comment) + .or_else(handle_try_comment) + .or_else(handle_class_comment) + .or_else(handle_method_comment) + .or_else(handle_for_comment) + .or_else(handle_root_comments) + .or_else(handle_array_hole_comment) + .or_else(handle_variable_declarator_comment) + .or_else(handle_parameter_comment) + .or_else(handle_labelled_statement_comment) + .or_else(handle_call_expression_comment) + .or_else(handle_continue_break_comment) + .or_else(handle_mapped_type_comment) + .or_else(handle_switch_default_case_comment) + .or_else(handle_import_export_specifier_comment), + CommentTextPosition::OwnLine => handle_member_expression_comment(comment) + .or_else(handle_function_declaration_comment) + .or_else(handle_if_statement_comment) + .or_else(handle_while_comment) + .or_else(handle_try_comment) + .or_else(handle_class_comment) + .or_else(handle_method_comment) + .or_else(handle_for_comment) + .or_else(handle_root_comments) + .or_else(handle_parameter_comment) + .or_else(handle_array_hole_comment) + .or_else(handle_labelled_statement_comment) + .or_else(handle_call_expression_comment) + .or_else(handle_mapped_type_comment) + .or_else(handle_continue_break_comment) + .or_else(handle_union_type_comment) + .or_else(handle_import_export_specifier_comment), + CommentTextPosition::SameLine => handle_if_statement_comment(comment) + .or_else(handle_while_comment) + .or_else(handle_for_comment) + .or_else(handle_root_comments) + .or_else(handle_after_arrow_param_comment) + .or_else(handle_array_hole_comment) + .or_else(handle_call_expression_comment) + .or_else(handle_continue_break_comment) + .or_else(handle_class_comment), + } + } +} + +/// Force end of line type cast comments to remain leading comments of the next node, if any +fn handle_typecast_comment(comment: DecoratedComment) -> CommentPlacement { + match comment.following_node() { + Some(following_node) if is_type_comment(comment.piece()) => { + CommentPlacement::leading(following_node.clone(), comment) + } + _ => CommentPlacement::Default(comment), + } +} + +fn handle_after_arrow_param_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let is_next_arrow = comment.following_token().kind() == JsSyntaxKind::FAT_ARROW; + + // Makes comments after the `(` and `=>` dangling comments + // ```javascript + // () /* comment */ => true + // ``` + if JsArrowFunctionExpression::can_cast(comment.enclosing_node().kind()) && is_next_arrow { + CommentPlacement::dangling(comment.enclosing_node().clone(), comment) + } else { + CommentPlacement::Default(comment) + } +} + +/// Handles array hole comments. Array holes have no token so all comments +/// become trailing comments by default. Override it that all comments are leading comments. +fn handle_array_hole_comment( + comment: DecoratedComment, +) -> CommentPlacement { + if let Some(array_hole) = comment.preceding_node().and_then(JsArrayHole::cast_ref) { + CommentPlacement::leading(array_hole.into_syntax(), comment) + } else { + CommentPlacement::Default(comment) + } +} + +fn handle_call_expression_comment( + comment: DecoratedComment, +) -> CommentPlacement { + // Make comments between the callee and the arguments leading comments of the first argument. + // ```javascript + // a /* comment */ (call) + // ``` + if let Some(arguments) = comment.following_node().and_then(JsCallArguments::cast_ref) { + return if let Some(Ok(first)) = arguments.args().first() { + CommentPlacement::leading(first.into_syntax(), comment) + } else { + CommentPlacement::dangling(arguments.into_syntax(), comment) + }; + } + + CommentPlacement::Default(comment) +} + +fn handle_continue_break_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let enclosing = comment.enclosing_node(); + + // Make comments between the `continue` and label token trailing comments + // ```javascript + // continue /* comment */ a; + // ``` + // This differs from Prettier because other ASTs use an identifier for the label whereas Rome uses + // a token. + match enclosing.kind() { + JsSyntaxKind::JS_CONTINUE_STATEMENT | JsSyntaxKind::JS_BREAK_STATEMENT => { + match enclosing.parent() { + // Make it the trailing of the parent if this is a single-statement body + // to prevent that the comment becomes a trailing comment of the parent when re-formatting + // ```javascript + // for (;;) continue /* comment */; + // ``` + Some(parent) + if matches!( + parent.kind(), + JsSyntaxKind::JS_FOR_STATEMENT + | JsSyntaxKind::JS_FOR_OF_STATEMENT + | JsSyntaxKind::JS_FOR_IN_STATEMENT + | JsSyntaxKind::JS_WHILE_STATEMENT + | JsSyntaxKind::JS_DO_WHILE_STATEMENT + | JsSyntaxKind::JS_IF_STATEMENT + | JsSyntaxKind::JS_WITH_STATEMENT + | JsSyntaxKind::JS_LABELED_STATEMENT + ) => + { + CommentPlacement::trailing(parent, comment) + } + _ => CommentPlacement::trailing(enclosing.clone(), comment), + } + } + _ => CommentPlacement::Default(comment), + } +} + +/// Moves line comments after the `default` keyword into the block statement: +/// +/// ```javascript +/// switch (x) { +/// default: // comment +/// { +/// break; +/// } +/// ``` +/// +/// All other same line comments become `Dangling` comments that are handled inside of the default case +/// formatting +fn handle_switch_default_case_comment( + comment: DecoratedComment, +) -> CommentPlacement { + match (comment.enclosing_node().kind(), comment.following_node()) { + (JsSyntaxKind::JS_DEFAULT_CLAUSE, Some(following)) => { + match JsBlockStatement::cast_ref(following) { + Some(block) if comment.kind().is_line() => { + place_block_statement_comment(block, comment) + } + _ => CommentPlacement::dangling(comment.enclosing_node().clone(), comment), + } + } + _ => CommentPlacement::Default(comment), + } +} + +fn handle_labelled_statement_comment( + comment: DecoratedComment, +) -> CommentPlacement { + match comment.enclosing_node().kind() { + JsSyntaxKind::JS_LABELED_STATEMENT => { + CommentPlacement::leading(comment.enclosing_node().clone(), comment) + } + _ => CommentPlacement::Default(comment), + } +} + +fn handle_class_comment(comment: DecoratedComment) -> CommentPlacement { + // Make comments following after the `extends` or `implements` keyword trailing comments + // of the preceding extends, type parameters, or id. + // ```javascript + // class a9 extends + // /* comment */ + // b { + // constructor() {} + // } + // ``` + if matches!( + comment.enclosing_node().kind(), + JsSyntaxKind::JS_EXTENDS_CLAUSE + | JsSyntaxKind::TS_IMPLEMENTS_CLAUSE + | JsSyntaxKind::TS_EXTENDS_CLAUSE + ) { + if comment.preceding_node().is_none() && !comment.text_position().is_same_line() { + if let Some(sibling) = comment.enclosing_node().prev_sibling() { + return CommentPlacement::trailing(sibling, comment); + } + } + + return CommentPlacement::Default(comment); + } + + let first_member = if let Some(class) = JsAnyClass::cast_ref(comment.enclosing_node()) { + class.members().first().map(AstNode::into_syntax) + } else if let Some(interface) = TsInterfaceDeclaration::cast_ref(comment.enclosing_node()) { + interface.members().first().map(AstNode::into_syntax) + } else { + return CommentPlacement::Default(comment); + }; + + if comment.text_position().is_same_line() { + // Handle same line comments in empty class bodies + // ```javascript + // class Test { /* comment */ } + // ``` + if comment.following_token().kind() == JsSyntaxKind::R_CURLY && first_member.is_none() { + return CommentPlacement::dangling(comment.enclosing_node().clone(), comment); + } else { + return CommentPlacement::Default(comment); + } + } + + if let Some(following) = comment.following_node() { + // Make comments preceding the first member leading comments of the member + // ```javascript + // class Test { /* comment */ + // prop; + // } + // ``` + if let Some(member) = first_member { + if following == &member { + return CommentPlacement::leading(member, comment); + } + } + + if let Some(preceding) = comment.preceding_node() { + // Make all comments between the id/type parameters and the extends clause trailing comments + // of the id/type parameters + // + // ```javascript + // class Test + // + // /* comment */ extends B {} + // ``` + if matches!( + following.kind(), + JsSyntaxKind::JS_EXTENDS_CLAUSE + | JsSyntaxKind::TS_IMPLEMENTS_CLAUSE + | JsSyntaxKind::TS_EXTENDS_CLAUSE + ) && !comment.text_position().is_same_line() + { + return CommentPlacement::trailing(preceding.clone(), comment); + } + } + } else if first_member.is_none() { + // Handle the case where there are no members, attach the comments as dangling comments. + // ```javascript + // class Test // comment + // { + // } + // ``` + return CommentPlacement::dangling(comment.enclosing_node().clone(), comment); + } + + CommentPlacement::Default(comment) +} + +fn handle_method_comment(comment: DecoratedComment) -> CommentPlacement { + let enclosing = comment.enclosing_node(); + + let is_method = matches!( + enclosing.kind(), + JsSyntaxKind::JS_METHOD_CLASS_MEMBER + | JsSyntaxKind::JS_METHOD_OBJECT_MEMBER + | JsSyntaxKind::JS_SETTER_CLASS_MEMBER + | JsSyntaxKind::JS_GETTER_CLASS_MEMBER + | JsSyntaxKind::JS_SETTER_OBJECT_MEMBER + | JsSyntaxKind::JS_GETTER_OBJECT_MEMBER + | JsSyntaxKind::JS_CONSTRUCTOR_CLASS_MEMBER + ); + + if !is_method { + return CommentPlacement::Default(comment); + } + + // Move end of line and own line comments before the method body into the function + // ```javascript + // class Test { + // method() /* test */ + // {} + // } + // ``` + if let Some(following) = comment.following_node() { + if let Some(body) = JsFunctionBody::cast_ref(following) { + if let Some(directive) = body.directives().first() { + return CommentPlacement::leading(directive.into_syntax(), comment); + } + + let first_non_empty = body + .statements() + .iter() + .find(|statement| !matches!(statement, JsAnyStatement::JsEmptyStatement(_))); + + return match first_non_empty { + None => CommentPlacement::dangling(body.into_syntax(), comment), + Some(statement) => CommentPlacement::leading(statement.into_syntax(), comment), + }; + } + } + + CommentPlacement::Default(comment) +} + +/// Handle a all comments document. +/// See `blank.js` +fn handle_root_comments(comment: DecoratedComment) -> CommentPlacement { + if let Some(root) = JsAnyRoot::cast_ref(comment.enclosing_node()) { + let is_blank = match &root { + JsAnyRoot::JsExpressionSnipped(_) => false, + JsAnyRoot::JsModule(module) => { + module.directives().is_empty() && module.items().is_empty() + } + JsAnyRoot::JsScript(script) => { + script.directives().is_empty() && script.statements().is_empty() + } + }; + + if is_blank { + return CommentPlacement::leading(root.into_syntax(), comment); + } + } + + CommentPlacement::Default(comment) +} + +fn handle_member_expression_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let following = match comment.following_node() { + Some(following) + if matches!( + comment.enclosing_node().kind(), + JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION + | JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION + ) => + { + following + } + _ => return CommentPlacement::Default(comment), + }; + + // ```javascript + // a + // /* comment */.b + // a + // /* comment */[b] + // ``` + if JsAnyName::can_cast(following.kind()) || JsIdentifierExpression::can_cast(following.kind()) { + CommentPlacement::leading(comment.enclosing_node().clone(), comment) + } else { + CommentPlacement::Default(comment) + } +} + +fn handle_function_declaration_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let is_function_declaration = matches!( + comment.enclosing_node().kind(), + JsSyntaxKind::JS_FUNCTION_DECLARATION + | JsSyntaxKind::JS_FUNCTION_EXPORT_DEFAULT_DECLARATION + ); + + let following = match comment.following_node() { + Some(following) if is_function_declaration => following, + _ => return CommentPlacement::Default(comment), + }; + + // Make comments between the `)` token and the function body leading comments + // of the first non empty statement or dangling comments of the body. + // ```javascript + // function test() /* comment */ { + // console.log("Hy"); + // } + // ``` + if let Some(body) = JsFunctionBody::cast_ref(following) { + match body + .statements() + .iter() + .find(|statement| !matches!(statement, JsAnyStatement::JsEmptyStatement(_))) + { + Some(first) => CommentPlacement::leading(first.into_syntax(), comment), + None => CommentPlacement::dangling(body.into_syntax(), comment), + } + } else { + CommentPlacement::Default(comment) + } +} + +fn handle_conditional_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let enclosing = comment.enclosing_node(); + + let (conditional, following) = match ( + JsAnyConditional::cast_ref(enclosing), + comment.following_node(), + ) { + (Some(conditional), Some(following)) => (conditional, following), + _ => { + return CommentPlacement::Default(comment); + } + }; + + // Make end of line comments that come after the operator leading comments of the consequent / alternate. + // ```javascript + // a + // // becomes leading of consequent + // ? { x: 5 } : + // {}; + // + // a + // ? { x: 5 } + // : // becomes leading of alternate + // {}; + // + // a // remains trailing, because it directly follows the node + // ? { x: 5 } + // : {}; + // ``` + let token = comment.piece().as_piece().token(); + let is_after_operator = conditional.colon_token().as_ref() == Ok(&token) + || conditional.question_mark_token().as_ref() == Ok(&token); + + if is_after_operator { + return CommentPlacement::leading(following.clone(), comment); + } + + CommentPlacement::Default(comment) +} + +fn handle_if_statement_comment( + comment: DecoratedComment, +) -> CommentPlacement { + fn handle_else_clause( + comment: DecoratedComment, + consequent: JsSyntaxNode, + if_statement: JsSyntaxNode, + ) -> CommentPlacement { + // Make all comments trailing comments of the `consequent` if the `consequent` is a `JsBlockStatement` + // ```javascript + // if (test) { + // + // } /* comment */ else if (b) { + // test + // } + // /* comment */ else if(c) { + // + // } /*comment */ else { + // + // } + // ``` + if consequent.kind() == JsSyntaxKind::JS_BLOCK_STATEMENT { + return CommentPlacement::trailing(consequent, comment); + } + + // Handle end of line comments that aren't stretching over multiple lines. + // Make them dangling comments of the consequent expression + // + // ```javascript + // if (cond1) expr1; // comment A + // else if (cond2) expr2; // comment A + // else expr3; + // + // if (cond1) expr1; /* comment */ else expr2; + // + // if (cond1) expr1; /* b */ + // else if (cond2) expr2; /* b */ + // else expr3; /* b*/ + // ``` + if !comment.kind().is_block() + && !comment.text_position().is_own_line() + && comment.preceding_node().is_some() + { + return CommentPlacement::dangling(consequent, comment); + } + + // ```javascript + // if (cond1) expr1; + // + // /* comment */ else expr2; + // + // if (cond) expr; /* + // a multiline comment */ + // else b; + // + // if (6) // comment + // true + // else // comment + // {true} + // ``` + CommentPlacement::dangling(if_statement, comment) + } + + match (comment.enclosing_node().kind(), comment.following_node()) { + (JsSyntaxKind::JS_IF_STATEMENT, Some(following)) => { + let if_statement = JsIfStatement::unwrap_cast(comment.enclosing_node().clone()); + + if let Some(preceding) = comment.preceding_node() { + // Test if this is a comment right before the condition's `)` + if comment.following_token().kind() == JsSyntaxKind::R_PAREN { + return CommentPlacement::trailing(preceding.clone(), comment); + } + + // Handle comments before `else` + if following.kind() == JsSyntaxKind::JS_ELSE_CLAUSE { + let consequent = preceding.clone(); + let if_statement = comment.enclosing_node().clone(); + return handle_else_clause(comment, consequent, if_statement); + } + } + + // Move comments coming before the `{` inside of the block + // + // ```javascript + // if (cond) /* test */ { + // } + // ``` + if let Some(block_statement) = JsBlockStatement::cast_ref(following) { + return place_block_statement_comment(block_statement, comment); + } + + // Don't attach comments to empty statements + // ```javascript + // if (cond) /* test */ ; + // ``` + if let Some(preceding) = comment.preceding_node() { + if JsEmptyStatement::can_cast(following.kind()) { + return CommentPlacement::trailing(preceding.clone(), comment); + } + } + + // Move comments coming before an if chain inside the body of the first non chain if. + // + // ```javascript + // if (cond1) /* test */ if (other) { a } + // ``` + if let Some(if_statement) = JsIfStatement::cast_ref(following) { + if let Ok(nested_consequent) = if_statement.consequent() { + return place_leading_statement_comment(nested_consequent, comment); + } + } + + // Make all comments after the condition's `)` leading comments + // ```javascript + // if (5) // comment + // true + // + // ``` + if let Ok(consequent) = if_statement.consequent() { + if consequent.syntax() == following { + return CommentPlacement::leading(following.clone(), comment); + } + } + } + (JsSyntaxKind::JS_ELSE_CLAUSE, _) => { + if let Some(if_statement) = comment + .enclosing_node() + .parent() + .and_then(JsIfStatement::cast) + { + if let Ok(consequent) = if_statement.consequent() { + return handle_else_clause( + comment, + consequent.into_syntax(), + if_statement.into_syntax(), + ); + } + } + } + _ => { + // fall through + } + } + + CommentPlacement::Default(comment) +} + +fn handle_while_comment(comment: DecoratedComment) -> CommentPlacement { + let (while_statement, following) = match ( + JsWhileStatement::cast_ref(comment.enclosing_node()), + comment.following_node(), + ) { + (Some(while_statement), Some(following)) => (while_statement, following), + _ => return CommentPlacement::Default(comment), + }; + + if let Some(preceding) = comment.preceding_node() { + // Test if this is a comment right before the condition's `)` + if comment.following_token().kind() == JsSyntaxKind::R_PAREN { + return CommentPlacement::trailing(preceding.clone(), comment); + } + } + + // Move comments coming before the `{` inside of the block + // + // ```javascript + // while (cond) /* test */ { + // } + // ``` + if let Some(block) = JsBlockStatement::cast_ref(following) { + return place_block_statement_comment(block, comment); + } + + // Don't attach comments to empty statements + // ```javascript + // if (cond) /* test */ ; + // ``` + if let Some(preceding) = comment.preceding_node() { + if JsEmptyStatement::can_cast(following.kind()) { + return CommentPlacement::trailing(preceding.clone(), comment); + } + } + + // Make all comments after the condition's `)` leading comments + // ```javascript + // while (5) // comment + // true + // + // ``` + if let Ok(body) = while_statement.body() { + if body.syntax() == following { + return CommentPlacement::leading(body.into_syntax(), comment); + } + } + + CommentPlacement::Default(comment) +} + +fn handle_try_comment(comment: DecoratedComment) -> CommentPlacement { + let following = match comment.following_node() { + Some(following) + if matches!( + comment.enclosing_node().kind(), + JsSyntaxKind::JS_TRY_STATEMENT | JsSyntaxKind::JS_TRY_FINALLY_STATEMENT + ) => + { + // Move comments before the `catch` or `finally` inside of the body + // ```javascript + // try { + // } + // catch(e) { + // } + // // Comment 7 + // finally {} + // ``` + let body = if let Some(catch) = JsCatchClause::cast_ref(following) { + catch.body() + } else if let Some(finally) = JsFinallyClause::cast_ref(following) { + finally.body() + } else { + // Use an err, so that the following code skips over it + Err(rome_rowan::SyntaxError::MissingRequiredChild) + }; + + // + // ```javascript + // try { + // } /* comment catch { + // } + // ``` + if let Ok(body) = body { + return place_block_statement_comment(body, comment); + } + + following + } + Some(following) + if matches!( + comment.enclosing_node().kind(), + JsSyntaxKind::JS_CATCH_CLAUSE | JsSyntaxKind::JS_FINALLY_CLAUSE + ) => + { + following + } + _ => return CommentPlacement::Default(comment), + }; + + // Move comments coming before the `{` inside of the block + // + // ```javascript + // try /* test */ { + // } + // ``` + if let Some(block) = JsBlockStatement::cast_ref(following) { + return place_block_statement_comment(block, comment); } - fn is_group_end_token(&self, kind: JsSyntaxKind) -> bool { + CommentPlacement::Default(comment) +} + +fn handle_for_comment(comment: DecoratedComment) -> CommentPlacement { + let enclosing = comment.enclosing_node(); + + let is_for_in_or_of = matches!( + enclosing.kind(), + JsSyntaxKind::JS_FOR_OF_STATEMENT | JsSyntaxKind::JS_FOR_IN_STATEMENT + ); + + if !is_for_in_or_of && !matches!(enclosing.kind(), JsSyntaxKind::JS_FOR_STATEMENT) { + return CommentPlacement::Default(comment); + } + + if comment.text_position().is_own_line() && is_for_in_or_of { + CommentPlacement::leading(enclosing.clone(), comment) + } + // Don't attach comments to empty statement + // ```javascript + // for /* comment */ (;;); + // for (;;a++) /* comment */; + // ``` + else if comment.following_node().map_or(false, |following| { + JsEmptyStatement::can_cast(following.kind()) + }) { + if let Some(preceding) = comment.preceding_node() { + CommentPlacement::trailing(preceding.clone(), comment) + } else { + CommentPlacement::dangling(comment.enclosing_node().clone(), comment) + } + } else { + CommentPlacement::Default(comment) + } +} + +fn handle_variable_declarator_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let following = match comment.following_node() { + Some(following) => following, + None => return CommentPlacement::Default(comment), + }; + + fn is_complex_value(value: &JsSyntaxNode) -> bool { matches!( - kind, - JsSyntaxKind::R_BRACK - | JsSyntaxKind::R_CURLY - | JsSyntaxKind::R_PAREN - | JsSyntaxKind::COMMA - | JsSyntaxKind::SEMICOLON - | JsSyntaxKind::DOT - | JsSyntaxKind::EOF + value.kind(), + JsSyntaxKind::JS_OBJECT_EXPRESSION + | JsSyntaxKind::JS_ARRAY_EXPRESSION + | JsSyntaxKind::JS_TEMPLATE + | JsSyntaxKind::TS_OBJECT_TYPE + | JsSyntaxKind::TS_UNION_TYPE ) } + + let enclosing = comment.enclosing_node(); + match enclosing.kind() { + JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION | JsSyntaxKind::TS_TYPE_ALIAS_DECLARATION => { + // Makes all comments preceding objects/arrays/templates or block comments leading comments of these nodes. + // ```javascript + // let a = // comment + // { }; + // ``` + if is_complex_value(following) || !comment.kind().is_line() { + return CommentPlacement::leading(following.clone(), comment); + } + } + JsSyntaxKind::JS_VARIABLE_DECLARATOR => { + let variable_declarator = JsVariableDeclarator::unwrap_cast(enclosing.clone()); + + match variable_declarator.initializer() { + // ```javascript + // let obj2 // Comment + // = { + // key: 'val' + // } + // ``` + Some(initializer) if initializer.syntax() == following => { + if let Ok(expression) = initializer.expression() { + return CommentPlacement::leading(expression.into_syntax(), comment); + } + } + _ => { + // fall through + } + } + } + JsSyntaxKind::JS_INITIALIZER_CLAUSE => { + if let Some(variable_declarator) = + enclosing.parent().and_then(JsVariableDeclarator::cast) + { + // Keep trailing comments with the id for variable declarators. Necessary because the value is wrapped + // inside of an initializer clause. + // ```javascript + // let a = // comment + // b; + // ``` + if !is_complex_value(following) + && !JsCommentStyle::is_suppression(comment.piece().text()) + && comment.kind().is_line() + && comment.preceding_node().is_none() + { + if let Ok(id) = variable_declarator.id() { + return CommentPlacement::trailing(id.into_syntax(), comment); + } + } + } + } + _ => { + // fall through + } + } + + CommentPlacement::Default(comment) +} + +fn handle_parameter_comment(comment: DecoratedComment) -> CommentPlacement { + // Make all own line comments leading comments of the parameter + // ```javascript + // function a( + // b + // // comment + // = c + // ) + // ``` + match comment.enclosing_node().kind() { + JsSyntaxKind::JS_FORMAL_PARAMETER if comment.text_position().is_own_line() => { + return CommentPlacement::leading(comment.enclosing_node().clone(), comment) + } + JsSyntaxKind::JS_INITIALIZER_CLAUSE => { + if let Some(parameter) = comment + .enclosing_node() + .parent() + .and_then(JsFormalParameter::cast) + { + // Keep end of line comments after the `=` trailing comments of the id + // ```javascript + // function a( + // b = // test + // c + // ) + // ``` + if comment.text_position().is_end_of_line() && comment.preceding_node().is_none() { + if let Ok(binding) = parameter.binding() { + return CommentPlacement::trailing(binding.into_syntax(), comment); + } + } else if comment.text_position().is_own_line() { + return CommentPlacement::leading(parameter.into_syntax(), comment); + } + } + } + _ => { + // fall through + } + } + + CommentPlacement::Default(comment) +} + +/// Format comments preceding the type parameter name in mapped types on the line above. +/// +/// This is achieved by making them dangling comments of the mapped type. +/// +/// ```javascript +/// type A = { +/// /* comment */ +/// [a in A]: string; +/// } +/// ``` +fn handle_mapped_type_comment( + comment: DecoratedComment, +) -> CommentPlacement { + if let (JsSyntaxKind::TS_MAPPED_TYPE, Some(following)) = + (comment.enclosing_node().kind(), comment.following_node()) + { + if following.kind() == JsSyntaxKind::TS_TYPE_PARAMETER_NAME { + return CommentPlacement::dangling(comment.enclosing_node().clone(), comment); + } + } + + CommentPlacement::Default(comment) +} + +fn handle_union_type_comment( + comment: DecoratedComment, +) -> CommentPlacement { + match (comment.enclosing_node().kind(), comment.preceding_node()) { + (JsSyntaxKind::TS_UNION_TYPE, Some(preceding)) => { + CommentPlacement::trailing(preceding.clone(), comment) + } + _ => CommentPlacement::Default(comment), + } +} + +fn handle_import_export_specifier_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let enclosing_node = comment.enclosing_node(); + + match enclosing_node.kind() { + // Make end of line or own line comments in the middle of an import specifier leading comments of that specifier + // ```javascript + // import { a //comment1 + // //comment2 + // //comment3 + // as b} from ""; + // ``` + JsSyntaxKind::JS_NAMED_IMPORT_SPECIFIER + | JsSyntaxKind::JS_EXPORT_NAMED_SPECIFIER + | JsSyntaxKind::JS_EXPORT_NAMED_SHORTHAND_SPECIFIER + | JsSyntaxKind::JS_EXPORT_NAMED_FROM_SPECIFIER => { + CommentPlacement::leading(enclosing_node.clone(), comment) + } + // Make end of line or own line comments in the middle of an import assertion a leading comment of the assertion + JsSyntaxKind::JS_IMPORT_ASSERTION_ENTRY => { + CommentPlacement::leading(enclosing_node.clone(), comment) + } + + JsSyntaxKind::JS_EXPORT_AS_CLAUSE => { + if let Some(parent) = enclosing_node.parent() { + CommentPlacement::leading(parent, comment) + } else { + CommentPlacement::Default(comment) + } + } + + _ => CommentPlacement::Default(comment), + } +} + +fn place_leading_statement_comment( + statement: JsAnyStatement, + comment: DecoratedComment, +) -> CommentPlacement { + match statement { + JsAnyStatement::JsBlockStatement(block) => place_block_statement_comment(block, comment), + statement => CommentPlacement::leading(statement.into_syntax(), comment), + } +} + +fn place_block_statement_comment( + block_statement: JsBlockStatement, + comment: DecoratedComment, +) -> CommentPlacement { + let first_non_empty = block_statement + .statements() + .iter() + .find(|statement| !matches!(statement, JsAnyStatement::JsEmptyStatement(_))); + + match first_non_empty { + None => CommentPlacement::dangling(block_statement.into_syntax(), comment), + Some(statement) => CommentPlacement::leading(statement.into_syntax(), comment), + } +} + +/// 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) +pub(crate) 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, + }) } diff --git a/crates/rome_js_formatter/src/context.rs b/crates/rome_js_formatter/src/context.rs index b627f8938b8..53655f227d9 100644 --- a/crates/rome_js_formatter/src/context.rs +++ b/crates/rome_js_formatter/src/context.rs @@ -1,8 +1,7 @@ -use crate::comments::{FormatJsLeadingComment, JsCommentStyle}; +use crate::comments::{FormatJsLeadingComment, JsCommentStyle, JsComments}; use rome_formatter::printer::PrinterOptions; use rome_formatter::{ - Comments, CstFormatContext, FormatContext, FormatOptions, IndentStyle, LineWidth, - TransformSourceMap, + CstFormatContext, FormatContext, FormatOptions, IndentStyle, LineWidth, TransformSourceMap, }; use rome_js_syntax::{JsLanguage, SourceType}; use std::fmt; @@ -15,13 +14,13 @@ pub struct JsFormatContext { options: JsFormatOptions, /// The comments of the nodes and tokens in the program. - comments: Rc>, + comments: Rc, source_map: Option, } impl JsFormatContext { - pub fn new(options: JsFormatOptions, comments: Comments) -> Self { + pub fn new(options: JsFormatOptions, comments: JsComments) -> Self { Self { options, comments: Rc::new(comments), @@ -65,13 +64,9 @@ impl FormatContext for JsFormatContext { impl CstFormatContext for JsFormatContext { type Language = JsLanguage; type Style = JsCommentStyle; - type LeadingCommentRule = FormatJsLeadingComment; + type CommentRule = FormatJsLeadingComment; - fn comment_style(&self) -> Self::Style { - JsCommentStyle - } - - fn comments(&self) -> &Comments { + fn comments(&self) -> &JsComments { &self.comments } } diff --git a/crates/rome_js_formatter/src/generated.rs b/crates/rome_js_formatter/src/generated.rs index 46293030ee2..d6423adec24 100644 --- a/crates/rome_js_formatter/src/generated.rs +++ b/crates/rome_js_formatter/src/generated.rs @@ -1,6 +1,8 @@ //! Generated file, do not edit by hand, see `xtask/codegen` -use crate::{AsFormat, FormatNodeRule, IntoFormat, JsFormatContext, JsFormatter}; +use crate::{ + AsFormat, FormatNodeRule, FormatUnknownNodeRule, IntoFormat, JsFormatContext, JsFormatter, +}; use rome_formatter::{FormatOwnedWithRule, FormatRefWithRule, FormatResult, FormatRule}; impl FormatRule for crate::js::auxiliary::script::FormatJsScript { type Context = JsFormatContext; @@ -10284,7 +10286,7 @@ impl FormatRule for crate::js::unknown::unknown::Form type Context = JsFormatContext; #[inline(always)] fn fmt(&self, node: &rome_js_syntax::JsUnknown, f: &mut JsFormatter) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknown { @@ -10322,7 +10324,7 @@ impl FormatRule node: &rome_js_syntax::JsUnknownStatement, f: &mut JsFormatter, ) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownStatement { @@ -10360,7 +10362,7 @@ impl FormatRule node: &rome_js_syntax::JsUnknownExpression, f: &mut JsFormatter, ) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownExpression { @@ -10394,7 +10396,7 @@ impl FormatRule type Context = JsFormatContext; #[inline(always)] fn fmt(&self, node: &rome_js_syntax::JsUnknownMember, f: &mut JsFormatter) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownMember { @@ -10432,7 +10434,7 @@ impl FormatRule node: &rome_js_syntax::JsUnknownBinding, f: &mut JsFormatter, ) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownBinding { @@ -10470,7 +10472,7 @@ impl FormatRule node: &rome_js_syntax::JsUnknownAssignment, f: &mut JsFormatter, ) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownAssignment { @@ -10508,7 +10510,7 @@ impl FormatRule node: &rome_js_syntax::JsUnknownParameter, f: &mut JsFormatter, ) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownParameter { @@ -10546,7 +10548,7 @@ impl FormatRule node: &rome_js_syntax::JsUnknownImportAssertionEntry, f: &mut JsFormatter, ) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownImportAssertionEntry { @@ -10578,7 +10580,7 @@ impl FormatRule node: &rome_js_syntax::JsUnknownNamedImportSpecifier, f: &mut JsFormatter, ) -> FormatResult<()> { - FormatNodeRule::::fmt(self, node, f) + FormatUnknownNodeRule::::fmt(self, node, f) } } impl<'a> AsFormat<'a> for rome_js_syntax::JsUnknownNamedImportSpecifier { diff --git a/crates/rome_js_formatter/src/lib.rs b/crates/rome_js_formatter/src/lib.rs index a4e823daa53..cf9e0abd760 100644 --- a/crates/rome_js_formatter/src/lib.rs +++ b/crates/rome_js_formatter/src/lib.rs @@ -261,7 +261,7 @@ mod syntax_rewriter; use rome_formatter::prelude::*; use rome_formatter::{ - write, Comments, CstFormatContext, Format, FormatLanguage, TransformSourceMap, + comments::Comments, write, CstFormatContext, Format, FormatLanguage, TransformSourceMap, }; use rome_formatter::{Buffer, FormatOwnedWithRule, FormatRefWithRule, Formatted, Printed}; use rome_js_syntax::{ @@ -271,11 +271,11 @@ use rome_rowan::SyntaxResult; use rome_rowan::TextRange; use rome_rowan::{AstNode, SyntaxNode}; -use crate::builders::{format_parenthesize, format_suppressed_node}; use crate::comments::JsCommentStyle; use crate::context::{JsFormatContext, JsFormatOptions}; use crate::cst::FormatJsSyntaxNode; use crate::syntax_rewriter::transform; +use rome_formatter::trivia::format_skipped_token_trivia; use std::iter::FusedIterator; use std::marker::PhantomData; @@ -419,23 +419,32 @@ where { } +/// Rule for formatting a JavaScript [AstNode]. pub trait FormatNodeRule where N: AstNode, { fn fmt(&self, node: &N, f: &mut JsFormatter) -> FormatResult<()> { - let syntax = node.syntax(); + if self.is_suppressed(node, f) { + return write!(f, [format_suppressed_node(node.syntax())]); + } + + self.fmt_leading_comments(node, f)?; + self.fmt_node(node, f)?; + self.fmt_dangling_comments(node, f)?; + self.fmt_trailing_comments(node, f) + } - if f.context().comments().is_suppressed(syntax) { - write!(f, [format_suppressed_node(syntax)]) - } else if self.needs_parentheses(node) { + /// Formats the node without comments. Ignores any suppression comments. + fn fmt_node(&self, node: &N, f: &mut JsFormatter) -> FormatResult<()> { + if self.needs_parentheses(node) { write!( f, - [format_parenthesize( - node.syntax().first_token().as_ref(), - &format_once(|f| self.fmt_fields(node, f)), - node.syntax().last_token().as_ref(), - )] + [ + text("("), + format_once(|f| self.fmt_fields(node, f)), + text(")"), + ] ) } else { self.fmt_fields(node, f) @@ -450,6 +459,50 @@ where let _ = item; false } + + /// Returns `true` if the node has a suppression comment and should use the same formatting as in the source document. + fn is_suppressed(&self, node: &N, f: &JsFormatter) -> bool { + f.context().comments().is_suppressed(node.syntax()) + } + + /// Formats the [leading comments](rome_formatter::comments#leading-comments) of the node. + /// + /// You may want to override this method if you want to manually handle the formatting of comments + /// inside of the `fmt_fields` method or customize the formatting of the leading comments. + fn fmt_leading_comments(&self, node: &N, f: &mut JsFormatter) -> FormatResult<()> { + format_leading_comments(node.syntax()).fmt(f) + } + + /// Formats the [dangling comments](rome_formatter::comments#dangling-comments) of the node. + /// + /// You should override this method if the node handled by this rule can have dangling comments because the + /// default implementation formats the dangling comments at the end of the node, which isn't ideal but ensures that + /// no comments are dropped. + /// + /// A node can have dangling comments if all its children are tokens or if all node childrens are optional. + fn fmt_dangling_comments(&self, node: &N, f: &mut JsFormatter) -> FormatResult<()> { + format_dangling_comments(node.syntax()) + .with_soft_block_indent() + .fmt(f) + } + + /// Formats the [trailing comments](rome_formatter::comments#trailing-comments) of the node. + /// + /// You may want to override this method if you want to manually handle the formatting of comments + /// inside of the `fmt_fields` method or customize the formatting of the trailing comments. + fn fmt_trailing_comments(&self, node: &N, f: &mut JsFormatter) -> FormatResult<()> { + format_trailing_comments(node.syntax()).fmt(f) + } +} + +/// Rule for formatting an unknown node. +pub trait FormatUnknownNodeRule +where + N: AstNode, +{ + fn fmt(&self, node: &N, f: &mut JsFormatter) -> FormatResult<()> { + format_unknown_node(node.syntax()).fmt(f) + } } /// Format implementation specific to JavaScript tokens. @@ -464,9 +517,8 @@ impl FormatRule for FormatJsSyntaxToken { write!( f, [ - format_leading_trivia(token), + format_skipped_token_trivia(token), format_trimmed_token(token), - format_trailing_trivia(token), ] ) } @@ -503,10 +555,6 @@ impl FormatLanguage for JsFormatLanguage { type CommentStyle = JsCommentStyle; type FormatRule = FormatJsSyntaxNode; - fn comment_style(&self) -> Self::CommentStyle { - JsCommentStyle - } - fn transform( &self, root: &SyntaxNode, @@ -746,6 +794,42 @@ function() { assert_eq!(result.range(), Some(TextRange::new(range_start, range_end))); } + #[test] + fn test_range_formatting_middle_of_token() { + let input = r#"/* */ function Foo(){ +/**/ +} +"#; + + let range = TextRange::new(TextSize::from(16), TextSize::from(28)); + + debug_assert_eq!( + &input[range], + r#"oo(){ +/**/ +}"# + ); + + let tree = parse_script(input, 0); + let result = format_range( + JsFormatOptions::new(SourceType::js_script()).with_indent_style(IndentStyle::Space(4)), + &tree.syntax(), + range, + ) + .expect("Range formatting failed"); + + assert_eq!( + result.as_code(), + r#"/* */ function Foo() { + /**/ +}"# + ); + assert_eq!( + result.range(), + Some(TextRange::new(TextSize::from(0), TextSize::from(28))) + ) + } + #[ignore] #[test] // use this test check if your snippet prints as you wish, without using a snapshot diff --git a/crates/rome_js_formatter/src/prelude.rs b/crates/rome_js_formatter/src/prelude.rs index 7198ad90739..77801975f6c 100644 --- a/crates/rome_js_formatter/src/prelude.rs +++ b/crates/rome_js_formatter/src/prelude.rs @@ -2,17 +2,12 @@ //! when implementing the [crate::FormatNodeRule] trait. pub(crate) use crate::{ - AsFormat as _, FormatNodeRule, FormattedIterExt, JsFormatContext, JsFormatter, + builders::format_or_verbatim, comments::JsComments, AsFormat as _, FormatNodeRule, + FormattedIterExt, JsFormatContext, JsFormatter, }; pub use rome_formatter::prelude::*; pub use rome_rowan::{AstNode as _, AstNodeList as _, AstSeparatedList as _}; -pub use crate::builders::{ - format_delimited, format_inserted, format_inserted_close_paren, format_inserted_open_paren, - format_or_verbatim, format_parenthesize, format_suppressed_node, format_unknown_node, - format_verbatim_node, -}; - pub use crate::separated::{ FormatAstSeparatedListExtension, FormatSeparatedOptions, TrailingSeparator, }; diff --git a/crates/rome_js_formatter/src/separated.rs b/crates/rome_js_formatter/src/separated.rs index f0f207c0271..d6584a3a2d6 100644 --- a/crates/rome_js_formatter/src/separated.rs +++ b/crates/rome_js_formatter/src/separated.rs @@ -1,7 +1,7 @@ use crate::prelude::*; use crate::AsFormat; use rome_formatter::{write, GroupId}; -use rome_js_syntax::{JsLanguage, JsSyntaxKind}; +use rome_js_syntax::JsLanguage; use rome_rowan::{ AstNode, AstSeparatedElement, AstSeparatedList, AstSeparatedListElementsIterator, Language, }; @@ -13,7 +13,7 @@ pub struct FormatSeparatedElement { element: AstSeparatedElement, is_last: bool, /// The separator to write if the element has no separator yet. - separator: JsSyntaxKind, + separator: &'static str, options: FormatSeparatedOptions, } @@ -63,12 +63,12 @@ where TrailingSeparator::Allowed => { write!( f, - [if_group_breaks(&format_inserted(self.separator)) + [if_group_breaks(&text(self.separator)) .with_group_id(self.options.group_id)] )?; } TrailingSeparator::Mandatory => { - format_inserted(self.separator).fmt(f)?; + text(self.separator).fmt(f)?; } TrailingSeparator::Omit | TrailingSeparator::Disallowed => { /* no op */ } } @@ -90,7 +90,7 @@ where { next: Option>, inner: I, - separator: JsSyntaxKind, + separator: &'static str, options: FormatSeparatedOptions, } @@ -98,7 +98,7 @@ impl FormatSeparatedIter where L: Language, { - fn new(inner: I, separator: JsSyntaxKind) -> Self { + fn new(inner: I, separator: &'static str) -> Self { Self { inner, separator, @@ -166,7 +166,7 @@ pub trait FormatAstSeparatedListExtension: AstSeparatedList FormatSeparatedIter< AstSeparatedListElementsIterator, JsLanguage, diff --git a/crates/rome_js_formatter/src/syntax_rewriter.rs b/crates/rome_js_formatter/src/syntax_rewriter.rs index 66217b26b73..4fdd26c3c23 100644 --- a/crates/rome_js_formatter/src/syntax_rewriter.rs +++ b/crates/rome_js_formatter/src/syntax_rewriter.rs @@ -1,3 +1,4 @@ +use crate::comments::is_type_comment; use crate::parentheses::JsAnyParenthesized; use crate::TextRange; use rome_formatter::{TransformSourceMap, TransformSourceMapBuilder}; @@ -7,8 +8,7 @@ use rome_js_syntax::{ }; use rome_rowan::syntax::SyntaxTrivia; use rome_rowan::{ - AstNode, SyntaxKind, SyntaxRewriter, SyntaxToken, SyntaxTriviaPiece, SyntaxTriviaPieceComments, - VisitNodeSignal, + AstNode, SyntaxKind, SyntaxRewriter, SyntaxToken, SyntaxTriviaPiece, VisitNodeSignal, }; use std::iter::FusedIterator; @@ -174,10 +174,26 @@ impl JsFormatSyntaxRewriter { self.source_map .add_deleted_range(l_paren.text_trimmed_range()); - let l_paren_trivia = chain_pieces( - l_paren.leading_trivia().pieces(), - l_paren.trailing_trivia().pieces(), - ); + let mut l_paren_trailing = l_paren.trailing_trivia().pieces().peekable(); + + // Skip over leading whitespace + while let Some(piece) = l_paren_trailing.peek() { + if piece.is_whitespace() { + self.source_map + .add_deleted_range(TextRange::at(inner_offset, piece.text_len())); + inner_offset += piece.text_len(); + l_paren_trailing.next(); + } else { + break; + } + } + + let l_paren_trailing_non_whitespace_trivia = l_paren_trailing + .peek() + .map_or(false, |piece| piece.is_skipped() || piece.is_comments()); + + let l_paren_trivia = + chain_pieces(l_paren.leading_trivia().pieces(), l_paren_trailing); let mut leading_trivia = first_token.leading_trivia().pieces().peekable(); let mut first_new_line = None; @@ -185,8 +201,9 @@ impl JsFormatSyntaxRewriter { // The leading whitespace before the opening parens replaces the whitespace before the node. while let Some(trivia) = leading_trivia.peek() { if trivia.is_newline() && first_new_line.is_none() { - inner_offset += trivia.text_len(); + let trivia_len = trivia.text_len(); first_new_line = Some((inner_offset, leading_trivia.next().unwrap())); + inner_offset += trivia_len; } else if trivia.is_whitespace() || trivia.is_newline() { let trivia_len = trivia.text_len(); self.source_map @@ -199,7 +216,10 @@ impl JsFormatSyntaxRewriter { } // Remove all leading new lines directly in front of the token but keep the leading new-line if it precedes a skipped token trivia or a comment. - if leading_trivia.peek().is_none() && first_new_line.is_some() { + if !l_paren_trailing_non_whitespace_trivia + && leading_trivia.peek().is_none() + && first_new_line.is_some() + { let (inner_offset, new_line) = first_new_line.take().unwrap(); self.source_map @@ -380,25 +400,6 @@ fn has_type_cast_comment_or_skipped(trivia: &SyntaxTrivia) -> bool { }) } -/// 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 where F: Iterator>, @@ -499,14 +500,14 @@ where #[cfg(test)] mod tests { use super::JsFormatSyntaxRewriter; - use crate::{format_node, JsFormatOptions}; + use crate::{format_node, JsFormatOptions, TextRange}; use rome_diagnostics::file::FileId; use rome_formatter::{SourceMarker, TransformSourceMap}; use rome_js_parser::parse_module; use rome_js_syntax::{ JsArrayExpression, JsBinaryExpression, JsExpressionStatement, JsIdentifierExpression, JsLogicalExpression, JsSequenceExpression, JsStringLiteralExpression, JsSyntaxNode, - SourceType, + JsUnaryExpression, SourceType, }; use rome_rowan::{AstNode, SyntaxRewriter, TextSize}; @@ -760,6 +761,79 @@ mod tests { ); } + #[test] + fn parentheses() { + let (transformed, source_map) = source_map_test( + r#"!( + /* foo */ + x +); +!( + x // foo +); +!( + /* foo */ + x + y +); +!( + x + y + /* foo */ +); +!( + x + y // foo +);"#, + ); + + let unary_expressions = transformed + .descendants() + .filter_map(JsUnaryExpression::cast) + .collect::>(); + assert_eq!(unary_expressions.len(), 5); + + assert_eq!( + source_map.trimmed_source_text(unary_expressions[0].syntax()), + r#"!( + /* foo */ + x +)"# + ); + + assert_eq!( + source_map.source_range(unary_expressions[1].syntax().text_range()), + TextRange::new(TextSize::from(21), TextSize::from(36)) + ); + + assert_eq!( + source_map.trimmed_source_text(unary_expressions[1].syntax()), + r#"!( + x // foo +)"# + ); + + assert_eq!( + source_map.trimmed_source_text(unary_expressions[2].syntax()), + r#"!( + /* foo */ + x + y +)"# + ); + + assert_eq!( + source_map.trimmed_source_text(unary_expressions[3].syntax()), + r#"!( + x + y + /* foo */ +)"# + ); + + assert_eq!( + source_map.trimmed_source_text(unary_expressions[4].syntax()), + r#"!( + x + y // foo +)"# + ); + } + fn source_map_test(input: &str) -> (JsSyntaxNode, TransformSourceMap) { let tree = parse_module(input, FileId::zero()).syntax();