Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 26, 2025

  • Understand the issue: preserve comments between CatchClause's param and body
  • Identify the problematic code location: CatchClause gen in crates/oxc_codegen/src/gen.rs
  • Implement the fix: add print_comments_at(self.body.span.start) before printing the block statement, and consume the pending space flag
  • Add test case to verify the fix
  • Run tests and verify the fix works
  • Run code review and address feedback
  • Run security checks
  • Address review feedback: remove .serena files
  • Address review feedback: add test case with multiple comments
  • Revert RParen change and document that line comments are not supported (block comments work)
  • Add !p.options.minify guard to space handling and add minify test
Original prompt

This section details on the original issue you should resolve

<issue_title>codegen: preserve comments between CatchClause's param and body</issue_title>
<issue_description>Code coverage tooling that support ignore next ignore hint need code transforms to preserve comments between catch(param) { in order to be able to ignore catch block's body.

catch (err) /* v8 ignore next */ {
// instructs coverage tooling    ^
// to ignore this BlockStatement ^

Input:

try {
  console.log('test');
}
catch (err) /* v8 ignore next */ {
  console.error(err);
}

Expected output:

try {
  console.log('test');
}
catch (err) /* v8 ignore next */ {
  console.error(err);
}

Actual output:

try {
  console.log('test');
}
catch (err) {
  console.error(err);
}

https://playground.oxc.rs/#eNptU01v2...

Initial coverage ignore hint support added in:

Downstream Vitest issue:

Comments on the Issue (you are @copilot in this section)


✨ 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 26, 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] Fix codegen to preserve comments in CatchClause feat(codegen): preserve comments between CatchClause's param and body Nov 26, 2025
Copilot finished work on behalf of sapphi-red November 26, 2025 14:31
Copilot AI requested a review from sapphi-red November 26, 2025 14:31
@sapphi-red sapphi-red marked this pull request as ready for review November 27, 2025 08:27
Copilot AI review requested due to automatic review settings November 27, 2025 08:27
@github-actions github-actions bot added A-parser Area - Parser A-codegen Area - Code Generation C-enhancement Category - New feature or request labels Nov 27, 2025
Copilot finished reviewing on behalf of sapphi-red November 27, 2025 08:34
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 27, 2025

CodSpeed Performance Report

Merging #16167 will not alter performance

Comparing copilot/preserve-comments-in-catch-clause (4674fd9) with main (d4a1581)1

Summary

✅ 42 untouched
⏩ 3 skipped2

Footnotes

  1. No successful run was found on main (78aa294) during the generation of this report, so d4a1581 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 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.

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 adds support for preserving comments between a catch clause's parameter and body, enabling code coverage tools that use ignore hints (like /* v8 ignore next */) to properly instrument catch blocks.

Key Changes:

  • Modified comment parsing to treat line comments after ) as leading rather than trailing
  • Added comment printing logic in CatchClause codegen before the block body
  • Added manual handling of the print_next_indent_as_space flag to ensure proper spacing

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
crates/oxc_parser/src/lexer/trivia_builder.rs Updated trailing comment detection to exclude Kind::RParen, making line comments after ) be treated as leading comments attached to the next token
crates/oxc_codegen/src/gen.rs Added print_comments_at(self.body.span.start) before printing catch block body, with manual handling of spacing flag for inline comments
crates/oxc_codegen/tests/integration/comments.rs Added three test cases covering inline block comments, multiple comments, and line comments between catch parameter and body
crates/oxc_codegen/tests/integration/snapshots/coverage.snap Added snapshot expectations for the three new test cases showing proper comment preservation

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

@sapphi-red sapphi-red marked this pull request as draft November 27, 2025 09:53
Copilot AI requested a review from sapphi-red November 27, 2025 10:00
Copilot finished work on behalf of sapphi-red November 27, 2025 10:00
Co-authored-by: sapphi-red <[email protected]>
@sapphi-red sapphi-red force-pushed the copilot/preserve-comments-in-catch-clause branch from 78c730b to bc81ea4 Compare November 27, 2025 10:18
p.print_soft_space();
p.print_comments_at(self.body.span.start);
// Consume the space flag set by comment printing to ensure proper spacing before the opening brace
if p.print_next_indent_as_space {
Copy link
Member

Choose a reason for hiding this comment

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

@copilot add !p.options.minify and add tests for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added !p.options.minify guard to the space handling logic and added tests for both minify mode with comments enabled (preserves comments) and true minify mode with comments disabled (strips comments). See 4674fd9.

Copilot AI requested a review from sapphi-red November 28, 2025 02:09
Copilot finished work on behalf of sapphi-red November 28, 2025 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

codegen: preserve comments between CatchClause's param and body

2 participants