From 485d9c13dde05cd00ae77fddae8ab0ff11445dc0 Mon Sep 17 00:00:00 2001 From: Matthew Hughes Date: Mon, 7 Sep 2026 20:00:26 +0100 Subject: [PATCH 1/3] refactor(chain): refactor chain parsing Some general code cleanup, things like: * Replace some `ref` patterns with `&` ones * Only pass down only required field to helper function * Add comments for span attribute --- src/chains.rs | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 45d9d488c76..0fa79cb85d8 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -175,6 +175,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, } @@ -421,7 +423,7 @@ impl Chain { prev_span_end: &mut BytePos, children: &mut Vec, ) { - let white_spaces: &[_] = &[' ', '\t']; + let white_spaces = &[' ', '\t']; if post_comment_snippet .trim_matches(white_spaces) .starts_with('\n') @@ -445,6 +447,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 +470,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(), + pre_comment.to_owned(), CommentPosition::Top, )); } - _ => (), } } @@ -509,7 +511,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 +521,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 { - match expr.expr.kind { - ast::ExprKind::MethodCall(ref call) => Some(SubExpr { + fn pop_expr_chain(expr: &ast::Expr, context: &RewriteContext<'_>) -> Option { + 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 +543,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(), } From a970750b1c63b3403ec1bcf8f907b6f291a2971d Mon Sep 17 00:00:00 2001 From: Matthew Hughes Date: Sat, 12 Sep 2026 22:19:45 +0100 Subject: [PATCH 2/3] refactor(chain): drop `CommentPosition` I found the naming in this type quite confusing: all it was tracking was "does this comment belong on the same line", so use a bool for this purpose. --- src/chains.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 0fa79cb85d8..8ffb9af18e5 100644 --- a/src/chains.rs +++ b/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, @@ -204,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 { @@ -341,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, } @@ -436,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(); } @@ -475,7 +472,7 @@ impl Chain { children.push(ChainItem::comment( pre_comment_span, pre_comment.to_owned(), - CommentPosition::Top, + false, )); } } @@ -838,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); From 066e0bc0af41a4498929f8400c172f59dc9183e7 Mon Sep 17 00:00:00 2001 From: Matthew Hughes Date: Sun, 13 Sep 2026 19:44:35 +0100 Subject: [PATCH 3/3] Add some more chain comment tests So that I can be confidence that my future refactors aren't changing some untested behaviour. --- tests/source/chain_yield_with_comments.rs | 8 ++++++ tests/source/chains_with_comment.rs | 26 ++++++++++++++++++ tests/target/chain_yield_with_comments.rs | 10 +++++++ tests/target/chains_with_comment.rs | 32 +++++++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 tests/source/chain_yield_with_comments.rs create mode 100644 tests/target/chain_yield_with_comments.rs diff --git a/tests/source/chain_yield_with_comments.rs b/tests/source/chain_yield_with_comments.rs new file mode 100644 index 00000000000..41e1db18edb --- /dev/null +++ b/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; +} diff --git a/tests/source/chains_with_comment.rs b/tests/source/chains_with_comment.rs index 91160711b89..19f959f5484 100644 --- a/tests/source/chains_with_comment.rs +++ b/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; } diff --git a/tests/target/chain_yield_with_comments.rs b/tests/target/chain_yield_with_comments.rs new file mode 100644 index 00000000000..1210759920f --- /dev/null +++ b/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; +} diff --git a/tests/target/chains_with_comment.rs b/tests/target/chains_with_comment.rs index 522d70713bc..00136f75db2 100644 --- a/tests/target/chains_with_comment.rs +++ b/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; }