From 547577fa5d309d90292ca3a58fef1bf0d9325cc0 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Mon, 7 Aug 2023 12:57:20 -0400 Subject: [PATCH] implement sinlge line let-chain rules for now, let-chains can only be formatted on a single line if the chain consits of 2 expressions where the first is an identifier proceeded by any number of unary operators and the second is a let-expr. --- src/expr.rs | 3 ++ src/pairs.rs | 41 ++++++++++++-- tests/source/let_chains.rs | 107 ++++++++++++++++++++++++++++++++++++- tests/target/let_chains.rs | 91 ++++++++++++++++++++++++++++++- 4 files changed, 236 insertions(+), 6 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index c15e680333b..25226991fbc 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1842,11 +1842,14 @@ fn rewrite_let( ) -> Option { let mut result = "let ".to_owned(); + // TODO(ytmimi) comments could appear between `let` and the `pat` + // 4 = "let ".len() let pat_shape = shape.offset_left(4)?; let pat_str = pat.rewrite(context, pat_shape)?; result.push_str(&pat_str); + // TODO(ytmimi) comments could appear between `pat` and `=` result.push_str(" ="); let comments_lo = context diff --git a/src/pairs.rs b/src/pairs.rs index 96f023b3b0e..9dac20d3699 100644 --- a/src/pairs.rs +++ b/src/pairs.rs @@ -42,9 +42,13 @@ pub(crate) fn rewrite_all_pairs( context: &RewriteContext<'_>, ) -> Option { expr.flatten(context, shape).and_then(|list| { - // First we try formatting on one line. - rewrite_pairs_one_line(&list, shape, context) - .or_else(|| rewrite_pairs_multiline(&list, shape, context)) + if list.let_chain_count() > 0 && !list.can_rewrite_let_chain_single_line() { + rewrite_pairs_multiline(&list, shape, context) + } else { + // First we try formatting on one line. + rewrite_pairs_one_line(&list, shape, context) + .or_else(|| rewrite_pairs_multiline(&list, shape, context)) + } }) } @@ -255,6 +259,37 @@ struct PairList<'a, 'b, T: Rewrite> { separators: Vec<&'a str>, } +fn is_ident(expr: &ast::Expr) -> bool { + match &expr.kind { + ast::ExprKind::Path(None, path) if path.segments.len() == 1 => true, + ast::ExprKind::Unary(_, expr) + | ast::ExprKind::AddrOf(_, _, expr) + | ast::ExprKind::Paren(expr) + | ast::ExprKind::Try(expr) => is_ident(expr), + _ => false, + } +} + +impl<'a, 'b> PairList<'a, 'b, ast::Expr> { + fn let_chain_count(&self) -> usize { + self.list + .iter() + .filter(|(expr, _)| matches!(expr.kind, ast::ExprKind::Let(_, _, _))) + .count() + } + + fn can_rewrite_let_chain_single_line(&self) -> bool { + if self.list.len() != 2 { + return false; + } + + let fist_item_is_ident = is_ident(self.list[0].0); + let second_item_is_let_chain = matches!(self.list[1].0.kind, ast::ExprKind::Let(_, _, _)); + + fist_item_is_ident && second_item_is_let_chain + } +} + impl FlattenPair for ast::Expr { fn flatten( &self, diff --git a/tests/source/let_chains.rs b/tests/source/let_chains.rs index f5f8285aa10..88ee126bfbd 100644 --- a/tests/source/let_chains.rs +++ b/tests/source/let_chains.rs @@ -13,4 +13,109 @@ fn main() { if let XXXXXXXXX { xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, yyyyyyyyyyyyy, zzzzzzzzzzzzz} = xxxxxxx() && let Foo = bar() { todo!() } -} \ No newline at end of file +} + +fn test_single_line_let_chain() { + // first item in let-chain is an ident + if a && let Some(b) = foo() { + } + + // first item in let-chain is a unary ! with an ident + let unary_not = if !from_hir_call + && let Some(p) = parent + { + }; + + // first item in let-chain is a unary * with an ident + let unary_deref = if *some_deref + && let Some(p) = parent + { + }; + + // first item in let-chain is a unary - (neg) with an ident + let unary_neg = if -some_ident + && let Some(p) = parent + { + }; + + // first item in let-chain is a try (?) with an ident + let try_ = if some_try? + && let Some(p) = parent + { + }; + + // first item in let-chain is an ident wrapped in parens + let in_parens = if (some_ident) + && let Some(p) = parent + { + }; + + // first item in let-chain is a ref & with an ident + let _ref = if &some_ref + && let Some(p) = parent + { + }; + + // first item in let-chain is a ref &mut with an ident + let mut_ref = if &mut some_ref + && let Some(p) = parent + { + }; + + // chain unary ref and try + let chain_of_unary_ref_and_try = if !&*some_ref? + && let Some(p) = parent { + }; +} + +fn test_multi_line_let_chain() { + // Can only single line the let-chain if the first item is an ident + if let Some(x) = y && a { + + } + + // More than one let-chain must be formatted on multiple lines + if let Some(x) = y && let Some(a) = b { + + } + + // The ident isn't long enough so we don't wrap the first let-chain + if a && let Some(x) = y && let Some(a) = b { + + } + + // The ident is long enough so both let-chains are wrapped + if aaa && let Some(x) = y && let Some(a) = b { + + } + + // function call + if a() && let Some(x) = y { + + } + + // bool literal + if true && let Some(x) = y { + + } + + // cast to a bool + if 1 as bool && let Some(x) = y { + + } + + // matches! macro call + if matches!(a, some_type) && let Some(x) = y { + + } + + // block expression returning bool + if { true } && let Some(x) = y { + + } + + // field access + if a.x && let Some(x) = y { + + } +} diff --git a/tests/target/let_chains.rs b/tests/target/let_chains.rs index 36fd0ba1590..165641521cf 100644 --- a/tests/target/let_chains.rs +++ b/tests/target/let_chains.rs @@ -1,5 +1,7 @@ fn main() { - if let x = x && x {} + if let x = x + && x + {} if xxx && let x = x {} @@ -12,7 +14,10 @@ fn main() { {} if aaaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaa - || aaaaaaaaa && let Some(x) = xxxxxxxxxxxx && aaaaaaa && let None = aaaaaaaaaa + || aaaaaaaaa + && let Some(x) = xxxxxxxxxxxx + && aaaaaaa + && let None = aaaaaaaaaa {} if let Some(Struct { x: TS(1, 2) }) = path::to::<_>(hehe) @@ -39,3 +44,85 @@ fn main() { todo!() } } + +fn test_single_line_let_chain() { + // first item in let-chain is an ident + if a && let Some(b) = foo() {} + + // first item in let-chain is a unary ! with an ident + let unary_not = if !from_hir_call && let Some(p) = parent {}; + + // first item in let-chain is a unary * with an ident + let unary_deref = if *some_deref && let Some(p) = parent {}; + + // first item in let-chain is a unary - (neg) with an ident + let unary_neg = if -some_ident && let Some(p) = parent {}; + + // first item in let-chain is a try (?) with an ident + let try_ = if some_try? && let Some(p) = parent {}; + + // first item in let-chain is an ident wrapped in parens + let in_parens = if (some_ident) && let Some(p) = parent {}; + + // first item in let-chain is a ref & with an ident + let _ref = if &some_ref && let Some(p) = parent {}; + + // first item in let-chain is a ref &mut with an ident + let mut_ref = if &mut some_ref && let Some(p) = parent {}; + + // chain unary ref and try + let chain_of_unary_ref_and_try = if !&*some_ref? && let Some(p) = parent {}; +} + +fn test_multi_line_let_chain() { + // Can only single line the let-chain if the first item is an ident + if let Some(x) = y + && a + {} + + // More than one let-chain must be formatted on multiple lines + if let Some(x) = y + && let Some(a) = b + {} + + // The ident isn't long enough so we don't wrap the first let-chain + if a && let Some(x) = y + && let Some(a) = b + {} + + // The ident is long enough so both let-chains are wrapped + if aaa + && let Some(x) = y + && let Some(a) = b + {} + + // function call + if a() + && let Some(x) = y + {} + + // bool literal + if true + && let Some(x) = y + {} + + // cast to a bool + if 1 as bool + && let Some(x) = y + {} + + // matches! macro call + if matches!(a, some_type) + && let Some(x) = y + {} + + // block expression returning bool + if { true } + && let Some(x) = y + {} + + // field access + if a.x + && let Some(x) = y + {} +}