feat(codegen): improve printing of comments (#7108)

oxc-project · Nov 4, 2024 · caa4b1f · caa4b1f
1 parent b73cfd9
commit caa4b1f
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 44 deletions.
diff --git a/crates/oxc_codegen/src/comment.rs b/crates/oxc_codegen/src/comment.rs
@@ -162,34 +162,21 @@ impl<'a> Codegen<'a> {
         }
     }
 
-    pub(crate) fn try_print_eof_legal_comments(&mut self) {
-        match self.options.legal_comments.clone() {
-            LegalComment::Eof => {
-                let comments = self.legal_comments.drain(..).collect::<Vec<_>>();
-                for c in comments {
-                    self.print_comment(&c);
-                    self.print_hard_newline();
-                }
-            }
-            LegalComment::Linked(path) => {
-                self.print_str("/*! For license information please see ");
-                self.print_str(&path);
-                self.print_str(" */");
-            }
-            _ => {}
-        }
-    }
-
     fn print_comments(&mut self, start: u32, comments: &[Comment], unused_comments: Vec<Comment>) {
-        if comments.first().is_some_and(|c| c.preceded_by_newline) {
-            // Skip printing newline if this comment is already on a newline.
-            if self.last_byte().is_some_and(|b| b != b'\n' && b != b'\t') {
-                self.print_hard_newline();
-                self.print_indent();
-            }
-        }
-
         for (i, comment) in comments.iter().enumerate() {
+            if i == 0 && comment.preceded_by_newline {
+                // Skip printing newline if this comment is already on a newline.
+                if let Some(b) = self.last_byte() {
+                    match b {
+                        b'\n' => self.print_indent(),
+                        b'\t' => { /* noop */ }
+                        _ => {
+                            self.print_hard_newline();
+                            self.print_indent();
+                        }
+                    }
+                }
+            }
             if i >= 1 {
                 if comment.preceded_by_newline {
                     self.print_hard_newline();
@@ -198,13 +185,10 @@ impl<'a> Codegen<'a> {
                     self.print_hard_newline();
                 }
             }
-
             self.print_comment(comment);
-        }
-
-        if comments.last().is_some_and(|c| c.is_line() || c.followed_by_newline) {
-            self.print_hard_newline();
-            self.print_indent();
+            if i == comments.len() - 1 && (comment.is_line() || comment.followed_by_newline) {
+                self.print_hard_newline();
+            }
         }
 
         if !unused_comments.is_empty() {
@@ -233,4 +217,22 @@ impl<'a> Codegen<'a> {
             }
         }
     }
+
+    pub(crate) fn try_print_eof_legal_comments(&mut self) {
+        match self.options.legal_comments.clone() {
+            LegalComment::Eof => {
+                let comments = self.legal_comments.drain(..).collect::<Vec<_>>();
+                for c in comments {
+                    self.print_comment(&c);
+                    self.print_hard_newline();
+                }
+            }
+            LegalComment::Linked(path) => {
+                self.print_str("/*! For license information please see ");
+                self.print_str(&path);
+                self.print_str(" */");
+            }
+            _ => {}
+        }
+    }
 }
diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs
@@ -2186,6 +2186,7 @@ impl<'a> Gen for ClassBody<'a> {
         p.print_curly_braces(self.span, self.body.is_empty(), |p| {
             for item in &self.body {
                 p.print_semicolon_if_needed();
+                p.print_leading_comments(item.span().start);
                 p.print_indent();
                 item.print(p, ctx);
             }
@@ -2197,27 +2198,22 @@ impl<'a> Gen for ClassElement<'a> {
     fn gen(&self, p: &mut Codegen, ctx: Context) {
         match self {
             Self::StaticBlock(elem) => {
-                p.print_leading_comments(elem.span.start);
                 elem.print(p, ctx);
                 p.print_soft_newline();
             }
             Self::MethodDefinition(elem) => {
-                p.print_leading_comments(elem.span.start);
                 elem.print(p, ctx);
                 p.print_soft_newline();
             }
             Self::PropertyDefinition(elem) => {
-                p.print_leading_comments(elem.span.start);
                 elem.print(p, ctx);
                 p.print_semicolon_after_statement();
             }
             Self::AccessorProperty(elem) => {
-                p.print_leading_comments(elem.span.start);
                 elem.print(p, ctx);
                 p.print_semicolon_after_statement();
             }
             Self::TSIndexSignature(elem) => {
-                p.print_leading_comments(elem.span.start);
                 elem.print(p, ctx);
                 p.print_semicolon_after_statement();
             }
@@ -3547,8 +3543,8 @@ impl<'a> Gen for TSInterfaceDeclaration<'a> {
         p.print_soft_space();
         p.print_curly_braces(self.body.span, self.body.body.is_empty(), |p| {
             for item in &self.body.body {
-                p.print_indent();
                 p.print_leading_comments(item.span().start);
+                p.print_indent();
                 item.print(p, ctx);
                 p.print_semicolon();
                 p.print_soft_newline();
@@ -3581,6 +3577,7 @@ impl<'a> Gen for TSEnumDeclaration<'a> {
         p.print_space_before_identifier();
         p.print_curly_braces(self.span, self.members.is_empty(), |p| {
             for member in &self.members {
+                p.print_leading_comments(member.span().start);
                 p.print_indent();
                 member.print(p, ctx);
                 p.print_comma();
@@ -3592,7 +3589,6 @@ impl<'a> Gen for TSEnumDeclaration<'a> {
 
 impl<'a> Gen for TSEnumMember<'a> {
     fn gen(&self, p: &mut Codegen, ctx: Context) {
-        p.print_leading_comments(self.span.start);
         match &self.id {
             TSEnumMemberName::StaticIdentifier(decl) => decl.print(p, ctx),
             TSEnumMemberName::StaticStringLiteral(decl) => decl.print(p, ctx),

diff --git a/crates/oxc_codegen/tests/integration/legal_comments.rs b/crates/oxc_codegen/tests/integration/legal_comments.rs
@@ -9,6 +9,13 @@ fn cases() -> Vec<&'static str> {
         "/* @license */\n//! KEEP\nfoo;bar;",
         "/* @license */\n/*! KEEP */\nfoo;bar;",
         "/* @license *//*! KEEP */\nfoo;bar;",
+        "function () {
+    /*
+    * @license
+    * Copyright notice 2
+    */
+    bar;
+}",
     ]
 }
 

diff --git a/crates/oxc_codegen/tests/integration/snapshots/jsodc.snap b/crates/oxc_codegen/tests/integration/snapshots/jsodc.snap
@@ -163,8 +163,8 @@ exports.button = function() {};
 /** Unbutton the shirt. */
 exports.unbutton = function() {};
 this.Book = function(title) {
-/** The title of the book. */
-		this.title = title;
+	/** The title of the book. */
+	this.title = title;
 };
 export enum DefinitionKind {
 	/**

diff --git a/crates/oxc_codegen/tests/integration/snapshots/legal_eof_comments.snap b/crates/oxc_codegen/tests/integration/snapshots/legal_eof_comments.snap
@@ -1,6 +1,5 @@
 ---
 source: crates/oxc_codegen/tests/integration/main.rs
-snapshot_kind: text
 ---
 ########## 0
 /* @license */
@@ -50,3 +49,20 @@ foo;
 bar;
 /* @license */
 /*! KEEP */
+
+########## 5
+function () {
+    /*
+    * @license
+    * Copyright notice 2
+    */
+    bar;
+}
+----------
+function() {
+	bar;
+}
+/*
+* @license
+* Copyright notice 2
+*/
diff --git a/crates/oxc_codegen/tests/integration/snapshots/legal_inline_comments.snap b/crates/oxc_codegen/tests/integration/snapshots/legal_inline_comments.snap
@@ -1,6 +1,5 @@
 ---
 source: crates/oxc_codegen/tests/integration/main.rs
-snapshot_kind: text
 ---
 ########## 0
 /* @license */
@@ -50,3 +49,20 @@ foo;bar;
 /*! KEEP */
 foo;
 bar;
+
+########## 5
+function () {
+    /*
+    * @license
+    * Copyright notice 2
+    */
+    bar;
+}
+----------
+function() {
+	/*
+	* @license
+	* Copyright notice 2
+	*/
+	bar;
+}
diff --git a/crates/oxc_codegen/tests/integration/snapshots/legal_linked_comments.snap b/crates/oxc_codegen/tests/integration/snapshots/legal_linked_comments.snap
@@ -1,6 +1,5 @@
 ---
 source: crates/oxc_codegen/tests/integration/main.rs
-snapshot_kind: text
 ---
 ########## 0
 /* @license */
@@ -41,3 +40,16 @@ foo;bar;
 foo;
 bar;
 /*! For license information please see test.js */
+########## 5
+function () {
+    /*
+    * @license
+    * Copyright notice 2
+    */
+    bar;
+}
+----------
+function() {
+	bar;
+}
+/*! For license information please see test.js */