fix(codegen): preserve comments before expression operands (#24510)

Coverage directives attached to expression operands were silently dropped by codegen in logical right-hand sides, conditional consequents, object property values, and template literal interpolations. The AST expressions survive, but those emitters never consume their leading comment groups.

Print the comment group at each surviving expression boundary, probing parenthesized layers (`a || /* c */ (x)` anchors the comment at the `(`, `a || (/* c */ x)` at `x`). Parenthesized function and arrow IIFEs keep comments inside their generated wrapper.

Logical right-hand sides and conditional consequents only print groups containing annotations. This is codegen policy for mutated ASTs: comments are anchored by source position, and mutating consumers (minifier statement merging, rolldown finalization) move statements into exactly these positions — a dissolved statement's leading normal-comment group can coincide with the moved operand's span start, and printing it there misplaces statement-level trivia and breaks minify-twice idempotency (test262 `language/asi/S7.9_A5.8_T1.js` and friends). Annotations are the one comment kind with expression-level meaning, so they pass through.

Printing comments at expression boundaries also exposed three latent spacing bugs in the shared comment printer, surfaced by monitor-oxc over real node_modules: a full indent was injected before mid-line groups (doubling indentation on every pass, invisible at indent 0), a group ending in a newline left the next operand at column 0, and minify-with-comments mode emitted a glue space or not depending on which printer consumed the group. All three are fixed with regression tests.

## Example

Input:

```js
const value = a ?? /* istanbul ignore next */ [];
```

Before:

```js
const value = a ?? [];
```

After:

```js
const value = a ?? /* istanbul ignore next */ [];
```

Verified with `cargo test -p oxc_codegen` (134 tests), full `cargo coverage` (minifier at baseline, zero snapshot drift), `--twice` idempotency over webpack/typescript/eslint-utils/storybook/sigstore files in plain, dce, and minify-with-comments modes, and monitor-oxc green on codegen/dce/whitespace/transformer (remaining failures are main's pre-existing baseline).

closes #21301
closes #22049
closes #24324

This PR was assisted by OpenAI Codex and Claude.

crates/oxc_codegen/src/binary_expr_visitor.rs

@@ -230,6 +230,13 @@ impl<'a> BinaryExpressionVisitor<'a> {
         self.operator.r#gen(p);
         p.print_soft_space();
         let right = self.e.right();
+        if let Binaryish::Logical(e) = self.e {
+            // Annotation-gated (see the helper's doc): statements get merged
+            // into logical RHS positions on mutated ASTs. Pass the unstripped
+            // right — `Binaryish::right()` removes the paren layers the helper
+            // needs to probe.
+            p.print_annotation_comments_before_expression(&e.right);
+        }
         if !cjs_module_lexer::try_print_equality_string(p, self.operator, right) {
             right.gen_expr(p, self.right_precedence, self.ctx);
         }

crates/oxc_codegen/src/comment.rs

@@ -2,13 +2,27 @@ use std::borrow::Cow;
 
 use rustc_hash::{FxHashMap, FxHashSet};
 
-use oxc_ast::{Comment, CommentKind, ast::Program};
+use oxc_ast::{
+    Comment, CommentKind,
+    ast::{Expression, Program},
+};
+use oxc_span::GetSpan;
 use oxc_syntax::line_terminator::LineTerminatorSplitter;
 
 use crate::{Codegen, LegalComment, options::CommentOptions};
 
 pub type CommentsMap = FxHashMap</* attached_to */ u32, Vec<Comment>>;
 
+/// A `pife`-marked arrow or function expression prints its leading comments
+/// inside its own `(` wrap, so operand emission sites must not consume them.
+fn is_pife_function(expression: &Expression<'_>) -> bool {
+    match expression {
+        Expression::ArrowFunctionExpression(arrow) => arrow.pife,
+        Expression::FunctionExpression(function) => function.pife,
+        _ => false,
+    }
+}
+
 /// Which annotation kind an emission site expects to recover from
 /// [`Codegen::annotation_comments`].
 ///
@@ -158,13 +172,70 @@ impl Codegen<'_> {
         }
     }
 
-    /// Print leading comments at `start` and consume any pending indent-as-space,
-    /// so the next token glues to the comment instead of breaking onto a new line.
+    /// Print leading comments at `start` and glue the next token to them: after a
+    /// group ending in a newline (line comment / `followed_by_newline`), print the
+    /// indent — mid-expression callers have no statement machinery to do it, and an
+    /// unindented next token renders differently once the parser re-anchors the
+    /// comments to it (codegen would no longer be idempotent). Otherwise consume the
+    /// pending indent-as-space so the token glues with a single space.
     #[inline]
     pub(crate) fn print_leading_comments_anchored_to_self(&mut self, start: u32) {
         if let Some(comments) = self.get_comments(start) {
             self.print_comments(&comments);
-            self.consume_pending_indent_space();
+            if self.last_byte() == Some(b'\n') {
+                self.print_indent();
+            } else {
+                self.consume_pending_indent_space();
+            }
+        }
+    }
+
+    /// Print comments attached to an expression that survives codegen.
+    ///
+    /// Probes the parenthesized layers too: `a || /* c */ (x)` anchors the
+    /// comment at the `(`, `a || (/* c */ x)` at `x` — an operand printer only
+    /// sees one node, so the walk happens here for every emission site.
+    pub(crate) fn print_leading_comments_before_expression(&mut self, expression: &Expression<'_>) {
+        if self.comments.is_empty() || is_pife_function(expression) {
+            return;
+        }
+        self.print_leading_comments_anchored_to_self(expression.span().start);
+        if let Expression::ParenthesizedExpression(paren) = expression {
+            self.print_leading_comments_before_expression(&paren.expression);
+        }
+    }
+
+    /// Print an expression's comment group only when it contains an annotation
+    /// comment, probing parenthesized layers like
+    /// [`Self::print_leading_comments_before_expression`].
+    ///
+    /// This is the variant for emission sites that mutating consumers move
+    /// statements into (the minifier merges `if(a)x;if(b)x;` into
+    /// `if(a||(b,..))x`; rolldown finalizes moved nodes with their original
+    /// spans). Comments are anchored by source position, so a dissolved
+    /// statement's leading normal-comment group can coincide with the moved
+    /// operand's span start — printing it there misplaces statement-level
+    /// trivia inside an expression and is not idempotent
+    /// (`test_normal_comment_before_logical_rhs_not_printed` documents the
+    /// falsifier). Annotations are the one comment kind with expression-level
+    /// meaning, so they still pass through.
+    pub(crate) fn print_annotation_comments_before_expression(
+        &mut self,
+        expression: &Expression<'_>,
+    ) {
+        if self.comments.is_empty() || is_pife_function(expression) {
+            return;
+        }
+        let start = expression.span().start;
+        if self
+            .comments
+            .get(&start)
+            .is_some_and(|comments| comments.iter().any(|comment| comment.is_annotation()))
+        {
+            self.print_leading_comments_anchored_to_self(start);
+        }
+        if let Expression::ParenthesizedExpression(paren) = expression {
+            self.print_annotation_comments_before_expression(&paren.expression);
         }
     }
 
@@ -267,7 +338,12 @@ impl Codegen<'_> {
                     }
                 }
             }
