More chain code refactoring

src/chains.rs

@@ -158,12 +158,6 @@ pub(crate) fn rewrite_chain(
     chain.rewrite_result(context, shape)
 }
 
-#[derive(Debug)]
-enum CommentPosition {
-    Back,
-    Top,
-}
-
 /// Information about an expression in a chain.
 struct SubExpr {
     expr: ast::Expr,
@@ -175,6 +169,8 @@ struct SubExpr {
 struct ChainItem {
     kind: ChainItemKind,
     tries: usize,
+    // The entire span of the chain item, including the trailing dot and any comments, e.g.
+    // `.some_method(arg, arg)`, or  `. /* a comment */ my_attribute`.
     span: Span,
 }
 
@@ -202,7 +198,10 @@ enum ChainItemKind {
     Await,
     Use,
     Yield,
-    Comment(String, CommentPosition),
+    /// A comment within a chain, e.g. `parent. item /* comment */.rest`.
+    /// The bool is whether this comment should be on the same line as the previous item in the
+    /// chain.
+    Comment(String, bool),
 }
 
 impl ChainItemKind {
@@ -339,9 +338,9 @@ impl ChainItem {
         ChainItem { kind, tries, span }
     }
 
-    fn comment(span: Span, comment: String, pos: CommentPosition) -> ChainItem {
+    fn comment(span: Span, comment: String, on_same_line: bool) -> ChainItem {
         ChainItem {
-            kind: ChainItemKind::Comment(comment, pos),
+            kind: ChainItemKind::Comment(comment, on_same_line),
             tries: 0,
             span,
         }
@@ -421,7 +420,7 @@ impl Chain {
             prev_span_end: &mut BytePos,
             children: &mut Vec<ChainItem>,
         ) {
-            let white_spaces: &[_] = &[' ', '\t'];
+            let white_spaces = &[' ', '\t'];
             if post_comment_snippet
                 .trim_matches(white_spaces)
                 .starts_with('\n')
@@ -434,7 +433,7 @@ impl Chain {
                 children.push(ChainItem::comment(
                     post_comment_span,
                     trimmed_snippet.trim().to_owned(),
-                    CommentPosition::Back,
+                    true,
                 ));
                 *prev_span_end = post_comment_span.hi();
             }
@@ -445,6 +444,8 @@ impl Chain {
         let mut prev_span_end = parent.span.hi();
         let mut iter = rev_children.into_iter().rev().peekable();
         if let Some(first_chain_item) = iter.peek() {
+            // `parent? /* maybe comment */ . /* maybe comment */ first_child`
+            //        ^------------------- ^ comment_span
             let comment_span = mk_sp(prev_span_end, first_chain_item.span.lo());
             let comment_snippet = context.snippet(comment_span);
             if !is_tries(comment_snippet.trim()) {
@@ -466,16 +467,14 @@ impl Chain {
             if handle_comment {
                 let pre_comment_span = mk_sp(prev_span_end, chain_item.span.lo());
                 let pre_comment_snippet = trim_tries(context.snippet(pre_comment_span));
-                let (pre_comment, _) = extract_pre_comment(&pre_comment_snippet);
-                match pre_comment {
-                    Some(ref comment) if !comment.is_empty() => {
+                if let (Some(pre_comment), _) = extract_pre_comment(&pre_comment_snippet) {
+                    if !pre_comment.is_empty() {
                         children.push(ChainItem::comment(
                             pre_comment_span,
-                            comment.to_owned(),
-                            CommentPosition::Top,
+                            pre_comment.to_owned(),
+                            false,
                         ));
                     }
-                    _ => (),
                 }
             }
 
@@ -509,7 +508,8 @@ impl Chain {
             is_postfix_receiver: false,
         }];
 
-        while let Some(subexpr) = Self::pop_expr_chain(subexpr_list.last().unwrap(), context) {
+        while let Some(subexpr) = Self::pop_expr_chain(&subexpr_list.last().unwrap().expr, context)
+        {
             subexpr_list.push(subexpr);
         }
 
@@ -518,20 +518,20 @@ impl Chain {
 
     // Returns the expression's subexpression, if it exists. When the subexpr
     // is a try! macro, we'll convert it to shorthand when the option is set.
-    fn pop_expr_chain(expr: &SubExpr, context: &RewriteContext<'_>) -> Option<SubExpr> {
-        match expr.expr.kind {
-            ast::ExprKind::MethodCall(ref call) => Some(SubExpr {
+    fn pop_expr_chain(expr: &ast::Expr, context: &RewriteContext<'_>) -> Option<SubExpr> {
+        match &expr.kind {
+            ast::ExprKind::MethodCall(call) => Some(SubExpr {
                 expr: Self::convert_try(&call.receiver, context),
                 is_postfix_receiver: true,
             }),
-            ast::ExprKind::Field(ref subexpr, _)
-            | ast::ExprKind::Await(ref subexpr, _)
-            | ast::ExprKind::Use(ref subexpr, _)
-            | ast::ExprKind::Yield(ast::YieldKind::Postfix(ref subexpr)) => Some(SubExpr {
+            ast::ExprKind::Field(subexpr, _)
+            | ast::ExprKind::Await(subexpr, _)
+            | ast::ExprKind::Use(subexpr, _)
+            | ast::ExprKind::Yield(ast::YieldKind::Postfix(subexpr)) => Some(SubExpr {
                 expr: Self::convert_try(subexpr, context),
                 is_postfix_receiver: true,
             }),
-            ast::ExprKind::Try(ref subexpr) => Some(SubExpr {
+            ast::ExprKind::Try(subexpr) => Some(SubExpr {
                 expr: Self::convert_try(subexpr, context),
                 is_postfix_receiver: false,
             }),
@@ -540,13 +540,9 @@ impl Chain {
     }
 
     fn convert_try(expr: &ast::Expr, context: &RewriteContext<'_>) -> ast::Expr {
-        match expr.kind {
-            ast::ExprKind::MacCall(ref mac) if context.config.use_try_shorthand() => {
-                if let Some(subexpr) = convert_try_mac(mac, context) {
-                    subexpr
-                } else {
-                    expr.clone()
-                }
+        match &expr.kind {
+            ast::ExprKind::MacCall(mac) if context.config.use_try_shorthand() => {
+                convert_try_mac(mac, context).unwrap_or(expr.clone())
             }
             _ => expr.clone(),
         }
@@ -839,8 +835,8 @@ impl<'a> ChainFormatterShared<'a> {
 
         for (rewrite, chain_item) in iter {
             match chain_item.kind {
-                ChainItemKind::Comment(_, CommentPosition::Back) => result.push(' '),
-                ChainItemKind::Comment(_, CommentPosition::Top) => result.push_str(&connector),
+                ChainItemKind::Comment(_, true) => result.push_str(" "),
+                ChainItemKind::Comment(_, false) => result.push_str(&connector),
                 _ => result.push_str(&connector),
             }
             result.push_str(rewrite);

tests/source/chain_yield_with_comments.rs

@@ -0,0 +1,8 @@
+// rustfmt-edition: 2024
+
+fn f() {
+    parent /* comment */ .yield /* some comment */?// comment
+    .yield /* some
+       long
+    comment */.yield;
+}

tests/source/chains_with_comment.rs

@@ -118,4 +118,30 @@ fn foo() {
     // comment
         ? ? ?
         .baz;
+
+    parent /* comment */ .child;
+    parent /* comment1 */     /* comment2 */ .child;
+    parent /* comment1 */   // comment 2
+            .child;
+    parent /* ???no tries here */ // nor ??? here ????
+    ???.child;
+    parent/*
+        some interesting shaped comment
+    */.child;
+
+    parent /* comment */
+    // another comment
+        .child1(some, args) /* comment */ // again
+    .await /* longer
+         comment
+    */ // more comments!
+        .use // here's a comment that reaches right to the default width limit aaaaaaaaaaaaaaaaaaaa
+    .end;
+
+    parent? /* spacing is interesting */    ?   /* blah */.child;
+
+    // NB: recording the current state of things
+    // but is possibly a bug: rustfmt/issues/6433
+    parent //comment
+           .1 .2 .3;
 }

tests/target/chain_yield_with_comments.rs

@@ -0,0 +1,10 @@
+// rustfmt-edition: 2024
+
+fn f() {
+    parent /* comment */
+        .yield? /* some comment */// comment
+        .yield /* some
+           long
+        comment */
+        .yield;
+}

tests/target/chains_with_comment.rs

@@ -134,4 +134,36 @@ fn foo() {
         // comment
         // comment
         .baz;
+
+    parent /* comment */
+        .child;
+    parent /* comment1 */     /* comment2 */
+        .child;
+    parent /* comment1 */   // comment 2
+        .child;
+    parent??? /* ???no tries here */ // nor ??? here ????
+        .child;
+    parent /*
+            some interesting shaped comment
+        */
+        .child;
+
+    parent /* comment */
+        // another comment
+        .child1(some, args) /* comment */ // again
+        .await /* longer
+             comment
+        */ // more comments!
+        .use // here's a comment that reaches right to the default width limit aaaaaaaaaaaaaaaaaaaa
+        .end;
+
+    parent?? /* spacing is interesting */       /* blah */
+        .child;
+
+    // NB: recording the current state of things
+    // but is possibly a bug: rustfmt/issues/6433
+    parent //comment
+        .1
+         .2
+         .3;
 }