From b473491d490e53b5dcf4499364a944dce69dc7e3 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 15 Sep 2022 09:49:45 +0200 Subject: [PATCH] Format import & export comments --- crates/rome_formatter/src/comments/builder.rs | 18 ++- crates/rome_js_formatter/src/comments.rs | 65 +++++++- .../src/js/module/export_named_clause.rs | 39 +++-- .../src/js/module/import_assertion_entry.rs | 31 +++- crates/rome_js_formatter/src/lib.rs | 12 +- .../specs/js/module/import/bare_import.js | 17 +- .../js/module/import/bare_import.js.snap | 26 +-- .../js/comments/export-and-import.js.snap | 93 ----------- .../specs/prettier/js/comments/export.js.snap | 34 ++-- .../specs/prettier/js/import/comments.js.snap | 149 ------------------ 10 files changed, 172 insertions(+), 312 deletions(-) delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments/export-and-import.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/import/comments.js.snap diff --git a/crates/rome_formatter/src/comments/builder.rs b/crates/rome_formatter/src/comments/builder.rs index 022befbac03..97f066801be 100644 --- a/crates/rome_formatter/src/comments/builder.rs +++ b/crates/rome_formatter/src/comments/builder.rs @@ -581,7 +581,7 @@ impl<'a> SourceParentheses<'a> { #[cfg(test)] mod tests { - use crate::comments::builder::{CommentsBuilder, CommentsBuilderVisitor}; + use crate::comments::builder::CommentsBuilderVisitor; use crate::comments::map::CommentsMap; use crate::comments::CommentPosition; use crate::{ @@ -854,7 +854,7 @@ b;"#; let transformed = mutation.commit(); let style = TestCommentStyle::default(); - let mut comments_builder = CommentsBuilderVisitor::new(&style, Some(&source_map)); + let comments_builder = CommentsBuilderVisitor::new(&style, Some(&source_map)); let (comments, _) = comments_builder.visit(&transformed); let decorated_comments = style.finish(); @@ -914,11 +914,14 @@ b;"#; assert_eq!(decorated.len(), 2); let first = &decorated[0]; - assert_eq!(first.position(), CommentPosition::OwnLine); + assert_eq!(first.position(), CommentPosition::EndOfLine); assert_eq!(first.lines_before(), 0); assert_eq!(first.lines_after(), 2); assert_eq!(first.preceding_node(), None); - assert_eq!(first.following_node(), None); + assert_eq!( + first.following_node().map(SyntaxNode::kind), + Some(JsSyntaxKind::JS_MODULE) + ); assert_eq!(first.enclosing_node().kind(), JsSyntaxKind::JS_MODULE); let second = &decorated[1]; @@ -926,10 +929,13 @@ b;"#; assert_eq!(second.lines_before(), 2); assert_eq!(second.lines_after(), 0); assert_eq!(second.preceding_node(), None); - assert_eq!(second.following_node(), None); + assert_eq!( + first.following_node().map(SyntaxNode::kind), + Some(JsSyntaxKind::JS_MODULE) + ); assert_eq!(second.enclosing_node().kind(), JsSyntaxKind::JS_MODULE); - assert!(!comments.dangling(&root.key()).is_empty()); + assert!(!comments.leading(&root.key()).is_empty()); } fn extract_comments( diff --git a/crates/rome_js_formatter/src/comments.rs b/crates/rome_js_formatter/src/comments.rs index d222448d64d..dff65f602da 100644 --- a/crates/rome_js_formatter/src/comments.rs +++ b/crates/rome_js_formatter/src/comments.rs @@ -167,7 +167,8 @@ impl CommentStyle for JsCommentStyle { .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_switch_default_case_comment) + .or_else(handle_import_export_specifier_comment), CommentPosition::OwnLine => handle_member_expression_comment(comment) .or_else(handle_function_declaration_comment) .or_else(handle_if_statement_comment) @@ -183,7 +184,8 @@ impl CommentStyle for JsCommentStyle { .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_union_type_comment) + .or_else(handle_import_export_specifier_comment), CommentPosition::SameLine => handle_if_statement_comment(comment) .or_else(handle_while_comment) .or_else(handle_for_comment) @@ -1048,6 +1050,65 @@ fn handle_union_type_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), + } + + // if ( + // enclosingNode?.type === "ImportSpecifier" || + // enclosingNode?.type === "ExportSpecifier" + // ) { + // addLeadingComment(enclosingNode, comment); + // return true; + // } + // + // const isImportDeclaration = + // precedingNode?.type === "ImportSpecifier" && + // enclosingNode?.type === "ImportDeclaration"; + // const isExportDeclaration = + // precedingNode?.type === "ExportSpecifier" && + // enclosingNode?.type === "ExportNamedDeclaration"; + // if ( + // (isImportDeclaration || isExportDeclaration) && + // hasNewline(text, locEnd(comment)) + // ) { + // addTrailingComment(precedingNode, comment); + // return true; + // } + // return false; +} + fn place_leading_statement_comment( statement: JsAnyStatement, comment: DecoratedComment, diff --git a/crates/rome_js_formatter/src/js/module/export_named_clause.rs b/crates/rome_js_formatter/src/js/module/export_named_clause.rs index b626bb05608..d0a7d9d9817 100644 --- a/crates/rome_js_formatter/src/js/module/export_named_clause.rs +++ b/crates/rome_js_formatter/src/js/module/export_named_clause.rs @@ -1,9 +1,8 @@ use crate::prelude::*; -use rome_formatter::write; +use rome_formatter::{format_args, write}; use crate::utils::FormatWithSemicolon; -use crate::builders::format_delimited; use rome_js_syntax::JsExportNamedClause; use rome_js_syntax::JsExportNamedClauseFields; @@ -25,15 +24,24 @@ impl FormatNodeRule for FormatJsExportNamedClause { write!(f, [type_token.format(), space()])?; } - write!( - f, - [format_delimited( - l_curly_token.as_ref()?, - &specifiers.format(), - r_curly_token.as_ref()? - ) - .soft_block_spaces()] - ) + write!(f, [l_curly_token.format()])?; + + if specifiers.is_empty() { + write!( + f, + [format_dangling_comments(node.syntax()).with_block_indent()] + )?; + } else { + write!( + f, + [group(&format_args![ + soft_line_indent_or_space(&specifiers.format()), + soft_line_break_or_space(), + ])] + )?; + } + + write!(f, [r_curly_token.format()]) }); write!( @@ -41,4 +49,13 @@ impl FormatNodeRule for FormatJsExportNamedClause { [FormatWithSemicolon::new(&content, semicolon_token.as_ref())] ) } + + fn fmt_dangling_comments( + &self, + _: &JsExportNamedClause, + _: &mut JsFormatter, + ) -> FormatResult<()> { + // Handled as part of `fmt_fields` + Ok(()) + } } diff --git a/crates/rome_js_formatter/src/js/module/import_assertion_entry.rs b/crates/rome_js_formatter/src/js/module/import_assertion_entry.rs index 192f6a96ec1..8a58e4108ba 100644 --- a/crates/rome_js_formatter/src/js/module/import_assertion_entry.rs +++ b/crates/rome_js_formatter/src/js/module/import_assertion_entry.rs @@ -34,13 +34,30 @@ impl FormatNodeRule for FormatJsImportAssertionEntry { } }; - write![ + write![f, [colon_token.format(), space()]]?; + + if f.comments().has_dangling_comments(node.syntax()) { + write!( + f, + [space(), format_dangling_comments(node.syntax()), space()] + )?; + } + + write!( f, - [ - colon_token.format(), - space(), - FormatLiteralStringToken::new(&value_token?, StringLiteralParentKind::Expression), - ] - ] + [FormatLiteralStringToken::new( + &value_token?, + StringLiteralParentKind::Expression + )] + ) + } + + fn fmt_dangling_comments( + &self, + _: &JsImportAssertionEntry, + _: &mut JsFormatter, + ) -> FormatResult<()> { + // Handled inside `fmt_fields` + Ok(()) } } diff --git a/crates/rome_js_formatter/src/lib.rs b/crates/rome_js_formatter/src/lib.rs index 4e0b74f8c48..4eaefb3c709 100644 --- a/crates/rome_js_formatter/src/lib.rs +++ b/crates/rome_js_formatter/src/lib.rs @@ -814,10 +814,14 @@ function() { // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" -interface ConstructorInline { - // https://github.com/prettier/prettier/issues/2163 - (i): any; -}"#; +export { + foooo, + + barrr + // rome-ignore format: test + as // comment + a, +} from 'foo'"#; let syntax = SourceType::tsx(); let tree = parse(src, 0, syntax); diff --git a/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js b/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js index f3b3c795033..f424f1f08eb 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js +++ b/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js @@ -11,10 +11,13 @@ import "very_long_import_very_long_import_very" assert { } import "very_long_import_very_long_import_very" assert { - // something good is here - "type": /****/ "json", - "type2": /****/ "json", - "type3": /****/ "json", - "type4": /****/ "json", - "typetypetypetypetypetypetypetypetypetypetype": /****/ "typetypetypetypetypetypetypetypetypetypetypetypetypetype", - } \ No newline at end of file + // something good is here + "type": /****/ "json", + "type2" /****/ : "json", + /****/ + "type4" /* dangling 1 */: /* danling 2 */ // line + "json", + /****/ + "typetypetypetypetypetypetypetypetypetypetype": /****/ "typetypetypetypetypetypetypetypetypetypetypetypetypetype", + } + diff --git a/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js.snap b/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js.snap index b9e41a59712..7046c757435 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/import/bare_import.js.snap @@ -16,13 +16,17 @@ import "very_long_import_very_long_import_very" assert { } import "very_long_import_very_long_import_very" assert { - // something good is here - "type": /****/ "json", - "type2": /****/ "json", - "type3": /****/ "json", - "type4": /****/ "json", - "typetypetypetypetypetypetypetypetypetypetype": /****/ "typetypetypetypetypetypetypetypetypetypetypetypetypetype", - } + // something good is here + "type": /****/ "json", + "type2" /****/ : "json", + /****/ + "type4" /* dangling 1 */: /* danling 2 */ // line + "json", + /****/ + "typetypetypetypetypetypetypetypetypetypetype": /****/ "typetypetypetypetypetypetypetypetypetypetypetypetypetype", + } + + ============================= # Outputs ## Output 1 @@ -47,8 +51,10 @@ import "very_long_import_very_long_import_very" assert { // something good is here "type": /****/ "json", "type2": /****/ "json", - "type3": /****/ "json", - "type4": /****/ "json", + /****/ + /* danling 2 */ // line + "type4": /* dangling 1 */ "json", + /****/ "typetypetypetypetypetypetypetypetypetypetype": /****/ "typetypetypetypetypetypetypetypetypetypetypetypetypetype", }; @@ -57,5 +63,5 @@ import "very_long_import_very_long_import_very" assert { 1: import "very_long_import_very_long_import_very_long_import_very_long_import_very_long_import_very_long_import_very_long_import_"; 2: import "very_long_import_very_long_import_very_long_import_very_long_import_very_long_import_very_long" assert { - 18: "typetypetypetypetypetypetypetypetypetypetype": /****/ "typetypetypetypetypetypetypetypetypetypetypetypetypetype", + 20: "typetypetypetypetypetypetypetypetypetypetype": /****/ "typetypetypetypetypetypetypetypetypetypetypetypetypetype", diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments/export-and-import.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments/export-and-import.js.snap deleted file mode 100644 index 2af128680c9..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments/export-and-import.js.snap +++ /dev/null @@ -1,93 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -// These are tests to compare comment formats in `export` and `import`. - -export { - foo, - - bar as // comment - baz, -} from 'foo' - -const fooo = "" -const barr = "" - -export { - fooo, - - barr as // comment - bazz, -} - -import { - foo, - - bar as // comment - baz, -} from 'foo' -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -2,8 +2,7 @@ - - export { - foo, -- // comment -- bar as baz, -+ bar as baz, // comment - } from "foo"; - - const fooo = ""; -@@ -11,12 +10,10 @@ - - export { - fooo, -- // comment -- barr as bazz, -+ barr as bazz, // comment - }; - - import { - foo, -- // comment -- bar as baz, -+ bar as baz, // comment - } from "foo"; -``` - -# Output - -```js -// These are tests to compare comment formats in `export` and `import`. - -export { - foo, - bar as baz, // comment -} from "foo"; - -const fooo = ""; -const barr = ""; - -export { - fooo, - barr as bazz, // comment -}; - -import { - foo, - bar as baz, // comment -} from "foo"; -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments/export.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments/export.js.snap index 6d70131d25f..5734e41fd5f 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments/export.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/comments/export.js.snap @@ -1,5 +1,7 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +info: + test_file: js/comments/export.js --- # Input @@ -53,36 +55,20 @@ export { ```diff --- Prettier +++ Rome -@@ -1,5 +1,4 @@ --export //comment +@@ -1,5 +1,5 @@ + export //comment - {}; -+export {}; //comment ++{}; export /* comment */ {}; -@@ -25,14 +24,12 @@ - const barrr = ""; - export { - foooo, -- // comment -- barrr as baz, -+ barrr as baz, // comment - } from "foo"; - - const fooooo = ""; - const barrrr = ""; - export { - fooooo, -- // comment -- barrrr as bazz, -+ barrrr as bazz, // comment - }; ``` # Output ```js -export {}; //comment +export //comment +{}; export /* comment */ {}; @@ -108,14 +94,16 @@ const foooo = ""; const barrr = ""; export { foooo, - barrr as baz, // comment + // comment + barrr as baz, } from "foo"; const fooooo = ""; const barrrr = ""; export { fooooo, - barrrr as bazz, // comment + // comment + barrrr as bazz, }; ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/import/comments.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/import/comments.js.snap deleted file mode 100644 index f1306008ae0..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/import/comments.js.snap +++ /dev/null @@ -1,149 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -import { a //comment1 -//comment2 -//comment3 -as b} from ""; - -import { - a as //comment1 - //comment2 - //comment3 - b1 -} from ""; - -import { - a as //comment2 //comment1 - //comment3 - b2 -} from ""; - -import { - a as //comment3 //comment2 //comment1 - b3 -} from ""; - -import { - // comment 1 - FN1, // comment 2 - /* comment 3 */ FN2, - // FN3, - FN4 /* comment 4 */ - // FN4, - // FN5 -} from "./module"; - -import { - ExecutionResult, - DocumentNode, - /* tslint:disable */ - SelectionSetNode, - /* tslint:enable */ -} from 'graphql'; - -import x, { - // comment - y -} from 'z'; -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,26 +1,25 @@ - import { -- //comment1 -+ a //comment1 - //comment2 - //comment3 -- a as b, -+ as b, - } from ""; - - import { -- //comment1 -+ a as //comment1 - //comment2 - //comment3 -- a as b1, -+ b1, - } from ""; - - import { -- //comment2 //comment1 -+ a as //comment2 //comment1 - //comment3 -- a as b2, -+ b2, - } from ""; - - import { -- //comment3 //comment2 //comment1 -- a as b3, -+ a as b3, //comment3 //comment2 //comment1 - } from ""; - - import { -``` - -# Output - -```js -import { - a //comment1 - //comment2 - //comment3 - as b, -} from ""; - -import { - a as //comment1 - //comment2 - //comment3 - b1, -} from ""; - -import { - a as //comment2 //comment1 - //comment3 - b2, -} from ""; - -import { - a as b3, //comment3 //comment2 //comment1 -} from ""; - -import { - // comment 1 - FN1, // comment 2 - /* comment 3 */ FN2, - // FN3, - FN4 /* comment 4 */, - // FN4, - // FN5 -} from "./module"; - -import { - ExecutionResult, - DocumentNode, - /* tslint:disable */ - SelectionSetNode, - /* tslint:enable */ -} from "graphql"; - -import x, { - // comment - y, -} from "z"; -``` - - -