Skip to content

Commit 068791e

Browse files
authored
fix(fmt): preserve comment position in disabled lines (#11922)
fix(fmt): preserve cmnt possition in disabled lines
1 parent 2bb29fb commit 068791e

File tree

5 files changed

+27
-9
lines changed

5 files changed

+27
-9
lines changed

crates/fmt/src/state/common.rs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -407,18 +407,32 @@ impl<'ast> State<'_, 'ast> {
407407

408408
print(self, value);
409409

410+
let next_span = if is_last { None } else { Some(get_span(&values[i + 1])) };
411+
let next_pos = next_span.map(Span::lo).unwrap_or(pos_hi);
412+
let cmnt_before_next =
413+
self.peek_comment_before(next_pos).map(|cmnt| (cmnt.span, cmnt.style));
414+
410415
if !is_last {
416+
// Handle disabled lines with comments after the value, but before the comma.
417+
if cmnt_before_next.is_some_and(|(cmnt_span, _)| {
418+
let span = self.cursor.span(cmnt_span.lo());
419+
self.inline_config.is_disabled(span)
420+
// NOTE: necessary workaround to patch this edgecase due to lack of spans for the commas.
421+
&& self.sm.span_to_snippet(span).is_ok_and(|snip| !snip.contains(','))
422+
}) {
423+
self.print_comments(
424+
next_pos,
425+
CommentConfig::skip_ws().mixed_no_break().mixed_prev_space(),
426+
);
427+
}
411428
self.print_word(",");
412429
}
413430

414-
let next_span = if is_last { None } else { Some(get_span(&values[i + 1])) };
415-
let next_pos = next_span.map(Span::lo).unwrap_or(pos_hi);
416-
417431
if !is_last
418432
&& format.breaks_cmnts
419-
&& self.peek_comment_before(next_pos).is_some_and(|cmnt| {
420-
let disabled = self.inline_config.is_disabled(cmnt.span);
421-
(cmnt.style.is_mixed() && !disabled) || (cmnt.style.is_isolated() && disabled)
433+
&& cmnt_before_next.is_some_and(|(cmnt_span, cmnt_style)| {
434+
let disabled = self.inline_config.is_disabled(cmnt_span);
435+
(cmnt_style.is_mixed() && !disabled) || (cmnt_style.is_isolated() && disabled)
422436
})
423437
{
424438
self.hardbreak(); // trailing and isolated comments already hardbreak

crates/fmt/testdata/Repros/fmt.sol

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,14 @@ contract DbgFmtTest is Test {
166166
}
167167
}
168168

169+
// https://github.com/foundry-rs/foundry/issues/8557
169170
// https://github.com/foundry-rs/foundry/issues/11249
170171
function argListRepro(address tokenIn, uint256 amountIn, bool data) {
171172
maverickV2SwapCallback(
172173
tokenIn,
173174
amountIn, // forgefmt: disable-line
174175
// forgefmt: disable-next-line
175-
0,/* we didn't bother loading `amountOut` because we don't use it */
176+
0 /* we didn't bother loading `amountOut` because we don't use it */,
176177
data
177178
);
178179
}

crates/fmt/testdata/Repros/original.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ contract DbgFmtTest is Test {
167167
}
168168
}
169169

170+
// https://github.com/foundry-rs/foundry/issues/8557
170171
// https://github.com/foundry-rs/foundry/issues/11249
171172
function argListRepro(address tokenIn, uint256 amountIn, bool data) {
172173
maverickV2SwapCallback(

crates/fmt/testdata/Repros/sorted.fmt.sol

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,14 @@ contract DbgFmtTest is Test {
167167
}
168168
}
169169

170+
// https://github.com/foundry-rs/foundry/issues/8557
170171
// https://github.com/foundry-rs/foundry/issues/11249
171172
function argListRepro(address tokenIn, uint256 amountIn, bool data) {
172173
maverickV2SwapCallback(
173174
tokenIn,
174175
amountIn, // forgefmt: disable-line
175176
// forgefmt: disable-next-line
176-
0,/* we didn't bother loading `amountOut` because we don't use it */
177+
0 /* we didn't bother loading `amountOut` because we don't use it */,
177178
data
178179
);
179180
}

crates/fmt/testdata/Repros/tab.fmt.sol

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,14 @@ contract DbgFmtTest is Test {
167167
}
168168
}
169169

170+
// https://github.com/foundry-rs/foundry/issues/8557
170171
// https://github.com/foundry-rs/foundry/issues/11249
171172
function argListRepro(address tokenIn, uint256 amountIn, bool data) {
172173
maverickV2SwapCallback(
173174
tokenIn,
174175
amountIn, // forgefmt: disable-line
175176
// forgefmt: disable-next-line
176-
0,/* we didn't bother loading `amountOut` because we don't use it */
177+
0 /* we didn't bother loading `amountOut` because we don't use it */,
177178
data
178179
);
179180
}

0 commit comments

Comments
 (0)