-        } else {
+        } else if !self.consume_pending_indent_space()
+            && matches!(self.last_byte(), None | Some(b'\n'))
+        {
+            // Only indent at a line start. Mid-line emission sites (`a ?? /* c */ b`,
+            // `key: /* c */ value`, `${/* c */ expr}`) would otherwise get a full
+            // indent injected mid-line, growing indentation on every codegen pass.
             self.print_indent();
         }
         self.print_comment(first);

crates/oxc_codegen/src/gen.rs

@@ -1792,6 +1792,7 @@ impl Gen for ObjectProperty<'_> {
             p.print_soft_space();
         }
 
+        p.print_leading_comments_before_expression(&self.value);
         self.value.print_expr(p, Precedence::Comma, Context::empty());
     }
 }
@@ -1999,31 +2000,19 @@ impl GenExpr for ConditionalExpression<'_> {
             p.print_soft_space();
             p.print_ascii_byte(b'?');
             p.print_soft_space();
+            // Annotation-gated (see the helper's doc): `if/else` bodies get
+            // merged into consequents on mutated ASTs.
+            p.print_annotation_comments_before_expression(&self.consequent);
             self.consequent.print_expr(p, Precedence::Yield, Context::empty());
             p.print_soft_space();
             p.print_colon();
             p.print_soft_space();
-            // Skip when the alternate is a `pife` arrow/function — its own
-            // gen_expr prints the leading comment inside its `(` wrap so the
-            // source position `: ( /* c */ arrow )` is preserved.
-            if !is_pife_arrow_or_function(&self.alternate) {
-                p.print_leading_comments_anchored_to_self(self.alternate.span().start);
-            }
+            p.print_leading_comments_before_expression(&self.alternate);
             self.alternate.print_expr(p, Precedence::Yield, ctx & Context::FORBID_IN);
         });
     }
 }
 
-/// `true` if `expr` is a `pife`-marked arrow or function expression — its
-/// own `gen_expr` adds a `(` wrap and prints leading comments inside it.
-fn is_pife_arrow_or_function(expr: &Expression<'_>) -> bool {
-    match expr {
-        Expression::ArrowFunctionExpression(arrow) => arrow.pife,
-        Expression::FunctionExpression(func) => func.pife,
-        _ => false,
-    }
-}
-
 impl GenExpr for AssignmentExpression<'_> {
     fn gen_expr(&self, p: &mut Codegen, precedence: Precedence, ctx: Context) {
         let n = p.code_len();
@@ -2314,6 +2303,7 @@ impl Gen for TemplateLiteral<'_> {
         p.print_str_escaping_script_close_tag(first_quasi.value.raw.as_str());
         for (expr, quasi) in self.expressions.iter().zip(remaining_quasis) {
             p.print_str("${");
+            p.print_leading_comments_before_expression(expr);
             p.print_expression(expr);
             p.print_ascii_byte(b'}');
             p.add_source_mapping(quasi.span);

crates/oxc_codegen/src/lib.rs

@@ -565,7 +565,7 @@ impl<'a> Codegen<'a> {
     #[inline]
     fn consume_pending_indent_space(&mut self) -> bool {
         if self.print_next_indent_as_space {
-            self.print_hard_space();
+            self.print_soft_space();
             self.print_next_indent_as_space = false;
             true
         } else {

crates/oxc_codegen/tests/integration/comments.rs

@@ -36,6 +36,147 @@ fn test_line_comment_after_conditional_colon() {
     test_idempotency("x = (_a = a) === null || _a === void 0 ? void 0 : // c1\n// c2\n_a.b();");
 }
 
+#[test]
+fn test_comments_before_expression_operands() {
+    // https://github.com/oxc-project/oxc/issues/21301
+    // https://github.com/oxc-project/oxc/issues/24324
+    test(
+        "const value = a ?? /* istanbul ignore next */ [];",
+        "const value = a ?? /* istanbul ignore next */ [];\n",
+    );
+    test(
+        "const value = a && /* istanbul ignore next */ otherValue;",
+        "const value = a && /* istanbul ignore next */ otherValue;\n",
+    );
+    test(
+        "const value = a ? /* istanbul ignore next */ first : second;",
+        "const value = a ? /* istanbul ignore next */ first : second;\n",
+    );
+    test(
+        "condition ?\n  /* v8 ignore next */ uncovered() :\n  coveredAlternate();",
+        "condition ? \n/* v8 ignore next */ uncovered() : coveredAlternate();\n",
+    );
+    test(
+        "const value = { aFunction: /* istanbul ignore next */ () => {} };",
+        "const value = { aFunction: /* istanbul ignore next */ () => {} };\n",
+    );
+}
+
+#[test]
+fn test_comments_before_expression_operands_idempotency() {
+    test_idempotency("const value = a ?? /* istanbul ignore next */ [];");
+    test_idempotency("const value = a && /* istanbul ignore next */ otherValue;");
+    test_idempotency("const value = a ? /* istanbul ignore next */ first : second;");
+    test_idempotency("condition ?\n  /* v8 ignore next */ uncovered() :\n  coveredAlternate();");
+    test_idempotency("const value = { aFunction: /* istanbul ignore next */ () => {} };");
+}
+
+// A mid-line comment group must not receive a full indent: `print_comments`
+// used to inject `indent` tabs before any group whose first comment was not
+// preceded by a newline, so indented emission sites (`key: /** c */ value`)
+// grew indentation on every codegen pass (monitor-oxc caught this on
+// webpack's `source: /** @type {string} */ (source)` shorthand collapse).
+#[test]
+fn test_comment_before_indented_object_property_value() {
+    test(
+        "function f(source) {\n\treturn {\n\t\tsource: /** @type {string} */ (source),\n\t\tother: 1\n\t};\n}",
+        "function f(source) {\n\treturn {\n\t\t/** @type {string} */ source,\n\t\tother: 1\n\t};\n}\n",
+    );
+    test_idempotency(
+        "function f(source) {\n\treturn {\n\t\tsource: /** @type {string} */ (source),\n\t\tother: 1\n\t};\n}",
+    );
+}
+
+// A comment group ending in a newline before a mid-expression operand must
+// re-indent the operand: pass 1 otherwise leaves it at column 0 while the
+// reparse anchors the group to the item and indents it (monitor-oxc dce
+// caught this on storybook's `argument: /** @type {Expression} */ (argument)`
+// with the group on its own lines).
+#[test]
+fn test_newline_comment_group_before_object_property_value() {
+    test(
+        "function f(argument) {\n\treturn {\n\t\ttype: 1,\n\t\targument:\n\t\t// c1\n\t\t/** @type {Expression} */\n\t\t(argument)\n\t};\n}",
+        "function f(argument) {\n\treturn {\n\t\ttype: 1,\n\t\t// c1\n\t\t/** @type {Expression} */\n\t\targument\n\t};\n}\n",
+    );
+    test_idempotency(
+        "function f(argument) {\n\treturn {\n\t\ttype: 1,\n\t\targument:\n\t\t// c1\n\t\t/** @type {Expression} */\n\t\t(argument)\n\t};\n}",
+    );
+}
+
+// An annotation before a parenthesized logical RHS anchors to the `(`, which
+// `Binaryish::right()` strips — the lookup must check the unstripped span too.
+// This is the reparse shape of a lowered optional chain (monitor-oxc
+// transformer caught it on `a?.x || /* istanbul ignore next */ a?.y`).
+#[test]
+fn test_annotation_before_parenthesized_logical_rhs() {
+    test(
+        "x = a || /* istanbul ignore next */ (b ? c : d);",
+        "x = a || /* istanbul ignore next */ (b ? c : d);\n",
+    );
+    test_idempotency("x = a || /* istanbul ignore next */ (b ? c : d);");
+    // The comment can also anchor INSIDE the parens — the helpers probe
+    // every parenthesized layer, at all emission sites. It hoists before the
+    // `(`, where the reparse re-anchors it, so the rendering is stable.
+    test(
+        "x = a || (/* istanbul ignore next */ b ? c : d);",
+        "x = a || /* istanbul ignore next */ (b ? c : d);\n",
+    );
+    test_idempotency("x = a || (/* istanbul ignore next */ b ? c : d);");
+    test("y = { k: (/* lingui */ v) };", "y = { k: /* lingui */ v };\n");
+    test_idempotency("y = { k: (/* lingui */ v) };");
+}
+
+// In minify mode (comments kept, whitespace removed) a comment group glues to
+// the next token with no space: `print_indent` never consumes the pending
+// indent-as-space under minify, so an emitting consumer would make spacing
+// depend on which printer consumed the group (`*/ x` vs `*/x`) — monitor-oxc
+// whitespace caught this on webpack's `/** @type {string} */ (source)`.
+#[test]
+fn test_minify_comment_glue_idempotency() {
+    use oxc_codegen::CodegenOptions;
+    let minify_with_comments = CodegenOptions { minify: true, ..CodegenOptions::default() };
+    crate::tester::test_options(
+        "const x = { source: /** @type {string} */ (source), other: 1 };",
+        "const x={/** @type {string} */source,other:1};",
+        minify_with_comments.clone(),
+    );
+    test_idempotency_options(
+        "const x = { source: /** @type {string} */ (source), other: 1 };",
+        &minify_with_comments,
+    );
+}
+
+// Normal-comment groups must NOT be printed before a logical RHS: the minifier
+// merges statements into logical right-hand sides (`if(a)x;if(b)x;` ->
+// `if(a||(b,..))x`), which can anchor a removed statement's banner comments at
+// the RHS span start; printing them mid-expression breaks minify idempotency
+// (minifier_test262 `language/asi/S7.9_A5.8_T1.js`). Only annotation-bearing
+// groups (coverage directives etc.) are printed.
+#[test]
+fn test_normal_comment_before_logical_rhs_not_printed() {
+    test("const value = a ?? /* plain comment */ [];", "const value = a ?? [];\n");
+    test("const value = a || //\n////////\n(b, c);", "const value = a || (b, c);\n");
+}
+
+#[test]
+fn test_comment_before_template_literal_interpolation() {
+    // https://github.com/oxc-project/oxc/issues/22049
+    test(
+        "const value = `color: ${/* v8 ignore next -- @preserve */ ({ theme }) => theme.color}`;",
+        "const value = `color: ${/* v8 ignore next -- @preserve */ ({ theme }) => theme.color}`;\n",
+    );
+    test(
+        "const value = `color: ${\n  /* v8 ignore next -- @preserve */\n  ({ theme }) => theme.color\n};`;",
+        "const value = `color: ${\n/* v8 ignore next -- @preserve */\n({ theme }) => theme.color};`;\n",
+    );
+    test_idempotency(
+        "const value = `color: ${/* v8 ignore next -- @preserve */ ({ theme }) => theme.color}`;",
+    );
+    test_idempotency(
+        "const value = `color: ${\n  /* v8 ignore next -- @preserve */\n  ({ theme }) => theme.color\n};`;",
+    );
+}
+
 #[test]
 fn test_comment_at_top_of_file() {
     use oxc_allocator::Allocator;
@@ -71,10 +212,7 @@ fn unit() {
     test("console.log(<>{/*before*/ x}</>)", "console.log(<>{/*before*/ x}</>);\n");
     // https://lingui.dev/ref/macro#definemessage
     test("const message = /*i18n*/{};", "const message = (/*i18n*/ {});\n");
-    test(
-        "function foo() { return /*i18n*/ {} }",
-        "function foo() {\n\treturn (\t/*i18n*/ {});\n}\n",
-    );
+    test("function foo() { return /*i18n*/ {} }", "function foo() {\n\treturn (/*i18n*/ {});\n}\n");
 
     test_same("export { /** @deprecated */ parseAst } from \"rolldown/parseAst\";\n");
     test_same("export { /** @deprecated */ parseAst };\n");