refactor(codegen) print inner comments

oxc-project · Sep 25, 2024 · 2ee73ba · 2ee73ba
1 parent a4fdf1b
commit 2ee73ba
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 40 deletions.
diff --git a/crates/oxc_codegen/src/comment.rs b/crates/oxc_codegen/src/comment.rs
@@ -33,6 +33,10 @@ impl<'a> Codegen<'a> {
         })
     }
 
+    pub fn has_comment(&self, start: u32) -> bool {
+        self.comments.contains_key(&start)
+    }
+
     /// Weather to keep leading comments.
     fn is_leading_comments(comment: &Comment, source_text: &str) -> bool {
         (comment.is_jsdoc(source_text) || (comment.is_line() && Self::is_annotation_comments(comment, source_text)))
@@ -41,6 +45,28 @@ impl<'a> Codegen<'a> {
             && !comment.span.source_text(source_text).chars().all(|c| c == '*')
     }
 
+    fn print_comment(&mut self, comment: &Comment, source_text: &str) {
+        let comment_source = comment.real_span().source_text(source_text);
+        match comment.kind {
+            CommentKind::Line => {
+                self.print_str(comment_source);
+            }
+            CommentKind::Block => {
+                // Print block comments with our own indentation.
+                let lines = comment_source.split(is_line_terminator);
+                for line in lines {
+                    if !line.starts_with("/*") {
+                        self.print_indent();
+                    }
+                    self.print_str(line.trim_start());
+                    if !line.ends_with("*/") {
+                        self.print_hard_newline();
+                    }
+                }
+            }
+        }
+    }
+
     pub(crate) fn print_leading_comments(&mut self, start: u32) {
         if self.options.minify {
             return;
@@ -68,25 +94,7 @@ impl<'a> Codegen<'a> {
                 self.print_indent();
             }
 
-            let comment_source = comment.real_span().source_text(source_text);
-            match comment.kind {
-                CommentKind::Line => {
-                    self.print_str(comment_source);
-                }
-                CommentKind::Block => {
-                    // Print block comments with our own indentation.
-                    let lines = comment_source.split(is_line_terminator);
-                    for line in lines {
-                        if !line.starts_with("/*") {
-                            self.print_indent();
-                        }
-                        self.print_str(line.trim_start());
-                        if !line.ends_with("*/") {
-                            self.print_hard_newline();
-                        }
-                    }
-                }
-            }
+            self.print_comment(&comment, source_text);
         }
 
         if comments.last().is_some_and(|c| c.is_line() || c.followed_by_newline) {
@@ -123,4 +131,17 @@ impl<'a> Codegen<'a> {
             self.print_hard_space();
         }
     }
+
+    pub(crate) fn print_expr_comments(&mut self, start: u32) {
+        if self.options.minify {
+            return;
+        }
+        let Some(source_text) = self.source_text else { return };
+        let Some(comments) = self.comments.remove(&start) else { return };
+        for comment in comments {
+            self.print_hard_newline();
+            self.print_indent();
+            self.print_comment(&comment, source_text);
+        }
+    }
 }
diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs
@@ -1952,11 +1952,26 @@ impl<'a> GenExpr for ImportExpression<'a> {
         p.wrap(wrap, |p| {
             p.add_source_mapping(self.span.start);
             p.print_str("import(");
+            let has_comment = p.has_comment(self.source.span().start);
+            if has_comment {
+                p.indent();
+                p.print_expr_comments(self.source.span().start);
+                p.print_hard_newline();
+                p.print_indent();
+            }
             self.source.print_expr(p, Precedence::Comma, Context::empty());
             if !self.arguments.is_empty() {
                 p.print_comma();
+                if has_comment {
+                    p.print_soft_newline();
+                    p.print_indent();
+                }
                 p.print_expressions(&self.arguments, Precedence::Comma, Context::empty());
             }
+            if has_comment {
+                p.print_hard_newline();
+                p.dedent();
+            }
             p.print_char(b')');
         });
     }

diff --git a/crates/oxc_codegen/tests/integration/esbuild.rs b/crates/oxc_codegen/tests/integration/esbuild.rs
@@ -613,37 +613,37 @@ fn test_decorators() {
 fn test_import() {
     test("import('path');", "import(\"path\");\n"); // The semicolon must not be a separate statement
 
-    // FIXME
-    // Test preservation of Webpack-specific comments
+    // TODO: Maybe there has a bug on trailing comments
     // test(
-    // "import(// webpackFoo: 1\n // webpackBar: 2\n 'path');",
-    // "import(\n  // webpackFoo: 1\n  // webpackBar: 2\n  \"path\"\n);\n",
+    //     "import(// webpackFoo: 1\n // webpackBar: 2\n 'path');",
+    //     "import(\n  // webpackFoo: 1\n  // webpackBar: 2\n  \"path\"\n);\n",
     // );
     // test( "import(// webpackFoo: 1\n // webpackBar: 2\n 'path', {type: 'module'});", "import(\n  // webpackFoo: 1\n  // webpackBar: 2\n  \"path\",\n  { type: \"module\" }\n);\n");
+    test(
+        "import(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path');",
+        "import(\n\t/* webpackFoo: 1 */\n\t/* webpackBar: 2 */\n\t\"path\"\n);\n",
+    );
+    test( "import(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path', {type: 'module'});",
+        "import(\n\t/* webpackFoo: 1 */\n\t/* webpackBar: 2 */\n\t\"path\",\n\t{ type: \"module\" }\n);\n");
+    test(
+        "import(\n    /* multi\n     * line\n     * webpackBar: */ 'path');",
+        "import(\n\t/* multi\n\t* line\n\t* webpackBar: */\n\t\"path\"\n);\n",
+    );
     // test(
-    // "import(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path');",
-    // "import(\n  /* webpackFoo: 1 */\n  /* webpackBar: 2 */\n  \"path\"\n);\n",
-    // );
-    // test( "import(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path', {type: 'module'});", "import(\n  /* webpackFoo: 1 */\n  /* webpackBar: 2 */\n  \"path\",\n  { type: \"module\" }\n);\n");
-    // test(
-    // "import(\n    /* multi\n     * line\n     * webpackBar: */ 'path');",
-    // "import(\n  /* multi\n   * line\n   * webpackBar: */\n  \"path\"\n);\n",
-    // );
-    // test(
-    // "import(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */);",
-    // "import(\n  /* webpackFoo: 1 */\n  \"path\"\n  /* webpackBar:2 */\n);\n",
+    //     "import(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */);",
+    //     "import(\n\t/* webpackFoo: 1 */\n\t\"path\"\n\t/* webpackBar:2 */\n);\n",
     // );
     // test(
-    // "import(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);",
-    // "import(\n  /* webpackFoo: 1 */\n  \"path\"\n);\n",
+    //     "import(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);",
+    //     "import(\n\t/* webpackFoo: 1 */\n\t\"path\"\n);\n",
     // ); // Not currently handled
     // test(
     // "import(/* webpackFoo: 1 */ 'path', /* webpackBar:2 */ );",
-    // "import(\n  /* webpackFoo: 1 */\n  \"path\"\n  /* webpackBar:2 */\n);\n",
+    // "import(\n\t/* webpackFoo: 1 */\n\t\"path\"\n\t/* webpackBar:2 */\n);\n",
     // );
-    // test( "import(/* webpackFoo: 1 */ 'path', { type: 'module' } /* webpackBar:2 */ );", "import(\n  /* webpackFoo: 1 */\n  \"path\",\n  { type: \"module\" }\n  /* webpackBar:2 */\n);\n");
-    // test( "import(new URL('path', /* webpackFoo: these can go anywhere */ import.meta.url))",
-    // "import(new URL(\n  \"path\",\n  /* webpackFoo: these can go anywhere */\n  import.meta.url\n));\n");
+    // test( "import(/* webpackFoo: 1 */ 'path', { type: 'module' } /* webpackBar:2 */ );", "import(\n\t/* webpackFoo: 1 */\n\t\"path\",\n\t{ type: \"module\" }\n\t/* webpackBar:2 */\n);\n");
+    // test( "import(new URL('path', /* webpackFoo: these can go anywhe */ import.meta.url))",
+    // "import(new URL(\n  \"path\",\n  /* webpackFoo: these can go anywhere */\n\timport.meta.url\n));\n");
 }
 
 #[test]

diff --git a/crates/oxc_codegen/tests/integration/inner_comments.rs b/crates/oxc_codegen/tests/integration/inner_comments.rs
@@ -0,0 +1,8 @@
+use crate::snapshot;
+
+#[test]
+fn comment() {
+    let cases = vec![r"import(/* @vite-ignore */ dynamicVar)"];
+
+    snapshot("inner_comments", &cases);
+}
diff --git a/crates/oxc_codegen/tests/integration/main.rs b/crates/oxc_codegen/tests/integration/main.rs
@@ -1,5 +1,6 @@
 #![allow(clippy::missing_panics_doc)]
 pub mod esbuild;
+pub mod inner_comments;
 pub mod jsdoc;
 pub mod pure_comments;
 pub mod tester;

diff --git a/crates/oxc_codegen/tests/integration/snapshots/inner_comments.snap b/crates/oxc_codegen/tests/integration/snapshots/inner_comments.snap
@@ -0,0 +1,7 @@
+---
+source: crates/oxc_codegen/tests/integration/main.rs
+---
+########## 0
+import(/* @vite-ignore */ dynamicVar)
+----------
+import(/* @vite-ignore */ dynamicVar);