feat(parser): mark pure comments that cannot be applied#20687
Merged
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
d660597 to
eeb88ea
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb88ea0bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Dunqing
approved these changes
Mar 25, 2026
Member
Merge activity
|
## Summary Adds `CommentContent::PureNotApplied` variant to flag `#__PURE__` / `@__PURE__` annotations that cannot be applied, aligning with [Rollup's behavior](https://rollupjs.org/configuration-options/#pure) for invalid pure comment positions. ### Changes - `set_pure_on_call_or_new_expr` now returns `bool` so callers detect when the annotation couldn't be applied - Track the specific pure comment index (`Option<usize>`) instead of a boolean flag, fixing a bug where the wrong comment could be retagged when multiple pure comments exist (e.g. `/*#__PURE__*/ foo + /*#__PURE__*/ bar()`) - Mark pure comments as `PureNotApplied` in three contexts: - **Expression-level**: annotation not before a call/new expression (e.g. `/* #__PURE__ */ a().b`) - **Statement-level**: annotation before non-expression statements (e.g. `/* #__PURE__ */ function foo() {}`) - **Variable declarator**: annotation before `=` (e.g. `const foo /* #__PURE__ */ = bar()`) - Downstream consumers (codegen, minifier, Rolldown) can check for `PureNotApplied` to warn or strip Closes #20334 Rolldown will add custom logic to emit the comments that are `PureNotApplied` so it does not affect the rest of the compiler pipeline. ## Test plan - [x] Unit tests for all `PureNotApplied` cases (expression, statement, variable declarator) - [x] Regression test for correct comment targeting with multiple pure comments - [x] Codegen integration tests updated - [x] `cargo test -p oxc_parser -p oxc_codegen` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code)
5a821b7 to
59fd797
Compare
This was referenced May 13, 2026
h-a-n-a
pushed a commit
to rolldown/rolldown
that referenced
this pull request
May 14, 2026
Surfaces `/* #__PURE__ */` / `/* @__PURE__ */` annotations that the parser cannot apply, using Rollup's `INVALID_ANNOTATION` log code. The parser metadata comes from oxc-project/oxc#20687 (in `oxc@0.130.0`, already a dependency). `PreProcessEcmaAst::build` iterates `program.comments` after the semantic-warnings block and pushes one diagnostic per `PureNotApplied` comment. No AST mutation; codegen and tree-shaking unchanged. Five positive fixtures + one negative (a valid annotation that must produce zero warnings, guarding against false positives) cover the three contexts oxc marks plus the `@__PURE__` variant and a mixed valid/invalid case. Two existing snapshots (`remove_unused_pure_comment_calls`, `property_read_side_effects`) gained the warning because their inputs already contained misplaced annotations — their `# Assets` output is unchanged. One open question: Rollup also _strips_ invalid annotations from output (per its log text "The comment will be removed to avoid issues"). I left that out to avoid a behavior change during the RC phase; happy to follow up in a separate PR if desired. Fixes #8898.
IWANABETHATGUY
pushed a commit
to rolldown/rolldown
that referenced
this pull request
May 18, 2026
Surfaces `/* #__PURE__ */` / `/* @__PURE__ */` annotations that the parser cannot apply, using Rollup's `INVALID_ANNOTATION` log code. The parser metadata comes from oxc-project/oxc#20687 (in `oxc@0.130.0`, already a dependency). `PreProcessEcmaAst::build` iterates `program.comments` after the semantic-warnings block and pushes one diagnostic per `PureNotApplied` comment. No AST mutation; codegen and tree-shaking unchanged. Five positive fixtures + one negative (a valid annotation that must produce zero warnings, guarding against false positives) cover the three contexts oxc marks plus the `@__PURE__` variant and a mixed valid/invalid case. Two existing snapshots (`remove_unused_pure_comment_calls`, `property_read_side_effects`) gained the warning because their inputs already contained misplaced annotations — their `# Assets` output is unchanged. One open question: Rollup also _strips_ invalid annotations from output (per its log text "The comment will be removed to avoid issues"). I left that out to avoid a behavior change during the RC phase; happy to follow up in a separate PR if desired. Fixes #8898.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
CommentContent::PureNotAppliedvariant to flag#__PURE__/@__PURE__annotations that cannot be applied, aligning with Rollup's behavior for invalid pure comment positions.Changes
set_pure_on_call_or_new_exprnow returnsboolso callers detect when the annotation couldn't be appliedOption<usize>) instead of a boolean flag, fixing a bug where the wrong comment could be retagged when multiple pure comments exist (e.g./*#__PURE__*/ foo + /*#__PURE__*/ bar())PureNotAppliedin three contexts:/* #__PURE__ */ a().b)/* #__PURE__ */ function foo() {})=(e.g.const foo /* #__PURE__ */ = bar())PureNotAppliedto warn or stripCloses #20334
Rolldown will add custom logic to emit the comments that are
PureNotAppliedso it does not affect the rest of the compiler pipeline.Test plan
PureNotAppliedcases (expression, statement, variable declarator)cargo test -p oxc_parser -p oxc_codegenpasses🤖 Generated with Claude Code