From afaec3bafee5c1268fe69f2d562ba04a2064a8eb Mon Sep 17 00:00:00 2001 From: 0xrusowsky <0xrusowsky@proton.me> Date: Thu, 2 Oct 2025 09:53:06 +0200 Subject: [PATCH] fix(fmt): preserve cmnt possition in disabled lines --- crates/fmt/src/state/common.rs | 26 +++++++++++++++++------ crates/fmt/testdata/Repros/fmt.sol | 3 ++- crates/fmt/testdata/Repros/original.sol | 1 + crates/fmt/testdata/Repros/sorted.fmt.sol | 3 ++- crates/fmt/testdata/Repros/tab.fmt.sol | 3 ++- 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/crates/fmt/src/state/common.rs b/crates/fmt/src/state/common.rs index 15d8c32c224f8..60ce688216347 100644 --- a/crates/fmt/src/state/common.rs +++ b/crates/fmt/src/state/common.rs @@ -407,18 +407,32 @@ impl<'ast> State<'_, 'ast> { print(self, value); + let next_span = if is_last { None } else { Some(get_span(&values[i + 1])) }; + let next_pos = next_span.map(Span::lo).unwrap_or(pos_hi); + let cmnt_before_next = + self.peek_comment_before(next_pos).map(|cmnt| (cmnt.span, cmnt.style)); + if !is_last { + // Handle disabled lines with comments after the value, but before the comma. + if cmnt_before_next.is_some_and(|(cmnt_span, _)| { + let span = self.cursor.span(cmnt_span.lo()); + self.inline_config.is_disabled(span) + // NOTE: necessary workaround to patch this edgecase due to lack of spans for the commas. + && self.sm.span_to_snippet(span).is_ok_and(|snip| !snip.contains(',')) + }) { + self.print_comments( + next_pos, + CommentConfig::skip_ws().mixed_no_break().mixed_prev_space(), + ); + } self.print_word(","); } - let next_span = if is_last { None } else { Some(get_span(&values[i + 1])) }; - let next_pos = next_span.map(Span::lo).unwrap_or(pos_hi); - if !is_last && format.breaks_cmnts - && self.peek_comment_before(next_pos).is_some_and(|cmnt| { - let disabled = self.inline_config.is_disabled(cmnt.span); - (cmnt.style.is_mixed() && !disabled) || (cmnt.style.is_isolated() && disabled) + && cmnt_before_next.is_some_and(|(cmnt_span, cmnt_style)| { + let disabled = self.inline_config.is_disabled(cmnt_span); + (cmnt_style.is_mixed() && !disabled) || (cmnt_style.is_isolated() && disabled) }) { self.hardbreak(); // trailing and isolated comments already hardbreak diff --git a/crates/fmt/testdata/Repros/fmt.sol b/crates/fmt/testdata/Repros/fmt.sol index 4b9700f37afb5..3ccbaab706ea1 100644 --- a/crates/fmt/testdata/Repros/fmt.sol +++ b/crates/fmt/testdata/Repros/fmt.sol @@ -166,13 +166,14 @@ contract DbgFmtTest is Test { } } +// https://github.com/foundry-rs/foundry/issues/8557 // https://github.com/foundry-rs/foundry/issues/11249 function argListRepro(address tokenIn, uint256 amountIn, bool data) { maverickV2SwapCallback( tokenIn, amountIn, // forgefmt: disable-line // forgefmt: disable-next-line - 0,/* we didn't bother loading `amountOut` because we don't use it */ + 0 /* we didn't bother loading `amountOut` because we don't use it */, data ); } diff --git a/crates/fmt/testdata/Repros/original.sol b/crates/fmt/testdata/Repros/original.sol index b2923bfdfbba5..f9ae36eb3f2fe 100644 --- a/crates/fmt/testdata/Repros/original.sol +++ b/crates/fmt/testdata/Repros/original.sol @@ -167,6 +167,7 @@ contract DbgFmtTest is Test { } } +// https://github.com/foundry-rs/foundry/issues/8557 // https://github.com/foundry-rs/foundry/issues/11249 function argListRepro(address tokenIn, uint256 amountIn, bool data) { maverickV2SwapCallback( diff --git a/crates/fmt/testdata/Repros/sorted.fmt.sol b/crates/fmt/testdata/Repros/sorted.fmt.sol index b9dbb155a7f29..4a155b28c8b42 100644 --- a/crates/fmt/testdata/Repros/sorted.fmt.sol +++ b/crates/fmt/testdata/Repros/sorted.fmt.sol @@ -167,13 +167,14 @@ contract DbgFmtTest is Test { } } +// https://github.com/foundry-rs/foundry/issues/8557 // https://github.com/foundry-rs/foundry/issues/11249 function argListRepro(address tokenIn, uint256 amountIn, bool data) { maverickV2SwapCallback( tokenIn, amountIn, // forgefmt: disable-line // forgefmt: disable-next-line - 0,/* we didn't bother loading `amountOut` because we don't use it */ + 0 /* we didn't bother loading `amountOut` because we don't use it */, data ); } diff --git a/crates/fmt/testdata/Repros/tab.fmt.sol b/crates/fmt/testdata/Repros/tab.fmt.sol index ee7d041997141..e398157b75ae6 100644 --- a/crates/fmt/testdata/Repros/tab.fmt.sol +++ b/crates/fmt/testdata/Repros/tab.fmt.sol @@ -167,13 +167,14 @@ contract DbgFmtTest is Test { } } +// https://github.com/foundry-rs/foundry/issues/8557 // https://github.com/foundry-rs/foundry/issues/11249 function argListRepro(address tokenIn, uint256 amountIn, bool data) { maverickV2SwapCallback( tokenIn, amountIn, // forgefmt: disable-line // forgefmt: disable-next-line - 0,/* we didn't bother loading `amountOut` because we don't use it */ + 0 /* we didn't bother loading `amountOut` because we don't use it */, data ); }