Skip to content

Commit 1ec23c7

Browse files
committed
fix: preserve comments between node and comma in separated lists (#684)
`gen_node_with_separator` emitted the auto-inserted comma immediately after the node, then ran the regular trailing-comments path which only emits same-line comments. Multi-line comments living between the node end and the comma — e.g. foo( a // comment , b); — were therefore captured by `trailing_comments_with_previous` but filtered out by `get_trailing_comments_same_line` and lost. Collect the comma token's leading comments that sit on lines after the node end, mark them handled via `gen_comment_collection`, and emit them after the separator. Result: foo( a, // comment b, ); Applies to anything routed through `gen_node_with_separator`: call arguments, array literals, object literals, function parameters, etc.
1 parent f7dd1f3 commit 1ec23c7

2 files changed

Lines changed: 114 additions & 0 deletions

File tree

src/generation/generate.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8047,16 +8047,27 @@ fn gen_node_with_separator<'a>(value: Node<'a>, generated_separator: PrintItems,
80478047
PrintItems::new()
80488048
};
80498049

8050+
// comments that sit between the node and the comma on a subsequent line —
8051+
// the default trailing-comments path only emits same-line comments, so these
8052+
// would otherwise be dropped (issue #684).
8053+
let between_node_and_comma = if let Some(comma_token) = comma_token {
8054+
gen_comments_between_node_and_comma(value, comma_token, context)
8055+
} else {
8056+
PrintItems::new()
8057+
};
8058+
80508059
// if the current node is ignored and already has a semi-colon, then skip adding a separator
80518060
let is_ignored_with_semi_colon =
80528061
value.text_fast(context.program).ends_with(';') && get_has_ignore_comment(&value.leading_comments_fast(context.program), value, context);
80538062
if is_ignored_with_semi_colon {
80548063
items.extend(gen_node(value, context));
80558064
} else {
80568065
let generated_separator = generated_separator.into_rc_path();
8066+
let between_node_and_comma = between_node_and_comma.into_rc_path();
80578067
items.extend(gen_node_with_inner_gen(value, context, move |mut items, _| {
80588068
// this Rc clone is necessary because we can't move the captured generated_separator out of this closure
80598069
items.push_optional_path(generated_separator);
8070+
items.push_optional_path(between_node_and_comma);
80608071
items
80618072
}));
80628073
}
@@ -8073,6 +8084,38 @@ fn gen_node_with_separator<'a>(value: Node<'a>, generated_separator: PrintItems,
80738084
}
80748085
}
80758086

8087+
fn gen_comments_between_node_and_comma<'a>(value: Node<'a>, comma_token: &TokenAndSpan, context: &mut Context<'a>) -> PrintItems {
8088+
// collect comments between the node end and the comma — these would be classified
8089+
// as trailing comments of the node, but the existing path only emits ones on the
8090+
// same line, dropping anything that sits on a subsequent line.
8091+
let node_end = value.range().end;
8092+
let node_end_line = node_end.end_line_fast(context.program);
8093+
let mut comments_to_emit = Vec::new();
8094+
for comment in comma_token.range().start.leading_comments_fast(context.program) {
8095+
if context.has_handled_comment(&comment) {
8096+
continue;
8097+
}
8098+
if comment.start() < node_end {
8099+
continue;
8100+
}
8101+
// skip same-line trailing comments — those flow through the regular trailing path
8102+
if comment.start_line_fast(context.program) <= node_end_line {
8103+
continue;
8104+
}
8105+
comments_to_emit.push(comment);
8106+
}
8107+
8108+
if comments_to_emit.is_empty() {
8109+
return PrintItems::new();
8110+
}
8111+
8112+
let comma_range = comma_token.range();
8113+
let mut items = PrintItems::new();
8114+
items.push_signal(Signal::NewLine);
8115+
items.extend(gen_comment_collection(comments_to_emit.into_iter(), None, Some(&comma_range), context));
8116+
items
8117+
}
8118+
80768119
/// Some nodes don't have a TsTypeAnn, but instead a Box<TsType>
80778120
fn gen_type_ann_with_colon_if_exists_for_type<'a>(type_ann: Option<TsType<'a>>, context: &mut Context<'a>) -> PrintItems {
80788121
if let Some(type_ann) = type_ann {

tests/specs/issues/issue0684.txt

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
== should preserve comment between argument and following comma (issue #684) ==
2+
const x = useMemo(() =>
3+
doSomething()
4+
// eslint-disable-next-line react-hooks/exhaustive-deps
5+
, [dep]);
6+
7+
[expect]
8+
const x = useMemo(
9+
() => doSomething(),
10+
// eslint-disable-next-line react-hooks/exhaustive-deps
11+
[dep],
12+
);
13+
14+
== should preserve multiple comments between argument and comma ==
15+
foo(
16+
a
17+
// c1
18+
/* c2 */
19+
// c3
20+
, b);
21+
22+
[expect]
23+
foo(
24+
a,
25+
// c1
26+
/* c2 */
27+
// c3
28+
b,
29+
);
30+
31+
== should preserve comment before comma in array literal ==
32+
[
33+
1
34+
// c
35+
, 2
36+
];
37+
38+
[expect]
39+
[
40+
1,
41+
// c
42+
2,
43+
];
44+
45+
== should preserve comment before comma in object literal ==
46+
({
47+
a: 1
48+
// c
49+
, b: 2
50+
});
51+
52+
[expect]
53+
({
54+
a: 1,
55+
// c
56+
b: 2,
57+
});
58+
59+
== should preserve comment before comma in function parameters ==
60+
function f(
61+
a
62+
// c
63+
, b
64+
) {}
65+
66+
[expect]
67+
function f(
68+
a,
69+
// c
70+
b,
71+
) {}

0 commit comments

Comments
 (0)