Skip to content

Comments

fix(parser): preserve legal comments with @preserve/@license when preceded by other annotations#15929

Merged
graphite-app[bot] merged 1 commit intomainfrom
copilot/preserve-foo-comment-output
Nov 21, 2025
Merged

fix(parser): preserve legal comments with @preserve/@license when preceded by other annotations#15929
graphite-app[bot] merged 1 commit intomainfrom
copilot/preserve-foo-comment-output

Conversation

Copy link
Contributor

Copilot AI commented Nov 21, 2025

Legal comments like /* @foo @preserve */ were not being preserved when legal comment output was enabled. The parser only checked if the first @ annotation matched known patterns, missing @preserve/@license later in the comment.

Changes

Parser (oxc_parser/src/lexer/trivia_builder.rs)

  • Add fallback check using contains_license_or_preserve_comment after __PURE__/__NO_SIDE_EFFECTS__ checks
  • Ensures @license/@preserve detected anywhere in comment text

Example

// Before: Legal comment not detected by parser
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::None ❌

// After: Legal comment correctly detected
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::Legal ✅

Related: rolldown/rolldown#7160

Original prompt

/* @foo @preserve */ should be preserved when legal comment output is enabled even if comment output is disabled. rolldown/rolldown#7160 is related


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 21, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

Copilot AI changed the title [WIP] Preserve legal comment output for foo fix(parser, codegen): preserve legal comments with @preserve/@license when preceded by other annotations Nov 21, 2025
Copilot AI requested a review from sapphi-red November 21, 2025 02:51
Copilot AI changed the title fix(parser, codegen): preserve legal comments with @preserve/@license when preceded by other annotations fix(parser): preserve legal comments with @preserve/@license when preceded by other annotations Nov 21, 2025
Copilot AI requested a review from sapphi-red November 21, 2025 03:17
@github-actions github-actions bot added A-parser Area - Parser C-bug Category - Bug labels Nov 21, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 21, 2025

CodSpeed Performance Report

Merging #15929 will not alter performance

Comparing copilot/preserve-foo-comment-output (e359825) with main (4b9d8d2)

Summary

✅ 42 untouched
⏩ 3 skipped1

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.

@sapphi-red sapphi-red marked this pull request as ready for review November 21, 2025 03:54
Copilot AI review requested due to automatic review settings November 21, 2025 03:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where legal comments containing @preserve or @license were not being detected when preceded by other annotations like @foo. The parser was only checking if the first @ annotation matched known patterns, missing legal annotations that appeared later in the comment.

Key changes:

  • Added early returns after detecting __PURE__ and __NO_SIDE_EFFECTS__ annotations to avoid unnecessary fallthrough
  • Added fallback check using contains_license_or_preserve_comment to detect @license/@preserve anywhere in comment text
  • Added test coverage for comments with unknown annotations preceding legal annotations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sapphi-red sapphi-red requested a review from Boshen November 21, 2025 03:57
@sapphi-red sapphi-red assigned Boshen and unassigned sapphi-red Nov 21, 2025
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Nov 21, 2025
Copy link
Member

Boshen commented Nov 21, 2025

Merge activity

