From 275911d594e21f13de6e1c42a98171ab210bf359 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 17 Dec 2023 17:30:57 -0500 Subject: [PATCH] Add `let_chain_style` configuration option Now users have some control over how `let-chains` are formatted. The default value of `LegibleBindings` follows the style guide prescription defined in `r-l/rust 110568`. The `Tall` variant provides users an option to format all chain items on a single line if they fit. --- Configurations.md | 66 ++++++++++ src/config/mod.rs | 3 + src/config/options.rs | 14 ++ src/pairs.rs | 22 ++-- .../let_chain_style/legible_bindings.rs | 123 ++++++++++++++++++ .../let_chain_style/tall.rs} | 2 + .../let_chain_style/legible_bindings.rs} | 2 + tests/target/configs/let_chain_style/tall.rs | 108 +++++++++++++++ 8 files changed, 332 insertions(+), 8 deletions(-) create mode 100644 tests/source/configs/let_chain_style/legible_bindings.rs rename tests/source/{let_chains.rs => configs/let_chain_style/tall.rs} (98%) rename tests/target/{let_chains.rs => configs/let_chain_style/legible_bindings.rs} (98%) create mode 100644 tests/target/configs/let_chain_style/tall.rs diff --git a/Configurations.md b/Configurations.md index c9e908d3b47..7c152cf2f0e 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1128,6 +1128,72 @@ macro_rules! foo { See also [`format_macro_matchers`](#format_macro_matchers). +## `let_chain_style` + +Controls how rustfmt treats let-chains + +- **Default value**: `LegibleBindings` +- **Possible values**: `LegibleBindings`, `Tall` +- **Stability**: No (tracking issue: N/A) + +#### `LegibleBindings` (default): + +let-chain items are formatted on their own line to disambiguate the new bindings. +The let-chain may be arranged horizontally when the chain: +1. Only contains two items +2. The first item is an identifier +3. The second item is a let expressions. + +```rust +fn main() { + if let Some(x) = y + && a + {} + + if let Some(x) = y + && let Some(a) = b + {} + + if let Ok(name) = str::from_utf8(name) + && is_dyn_sym(name) + {} + + if condition() + && let Some(binding) = expr + && condition2(binding) + && condition3() + && let Some(binding2) = expr2 + && condition4(binding, binding2) + { + body(); + } +} +``` + +#### `Tall`: + +let-chain items are placed horizontally when there is sufficient space, otherwise the chain is formatted vertically. + +```rust +fn main() { + if let Some(x) = y && a {} + + if let Some(x) = y && let Some(a) = b {} + + if let Ok(name) = str::from_utf8(name) && is_dyn_sym(name) {} + + if condition() + && let Some(binding) = expr + && condition2(binding) + && condition3() + && let Some(binding2) = expr2 + && condition4(binding, binding2) + { + body(); + } +} +``` + ## `skip_macro_invocations` Skip formatting the bodies of macro invocations with the following names. diff --git a/src/config/mod.rs b/src/config/mod.rs index f1474fd3e61..7fe04b5766b 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -151,6 +151,8 @@ create_config! { "Write an item and its attribute on the same line \ if their combined width is below a threshold"; format_generated_files: bool, true, false, "Format generated files"; + let_chain_style: LetChainStyle, LetChainStyle::LegibleBindings, false, "Controls how rustfmt \ + lays out let-chains"; // Options that can change the source code beyond whitespace/blocks (somewhat linty things) merge_derives: bool, true, true, "Merge multiple `#[derive(...)]` into a single one"; @@ -680,6 +682,7 @@ edition = "2015" version = "One" inline_attribute_width = 0 format_generated_files = true +let_chain_style = "LegibleBindings" merge_derives = true use_try_shorthand = false use_field_init_shorthand = false diff --git a/src/config/options.rs b/src/config/options.rs index e7fd2cfbd31..5e12444bc42 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -494,3 +494,17 @@ pub enum StyleEdition { /// [Edition 2024](). Edition2024, } + +/// Controls how rustfmt treats let-chains +#[config_type] +pub enum LetChainStyle { + /// let-chain items are formatted on their own line to disambiguate the new bindings. + /// The let-chain may be arranged horizontally when the chain: + /// 1. Only contains two items + /// 2. The first item is an identifier + /// 3. The second item is a let expressions. + LegibleBindings, + /// let-chain items are placed horizontally when there is sufficient space, otherwise the chain + /// is formatted vertically. + Tall, +} diff --git a/src/pairs.rs b/src/pairs.rs index bfc2ffed383..ab43c22ed79 100644 --- a/src/pairs.rs +++ b/src/pairs.rs @@ -2,6 +2,7 @@ use rustc_ast::ast; use crate::config::lists::*; use crate::config::IndentStyle; +use crate::config::LetChainStyle; use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::Shape; use crate::utils::{ @@ -42,7 +43,7 @@ pub(crate) fn rewrite_all_pairs( context: &RewriteContext<'_>, ) -> Option { expr.flatten(context, shape).and_then(|list| { - if list.let_chain_count() > 0 && !list.can_rewrite_let_chain_single_line() { + if list.let_chain_count() > 0 && !list.can_rewrite_let_chain_single_line(context) { rewrite_pairs_multiline(&list, shape, context) } else { // First we try formatting on one line. @@ -278,15 +279,20 @@ impl<'a, 'b> PairList<'a, 'b, ast::Expr> { .count() } - fn can_rewrite_let_chain_single_line(&self) -> bool { - if self.list.len() != 2 { - return false; - } + fn can_rewrite_let_chain_single_line(&self, context: &RewriteContext<'_>) -> bool { + match context.config.let_chain_style() { + LetChainStyle::Tall => true, + LetChainStyle::LegibleBindings => { + 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(..)); + let fist_is_ident = is_ident(self.list[0].0); + let second_is_let_expr = matches!(self.list[1].0.kind, ast::ExprKind::Let(..)); - fist_item_is_ident && second_item_is_let_chain + fist_is_ident && second_is_let_expr + } + } } } diff --git a/tests/source/configs/let_chain_style/legible_bindings.rs b/tests/source/configs/let_chain_style/legible_bindings.rs new file mode 100644 index 00000000000..405014526e2 --- /dev/null +++ b/tests/source/configs/let_chain_style/legible_bindings.rs @@ -0,0 +1,123 @@ +// rustfmt-let_chain_style: LegibleBindings + +fn main() { + if let x = x && x {} + + if xxx && let x = x {} + + if aaaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaa && aaaaaaaaa && let Some(x) = xxxxxxxxxxxx && aaaaaaa && let None = aaaaaaaaaa {} + + if aaaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaa && aaaaaaaaa && let Some(x) = xxxxxxxxxxxx && aaaaaaa && let None = aaaaaaaaaa {} + + if let Some(Struct { x:TS(1,2) }) = path::to::<_>(hehe) + && let [Simple, people] = /* get ready */ create_universe(/* hi */ GreatPowers).initialize_badminton().populate_swamps() && + let everybody = (Loops { hi /*hi*/ , ..loopy() }) && summons::triumphantly() { todo!() } + + if let XXXXXXXXX { xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, yyyyyyyyyyyyy, zzzzzzzzzzzzz} = xxxxxxx() + && let Foo = bar() { 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 { + + } +} diff --git a/tests/source/let_chains.rs b/tests/source/configs/let_chain_style/tall.rs similarity index 98% rename from tests/source/let_chains.rs rename to tests/source/configs/let_chain_style/tall.rs index b7c1f811096..813483506ed 100644 --- a/tests/source/let_chains.rs +++ b/tests/source/configs/let_chain_style/tall.rs @@ -1,3 +1,5 @@ +// rustfmt-let_chain_style: Tall + fn main() { if let x = x && x {} diff --git a/tests/target/let_chains.rs b/tests/target/configs/let_chain_style/legible_bindings.rs similarity index 98% rename from tests/target/let_chains.rs rename to tests/target/configs/let_chain_style/legible_bindings.rs index 1ceecac8abc..99d35b17acd 100644 --- a/tests/target/let_chains.rs +++ b/tests/target/configs/let_chain_style/legible_bindings.rs @@ -1,3 +1,5 @@ +// rustfmt-let_chain_style: LegibleBindings + fn main() { if let x = x && x diff --git a/tests/target/configs/let_chain_style/tall.rs b/tests/target/configs/let_chain_style/tall.rs new file mode 100644 index 00000000000..e9e147f82a3 --- /dev/null +++ b/tests/target/configs/let_chain_style/tall.rs @@ -0,0 +1,108 @@ +// rustfmt-let_chain_style: Tall + +fn main() { + if let x = x && x {} + + if xxx && let x = x {} + + if aaaaaaaaaaaaaaaaaaaaa + && aaaaaaaaaaaaaaa + && aaaaaaaaa + && let Some(x) = xxxxxxxxxxxx + && aaaaaaa + && let None = aaaaaaaaaa + {} + + if aaaaaaaaaaaaaaaaaaaaa + && aaaaaaaaaaaaaaa + && aaaaaaaaa + && let Some(x) = xxxxxxxxxxxx + && aaaaaaa + && let None = aaaaaaaaaa + {} + + if let Some(Struct { x: TS(1, 2) }) = path::to::<_>(hehe) + && let [Simple, people] = /* get ready */ + create_universe(/* hi */ GreatPowers) + .initialize_badminton() + .populate_swamps() + && let everybody = (Loops { + hi, /*hi*/ + ..loopy() + }) + && summons::triumphantly() + { + todo!() + } + + if let XXXXXXXXX { + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + yyyyyyyyyyyyy, + zzzzzzzzzzzzz, + } = xxxxxxx() + && let Foo = bar() + { + 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 {} +}