chore(fmt): prefer_compact + indent bin expr w/ parenthesis (#12281)

* fix: indentation in operator exprs with parentheses

* docs: new param

* feat: prefer_compact

* fix: test

* style: typo

* fix: default config test

* Fix wrap comments test

* fix: emit and revert

---------

Co-authored-by: grandizzy <[email protected]>

Author: 0xrusowsky

crates/config/src/fmt.rs

@@ -39,8 +39,9 @@ pub struct FormatterConfig {
     pub sort_imports: bool,
     /// Whether to suppress spaces around the power operator (`**`).
     pub pow_no_space: bool,
-    /// Whether to compact call args in a single line when possible
-    pub call_compact_args: bool,
+    /// Style that determines if a broken list, should keep its elements together on their own
+    /// line, before breaking individually.
+    pub prefer_compact: PreferCompact,
 }
 
 /// Style of integer types.
@@ -186,6 +187,40 @@ impl MultilineFuncHeaderStyle {
     }
 }
 
+/// Style that determines if a broken list, should keep its elements together on their own line,
+/// before breaking individually.
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum PreferCompact {
+    /// All elements are preferred consistent.
+    None,
+    /// Calls are preferred compact. Events and errors break consistently.
+    Calls,
+    /// Events are preferred compact. Calls and errors break consistently.
+    Events,
+    /// Errors are preferred compact. Calls and events break consistently.
+    Errors,
+    /// Events and errors are preferred compact. Calls break consistently.
+    EventsErrors,
+    /// All elements are preferred compact.
+    #[default]
+    All,
+}
+
+impl PreferCompact {
+    pub fn calls(&self) -> bool {
+        matches!(self, Self::All | Self::Calls)
+    }
+
+    pub fn events(&self) -> bool {
+        matches!(self, Self::All | Self::Events | Self::EventsErrors)
+    }
+
+    pub fn errors(&self) -> bool {
+        matches!(self, Self::All | Self::Errors | Self::EventsErrors)
+    }
+}
+
 /// Style of indent
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
@@ -214,7 +249,7 @@ impl Default for FormatterConfig {
             contract_new_lines: false,
             sort_imports: false,
             pow_no_space: false,
-            call_compact_args: true,
+            prefer_compact: PreferCompact::default(),
             docs_style: DocCommentStyle::default(),
         }
     }

crates/fmt/README.md

@@ -116,6 +116,7 @@ The formatter supports multiple configuration options defined in `foundry.toml`.
 | `bracket_spacing` | `false` | Print spaces between brackets. |
 | `int_types` | `long` | Style for `uint256`/`int256` types. Options: `long`, `short`, `preserve`. |
 | `multiline_func_header` | `attributes_first` | The style of multiline function headers. Options: `attributes_first`, `params_always`, `params_first_multi`, `all`, `all_params`. |
+| `prefer_compact` | `calls` | Style that determines if a broken list, should keep its elements together on their own line, before breaking individually. Options: `calls`, `events`, `errors`, `events_errors`, `all`. |
 | `quote_style` | `double` | The style of quotation marks. Options: `double`, `single`, `preserve`. |
 | `number_underscore` | `preserve` | The style of underscores in number literals. Options: `preserve`, `remove`, `thousands`. |
 | `hex_underscore` | `remove` | The style of underscores in hex literals. Options: `preserve`, `remove`, `bytes`. |

crates/fmt/src/state/common.rs

@@ -524,7 +524,9 @@ impl<'ast> State<'_, 'ast> {
         for (pos, ident) in path.segments().iter().delimited() {
             self.print_ident(ident);
             if !pos.is_last {
-                self.zerobreak();
+                if !self.emit_or_revert {
+                    self.zerobreak();
+                }
                 self.word(".");
             }
         }

crates/fmt/src/state/sol.rs

@@ -725,15 +725,31 @@ impl<'ast> State<'_, 'ast> {
         let ast::ItemError { name, parameters } = err;
         self.word("error ");
         self.print_ident(name);
