fix(fmt): refine logic over comments between uninformed commasep (#12055)

Author: 0xrusowsky

crates/fmt/src/state/common.rs

@@ -285,6 +285,15 @@ impl<'ast> State<'_, 'ast> {
             return false;
         };
 
+        // If first item is uninformed (just a comma), and it has its own comment, skip it.
+        // It will be dealt with when printing the item in the main loop of `commasep`.
+        if span.is_dummy()
+            && let Some(next_pos) = values.get(1).map(|v| get_span(v).lo())
+            && self.peek_comment_before(next_pos).is_some()
+        {
+            return true;
+        }
+
         // Check for comments before the first item.
         if let Some((cmnt_span, cmnt_style)) =
             self.peek_comment_before(span.lo()).map(|c| (c.span, c.style))
@@ -406,7 +415,10 @@ impl<'ast> State<'_, 'ast> {
                 self.hardbreak(); // trailing and isolated comments already hardbreak
             }
 
-            print(self, value);
+            // Avoid printing the last uninformed item, so that we can handle line breaks.
+            if !(is_last && get_span(value).is_dummy()) {
+                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);
@@ -463,13 +475,9 @@ impl<'ast> State<'_, 'ast> {
             if let Some(next_span) = next_span
                 && !self.is_bol_or_only_ind()
                 && !self.inline_config.is_disabled(next_span)
+                && !next_span.is_dummy()
             {
-                if next_span.is_dummy() && !matches!(format.kind, ListFormatKind::AlwaysBreak) {
-                    // Don't add spaces between uninformed items (commas)
-                    self.zerobreak();
-                } else {
-                    format.print_break(false, values.len(), &mut self.s);
-                }
+                format.print_break(false, values.len(), &mut self.s);
             }
         }
 

crates/fmt/src/state/mod.rs

@@ -546,13 +546,15 @@ impl<'sess> State<'sess, '_> {
             // Handle mixed with follow-up comment
             if cmnt.style.is_mixed() {
                 if let Some(cmnt) = self.peek_comment_before(pos) {
-                    config.mixed_no_break = true;
+                    config.mixed_no_break_prev = true;
+                    config.mixed_no_break_post = true;
                     config.mixed_post_nbsp = cmnt.style.is_mixed();
                 }
 
                 // Ensure consecutive mixed comments don't have a double-space
                 if last_style.is_some_and(|s| s.is_mixed()) {
-                    config.mixed_no_break = true;
+                    config.mixed_no_break_prev = true;
+                    config.mixed_no_break_post = true;
                     config.mixed_prev_space = false;
                 }
             } else if config.offset != 0
@@ -693,7 +695,7 @@ impl<'sess> State<'sess, '_> {
                 let Some(prefix) = cmnt.prefix() else { return };
                 let never_break = self.last_token_is_neverbreak();
                 if !self.is_bol_or_only_ind() {
-                    match (never_break || config.mixed_no_break, config.mixed_prev_space) {
+                    match (never_break || config.mixed_no_break_prev, config.mixed_prev_space) {
                         (false, true) => config.space(&mut self.s),
                         (false, false) => config.zerobreak(&mut self.s),
                         (true, true) => self.nbsp(),
@@ -721,7 +723,7 @@ impl<'sess> State<'sess, '_> {
                 if config.mixed_post_nbsp {
                     config.nbsp_or_space(self.config.wrap_comments, &mut self.s);
                     self.cursor.advance(1);
-                } else if !config.mixed_no_break {
+                } else if !config.mixed_no_break_post {
                     config.space(&mut self.s);
                     self.cursor.advance(1);
                 }
@@ -996,7 +998,8 @@ pub(crate) struct CommentConfig {
     // Config: mixed comments
     mixed_prev_space: bool,
     mixed_post_nbsp: bool,
-    mixed_no_break: bool,
+    mixed_no_break_prev: bool,
+    mixed_no_break_post: bool,
 }
 
 impl CommentConfig {
@@ -1020,7 +1023,8 @@ impl CommentConfig {
     pub(crate) fn no_breaks(mut self) -> Self {
         self.iso_no_break = true;
         self.trailing_no_break = true;
-        self.mixed_no_break = true;
+        self.mixed_no_break_prev = true;
+        self.mixed_no_break_post = true;
         self
     }
 
@@ -1030,7 +1034,13 @@ impl CommentConfig {
     }
 
     pub(crate) fn mixed_no_break(mut self) -> Self {
-        self.mixed_no_break = true;
+        self.mixed_no_break_prev = true;
+        self.mixed_no_break_post = true;
+        self
+    }
+
+    pub(crate) fn mixed_no_break_post(mut self) -> Self {
+        self.mixed_no_break_post = true;
         self
     }
 

crates/fmt/src/state/sol.rs

@@ -1823,7 +1823,7 @@ impl<'ast> State<'_, 'ast> {
             |this, var| match var {
                 SpannedOption::Some(var) => this.print_var(var, true),
                 SpannedOption::None(span) => {
-                    this.print_comments(span.hi(), CommentConfig::skip_ws().no_breaks());
+                    this.print_comments(span.hi(), CommentConfig::skip_ws().mixed_no_break_post());
                 }
             },
             |var| match var {

crates/fmt/testdata/SimpleComments/fmt.sol

@@ -65,6 +65,19 @@ contract SimpleComments {
             /* hashMod */
         ) = _swapPre(2, TOTAL_SUPPLY / 1_000, false, zeroForOne1);
     }
+
+    // https://github.com/foundry-rs/foundry/issues/12045
+    function test6() {
+        (
+            // uint80 roundID
+            ,
+            int256 dataFeedAnswer,
+            // uint startedAt
+            ,
+            uint256 updatedAt,
+            // uint80 answeredInRound
+        ) = dataFeedContract.latestRoundData();
+    }
 }
 
 /*

crates/fmt/testdata/SimpleComments/original.sol

@@ -66,6 +66,19 @@ contract SimpleComments {
             /* hashMod */
         ) = _swapPre(2, TOTAL_SUPPLY / 1_000, false, zeroForOne1);
     }
+
+    // https://github.com/foundry-rs/foundry/issues/12045
+    function test6() {
+            (
+        // uint80 roundID
+        ,
+        int256 dataFeedAnswer,
+        // uint startedAt
+        ,
+        uint256 updatedAt,
+        // uint80 answeredInRound
+        ) = dataFeedContract.latestRoundData();
+    }
 }
 
 /*

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

@@ -78,6 +78,19 @@ contract SimpleComments {
             2, TOTAL_SUPPLY / 1_000, false, zeroForOne1
         );
     }
+
+    // https://github.com/foundry-rs/foundry/issues/12045
+    function test6() {
+        (
+            // uint80 roundID
+            ,
+            int256 dataFeedAnswer,
+            // uint startedAt
+            ,
+            uint256 updatedAt,
+            // uint80 answeredInRound
+        ) = dataFeedContract.latestRoundData();
+    }
 }
 
 /*