parser: preserve comments inside function parameter lists

src/ast/g.rs

@@ -337,6 +337,9 @@ pub struct Arg {
     pub is_typescript_ctor_field: bool,
 
     pub ts_metadata: TypeScript::Metadata,
+
+    // `function(/* token */ arg)` — kept for Function.prototype.toString()
+    pub leading_comments: StoreSlice<Comment>,
 }
 
 impl Default for Arg {
@@ -347,6 +350,7 @@ impl Default for Arg {
             default: None,
             is_typescript_ctor_field: false,
             ts_metadata: TypeScript::Metadata::MNone,
+            leading_comments: StoreSlice::EMPTY,
         }
     }
 }
@@ -368,6 +372,7 @@ impl Arg {
             },
             is_typescript_ctor_field: self.is_typescript_ctor_field,
             ts_metadata: self.ts_metadata.clone(),
+            leading_comments: self.leading_comments,
         })
     }
 }

src/js_parser/parse/parse_fn.rs

@@ -14,6 +14,11 @@ use bun_ast::{E, Expr, Flags, G, S, Stmt};
 
 type Error = crate::Error;
 
+#[inline]
+fn is_legal_comment(text: &[u8]) -> bool {
+    text.len() > 2 && text[2] == b'!'
+}
+
 impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_ONLY> {
     /// This assumes the "function" token has already been parsed
     pub(crate) fn parse_fn_stmt(
@@ -160,6 +165,34 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         Ok(p.s(S::Function { func }, loc))
     }
 
+    fn drain_arg_leading_comments_into(
+        &mut self,
+        out: &mut bun_alloc::ArenaVec<'a, G::Comment>,
+        base: usize,
+    ) {
+        let buf = &mut self.lexer.comments_to_preserve_before;
+        let mut i = base.min(buf.len());
+        while i < buf.len() {
+            if is_legal_comment(buf[i].text.slice()) {
+                i += 1;
+            } else {
+                out.push(buf.remove(i));
+            }
+        }
+    }
+
+    fn discard_non_legal_comments_from(&mut self, base: usize) {
+        let buf = &mut self.lexer.comments_to_preserve_before;
+        let mut i = base.min(buf.len());
+        while i < buf.len() {
+            if is_legal_comment(buf[i].text.slice()) {
+                i += 1;
+            } else {
+                buf.remove(i);
+            }
+        }
+    }
+
     pub(crate) fn parse_fn(
         &mut self,
         name: Option<js_ast::LocRef>,
@@ -185,6 +218,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             open_parens_loc: p.lexer.loc(),
             ..Default::default()
         };
+
+        // Keep `/* ... */` inside `(...)` for Function.prototype.toString().
+        let old_preserve_comments = p.lexer.preserve_all_comments_before;
+        let mut comments_base = p.lexer.comments_to_preserve_before.len();
+        p.lexer.preserve_all_comments_before = true;
         p.lexer.expect(T::TOpenParen)?;
 
         // Await and yield are not allowed in function arguments
@@ -216,6 +254,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         while p.lexer.token != T::TCloseParen {
             // Skip over "this" type annotations
             if Self::IS_TYPESCRIPT_ENABLED && p.lexer.token == T::TThis {
+                p.discard_non_legal_comments_from(comments_base);
+                p.lexer.preserve_all_comments_before = old_preserve_comments;
                 p.lexer.next()?;
                 if p.lexer.token == T::TColon {
                     p.lexer.next()?;
@@ -225,13 +265,21 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                     break;
                 }
 
+                comments_base = p.lexer.comments_to_preserve_before.len();
+                p.lexer.preserve_all_comments_before = true;
                 p.lexer.next()?;
                 continue;
             }
 
+            let mut leading = bun_alloc::ArenaVec::<G::Comment>::new_in(p.arena);
+
             let mut ts_decorators = bun_alloc::AstAlloc::vec();
             if opts.allow_ts_decorators {
+                p.drain_arg_leading_comments_into(&mut leading, comments_base);
+                p.lexer.preserve_all_comments_before = old_preserve_comments;
                 ts_decorators = p.parse_type_script_decorators()?;
+                comments_base = p.lexer.comments_to_preserve_before.len();
+                p.lexer.preserve_all_comments_before = true;
                 if ts_decorators.len_u32() > 0 {
                     arg_has_decorators = true;
                 }
@@ -244,6 +292,14 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 func.flags.insert(Flags::Function::HasRestArg);
             }
 
+            p.drain_arg_leading_comments_into(&mut leading, comments_base);
+            let leading_comments = if leading.is_empty() {
+                bun_ast::StoreSlice::EMPTY
+            } else {
+                bun_ast::StoreSlice::from_bump(leading)
+            };
+            p.lexer.preserve_all_comments_before = old_preserve_comments;
+
             let mut is_typescript_ctor_field = false;
             let is_identifier = p.lexer.token == T::TIdentifier;
             let mut text = p.lexer.identifier;
@@ -325,6 +381,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 // We need to track this because it affects code generation
                 is_typescript_ctor_field,
                 ts_metadata,
+                leading_comments,
             });
 
             if p.lexer.token != T::TComma {
@@ -343,9 +400,14 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 break;
             }
 