-        self.print_parameter_list(parameters, parameters.span, ListFormat::compact());
+        self.print_parameter_list(
+            parameters,
+            parameters.span,
+            if self.config.prefer_compact.errors() {
+                ListFormat::compact()
+            } else {
+                ListFormat::consistent()
+            },
+        );
         self.word(";");
     }
 
     fn print_event(&mut self, event: &'ast ast::ItemEvent<'ast>) {
         let ast::ItemEvent { name, parameters, anonymous } = event;
         self.word("event ");
         self.print_ident(name);
-        self.print_parameter_list(parameters, parameters.span, ListFormat::compact().break_cmnts());
+        self.print_parameter_list(
+            parameters,
+            parameters.span,
+            if self.config.prefer_compact.events() {
+                ListFormat::compact().break_cmnts()
+            } else {
+                ListFormat::consistent().break_cmnts()
+            },
+        );
         if *anonymous {
             self.word(" anonymous");
         }
@@ -1701,7 +1717,7 @@ impl<'ast> State<'_, 'ast> {
     }
 
     fn print_named_args(&mut self, args: &'ast [ast::NamedArg<'ast>], pos_hi: BytePos) {
-        let list_format = match (self.config.bracket_spacing, self.config.call_compact_args) {
+        let list_format = match (self.config.bracket_spacing, self.config.prefer_compact.calls()) {
             (false, true) => ListFormat::compact(),
             (false, false) => ListFormat::consistent(),
             (true, true) => ListFormat::compact().with_space(),
@@ -1738,7 +1754,7 @@ impl<'ast> State<'_, 'ast> {
                     .break_cmnts()
                     .break_single(true)
                     .without_ind(self.call_stack.is_chain())
-                    .with_delimiters(!(self.emit_or_revert || self.call_with_opts_and_args)),
+                    .with_delimiters(!self.call_with_opts_and_args),
             );
         } else if self.config.bracket_spacing {
             self.nbsp();
@@ -2264,17 +2280,14 @@ impl<'ast> State<'_, 'ast> {
             self.nbsp();
         };
         self.s.cbox(0);
-        self.print_path(path, false);
         self.emit_or_revert = path.segments().len() > 1;
-        self.print_call_args(
-            args,
-            if self.config.call_compact_args {
-                ListFormat::compact().break_cmnts().with_delimiters(args.len() == 1)
-            } else {
-                ListFormat::consistent().break_cmnts().with_delimiters(args.len() == 1)
-            },
-            path.to_string().len(),
-        );
+        self.print_path(path, false);
+        let format = if self.config.prefer_compact.calls() {
+            ListFormat::compact()
+        } else {
+            ListFormat::consistent()
+        };
+        self.print_call_args(args, format.break_cmnts(), path.to_string().len());
         self.emit_or_revert = false;
         self.end();
     }
@@ -2797,6 +2810,7 @@ fn has_complex_successor(expr_kind: &ast::ExprKind<'_>, left: bool) -> bool {
         }
         ast::ExprKind::Unary(_, expr) => has_complex_successor(&expr.kind, left),
         ast::ExprKind::Lit(..) | ast::ExprKind::Ident(_) => false,
+        ast::ExprKind::Tuple(..) => false,
         _ => true,
     }
 }

crates/fmt/testdata/DocComments/wrap-comments.fmt.sol

@@ -1,6 +1,5 @@
 // config: line_length = 40
 // config: wrap_comments = true
-// config: call_compact_args = false
 pragma solidity ^0.8.13;
 
 /// @title A Hello world example
