mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(parser): treat a line comment after ':' as leading, not trailing (#23515)
## Summary A line comment immediately after a `:` on the same line was treated as a *trailing* comment in `should_be_treated_as_trailing_comment`, which anchors it to position `0` and drops it during codegen. The `=` and `(` tokens were already exempted from this rule for the same reason; `:` was not. This surfaced as a transformer **idempotency** failure (caught by [monitor-oxc](https://github.com/oxc-project/monitor-oxc/actions/runs/27659196490/job/81800055426)): lowering optional chaining emits `? void 0 : // comment\n_obj.method()`, so the leading comment of the original expression lands right after the `:`. Codegen kept it on the first pass but dropped it on re-parse, so `transform(transform(x)) != transform(x)`. Treating such a comment as **leading** anchors it to the following operand — preserving it and restoring idempotency. Comments after `:` in switch cases and labeled statements are likewise no longer dropped. (Object-property values and type-annotation colons are unaffected: still printed where they already were, and verified idempotent.) ## Example Input: ```js x = cond ? a : // c b; ``` Before this PR the comment was dropped (`x = cond ? a : b;`); now it is preserved. ## Tests - `leading_comments_after_colon` (`oxc_parser`) — the comment anchors to the following token. - `test_line_comment_after_conditional_colon` (`oxc_codegen`) — exact output + idempotency, including the lowered optional-chaining shape. - Verified idempotent across every `:` context (ternary, object value, switch case, label, TS type annotation); no committed transform-conformance fixture contains the affected pattern, so no snapshots change.
This commit is contained in:
1 parent
80f16971c2
commit
837a395fba
2 files changed
+39
-3
No files matched your search
@@ -24,6 +24,18 @@ fn test_comment_inside_pife_function_alternate_of_conditional() {
|
||||
);
|
||||
}
|
||||
|
||||
// A line comment between a conditional `:` and a plain (non-pife) alternate must
|
||||
// be preserved. It was previously treated as a trailing comment of `:` and
|
||||
// dropped, which broke codegen idempotency once a transform emits
|
||||
// `? consequent : // comment\nalternate` (e.g. lowered optional chaining).
|
||||
#[test]
|
||||
fn test_line_comment_after_conditional_colon() {
|
||||
test("x = cond ? a : // c\nb;", "x = cond ? a : // c\nb;\n");
|
||||
test_idempotency("x = cond ? a : // c\nb;");
|
||||
// Real-world shape: `a?.b() ?? c` with a leading comment, after lowering.
|
||||
test_idempotency("x = (_a = a) === null || _a === void 0 ? void 0 : // c1\n// c2\n_a.b();");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_comment_at_top_of_file() {
|
||||
use oxc_allocator::Allocator;
|
||||
|
||||
@@ -150,8 +150,8 @@ impl<'a> TriviaBuilder<'a> {
|
||||
/// let x = 5;
|
||||
/// ```
|
||||
///
|
||||
/// 2. It does not immediately follow an `=` [`Kind::Eq`] or `(` [`Kind::LParen`]
|
||||
/// token.
|
||||
/// 2. It does not immediately follow an `=` [`Kind::Eq`], `(` [`Kind::LParen`]
|
||||
/// or `:` [`Kind::Colon`] token.
|
||||
///
|
||||
/// ```javascript
|
||||
/// let y = // This should not be treated as trailing (follows `=`)
|
||||
@@ -160,9 +160,16 @@ impl<'a> TriviaBuilder<'a> {
|
||||
/// function foo( // This should not be treated as trailing (follows `(`)
|
||||
/// param
|
||||
/// ) {}
|
||||
///
|
||||
/// let z = cond ? a : // This should not be treated as trailing (follows `:`)
|
||||
/// b;
|
||||
/// ```
|
||||
///
|
||||
/// Treating a comment after `:` as trailing drops it (it anchors to the
|
||||
/// previous token rather than the following operand), which breaks codegen
|
||||
/// idempotency once a transform emits `? consequent : // comment\nalternate`.
|
||||
fn should_be_treated_as_trailing_comment(&self) -> bool {
|
||||
!self.saw_newline && !matches!(self.previous_kind, Kind::Eq | Kind::LParen)
|
||||
!self.saw_newline && !matches!(self.previous_kind, Kind::Eq | Kind::LParen | Kind::Colon)
|
||||
}
|
||||
|
||||
fn should_stay_leading(comment: &Comment) -> bool {
|
||||
@@ -670,6 +677,23 @@ function bar() {}";
|
||||
assert_eq!(comments, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn leading_comments_after_colon() {
|
||||
// A line comment right after a conditional `:` anchors to the following
|
||||
// alternate (leading), not the previous token — otherwise it is dropped.
|
||||
let source_text = "v = cond ? a : // Leading comment\nb;";
|
||||
let comments = get_comments(source_text);
|
||||
let expected = vec![Comment {
|
||||
span: Span::new(15, 33),
|
||||
kind: CommentKind::Line,
|
||||
position: CommentPosition::Leading,
|
||||
attached_to: 34,
|
||||
newlines: CommentNewlines::Trailing,
|
||||
content: CommentContent::None,
|
||||
}];
|
||||
assert_eq!(comments, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pure_comment_not_applied() {
|
||||
let cases = [
|
||||
|
||||
Reference in new issue
Block a user