Format import & export comments

rome · Sep 15, 2022 · 4cc61c8 · 4cc61c8
1 parent 5524f76
commit 4cc61c8
Show file tree

Hide file tree

Showing 10 changed files with 172 additions and 312 deletions.
diff --git 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,22 +914,28 @@ 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];
         assert_eq!(second.position(), CommentPosition::OwnLine);
         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
@@ -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<JsLanguage>,
+) -> CommentPlacement<JsLanguage> {
+    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<JsLanguage>,

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
@@ -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,20 +24,38 @@ impl FormatNodeRule<JsExportNamedClause> 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!(
             f,
             [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
@@ -34,13 +34,30 @@ impl FormatNodeRule<JsImportAssertionEntry> 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
@@ -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
@@ -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",
-        }
+		// 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
@@ -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",