diff --git a/crates/oxc_codegen/src/comment.rs b/crates/oxc_codegen/src/comment.rs index 4126a98ea1ad2..a288bbc0a617a 100644 --- a/crates/oxc_codegen/src/comment.rs +++ b/crates/oxc_codegen/src/comment.rs @@ -26,21 +26,54 @@ impl<'a> Codegen<'a> { } } - pub fn has_annotation_comments(&self, start: u32) -> bool { + pub fn has_comment(&self, start: u32) -> bool { + self.comments.contains_key(&start) + } + + pub fn has_annotation_comment(&self, start: u32) -> bool { let Some(source_text) = self.source_text else { return false }; self.comments.get(&start).is_some_and(|comments| { - comments.iter().any(|comment| Self::is_annotation_comments(comment, source_text)) + comments.iter().any(|comment| Self::is_annotation_comment(comment, source_text)) + }) + } + + pub fn has_non_annotation_comment(&self, start: u32) -> bool { + let Some(source_text) = self.source_text else { return false }; + self.comments.get(&start).is_some_and(|comments| { + comments.iter().any(|comment| !Self::is_annotation_comment(comment, source_text)) }) } /// 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))) + (comment.is_jsdoc(source_text) || (comment.is_line() && Self::is_annotation_comment(comment, source_text))) && comment.preceded_by_newline // webpack comment `/*****/` && !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 +101,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) { @@ -99,7 +114,7 @@ impl<'a> Codegen<'a> { } } - fn is_annotation_comments(comment: &Comment, source_text: &str) -> bool { + fn is_annotation_comment(comment: &Comment, source_text: &str) -> bool { let comment_content = comment.span.source_text(source_text); ANNOTATION_MATCHER.find_iter(comment_content).count() != 0 } @@ -116,11 +131,40 @@ impl<'a> Codegen<'a> { let Some(comments) = self.comments.remove(&start) else { return }; for comment in comments { - if !Self::is_annotation_comments(&comment, source_text) { + if !Self::is_annotation_comment(&comment, source_text) { continue; } self.print_str(comment.real_span().source_text(source_text)); self.print_hard_space(); } } + + pub(crate) fn print_expr_comments(&mut self, start: u32) -> bool { + if self.options.minify { + return false; + } + let Some(source_text) = self.source_text else { return false }; + let Some(comments) = self.comments.remove(&start) else { return false }; + + let (annotation_comments, comments): (Vec<_>, Vec<_>) = comments + .into_iter() + .partition(|comment| Self::is_annotation_comment(comment, source_text)); + + if !annotation_comments.is_empty() { + self.comments.insert(start, annotation_comments); + } + + for comment in &comments { + self.print_hard_newline(); + self.print_indent(); + self.print_comment(comment, source_text); + } + + if comments.is_empty() { + false + } else { + self.print_hard_newline(); + true + } + } } diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs index 092f9fb42117c..f1401532e5ac0 100644 --- a/crates/oxc_codegen/src/gen.rs +++ b/crates/oxc_codegen/src/gen.rs @@ -562,7 +562,7 @@ impl<'a> Gen for VariableDeclaration<'a> { && p.start_of_annotation_comment.is_none() && matches!(self.kind, VariableDeclarationKind::Const) && matches!(self.declarations.first(), Some(VariableDeclarator { init: Some(init), .. }) if init.is_function()) - && p.has_annotation_comments(self.span.start) + && p.has_annotation_comment(self.span.start) { p.start_of_annotation_comment = Some(self.span.start); } @@ -834,7 +834,7 @@ impl<'a> Gen for ExportNamedDeclaration<'a> { if matches!(var_decl.kind, VariableDeclarationKind::Const) => { if matches!(var_decl.declarations.first(), Some(VariableDeclarator { init: Some(init), .. }) if init.is_function()) - && p.has_annotation_comments(self.span.start) + && p.has_annotation_comment(self.span.start) { p.start_of_annotation_comment = Some(self.span.start); } @@ -1368,7 +1368,7 @@ impl<'a> GenExpr for CallExpression<'a> { fn gen_expr(&self, p: &mut Codegen, precedence: Precedence, ctx: Context) { let is_export_default = p.start_of_default_export == p.code_len(); let mut wrap = precedence >= Precedence::New || ctx.intersects(Context::FORBID_CALL); - if p.has_annotation_comments(self.span.start) && precedence >= Precedence::Postfix { + if p.has_annotation_comment(self.span.start) && precedence >= Precedence::Postfix { wrap = true; } @@ -1386,7 +1386,19 @@ impl<'a> GenExpr for CallExpression<'a> { type_parameters.print(p, ctx); } p.print_char(b'('); - p.print_list(&self.arguments, ctx); + let has_comment = (self.span.end > 0 && p.has_comment(self.span.end - 1)) + || self.arguments.iter().any(|item| p.has_comment(item.span().start)); + if has_comment { + p.indent(); + p.print_list_with_comments(&self.arguments, ctx); + // Handle `/* comment */);` + if !p.print_expr_comments(self.span.end - 1) { + p.print_soft_newline(); + } + p.dedent(); + } else { + p.print_list(&self.arguments, ctx); + } p.print_char(b')'); p.add_source_mapping(self.span.end); }); @@ -1949,14 +1961,40 @@ impl<'a> GenExpr for SequenceExpression<'a> { impl<'a> GenExpr for ImportExpression<'a> { fn gen_expr(&self, p: &mut Codegen, precedence: Precedence, ctx: Context) { let wrap = precedence >= Precedence::New || ctx.intersects(Context::FORBID_CALL); + let has_comment = (self.span.end > 0 && p.has_comment(self.span.end - 1)) + || p.has_comment(self.source.span().start) + || self.arguments.first().is_some_and(|argument| p.has_comment(argument.span().start)); + p.wrap(wrap, |p| { p.add_source_mapping(self.span.start); p.print_str("import("); + if has_comment { + p.indent(); + } + if p.print_expr_comments(self.source.span().start) { + p.print_indent(); + } else if has_comment { + p.print_soft_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(); + } else { + p.print_soft_space(); + } p.print_expressions(&self.arguments, Precedence::Comma, Context::empty()); } + if has_comment { + // Handle `/* comment */);` + if !p.print_expr_comments(self.span.end - 1) { + p.print_soft_newline(); + } + p.dedent(); + } p.print_char(b')'); }); } @@ -2024,7 +2062,7 @@ impl<'a> GenExpr for ChainExpression<'a> { impl<'a> GenExpr for NewExpression<'a> { fn gen_expr(&self, p: &mut Codegen, precedence: Precedence, ctx: Context) { let mut wrap = precedence >= self.precedence(); - if p.has_annotation_comments(self.span.start) && precedence >= Precedence::Postfix { + if p.has_annotation_comment(self.span.start) && precedence >= Precedence::Postfix { wrap = true; } p.wrap(wrap, |p| { @@ -2034,7 +2072,19 @@ impl<'a> GenExpr for NewExpression<'a> { p.print_str("new "); self.callee.print_expr(p, Precedence::New, Context::FORBID_CALL); p.print_char(b'('); - p.print_list(&self.arguments, ctx); + let has_comment = p.has_comment(self.span.end - 1) + || self.arguments.iter().any(|item| p.has_comment(item.span().start)); + if has_comment { + p.indent(); + p.print_list_with_comments(&self.arguments, ctx); + // Handle `/* comment */);` + if !p.print_expr_comments(self.span.end - 1) { + p.print_soft_newline(); + } + p.dedent(); + } else { + p.print_list(&self.arguments, ctx); + } p.print_char(b')'); }); } diff --git a/crates/oxc_codegen/src/lib.rs b/crates/oxc_codegen/src/lib.rs index cbbf22cfc3f94..40871c85cc1b6 100644 --- a/crates/oxc_codegen/src/lib.rs +++ b/crates/oxc_codegen/src/lib.rs @@ -17,7 +17,7 @@ use oxc_ast::{ Trivias, }; use oxc_mangler::Mangler; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use oxc_syntax::{ identifier::is_identifier_part, operator::{BinaryOperator, UnaryOperator, UpdateOperator}, @@ -468,6 +468,22 @@ impl<'a> Codegen<'a> { } } + fn print_list_with_comments(&mut self, items: &[T], ctx: Context) { + for (index, item) in items.iter().enumerate() { + if index != 0 { + self.print_comma(); + } + if self.has_non_annotation_comment(item.span().start) { + self.print_expr_comments(item.span().start); + self.print_indent(); + } else { + self.print_soft_newline(); + self.print_indent(); + } + item.print(self, ctx); + } + } + fn print_expressions(&mut self, items: &[T], precedence: Precedence, ctx: Context) { for (index, item) in items.iter().enumerate() { if index != 0 { diff --git a/crates/oxc_codegen/tests/integration/esbuild.rs b/crates/oxc_codegen/tests/integration/esbuild.rs index 22657df5704d3..ed655a3e39778 100644 --- a/crates/oxc_codegen/tests/integration/esbuild.rs +++ b/crates/oxc_codegen/tests/integration/esbuild.rs @@ -180,32 +180,39 @@ fn test_new() { // test_minify("new x() ** 2", "new x**2;"); // Test preservation of Webpack-specific comments + // TODO: Not support trailing comments yet // test( // "new Worker(// webpackFoo: 1\n // webpackBar: 2\n 'path');", // "new Worker(\n // webpackFoo: 1\n // webpackBar: 2\n \"path\"\n);\n", // ); - // test( - // "new Worker(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path');", - // "new Worker(\n /* webpackFoo: 1 */\n /* webpackBar: 2 */\n \"path\"\n);\n", - // ); - // test( - // "new Worker(\n /* multi\n * line\n * webpackBar: */ 'path');", - // "new Worker(\n /* multi\n * line\n * webpackBar: */\n \"path\"\n);\n", - // ); - // test( - // "new Worker(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */);", - // "new Worker(\n /* webpackFoo: 1 */\n \"path\"\n /* webpackBar:2 */\n);\n", - // ); - // test( - // "new Worker(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);", - // "new Worker(\n /* webpackFoo: 1 */\n \"path\"\n);\n", - // ); // Not currently handled - // test( - // "new Worker(/* webpackFoo: 1 */ 'path', /* webpackBar:2 */ );", - // "new Worker(\n /* webpackFoo: 1 */\n \"path\"\n /* webpackBar:2 */\n);\n", - // ); - // test( "new Worker(new URL('path', /* webpackFoo: these can go anywhere */ import.meta.url))", - // "new Worker(new URL(\n \"path\",\n /* webpackFoo: these can go anywhere */\n import.meta.url\n));\n"); + test( + "new Worker(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path');", + "new Worker(\n\t/* webpackFoo: 1 */\n\t/* webpackBar: 2 */\n\t\"path\"\n);\n", + ); + test( + "new Worker(\n /* multi\n * line\n * webpackBar: */ 'path');", + "new Worker(\n\t/* multi\n\t* line\n\t* webpackBar: */\n\t\"path\"\n);\n", + ); + test( + "new Worker(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */);", + "new Worker(\n\t/* webpackFoo: 1 */\n\t\"path\"\n\t/* webpackBar:2 */\n);\n", + ); + test( + "new Worker(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);", + "new Worker(\n\t/* webpackFoo: 1 */\n\t\"path\"\n);\n", + ); // Not currently handled + test( + "new Worker(/* webpackFoo: 1 */ 'path', /* webpackBar:2 */ );", + "new Worker(\n\t/* webpackFoo: 1 */\n\t\"path\"\n\t/* webpackBar:2 */\n);\n", + ); + test( "new Worker(new URL('path', /* webpackFoo: these can go anywhere */ import.meta.url))", + "new Worker(new URL(\n\t\"path\",\n\t/* webpackFoo: these can go anywhere */\n\timport.meta.url\n));\n"); + + // non-webpack comments + test("new Worker(/* before */ foo)", "new Worker(\n\t/* before */\n\tfoo\n);\n"); + test("new Worker(/* before */ 'foo')", "new Worker(\n\t/* before */\n\t\"foo\"\n);\n"); + test("new Worker(foo /* after */)", "new Worker(\n\tfoo\n\t/* after */\n);\n"); + test("new Worker('foo' /* after */)", "new Worker(\n\t\"foo\"\n\t/* after */\n);\n"); } #[test] @@ -234,6 +241,43 @@ fn test_call() { test_minify("(1, eval)?.(x)", "(1,eval)?.(x);"); // testMangleMinify(t, "(1 ? eval : 2)(x)", "(0,eval)(x);"); // testMangleMinify(t, "(1 ? eval : 2)?.(x)", "eval?.(x);"); + + // Webpack-specific comments + // TODO: Not support trailing comments yet + // test( + // "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( + "require(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path');", + "require(\n\t/* webpackFoo: 1 */\n\t/* webpackBar: 2 */\n\t\"path\"\n);\n", + ); + test( "require(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path', {type: 'module'});", + "require(\n\t/* webpackFoo: 1 */\n\t/* webpackBar: 2 */\n\t\"path\",\n\t{ type: \"module\" }\n);\n"); + test( + "require(\n /* multi\n * line\n * webpackBar: */ 'path');", + "require(\n\t/* multi\n\t* line\n\t* webpackBar: */\n\t\"path\"\n);\n", + ); + test( + "require(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */);", + "require(\n\t/* webpackFoo: 1 */\n\t\"path\"\n\t/* webpackBar:2 */\n);\n", + ); + test( + "require(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);", + "require(\n\t/* webpackFoo: 1 */\n\t\"path\"\n);\n", + ); // Not currently handled + test( + "require(/* webpackFoo: 1 */ 'path', /* webpackBar:2 */ );", + "require(\n\t/* webpackFoo: 1 */\n\t\"path\"\n\t/* webpackBar:2 */\n);\n", + ); + test( "require(/* webpackFoo: 1 */ 'path', { type: 'module' } /* webpackBar:2 */ );", "require(\n\t/* webpackFoo: 1 */\n\t\"path\",\n\t{ type: \"module\" }\n\t/* webpackBar:2 */\n);\n"); + + // non-webpack comments + test("require(/* before */ foo)", "require(\n\t/* before */\n\tfoo\n);\n"); + test("require(/* before */ 'foo')", "require(\n\t/* before */\n\t\"foo\"\n);\n"); + test("require(foo /* after */)", "require(\n\tfoo\n\t/* after */\n);\n"); + test("require('foo' /* after */)", "require(\n\t\"foo\"\n\t/* after */\n);\n"); } #[test] @@ -613,37 +657,43 @@ 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: Not support trailing comments yet // 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 /* 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", - // ); - // test( - // "import(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);", - // "import(\n /* webpackFoo: 1 */\n \"path\"\n);\n", - // ); // Not currently handled - // test( - // "import(/* webpackFoo: 1 */ 'path', /* webpackBar:2 */ );", - // "import(\n /* webpackFoo: 1 */\n \"path\"\n /* 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 */ /* 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 */ '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\t/* webpackFoo: 1 */\n\t\"path\"\n);\n", + ); // Not currently handled + test( + "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', { 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 anywhere */ import.meta.url))", + "import(new URL(\n\t\"path\",\n\t/* webpackFoo: these can go anywhere */\n\timport.meta.url\n));\n"); + + // non-webpack comments + test("import(/* before */ foo)", "import(\n\t/* before */\n\tfoo\n);\n"); + test("import(/* before */ 'foo')", "import(\n\t/* before */\n\t\"foo\"\n);\n"); + test("import(foo /* after */)", "import(\n\tfoo\n\t/* after */\n);\n"); + test("import('foo' /* after */)", "import(\n\t\"foo\"\n\t/* after */\n);\n"); } #[test] diff --git a/crates/oxc_codegen/tests/integration/snapshots/pure_comments.snap b/crates/oxc_codegen/tests/integration/snapshots/pure_comments.snap index 51288eef6a92f..410915d5f06da 100644 --- a/crates/oxc_codegen/tests/integration/snapshots/pure_comments.snap +++ b/crates/oxc_codegen/tests/integration/snapshots/pure_comments.snap @@ -216,7 +216,9 @@ const builtInSymbols = new Set( ) ---------- -const builtInSymbols = new Set(/*#__PURE__*/ Object.getOwnPropertyNames(Symbol).filter((key) => key !== 'arguments' && key !== 'caller')); +const builtInSymbols = new Set( + /*#__PURE__*/ Object.getOwnPropertyNames(Symbol).filter((key) => key !== 'arguments' && key !== 'caller') +); ########## 14 (/* @__PURE__ */ new Foo()).bar(); diff --git a/crates/oxc_codegen/tests/integration/unit.rs b/crates/oxc_codegen/tests/integration/unit.rs index 6db532f87210e..203e2495b8759 100644 --- a/crates/oxc_codegen/tests/integration/unit.rs +++ b/crates/oxc_codegen/tests/integration/unit.rs @@ -255,3 +255,19 @@ fn equality() { test_minify("a, b == c , d", "a,b==c,d;"); test_minify("(a, b) == (c , d)", "(a,b)==(c,d);"); } + +#[test] +fn vite_special_comments() { + test( + "new URL(/* @vite-ignore */ 'non-existent', import.meta.url)", + "new URL(\n\t/* @vite-ignore */\n\t\"non-existent\",\n\timport.meta.url\n);\n", + ); + test( + "const importPromise = import(\n/* @vite-ignore */\nbase + '.js'\n);", + "const importPromise = import(\n\t/* @vite-ignore */\n\tbase + \".js\"\n);\n", + ); + test( + "import(/* @vite-ignore */ module1Url).then((module1) => {\nself.postMessage(module.default + module1.msg1 + import.meta.env.BASE_URL)})", + "import(\n\t/* @vite-ignore */\n\tmodule1Url\n).then((module1) => {\n\tself.postMessage(module.default + module1.msg1 + import.meta.env.BASE_URL);\n});\n", + ); +}