Skip to content

feat(parser): mark pure comments that cannot be applied#20687

Merged
graphite-app[bot] merged 1 commit intomainfrom
boshen/pure-comment-not-applied
Mar 25, 2026
Merged

feat(parser): mark pure comments that cannot be applied#20687
graphite-app[bot] merged 1 commit intomainfrom
boshen/pure-comment-not-applied

Conversation

@Boshen
Copy link
Copy Markdown
Member

@Boshen Boshen commented Mar 24, 2026

Summary

Adds CommentContent::PureNotApplied variant 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_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

  • Unit tests for all PureNotApplied cases (expression, statement, variable declarator)
  • Regression test for correct comment targeting with multiple pure comments
  • Codegen integration tests updated
  • cargo test -p oxc_parser -p oxc_codegen passes

🤖 Generated with Claude Code

@github-actions github-actions bot added A-parser Area - Parser A-ast Area - AST A-codegen Area - Code Generation C-enhancement Category - New feature or request labels Mar 24, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 24, 2026

Merging this PR will not alter performance

✅ 53 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing boshen/pure-comment-not-applied (5a821b7) with main (77abf54)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Boshen Boshen requested a review from Dunqing March 24, 2026 11:13
@Boshen Boshen force-pushed the boshen/pure-comment-not-applied branch from d660597 to eeb88ea Compare March 24, 2026 11:18
@Boshen Boshen marked this pull request as ready for review March 24, 2026 11:21
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 Dunqing added the 0-merge Merge with Graphite Merge Queue label Mar 25, 2026
Copy link
Copy Markdown
Member

Dunqing commented Mar 25, 2026

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)
@graphite-app graphite-app bot force-pushed the boshen/pure-comment-not-applied branch from 5a821b7 to 59fd797 Compare March 25, 2026 00:57
@graphite-app graphite-app bot merged commit 59fd797 into main Mar 25, 2026
27 checks passed
@graphite-app graphite-app bot deleted the boshen/pure-comment-not-applied branch March 25, 2026 01:01
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-codegen Area - Code Generation A-parser Area - Parser C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parser: add warnings to pure annotations which parser is not able to interpret

2 participants