Skip to content

feat(codegen): preserve comments between CatchClause's param and body #16167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
+79 −1
Draft

feat(codegen): preserve comments between CatchClause's param and body #16167

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0b95556
Initial analysis of issue
Copilot Nov 26, 2025
df85506
feat(codegen): preserve comments between CatchClause's param and body
Copilot Nov 26, 2025
14052b5
style: improve comment clarity in CatchClause gen
Copilot Nov 26, 2025
e4d70d4
Address review feedback: remove .serena files and add more test cases
Copilot Nov 26, 2025
1d5e8a9
fix(parser): treat line comments after `)` as leading comments for ca…
Copilot Nov 27, 2025
bc81ea4
revert(parser): remove RParen from trailing comment exclusion and add…
Copilot Nov 27, 2025
4674fd9
fix(codegen): add minify check to catch clause comment space handling…
Copilot Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/oxc_codegen/src/gen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,12 @@ impl Gen for CatchClause<'_> {
p.print_str(")");
}
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

@sapphi-red sapphi-red Nov 28, 2025

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

Copilot AI Nov 28, 2025

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.

p.print_hard_space();
p.print_next_indent_as_space = false;
}
p.print_block_statement(&self.body, ctx);
}
}
Expand Down
13 changes: 13 additions & 0 deletions crates/oxc_codegen/tests/integration/comments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,19 @@ catch(e) {
// should never happen
}
",
// Inline comment between catch param and body
"try { console.log('test'); }
catch (err) /* v8 ignore next */ { console.error(err); }",
// Multiple comments between catch param and body
"try { something(); }
catch (err) /* c8 ignore next */ /* istanbul ignore next */ { handle(err); }",
// Line comment between catch param and body.
// NOTE: Line comments after `)` are classified as trailing comments by the parser,
// so they are not preserved. Use block comments instead.
// See: https://github.com/oxc-project/oxc/pull/16167#discussion_r2567604139
"try { something(); }
catch (err) // v8 ignore next
{ handle(err); }",
];

snapshot("coverage", &cases);
Expand Down
32 changes: 32 additions & 0 deletions crates/oxc_codegen/tests/integration/snapshots/coverage.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,35 @@ try {
}
/* istanbul ignore next */
catch (e) {}

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

########## 21
try { something(); }
catch (err) /* c8 ignore next */ /* istanbul ignore next */ { handle(err); }
----------
try {
something();
} catch (err) /* c8 ignore next *//* istanbul ignore next */ {
handle(err);
}

########## 22
try { something(); }
catch (err) // v8 ignore next
{ handle(err); }
----------
try {
something();
} catch (err) {
handle(err);
}

Loading