…ceded by other annotations (#15929)

Legal comments like `/* @foo @preserve */` were not being preserved when legal comment output was enabled. The parser only checked if the first `@` annotation matched known patterns, missing `@preserve`/`@license` later in the comment.

## Changes

**Parser (`oxc_parser/src/lexer/trivia_builder.rs`)**
- Add fallback check using `contains_license_or_preserve_comment` after `__PURE__`/`__NO_SIDE_EFFECTS__` checks
- Ensures `@license`/`@preserve` detected anywhere in comment text

## Example

```javascript
// Before: Legal comment not detected by parser
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::None ❌

// After: Legal comment correctly detected
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::Legal ✅
```

Related: rolldown/rolldown#7160

<!-- START COPILOT CODING AGENT SUFFIX -->

<details>

<summary>Original prompt</summary>

> `/* @foo @preserve */` should be preserved when legal comment output is enabled even if comment output is disabled. rolldown/rolldown#7160 is related

</details>

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/oxc-project/oxc/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.
@graphite-app graphite-app bot force-pushed the copilot/preserve-foo-comment-output branch from e359825 to 622cb5e Compare November 21, 2025 14:50
@graphite-app graphite-app bot merged commit 622cb5e into main Nov 21, 2025
21 checks passed
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Nov 21, 2025
@graphite-app graphite-app bot deleted the copilot/preserve-foo-comment-output branch November 21, 2025 14:56
Copilot AI added a commit that referenced this pull request Nov 21, 2025
…ceded by other annotations (#15929)

Legal comments like `/* @foo @preserve */` were not being preserved when legal comment output was enabled. The parser only checked if the first `@` annotation matched known patterns, missing `@preserve`/`@license` later in the comment.

## Changes

**Parser (`oxc_parser/src/lexer/trivia_builder.rs`)**
- Add fallback check using `contains_license_or_preserve_comment` after `__PURE__`/`__NO_SIDE_EFFECTS__` checks
- Ensures `@license`/`@preserve` detected anywhere in comment text

## Example

```javascript
// Before: Legal comment not detected by parser
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::None ❌

// After: Legal comment correctly detected
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::Legal ✅
```

Related: rolldown/rolldown#7160

<!-- START COPILOT CODING AGENT SUFFIX -->

<details>

<summary>Original prompt</summary>

> `/* @foo @preserve */` should be preserved when legal comment output is enabled even if comment output is disabled. rolldown/rolldown#7160 is related

</details>

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/oxc-project/oxc/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.
overlookmotel pushed a commit that referenced this pull request Nov 24, 2025
### 💥 BREAKING CHANGES

- cbb27fd ast: [**BREAKING**] Add `TSGlobalDeclaration` type (#15712)
(overlookmotel)

### 🚀 Features

- 0c1f82b linter/plugins: Add `tokens` property to `Program` (#16020)
(overlookmotel)
- 6cff132 span: Add `Span::merge_within` method (#15869) (sapphi-red)
- 102365d allocator/vec: Add `Vec::into_bump_slice` method (#15770)
(Dunqing)

### 🐛 Bug Fixes

- e2ca770 codegen: Add support for printing type arguments in new
expressions (#15963) (Ives van Hoorne)
- 2bd3cb6 apps, editors, napi: Fix `oxlint-disable` comments (#16014)
(overlookmotel)
- 622cb5e parser: Preserve legal comments with @preserve/@license when
preceded by other annotations (#15929) (copilot-swe-agent)
- 7c46a9e transformer/tagged-template-transform: Handle `\n` escape
sequences (#15830) (Dunqing)
- f386efc minifier: Avoid generating invalid spans (#15778) (sapphi-red)
- d4ff004 parser: Forbid invalid modifiers on `module` and `global`
(#15723) (overlookmotel)
- 2191ae9 semantic: Allow reserved keywords in typescript ambient
contexts (#15495) (sapphi-red)
- 7d1ebad isolated-declarations: Incorrect nested namespace output in
isolated declarations (#15800) (copilot-swe-agent)

### ⚡ Performance

- b4b0ed8 transformer/typescript: Reverse order of checks (#15722)
(overlookmotel)

### 📚 Documentation

- c81a331 data_structures: Doc comments on fields of `Stack` (#15793)
(overlookmotel)
- cfae31d allocator: Use `allocator` as var name in examples (#15781)
(overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
…ceded by other annotations (oxc-project#15929)

Legal comments like `/* @foo @preserve */` were not being preserved when legal comment output was enabled. The parser only checked if the first `@` annotation matched known patterns, missing `@preserve`/`@license` later in the comment.

## Changes

**Parser (`oxc_parser/src/lexer/trivia_builder.rs`)**
- Add fallback check using `contains_license_or_preserve_comment` after `__PURE__`/`__NO_SIDE_EFFECTS__` checks
- Ensures `@license`/`@preserve` detected anywhere in comment text

## Example

```javascript
// Before: Legal comment not detected by parser
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::None ❌

// After: Legal comment correctly detected
/* @foo @preserve */ function foo() {}
// Parser classifies as: CommentContent::Legal ✅
```

Related: rolldown/rolldown#7160

<!-- START COPILOT CODING AGENT SUFFIX -->

<details>

<summary>Original prompt</summary>

> `/* @foo @preserve */` should be preserved when legal comment output is enabled even if comment output is disabled. rolldown/rolldown#7160 is related

</details>

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/oxc-project/oxc/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
### 💥 BREAKING CHANGES

- cbb27fd ast: [**BREAKING**] Add `TSGlobalDeclaration` type (oxc-project#15712)
(overlookmotel)

### 🚀 Features

- 0c1f82b linter/plugins: Add `tokens` property to `Program` (oxc-project#16020)
(overlookmotel)
- 6cff132 span: Add `Span::merge_within` method (oxc-project#15869) (sapphi-red)
- 102365d allocator/vec: Add `Vec::into_bump_slice` method (oxc-project#15770)
(Dunqing)

### 🐛 Bug Fixes

- e2ca770 codegen: Add support for printing type arguments in new
expressions (oxc-project#15963) (Ives van Hoorne)
- 2bd3cb6 apps, editors, napi: Fix `oxlint-disable` comments (oxc-project#16014)
(overlookmotel)
- 622cb5e parser: Preserve legal comments with @preserve/@license when
preceded by other annotations (oxc-project#15929) (copilot-swe-agent)
- 7c46a9e transformer/tagged-template-transform: Handle `\n` escape
sequences (oxc-project#15830) (Dunqing)
- f386efc minifier: Avoid generating invalid spans (oxc-project#15778) (sapphi-red)
- d4ff004 parser: Forbid invalid modifiers on `module` and `global`
(oxc-project#15723) (overlookmotel)
- 2191ae9 semantic: Allow reserved keywords in typescript ambient
contexts (oxc-project#15495) (sapphi-red)
- 7d1ebad isolated-declarations: Incorrect nested namespace output in
isolated declarations (oxc-project#15800) (copilot-swe-agent)

### ⚡ Performance

- b4b0ed8 transformer/typescript: Reverse order of checks (oxc-project#15722)
(overlookmotel)

### 📚 Documentation

- c81a331 data_structures: Doc comments on fields of `Stack` (oxc-project#15793)
(overlookmotel)
- cfae31d allocator: Use `allocator` as var name in examples (oxc-project#15781)
(overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants