nprasad2077/oxc
1
0 Fork 0
mirror of https://github.com/oxc-project/oxc.git synced 2026-11-04 10:29:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(codegen): preserve verbatim text of pure/no-side-effects comments (#22525)

Browse Source 
## Summary

- Stash `@__PURE__` / `@__NO_SIDE_EFFECTS__` (and `#`-prefixed) comments by `attached_to` during comment build, then emit the original source text at call / new / function-decl sites. Falls back to the canonical literal (`/* @__PURE__ */`, `/* @__NO_SIDE_EFFECTS__ */`) only when source text is unavailable or the stashed comment's kind doesn't match the emission site.
- Inputs like `/* #__PURE__ */`, `/*#__PURE__*/`, and `/* #__PURE__ -- @preserve */` round-trip verbatim instead of being normalized to `/* @__PURE__ */` ([rolldown#9408](https://github.com/rolldown/rolldown/issues/9408)).
- Verified by `cargo test -p oxc_codegen` (105 tests; includes new regression for mixed-kind `attached_to` collision in `pure_comment`).

## Problem

Codegen previously dropped pure / no-side-effects comments and emitted a canonical literal at every site marked with the AST `pure` flag. This lost the original text:

- `/* #__PURE__ */ foo()` → `/* @__PURE__ */ foo()` — bundlers / tools that prefer the `#` form lost it.
- `/*#__PURE__*/ foo()` (no spaces) was expanded to `/* @__PURE__ */ foo()`.
- `/* #__PURE__ -- @preserve */ foo()` lost the trailing `-- @preserve` marker that some tools key on (rolldown#9408).

## Fix

A new `annotation_comments: FxHashMap<u32, Comment>` stashes `is_pure()` / `is_no_side_effects()` comments keyed by `attached_to`. `Codegen::print_annotation_comment(start, kind, newline_after)` emits the verbatim source when available, otherwise prints the canonical literal owned by `AnnotationKind::canonical(newline_after)`.

Why a separate map from the existing `comments` (legal / JSDoc / not-applied / normal): applied pure annotations must be emitted **inline at the `pure` site** with a trailing space — not as a leading comment with a newline — and only when `node.pure == true`. Sharing storage would either double-print or conflate two formatting modes inside `print_comments`. Comments where the parser couldn't apply the annotation (e.g. `/* #__PURE__ */ function foo() {}` — wrong target; classified as `CommentContent::PureNotApplied`) still flow through `comments` and the existing leading-comment behavior is preserved.

### Mixed-kind collision

When multiple annotation comments share an `attached_to` (e.g. `/* @__PURE__ */ /* @__NO_SIDE_EFFECTS__ */ foo()`), the map is last-write-wins. The `AnnotationKind` filter on lookup ensures a `@__NO_SIDE_EFFECTS__` slot can't be emitted in place of `@__PURE__` (or vice versa); the dropped comment falls back to the canonical literal. Per the [compiler-notations-spec](https://github.com/javascript-compiler-hints/compiler-notations-spec), the two annotations only apply to their corresponding node kinds (call/new vs function declaration), so mixing them in front of a `CallExpression` would be a real semantic regression.

## Before / after

```js
// input
/* #__PURE__ -- @preserve */ pureOperation();
/*#__PURE__*/ foo(bar);
```

```js
// before
/* @__PURE__ */ pureOperation();
/* @__PURE__ */ foo(bar);

// after
/* #__PURE__ -- @preserve */ pureOperation();
/*#__PURE__*/ foo(bar);
```

AI assisted.
This commit is contained in:
Dunqing committed 2026-05-19 10:13:25 +00:00
1 parent 9f227fdaf7
commit d61e1d7caa
6 files changed
+182 -61
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/oxc_codegen/src/comment.rs
+90 -1
View File
@@ -9,14 +9,61 @@ use crate::{Codegen, LegalComment, options::CommentOptions};
pub type CommentsMap = FxHashMap</* attached_to */ u32, Vec<Comment>>;
/// Which annotation kind an emission site expects to recover from
/// [`Codegen::annotation_comments`].
///
/// `@__PURE__` / `#__PURE__` on a `CallExpression` or `NewExpression`, and
/// `@__NO_SIDE_EFFECTS__` / `#__NO_SIDE_EFFECTS__` on a function declaration or
/// expression, are not interchangeable: downstream tree-shakers only honor
/// each on its corresponding node kind. The filter prevents
/// [`Codegen::print_annotation_comment`] from emitting one kind where the
/// other was expected when both share an `attached_to`.
#[derive(Clone, Copy)]
pub enum AnnotationKind {
Pure,
NoSideEffects,
}
impl AnnotationKind {
#[inline]
fn matches(self, comment: &Comment) -> bool {
match self {
Self::Pure => comment.is_pure(),
Self::NoSideEffects => comment.is_no_side_effects(),
}
}
/// Canonical literal to emit when no verbatim source is available.
/// `newline_after = true` is used at statement-level emission sites
/// (function declarations, exports), `false` at inline emission sites
/// (call / new / function expressions).
#[inline]
fn canonical(self, newline_after: bool) -> &'static str {
match (self, newline_after) {
(Self::Pure, false) => "/* @__PURE__ */ ",
(Self::Pure, true) => "/* @__PURE__ */\n",
(Self::NoSideEffects, false) => "/* @__NO_SIDE_EFFECTS__ */ ",
(Self::NoSideEffects, true) => "/* @__NO_SIDE_EFFECTS__ */\n",
}
}
}
impl Codegen<'_> {
pub(crate) fn build_comments(&mut self, comments: &[Comment]) {
if self.options.comments == CommentOptions::disabled() {
return;
}
for comment in comments {
// Omit pure comments because they are handled separately.
// Stash pure / no-side-effects comments by `attached_to` so the
// emission site can recover the verbatim source text instead of
// falling back to the canonical literal (rolldown#9408).
// Best-effort: when several annotation comments share an
// `attached_to`, only the last survives; the emission site falls
// back to the canonical literal for the dropped ones.
if comment.is_pure() || comment.is_no_side_effects() {
if comment.is_leading() && self.options.print_annotation_comment() {
self.annotation_comments.insert(comment.attached_to, *comment);
}
continue;
}
let mut add = false;
@@ -49,6 +96,48 @@ impl Codegen<'_> {
self.comments.contains_key(&start)
}
/// Emit a pure / no-side-effects annotation comment for the AST node at
/// `start`, falling back to the canonical literal when no verbatim source
/// can be recovered.
///
/// The fallback covers four cases:
/// - no annotation comment is stashed at `start`,
/// - the stashed comment's kind doesn't match the emission site (e.g. a
/// `@__NO_SIDE_EFFECTS__` slot being queried by a `CallExpression`
/// site that needs `@__PURE__`),
/// - the comment is a line comment but the site can't break the line, or
/// - source text is unavailable (e.g. the [`Codegen::print_expression`]
/// path that skips [`Codegen::build_comments`]).
///
/// Export sites pass `self.span.start` and only recover verbatim when the
/// annotation precedes the `export` keyword. The rarer
/// `export /* @__NO_SIDE_EFFECTS__ */ function …` form (annotation between
/// `export` and `function`) attaches to the inner function's span and
/// falls back to canonical here.
pub(crate) fn print_annotation_comment(
&mut self,
start: u32,
kind: AnnotationKind,
newline_after: bool,
) {
if self.source_text.is_some()
&& let Some(comment) = self.annotation_comments.get(&start).copied()
&& kind.matches(&comment)
// Inline line comments would swallow the rest of the line.
&& (!comment.is_line() || newline_after)
{
self.annotation_comments.remove(&start);
self.print_comment(&comment);
if newline_after {
self.print_hard_newline();
} else {
self.print_str(" ");
}
return;
}
self.print_str(kind.canonical(newline_after));
}
pub(crate) fn print_leading_comments(&mut self, start: u32) {
if let Some(comments) = self.comments.remove(&start) {
self.print_comments(&comments);
crates/oxc_codegen/src/gen.rs
+24 -11
View File
@@ -13,12 +13,9 @@ use crate::{
Codegen, Context, Operator, Quote,
binary_expr_visitor::{BinaryExpressionVisitor, Binaryish, BinaryishOperator},
cjs_module_lexer,
comment::AnnotationKind,
};
const PURE_COMMENT: &str = "/* @__PURE__ */ ";
const NO_SIDE_EFFECTS_NEW_LINE_COMMENT: &str = "/* @__NO_SIDE_EFFECTS__ */\n";
const NO_SIDE_EFFECTS_COMMENT: &str = "/* @__NO_SIDE_EFFECTS__ */ ";
/// Generate source code for an AST node.
pub trait Gen: GetSpan {
/// Generate code for an AST node.
@@ -124,7 +121,11 @@ impl Gen for Statement<'_> {
p.print_comments_at(decl.span.start);
if decl.pure && p.options.print_annotation_comment() {
p.print_indent();
p.print_str(NO_SIDE_EFFECTS_NEW_LINE_COMMENT);
p.print_annotation_comment(
decl.span.start,
AnnotationKind::NoSideEffects,
true,
);
}
p.print_indent();
decl.print(p, ctx);
@@ -1002,7 +1003,10 @@ impl Gen for ExportNamedDeclaration<'_> {
&& func.pure
&& p.options.print_annotation_comment()
{
p.print_str(NO_SIDE_EFFECTS_NEW_LINE_COMMENT);
// Recover the verbatim annotation only when it sits before the
// `export` keyword; an annotation between `export` and `function`
// attaches to the inner function span and falls back to canonical.
p.print_annotation_comment(self.span.start, AnnotationKind::NoSideEffects, true);
}
p.print_indent();
p.add_source_mapping(self.span);
@@ -1165,7 +1169,8 @@ impl Gen for ExportDefaultDeclaration<'_> {
&& func.pure
&& p.options.print_annotation_comment()
{
p.print_str(NO_SIDE_EFFECTS_NEW_LINE_COMMENT);
// See [`ExportNamedDeclaration`] for the rationale.
p.print_annotation_comment(self.span.start, AnnotationKind::NoSideEffects, true);
}
p.print_indent();
p.add_source_mapping(self.span);
@@ -1223,13 +1228,21 @@ impl GenExpr for Expression<'_> {
// Function expressions
Self::ArrowFunctionExpression(func) => {
if func.pure && p.options.print_annotation_comment() {
p.print_str(NO_SIDE_EFFECTS_COMMENT);
p.print_annotation_comment(
func.span.start,
AnnotationKind::NoSideEffects,
false,
);
}
func.print_expr(p, precedence, ctx);
}
Self::FunctionExpression(func) => {
if func.pure && p.options.print_annotation_comment() {
p.print_str(NO_SIDE_EFFECTS_COMMENT);
p.print_annotation_comment(
func.span.start,
AnnotationKind::NoSideEffects,
false,
);
}
func.print(p, ctx);
}
@@ -1489,7 +1502,7 @@ impl GenExpr for CallExpression<'_> {
p.wrap(wrap, |p| {
if pure {
p.add_source_mapping(self.span);
p.print_str(PURE_COMMENT);
p.print_annotation_comment(self.span.start, AnnotationKind::Pure, false);
}
if is_export_default {
p.start_of_default_export = p.code_len();
@@ -2275,7 +2288,7 @@ impl GenExpr for NewExpression<'_> {
}
p.wrap(wrap, |p| {
if pure {
p.print_str(PURE_COMMENT);
p.print_annotation_comment(self.span.start, AnnotationKind::Pure, false);
}
p.print_space_before_identifier();
p.add_source_mapping(self.span);
crates/oxc_codegen/src/lib.rs
+11
View File
@@ -124,6 +124,16 @@ pub struct Codegen<'a> {
// Builders
comments: CommentsMap,
/// Pure / no-side-effects annotation comments keyed by `attached_to`,
/// so the emission site can recover verbatim source text instead of a
/// canonicalised literal (rolldown#9408). The emission site falls back
/// to the canonical literal when (a) no comment is stashed, (b) the
/// stashed comment's kind doesn't match the emission site (mixed
/// `@__PURE__` and `@__NO_SIDE_EFFECTS__` on the same `attached_to`),
/// (c) the comment is a line comment but the site can't break the line,
/// or (d) source text isn't available.
annotation_comments: FxHashMap<u32, Comment>,
/// Sorted, deduped `attached_to` keys for pending legal comments. Lets
/// `print_legal_orphans_before` flush via `partition_point` + `drain`.
legal_comment_keys: Vec<u32>,
@@ -180,6 +190,7 @@ impl<'a> Codegen<'a> {
indent: 0,
quote: Quote::Double,
comments: CommentsMap::default(),
annotation_comments: FxHashMap::default(),
legal_comment_keys: Vec::new(),
#[cfg(feature = "sourcemap")]
sourcemap_builder: None,
crates/oxc_codegen/tests/integration/esbuild.rs
+2 -2
View File
@@ -559,7 +559,7 @@ fn test_pure_comment() {
test("new (function() {})", "new (function() {})();\n");
test("new (function() {})()", "new (function() {})();\n");
test("/*@__PURE__*/new (function() {})()", "/* @__PURE__ */ new (function() {})();\n");
test("/*@__PURE__*/new (function() {})()", "/*@__PURE__*/ new (function() {})();\n");
test("export default (function() { foo() })", "export default (function() {\n\tfoo();\n});\n");
test(
@@ -568,7 +568,7 @@ fn test_pure_comment() {
);
test(
"export default /*@__PURE__*/(function() { foo() })()",
"export default /* @__PURE__ */ (function() {\n\tfoo();\n})();\n",
"export default /*@__PURE__*/ (function() {\n\tfoo();\n})();\n",
);
}
crates/oxc_codegen/tests/integration/js.rs
+9 -1
View File
@@ -437,8 +437,16 @@ fn pure_comment() {
test_same("/* @__PURE__ */ pureOperation();\n");
test_same("/* @__PURE__ */ new PureConsutrctor();\n");
test("/* @__PURE__ */\npureOperation();\n", "/* @__PURE__ */ pureOperation();\n");
test_same("/* @__PURE__ The comment may contain additional text */ pureOperation();\n");
test_same(
"/* #__PURE__ -- @preserve */ pureOperation();\n", // rolldown#9408
);
// A `@__NO_SIDE_EFFECTS__` comment sharing the call site's `attached_to`
// must not be emitted in place of the pure-call annotation. Without the
// kind filter, `FxHashMap` last-write-wins would print the wrong
// annotation kind in front of a CallExpression.
test(
"/* @__PURE__ The comment may contain additional text */ pureOperation();\n",
"/* @__PURE__ */ /* @__NO_SIDE_EFFECTS__ */ pureOperation();\n",
"/* @__PURE__ */ pureOperation();\n",
);
test("const foo /* #__PURE__ */ = pureOperation();", "const foo = pureOperation();\n"); // INVALID: "=" not allowed after annotation
crates/oxc_codegen/tests/integration/snapshots/pure_comments.snap
+46 -46
View File
@@ -15,14 +15,14 @@ x([
])
----------
x([
/* @__NO_SIDE_EFFECTS__ */ function() {},
/* @__NO_SIDE_EFFECTS__ */ function y() {},
/* @__NO_SIDE_EFFECTS__ */ function* () {},
/* @__NO_SIDE_EFFECTS__ */ function* y() {},
/* @__NO_SIDE_EFFECTS__ */ async function() {},
/* @__NO_SIDE_EFFECTS__ */ async function y() {},
/* @__NO_SIDE_EFFECTS__ */ async function* () {},
/* @__NO_SIDE_EFFECTS__ */ async function* y() {}
/* #__NO_SIDE_EFFECTS__ */ function() {},
/* #__NO_SIDE_EFFECTS__ */ function y() {},
/* #__NO_SIDE_EFFECTS__ */ function* () {},
/* #__NO_SIDE_EFFECTS__ */ function* y() {},
/* #__NO_SIDE_EFFECTS__ */ async function() {},
/* #__NO_SIDE_EFFECTS__ */ async function y() {},
/* #__NO_SIDE_EFFECTS__ */ async function* () {},
/* #__NO_SIDE_EFFECTS__ */ async function* y() {}
]);
########## 1
@@ -37,12 +37,12 @@ x([
])
----------
x([
/* @__NO_SIDE_EFFECTS__ */ (y) => y,
/* @__NO_SIDE_EFFECTS__ */ () => {},
/* @__NO_SIDE_EFFECTS__ */ (y) => y,
/* @__NO_SIDE_EFFECTS__ */ async (y) => y,
/* @__NO_SIDE_EFFECTS__ */ async () => {},
/* @__NO_SIDE_EFFECTS__ */ async (y) => y
/* #__NO_SIDE_EFFECTS__ */ (y) => y,
/* #__NO_SIDE_EFFECTS__ */ () => {},
/* #__NO_SIDE_EFFECTS__ */ (y) => y,
/* #__NO_SIDE_EFFECTS__ */ async (y) => y,
/* #__NO_SIDE_EFFECTS__ */ async () => {},
/* #__NO_SIDE_EFFECTS__ */ async (y) => y
]);
########## 2
@@ -57,12 +57,12 @@ x([
])
----------
x([
/* @__NO_SIDE_EFFECTS__ */ (y) => y,
/* @__NO_SIDE_EFFECTS__ */ () => {},
/* @__NO_SIDE_EFFECTS__ */ (y) => y,
/* @__NO_SIDE_EFFECTS__ */ async (y) => y,
/* @__NO_SIDE_EFFECTS__ */ async () => {},
/* @__NO_SIDE_EFFECTS__ */ async (y) => y
/* #__NO_SIDE_EFFECTS__ */ (y) => y,
/* #__NO_SIDE_EFFECTS__ */ () => {},
/* #__NO_SIDE_EFFECTS__ */ (y) => y,
/* #__NO_SIDE_EFFECTS__ */ async (y) => y,
/* #__NO_SIDE_EFFECTS__ */ async () => {},
/* #__NO_SIDE_EFFECTS__ */ async (y) => y
]);
########## 3
@@ -77,13 +77,13 @@ async function c() {}
async function* d() {}
----------
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
function a() {}
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
function* b() {}
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
async function c() {}
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
async function* d() {}
########## 4
@@ -98,13 +98,13 @@ async function c() {}
async function* d() {}
----------
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
function a() {}
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
function* b() {}
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
async function c() {}
/* @__NO_SIDE_EFFECTS__ */
// #__NO_SIDE_EFFECTS__
async function* d() {}
########## 5
@@ -189,13 +189,13 @@ isFunction(options)
: options
----------
isFunction(options) ? /* @__PURE__ */ (() => extend({ name: options.name }, extraOptions, { setup: options }))() : options;
isFunction(options) ? /*#__PURE__*/ (() => extend({ name: options.name }, extraOptions, { setup: options }))() : options;
########## 10
isFunction(options) ? /*#__PURE__*/ (() => extend({ name: options.name }, extraOptions, { setup: options }))() : options;
----------
isFunction(options) ? /* @__PURE__ */ (() => extend({ name: options.name }, extraOptions, { setup: options }))() : options;
isFunction(options) ? /*#__PURE__*/ (() => extend({ name: options.name }, extraOptions, { setup: options }))() : options;
########## 11
@@ -208,18 +208,18 @@ const obj = {
const p = /*#__PURE__*/ Promise.resolve();
----------
const obj = { props: /* @__PURE__ */ extend({}, TransitionPropsValidators, {
const obj = { props: /*#__PURE__*/ extend({}, TransitionPropsValidators, {
tag: String,
moveClass: String
}) };
const p = /* @__PURE__ */ Promise.resolve();
const p = /*#__PURE__*/ Promise.resolve();
########## 12
const staticCacheMap = /*#__PURE__*/ new WeakMap()
----------
const staticCacheMap = /* @__PURE__ */ new WeakMap();
const staticCacheMap = /*#__PURE__*/ new WeakMap();
########## 13
@@ -230,7 +230,7 @@ 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();
@@ -359,18 +359,18 @@ let at_yes = /* @__PURE__ */ foo(bar);
let at_no = /* @__PURE__ */ foo(bar());
let new_at_yes = /* @__PURE__ */ new foo(bar);
let new_at_no = /* @__PURE__ */ new foo(bar());
let nospace_at_yes = /* @__PURE__ */ foo(bar);
let nospace_at_no = /* @__PURE__ */ foo(bar());
let nospace_new_at_yes = /* @__PURE__ */ new foo(bar);
let nospace_new_at_no = /* @__PURE__ */ new foo(bar());
let num_yes = /* @__PURE__ */ foo(bar);
let num_no = /* @__PURE__ */ foo(bar());
let new_num_yes = /* @__PURE__ */ new foo(bar);
let new_num_no = /* @__PURE__ */ new foo(bar());
let nospace_num_yes = /* @__PURE__ */ foo(bar);
let nospace_num_no = /* @__PURE__ */ foo(bar());
let nospace_new_num_yes = /* @__PURE__ */ new foo(bar);
let nospace_new_num_no = /* @__PURE__ */ new foo(bar());
let nospace_at_yes = /*@__PURE__*/ foo(bar);
let nospace_at_no = /*@__PURE__*/ foo(bar());
let nospace_new_at_yes = /*@__PURE__*/ new foo(bar);
let nospace_new_at_no = /*@__PURE__*/ new foo(bar());
let num_yes = /* #__PURE__ */ foo(bar);
let num_no = /* #__PURE__ */ foo(bar());
let new_num_yes = /* #__PURE__ */ new foo(bar);
let new_num_no = /* #__PURE__ */ new foo(bar());
let nospace_num_yes = /*#__PURE__*/ foo(bar);
let nospace_num_no = /*#__PURE__*/ foo(bar());
let nospace_new_num_yes = /*#__PURE__*/ new foo(bar);
let nospace_new_num_no = /*#__PURE__*/ new foo(bar());
let dot_yes = /* @__PURE__ */ foo(sideEffect()).dot(bar);
let dot_no = /* @__PURE__ */ foo(sideEffect()).dot(bar());
let new_dot_yes = /* @__PURE__ */ new foo(sideEffect()).dot(bar);