fix(fmt): don't break var assignments when callee fits (#12323)

* fix(fmt): don't break var assignments when callee fits

* fix: deindent calls (exception)

Author: 0xrusowsky

crates/fmt/src/state/sol.rs

@@ -835,24 +835,48 @@ 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
+                        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);
+                    }
+                }
+                // Otherwise, if expr fits, ensure no breaks
+                else {
+                    print_inline(self);
                 }
             }
             _ => {
@@ -2926,6 +2950,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,

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,18 @@ 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;
+
+        vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak
+            + vars.previousOngoingDebtScaled;
+
+        vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak()
+            .previousOngoingDebtScaled();
+    }
 }

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,18 @@ 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;
+
+        vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak
+            + vars.previousOngoingDebtScaled;
+
+        vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak()
+            .previousOngoingDebtScaled();
+    }
 }

crates/fmt/testdata/VariableAssignment/original.sol

@@ -37,10 +37,19 @@ 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;
+
+        vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak + vars.previousOngoingDebtScaled;
+
+        vars.previousTotalDebt = vars.reallyLongVarThatCausesALineBreak() .previousOngoingDebtScaled();
+    }
 }