From 46f0834cbf4c04d06ffd654a56154de4b54049c7 Mon Sep 17 00:00:00 2001 From: 0xrusowsky <0xrusowsky@proton.me> Date: Tue, 28 Oct 2025 10:16:26 +0100 Subject: [PATCH 1/2] fix(fmt): don't break var assignments when callee fits --- crates/fmt/src/state/sol.rs | 47 +++++++++++++------ .../bracket-spacing.fmt.sol | 10 +++- .../fmt/testdata/VariableAssignment/fmt.sol | 10 +++- .../testdata/VariableAssignment/original.sol | 9 +++- 4 files changed, 59 insertions(+), 17 deletions(-) diff --git a/crates/fmt/src/state/sol.rs b/crates/fmt/src/state/sol.rs index 7303d6fb7d3d3..90e0fc602ddf2 100644 --- a/crates/fmt/src/state/sol.rs +++ b/crates/fmt/src/state/sol.rs @@ -818,24 +818,42 @@ impl<'ast> State<'_, 'ast> { self.s.offset(self.ind); self.print_expr(rhs); } - ast::ExprKind::Binary(_, op, _) => { - // Binary expressions: check if we need to break and indent - if force_break || self.estimate_lhs_size(rhs, op) + lhs_size > space_left { - if !self.is_bol_or_only_ind() { + ast::ExprKind::Binary(lhs, op, _) => { + let print_inline = |this: &mut Self| { + this.print_sep(Separator::Nbsp); + this.neverbreak(); + this.print_expr(rhs); + }; + let print_with_break = |this: &mut Self, force_break: bool| { + if !this.is_bol_or_only_ind() { if force_break { - self.print_sep(Separator::Hardbreak); + this.print_sep(Separator::Hardbreak); } else { - self.print_sep(Separator::Space); + this.print_sep(Separator::Space); } } - self.s.offset(self.ind); - self.s.ibox(self.ind); - self.print_expr(rhs); - self.end(); - } else { - self.print_sep(Separator::Nbsp); - self.neverbreak(); - self.print_expr(rhs); + this.s.offset(this.ind); + this.s.ibox(this.ind); + this.print_expr(rhs); + this.end(); + }; + + // Binary expressions: check if we need to break and indent + if force_break { + print_with_break(self, true); + } else if self.estimate_lhs_size(rhs, op) + lhs_size > space_left { + if has_complex_successor(&rhs.kind, true) + && get_callee_head_size(lhs) + lhs_size <= space_left + { + // Keep complex exprs (where callee fits) inline, as they will have breaks + print_inline(self); + } else { + print_with_break(self, false); + } + } + // Otherwise, if expr fits, ensure no breaks + else { + print_inline(self); } } _ => { @@ -2908,6 +2926,7 @@ pub(super) fn get_callee_head_size(callee: &ast::Expr<'_>) -> usize { _ => member_ident.as_str().len(), } } + ast::ExprKind::Binary(lhs, _, _) => get_callee_head_size(lhs), // If the callee is not an identifier or member access, it has no "head" _ => 0, diff --git a/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol b/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol index 69a914993d50b..32ead788f74db 100644 --- a/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol +++ b/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol @@ -39,8 +39,8 @@ contract TestContract { "0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"; } + // https://github.com/foundry-rs/foundry/issues/12254 function test_longIndexedCall() { - // https://github.com/foundry-rs/foundry/issues/12254 bytes memory message = mailboxes[destinationDomain].buildMessage( originDomain, bytes32(0), @@ -55,4 +55,12 @@ contract TestContract { abi.encode(orderId, bytes32(0), address(0)) ); } + + // https://github.com/foundry-rs/foundry/issues/12322 + function test_longComplexBinExpr() { + vars.previousTotalDebt = getDescaledAmount( + flow.getSnapshotDebtScaled(streamId), + flow.getTokenDecimals(streamId) + ) + vars.previousOngoingDebtScaled; + } } diff --git a/crates/fmt/testdata/VariableAssignment/fmt.sol b/crates/fmt/testdata/VariableAssignment/fmt.sol index be4b21b4e2b2d..df249bfd7aaf4 100644 --- a/crates/fmt/testdata/VariableAssignment/fmt.sol +++ b/crates/fmt/testdata/VariableAssignment/fmt.sol @@ -38,8 +38,8 @@ contract TestContract { "0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"; } + // https://github.com/foundry-rs/foundry/issues/12254 function test_longIndexedCall() { - // https://github.com/foundry-rs/foundry/issues/12254 bytes memory message = mailboxes[destinationDomain].buildMessage( originDomain, bytes32(0), @@ -54,4 +54,12 @@ contract TestContract { abi.encode(orderId, bytes32(0), address(0)) ); } + + // https://github.com/foundry-rs/foundry/issues/12322 + function test_longComplexBinExpr() { + vars.previousTotalDebt = getDescaledAmount( + flow.getSnapshotDebtScaled(streamId), + flow.getTokenDecimals(streamId) + ) + vars.previousOngoingDebtScaled; + } } diff --git a/crates/fmt/testdata/VariableAssignment/original.sol b/crates/fmt/testdata/VariableAssignment/original.sol index 55e8d937e0854..e6bcf11ec8689 100644 --- a/crates/fmt/testdata/VariableAssignment/original.sol +++ b/crates/fmt/testdata/VariableAssignment/original.sol @@ -37,10 +37,17 @@ contract TestContract { "0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"; } + // https://github.com/foundry-rs/foundry/issues/12254 function test_longIndexedCall() { - // https://github.com/foundry-rs/foundry/issues/12254 bytes memory message = mailboxes[destinationDomain].buildMessage(originDomain, bytes32(0), address(inbox).toBytes32(), abi.encode(orderId, bytes32(0), address(0))); // should have identicall behavior when call of the same size without indexing bytes memory message = mailboxes_destinationDomains.buildMessage(originDomain, bytes32(0), address(inbox).toBytes32(), abi.encode(orderId, bytes32(0), address(0))); } + + // https://github.com/foundry-rs/foundry/issues/12322 + function test_longComplexBinExpr() { + vars.previousTotalDebt = + getDescaledAmount(flow.getSnapshotDebtScaled(streamId), flow.getTokenDecimals(streamId)) + + vars.previousOngoingDebtScaled; + } } From 9699d4f1383f2bc7ebc809b81503bd0433bb3516 Mon Sep 17 00:00:00 2001 From: 0xrusowsky <0xrusowsky@proton.me> Date: Tue, 28 Oct 2025 11:10:11 +0100 Subject: [PATCH 2/2] fix: deindent calls (exception) --- crates/fmt/src/state/sol.rs | 8 +++++++- .../VariableAssignment/bracket-spacing.fmt.sol | 12 +++++++++--- crates/fmt/testdata/VariableAssignment/fmt.sol | 12 +++++++++--- crates/fmt/testdata/VariableAssignment/original.sol | 8 +++++--- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/crates/fmt/src/state/sol.rs b/crates/fmt/src/state/sol.rs index 90e0fc602ddf2..649e1165bc27a 100644 --- a/crates/fmt/src/state/sol.rs +++ b/crates/fmt/src/state/sol.rs @@ -846,7 +846,13 @@ impl<'ast> State<'_, 'ast> { && get_callee_head_size(lhs) + lhs_size <= space_left { // Keep complex exprs (where callee fits) inline, as they will have breaks - print_inline(self); + if matches!(lhs.kind, ast::ExprKind::Call(..)) { + self.s.ibox(-self.ind); + print_inline(self); + self.end(); + } else { + print_inline(self); + } } else { print_with_break(self, false); } diff --git a/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol b/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol index 32ead788f74db..07b4866e4d6cb 100644 --- a/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol +++ b/crates/fmt/testdata/VariableAssignment/bracket-spacing.fmt.sol @@ -59,8 +59,14 @@ contract TestContract { // https://github.com/foundry-rs/foundry/issues/12322 function test_longComplexBinExpr() { vars.previousTotalDebt = getDescaledAmount( - flow.getSnapshotDebtScaled(streamId), - flow.getTokenDecimals(streamId) - ) + vars.previousOngoingDebtScaled; + flow.getSnapshotDebtScaled(streamId), + flow.getTokenDecimals(streamId) + ) + vars.previousOngoingDebtScaled; + + vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak + + vars.previousOngoingDebtScaled; + + vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak() + .previousOngoingDebtScaled(); } } diff --git a/crates/fmt/testdata/VariableAssignment/fmt.sol b/crates/fmt/testdata/VariableAssignment/fmt.sol index df249bfd7aaf4..080992fe050b0 100644 --- a/crates/fmt/testdata/VariableAssignment/fmt.sol +++ b/crates/fmt/testdata/VariableAssignment/fmt.sol @@ -58,8 +58,14 @@ contract TestContract { // https://github.com/foundry-rs/foundry/issues/12322 function test_longComplexBinExpr() { vars.previousTotalDebt = getDescaledAmount( - flow.getSnapshotDebtScaled(streamId), - flow.getTokenDecimals(streamId) - ) + vars.previousOngoingDebtScaled; + flow.getSnapshotDebtScaled(streamId), + flow.getTokenDecimals(streamId) + ) + vars.previousOngoingDebtScaled; + + vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak + + vars.previousOngoingDebtScaled; + + vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak() + .previousOngoingDebtScaled(); } } diff --git a/crates/fmt/testdata/VariableAssignment/original.sol b/crates/fmt/testdata/VariableAssignment/original.sol index e6bcf11ec8689..2ccc26c97df29 100644 --- a/crates/fmt/testdata/VariableAssignment/original.sol +++ b/crates/fmt/testdata/VariableAssignment/original.sol @@ -46,8 +46,10 @@ contract TestContract { // https://github.com/foundry-rs/foundry/issues/12322 function test_longComplexBinExpr() { - vars.previousTotalDebt = - getDescaledAmount(flow.getSnapshotDebtScaled(streamId), flow.getTokenDecimals(streamId)) - + vars.previousOngoingDebtScaled; + vars.previousTotalDebt = getDescaledAmount(flow.getSnapshotDebtScaled(streamId), flow.getTokenDecimals(streamId)) + vars.previousOngoingDebtScaled; + + vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak + vars.previousOngoingDebtScaled; + + vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak() .previousOngoingDebtScaled(); } }