+            comments_base = p.lexer.comments_to_preserve_before.len();
+            p.lexer.preserve_all_comments_before = true;
             p.lexer.next()?;
             rest_arg = false;
         }
+        p.lexer.preserve_all_comments_before = old_preserve_comments;
+        p.discard_non_legal_comments_from(comments_base);
+
         if !args.is_empty() {
             func.args = bun_ast::StoreSlice::new_mut(args.into_bump_slice_mut());
         }

src/js_printer/lib.rs

@@ -2108,6 +2108,23 @@ pub(crate) mod __gated_printer {
                     self.print_space();
                 }
 
+                if !self.options.minify_whitespace {
+                    for comment in arg.leading_comments.slice() {
+                        let text = comment.text.slice();
+                        if text.starts_with(b"//") {
+                            let body = &text[2..];
+                            if !strings::contains(body, b"*/") {
+                                self.print(b"/*");
+                                self.print(body);
+                                self.print(b" */ ");
+                            }
+                        } else {
+                            self.print(text);
+                            self.print(b" ");
+                        }
+                    }
+                }
+
                 if has_rest_arg && i + 1 == args.len() {
                     self.print(b"...");
                 }

test/regression/issue/13451/13451.test.ts

@@ -0,0 +1,175 @@
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe, tempDir } from "harness";
+
+// https://github.com/oven-sh/bun/issues/13451
+//
+// Karma's `di` package (and AngularJS $injector) parse Function.prototype.toString()
+// to extract DI tokens from /* comment */-annotated parameter names, e.g.
+//
+//   exports.create = function (/* config */ config, /* config.proxies */ proxies) { ... }
+//
+// Bun's runtime transpiler was discarding those comments, so `di` saw `proxies`
+// instead of `config.proxies` and failed with "No provider for "proxies"!".
+
+const FN_ARGS = /^function\s*[^\(]*\(\s*([^\)]*)\)/m;
+const FN_ARG = /\/\*([^\*]*)\*\//m;
+
+function diParse(src: string): string[] {
+  const match = src.match(FN_ARGS);
+  if (!match || !match[1]) return [];
+  return match[1].split(",").map(arg => {
+    const m = arg.match(FN_ARG);
+    return m ? m[1].trim() : arg.trim();
+  });
+}
+
+test.concurrent("inline /* */ comments in function parameter lists survive the runtime transpiler", async () => {
+  using dir = tempDir("issue-13451", {
+    "mod.cjs": `
+exports.anonExpr = function (/* a */ x, /* b.c */ y, /* d */ z) {
+  return [x, y, z];
+};
+
+function decl(/* one */ p, /* two */ q) {
+  return [p, q];
+}
+exports.decl = decl;
+
+exports.obj = {
+  method(/* inner */ v) {
+    return v;
+  },
+};
+
+exports.karma = function (/* config */ config, /* config.proxies */ proxies, /* emitter */ emitter) {
+  return [config, proxies, emitter];
+};
+
+exports.lineComment = function (
+  // token
+  value
+) {
+  return value;
+};
+
+exports.noMigrateDefault = function (a = /* x */ 1, b) {
+  return [a, b];
+};
+
+exports.noMigrateTrailing = function (a /* trail */, b) {
+  return [a, b];
+};
+
+exports.noLeakIntoBody = function (a = () => { return 1 /* mid */ + 2; }, b) {
+  return [a, b];
+};
+`,
+    "main.cjs": `
+const m = require("./mod.cjs");
+console.log(JSON.stringify({
+  anonExpr: m.anonExpr.toString(),
+  decl: m.decl.toString(),
+  method: m.obj.method.toString(),
+  karma: m.karma.toString(),
+  lineComment: m.lineComment.toString(),
+  noMigrateDefault: m.noMigrateDefault.toString(),
+  noMigrateTrailing: m.noMigrateTrailing.toString(),
+  noLeakIntoBody: m.noLeakIntoBody.toString(),
+}));
+`,
+  });
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "main.cjs"],
+    env: bunEnv,
+    cwd: String(dir),
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stderr).toBe("");
+
+  const results = JSON.parse(stdout);
+
+  // anonymous function expression
+  expect(results.anonExpr).toContain("/* a */");
+  expect(results.anonExpr).toContain("/* b.c */");
+  expect(results.anonExpr).toContain("/* d */");
+  expect(diParse(results.anonExpr)).toEqual(["a", "b.c", "d"]);
+
+  // named function declaration
+  expect(results.decl).toContain("/* one */");
+  expect(results.decl).toContain("/* two */");
+  expect(diParse(results.decl)).toEqual(["one", "two"]);
+
+  // object method shorthand
+  expect(results.method).toContain("/* inner */");
+
+  // karma's exact pattern from middleware/proxy.js
+  expect(results.karma).toContain("/* config */");
+  expect(results.karma).toContain("/* config.proxies */");
+  expect(results.karma).toContain("/* emitter */");
+  expect(diParse(results.karma)).toEqual(["config", "config.proxies", "emitter"]);
+
+  // line comment converted to block (otherwise it would swallow the arg list)
+  expect(results.lineComment).not.toMatch(/\/\/[^\n]*\)/);
+  expect(results.lineComment).toContain("token");
+
+  // Comments inside a default value / after a binding must not migrate to the
+  // next argument (that would change the DI token), nor surface as statements
+  // inside a nested body.
+  expect(results.noMigrateDefault).not.toContain("/* x */");
+  expect(diParse(results.noMigrateDefault)).toEqual(["a = 1", "b"]);
+  expect(results.noMigrateTrailing).not.toContain("/* trail */");
+  expect(diParse(results.noMigrateTrailing)).toEqual(["a", "b"]);
+  expect(results.noLeakIntoBody).not.toContain("/* mid */");
+
+  expect(exitCode).toBe(0);
+});
+
+test.concurrent("comments inside erased TS this-params/decorators are not relocated onto real args", async () => {
+  using dir = tempDir("issue-13451-ts", {
+    "in.ts": `
+export function f1(this: /* Self */ number, x: number) { return x; }
+export function f2(/* ctx */ this: number, x: number) { return x; }
+declare const dec: any;
+export class C {
+  constructor(@dec(/* opts */) x: number) {}
+}
+`,
+  });
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "build", "--target=bun", "in.ts"],
+    env: bunEnv,
+    cwd: String(dir),
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stderr).toBe("");
+  expect(stdout).not.toContain("/* Self */");
+  expect(stdout).not.toContain("/* ctx */");
+  expect(stdout).not.toContain("/* opts */");
+  expect(exitCode).toBe(0);
+});
+
+test.concurrent("legal comments in parameter lists survive minification", async () => {
+  using dir = tempDir("issue-13451-legal", {
+    "in.js": `
+export const f = function(/*! @license MIT */ x) { return x; };
+export const g = function(/*! @preserve */) { return 1; };
+`,
+  });
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "build", "--minify", "in.js"],
+    env: bunEnv,
+    cwd: String(dir),
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stderr).toBe("");
+  expect(stdout).toContain("/*! @license MIT */");
+  expect(stdout).toContain("/*! @preserve */");
+  expect(exitCode).toBe(0);
+});