@@ -24,8 +23,7 @@ contract HelloWorld {
     /// @param age The dude's age
     constructor(uint256 age) {
         theDude = Person({
-            age: age,
-            wallet: msg.sender
+            age: age, wallet: msg.sender
         });
     }
 

crates/fmt/testdata/EmitStatement/120.compact.fmt.sol

@@ -0,0 +1,48 @@
+// config: line_length = 120
+event NewEvent(address beneficiary, uint256 index, uint64 timestamp, uint64 endTimestamp);
+
+function emitEvent() {
+    emit NewEvent(beneficiary, _vestingBeneficiaries.length - 1, uint64(block.timestamp), endTimestamp);
+
+    emit NewEvent( /* beneficiary */
+        beneficiary,
+        /* index */
+        _vestingBeneficiaries.length - 1,
+        /* timestamp */
+        uint64(block.timestamp),
+        /* end timestamp */
+        endTimestamp
+    );
+
+    emit NewEvent(
+        beneficiary, // beneficiary
+        _vestingBeneficiaries.length - 1, // index
+        uint64(block.timestamp), // timestamp
+        endTimestamp // end timestamp
+    );
+
+    // https://github.com/foundry-rs/foundry/issues/12029
+    emit OperatorSharesDecreased(
+        defaultOperator,
+        address(0),
+        strategyMock,
+        depositAmount / 6 // 1 withdrawal not queued so decreased
+    );
+
+    // https://github.com/foundry-rs/foundry/issues/12146
+    emit ISablierComptroller.DisableCustomFeeUSD(
+        protocol_protocol, caller_caller, user_users.sender, previousMinFeeUSD_0, newMinFeeUSD_feeUSD
+    );
+    emit ISablierComptroller.DisableCustomFeeUSD({
+        protocol: protocol, caller: caller, user: users.sender, previousMinFeeUSD: 0, newMinFeeUSD: feeUSD
+    });
+
+    emit ISablierLockupLinear.CreateLockupLinearStream({
+        streamId: streamId,
+        commonParams: Lockup.CreateEventCommon({
+            funder: msg.sender, sender: sender, recipient: recipient, depositAmount: depositAmount
+        }),
+        cliffTime: cliffTime,
+        unlockAmounts: unlockAmounts
+    });
+}

crates/fmt/testdata/EmitStatement/120.fmt.sol

@@ -1,4 +1,5 @@
 // config: line_length = 120
+// config: prefer_compact = "none"
 event NewEvent(address beneficiary, uint256 index, uint64 timestamp, uint64 endTimestamp);
 
 function emitEvent() {
@@ -30,9 +31,19 @@ function emitEvent() {
     );
 
     // https://github.com/foundry-rs/foundry/issues/12146
-    emit ISablierComptroller.DisableCustomFeeUSD(protocol, caller, users.sender, 0, feeUSD);
+    emit ISablierComptroller.DisableCustomFeeUSD(
+        protocol_protocol,
+        caller_caller,
+        user_users.sender,
+        previousMinFeeUSD_0,
+        newMinFeeUSD_feeUSD
+    );
     emit ISablierComptroller.DisableCustomFeeUSD({
-        protocol: protocol, caller: caller, user: users.sender, previousMinFeeUSD: 0, newMinFeeUSD: feeUSD
+        protocol: protocol,
+        caller: caller,
+        user: users.sender,
+        previousMinFeeUSD: 0,
+        newMinFeeUSD: feeUSD
     });
 
     emit ISablierLockupLinear.CreateLockupLinearStream({
@@ -41,12 +52,7 @@ function emitEvent() {
             funder: msg.sender,
             sender: sender,
             recipient: recipient,
-            depositAmount: depositAmount,
-            token: token,
-            cancelable: cancelable,
-            transferable: transferable,
-            timestamps: timestamps,
-            shape: shape
+            depositAmount: depositAmount
         }),
         cliffTime: cliffTime,
         unlockAmounts: unlockAmounts

crates/fmt/testdata/EmitStatement/fmt.sol

@@ -38,7 +38,11 @@ function emitEvent() {
 
     // https://github.com/foundry-rs/foundry/issues/12146
     emit ISablierComptroller.DisableCustomFeeUSD(
-        protocol, caller, users.sender, 0, feeUSD
+        protocol_protocol,
+        caller_caller,
+        user_users.sender,
+        previousMinFeeUSD_0,
+        newMinFeeUSD_feeUSD
     );
     emit ISablierComptroller.DisableCustomFeeUSD({
         protocol: protocol,
@@ -54,12 +58,7 @@ function emitEvent() {
             funder: msg.sender,
             sender: sender,
             recipient: recipient,
-            depositAmount: depositAmount,
-            token: token,
-            cancelable: cancelable,
-            transferable: transferable,
-            timestamps: timestamps,
-            shape: shape
+            depositAmount: depositAmount
         }),
         cliffTime: cliffTime,
         unlockAmounts: unlockAmounts

crates/fmt/testdata/EmitStatement/original.sol

@@ -31,7 +31,7 @@ function emitEvent() {
     );
 
     // https://github.com/foundry-rs/foundry/issues/12146
-    emit ISablierComptroller.DisableCustomFeeUSD(protocol, caller, users.sender, 0, feeUSD);
+    emit ISablierComptroller.DisableCustomFeeUSD(protocol_protocol, caller_caller, user_users.sender, previousMinFeeUSD_0, newMinFeeUSD_feeUSD);
     emit ISablierComptroller.DisableCustomFeeUSD({ protocol: protocol, caller: caller, user: users.sender, previousMinFeeUSD: 0, newMinFeeUSD: feeUSD });
 
     emit ISablierLockupLinear.CreateLockupLinearStream({
@@ -40,12 +40,7 @@ function emitEvent() {
             funder: msg.sender,
             sender: sender,
             recipient: recipient,
-            depositAmount: depositAmount,
-            token: token,
-            cancelable: cancelable,
-            transferable: transferable,
-            timestamps: timestamps,
-            shape: shape
+            depositAmount: depositAmount
         }),
         cliffTime: cliffTime,
         unlockAmounts: unlockAmounts

crates/fmt/testdata/NamedFunctionCallExpression/fmt.sol

@@ -1,4 +1,4 @@
-// config: call_compact_args = false
+// config: prefer_compact = "events_errors"
 contract NamedFunctionCallExpression {
     struct SimpleStruct {
         uint256 val;