From 7320daea4cac81602f6f75b0b446461fdc921710 Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Thu, 15 Jul 2021 19:31:25 +0530 Subject: [PATCH 1/8] refactor: soft deprecate match_arm_blocks in favour of match_arm_wrapping (#4896) Map `match_arm_blocks` option as `match_arm_wrapping` variants. --- src/config/config_type.rs | 24 +++++++++++++++++++++-- src/config/mod.rs | 11 +++++++++-- src/config/options.rs | 9 +++++++++ src/matches.rs | 40 +++++++++++++++++++++------------------ 4 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 7fc4486ddcd..5db0352107a 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -106,6 +106,7 @@ macro_rules! create_config { | "chain_width" => self.0.set_heuristics(), "license_template_path" => self.0.set_license_template(), "merge_imports" => self.0.set_merge_imports(), + "match_arm_blocks" => self.0.set_match_arm_blocks(), &_ => (), } } @@ -166,6 +167,7 @@ macro_rules! create_config { self.set_license_template(); self.set_ignore(dir); self.set_merge_imports(); + self.set_match_arm_blocks(); self } @@ -249,14 +251,16 @@ macro_rules! create_config { | "chain_width" => self.set_heuristics(), "license_template_path" => self.set_license_template(), "merge_imports" => self.set_merge_imports(), + "match_arm_blocks" => self.set_match_arm_blocks(), &_ => (), } } #[allow(unreachable_pub)] pub fn is_hidden_option(name: &str) -> bool { - const HIDE_OPTIONS: [&str; 5] = - ["verbose", "verbose_diff", "file_lines", "width_heuristics", "merge_imports"]; + const HIDE_OPTIONS: [&str; 6] = + ["verbose", "verbose_diff", "file_lines", "width_heuristics", "merge_imports", + "match_arm_blocks"]; HIDE_OPTIONS.contains(&name) } @@ -421,6 +425,22 @@ macro_rules! create_config { } } + fn set_match_arm_blocks(&mut self) { + if self.was_set().match_arm_blocks() { + eprintln!( + "Warning: the `match_arm_blocks` option is deprecated. \ + Use `match_arm_wrapping` instead" + ); + if !self.was_set().match_arm_wrapping() { + self.match_arm_wrapping.2 = if self.match_arm_blocks() { + MatchArmWrapping::Default + } else { + MatchArmWrapping::NoBlockFirstLine + }; + } + } + } + #[allow(unreachable_pub)] /// Returns `true` if the config key was explicitly set and is the default value. pub fn is_default(&self, key: &str) -> bool { diff --git a/src/config/mod.rs b/src/config/mod.rs index 8c04363b1fd..7c38f515f22 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -110,8 +110,9 @@ create_config! { "Align struct fields if their diffs fits within threshold"; enum_discrim_align_threshold: usize, 0, false, "Align enum variants discrims, if their diffs fit within threshold"; - match_arm_blocks: bool, true, false, "Wrap the body of arms in blocks when it does not fit on \ - the same line with the pattern of arms"; + match_arm_wrapping: MatchArmWrapping, MatchArmWrapping::Default, false, + "Wrap the body of match arms according to the given options"; + match_arm_blocks: bool, true, false, "(deprecated: use match_arm_wrapping instead)"; match_arm_leading_pipes: MatchArmLeadingPipe, MatchArmLeadingPipe::Never, true, "Determines whether leading pipes are emitted on match arms"; force_multiline_blocks: bool, false, false, @@ -426,6 +427,11 @@ mod test { "Merge imports"; merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)"; + // match_arm_blocks deprecation + match_arm_blocks: bool, true, false, "(deprecated: use match_arm_wrapping instead)"; + match_arm_wrapping: MatchArmWrapping, MatchArmWrapping::Default, false, + "Wrap the body of match arms according to the given options"; + // Width Heuristics use_small_heuristics: Heuristics, Heuristics::Default, true, "Whether to use different formatting for items and \ @@ -590,6 +596,7 @@ combine_control_expr = true overflow_delimited_expr = false struct_field_align_threshold = 0 enum_discrim_align_threshold = 0 +match_arm_wrapping = "Default" match_arm_blocks = true match_arm_leading_pipes = "Never" force_multiline_blocks = false diff --git a/src/config/options.rs b/src/config/options.rs index 3b91021813c..6ab31eb33f7 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -442,3 +442,12 @@ pub enum MatchArmLeadingPipe { /// Preserve any existing leading pipes Preserve, } + +/// Controls wrapping for match arm bodies +#[config_type] +pub enum MatchArmWrapping { + /// Follow the Style Guide Prescription + Default, + /// Don't block wrap when the first line can't fit + NoBlockFirstLine, +} diff --git a/src/matches.rs b/src/matches.rs index 140ec226c02..686729aba1d 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -7,7 +7,9 @@ use rustc_span::{BytePos, Span}; use crate::comment::{combine_strs_with_missing_comments, rewrite_comment}; use crate::config::lists::*; -use crate::config::{Config, ControlBraceStyle, IndentStyle, MatchArmLeadingPipe, Version}; +use crate::config::{ + Config, ControlBraceStyle, IndentStyle, MatchArmLeadingPipe, MatchArmWrapping, Version, +}; use crate::expr::{ format_expr, is_empty_block, is_simple_block, is_unsafe_block, prefer_next_line, rewrite_cond, ExprType, RhsTactics, @@ -413,26 +415,28 @@ fn rewrite_match_body( } let indent_str = shape.indent.to_string_with_newline(context.config); - let (body_prefix, body_suffix) = - if context.config.match_arm_blocks() && !context.inside_macro() { - let comma = if context.config.match_block_trailing_comma() { - "," + let (body_prefix, body_suffix) = if context.config.match_arm_wrapping() + == MatchArmWrapping::Default + && !context.inside_macro() + { + let comma = if context.config.match_block_trailing_comma() { + "," + } else { + "" + }; + let semicolon = if context.config.version() == Version::One { + "" + } else { + if semicolon_for_expr(context, body) { + ";" } else { "" - }; - let semicolon = if context.config.version() == Version::One { - "" - } else { - if semicolon_for_expr(context, body) { - ";" - } else { - "" - } - }; - ("{", format!("{}{}}}{}", semicolon, indent_str, comma)) - } else { - ("", String::from(",")) + } }; + ("{", format!("{}{}}}{}", semicolon, indent_str, comma)) + } else { + ("", String::from(",")) + }; let block_sep = match context.config.control_brace_style() { ControlBraceStyle::AlwaysNextLine => format!("{}{}", alt_block_sep, body_prefix), From 5c63ecdcb4c2562df9b0c5b906bf0d006981c6f5 Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Thu, 15 Jul 2021 19:51:43 +0530 Subject: [PATCH 2/8] test: add tests for match_arm_blocks deprecation --- src/config/mod.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/config/mod.rs b/src/config/mod.rs index 7c38f515f22..fd3c27237ca 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -730,6 +730,74 @@ make_backup = false } } + #[cfg(test)] + mod deprecated_option_match_arm_blocks { + use super::*; + + #[test] + fn test_old_option_set() { + if !crate::is_nightly_channel!() { + return; + } + // Old option defaults to true - set it to false + let toml = r#" + unstable_features = true + match_arm_blocks = false + "#; + let config = Config::from_toml(toml, Path::new("")).unwrap(); + assert_eq!( + config.match_arm_wrapping(), + MatchArmWrapping::NoBlockFirstLine + ); + } + + #[test] + fn test_both_set() { + if !crate::is_nightly_channel!() { + return; + } + let toml = r#" + unstable_features = true + match_arm_blocks = false + match_arm_wrapping = "Default" + "#; + let config = Config::from_toml(toml, Path::new("")).unwrap(); + assert_eq!(config.match_arm_wrapping(), MatchArmWrapping::Default); + } + + #[test] + fn test_new_overridden() { + if !crate::is_nightly_channel!() { + return; + } + let toml = r#" + unstable_features = true + merge_imports = false + "#; + let mut config = Config::from_toml(toml, Path::new("")).unwrap(); + config.override_value("match_arm_wrapping", "Default"); + assert_eq!(config.match_arm_wrapping(), MatchArmWrapping::Default); + } + + #[test] + fn test_old_overridden() { + if !crate::is_nightly_channel!() { + return; + } + let toml = r#" + unstable_features = true + match_arm_wrapping = "NoBlockFirstLine" + "#; + let mut config = Config::from_toml(toml, Path::new("")).unwrap(); + config.override_value("match_arm_blocks", "false"); + // no effect: the new option always takes precedence + assert_eq!( + config.match_arm_wrapping(), + MatchArmWrapping::NoBlockFirstLine + ); + } + } + #[cfg(test)] mod use_small_heuristics { use super::*; From 77193eb89f5cfafcf19a5730a95d8aea08dd1b64 Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Tue, 20 Jul 2021 15:18:16 +0530 Subject: [PATCH 3/8] test: add tests for match_arm_wrapping --- tests/source/configs/match_arm_wrapping/default.rs | 11 +++++++++++ .../match_arm_wrapping/no_block_first_line.rs | 11 +++++++++++ tests/target/configs/match_arm_wrapping/default.rs | 13 +++++++++++++ .../match_arm_wrapping/no_block_first_line.rs | 12 ++++++++++++ 4 files changed, 47 insertions(+) create mode 100644 tests/source/configs/match_arm_wrapping/default.rs create mode 100644 tests/source/configs/match_arm_wrapping/no_block_first_line.rs create mode 100644 tests/target/configs/match_arm_wrapping/default.rs create mode 100644 tests/target/configs/match_arm_wrapping/no_block_first_line.rs diff --git a/tests/source/configs/match_arm_wrapping/default.rs b/tests/source/configs/match_arm_wrapping/default.rs new file mode 100644 index 00000000000..03f29d373bc --- /dev/null +++ b/tests/source/configs/match_arm_wrapping/default.rs @@ -0,0 +1,11 @@ +// rustfmt-match_arm_wrapping: Default +// Wrap match-arms + +fn main() { + match lorem { + true => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + false => { + println!("{}", sit) + } + } +} diff --git a/tests/source/configs/match_arm_wrapping/no_block_first_line.rs b/tests/source/configs/match_arm_wrapping/no_block_first_line.rs new file mode 100644 index 00000000000..25383169106 --- /dev/null +++ b/tests/source/configs/match_arm_wrapping/no_block_first_line.rs @@ -0,0 +1,11 @@ +// rustfmt-match_arm_wrapping: NoBlockFirstLine +// Wrap match-arms + +fn main() { + match lorem { + true => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + false => { + println!("{}", sit) + } + } +} diff --git a/tests/target/configs/match_arm_wrapping/default.rs b/tests/target/configs/match_arm_wrapping/default.rs new file mode 100644 index 00000000000..c1812ca1fb7 --- /dev/null +++ b/tests/target/configs/match_arm_wrapping/default.rs @@ -0,0 +1,13 @@ +// rustfmt-match_arm_wrapping: Default +// Wrap match-arms + +fn main() { + match lorem { + true => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + false => { + println!("{}", sit) + } + } +} diff --git a/tests/target/configs/match_arm_wrapping/no_block_first_line.rs b/tests/target/configs/match_arm_wrapping/no_block_first_line.rs new file mode 100644 index 00000000000..6d6aeb50539 --- /dev/null +++ b/tests/target/configs/match_arm_wrapping/no_block_first_line.rs @@ -0,0 +1,12 @@ +// rustfmt-match_arm_wrapping: NoBlockFirstLine +// Wrap match-arms + +fn main() { + match lorem { + true => + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + false => { + println!("{}", sit) + } + } +} From 14a4d02f876b4cb06c407244c3fcc7af5ea0ebd9 Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Wed, 21 Jul 2021 15:05:00 +0530 Subject: [PATCH 4/8] feat: add Always option to match_arm_wrapping, with tests (#4896) --- src/config/options.rs | 2 + src/matches.rs | 41 ++++++++++--------- .../configs/match_arm_wrapping/always.rs | 18 ++++++++ .../configs/match_arm_wrapping/always.rs | 24 +++++++++++ 4 files changed, 65 insertions(+), 20 deletions(-) create mode 100644 tests/source/configs/match_arm_wrapping/always.rs create mode 100644 tests/target/configs/match_arm_wrapping/always.rs diff --git a/src/config/options.rs b/src/config/options.rs index 6ab31eb33f7..10c49977ef6 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -450,4 +450,6 @@ pub enum MatchArmWrapping { Default, /// Don't block wrap when the first line can't fit NoBlockFirstLine, + /// Always wrap match arms + Always, } diff --git a/src/matches.rs b/src/matches.rs index 686729aba1d..072c436def8 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -415,27 +415,23 @@ fn rewrite_match_body( } let indent_str = shape.indent.to_string_with_newline(context.config); - let (body_prefix, body_suffix) = if context.config.match_arm_wrapping() - == MatchArmWrapping::Default - && !context.inside_macro() - { - let comma = if context.config.match_block_trailing_comma() { - "," - } else { - "" - }; - let semicolon = if context.config.version() == Version::One { - "" - } else { - if semicolon_for_expr(context, body) { - ";" + let (body_prefix, body_suffix) = match context.config.match_arm_wrapping() { + MatchArmWrapping::Default | MatchArmWrapping::Always if !context.inside_macro() => { + let comma = if context.config.match_block_trailing_comma() { + "," } else { "" - } - }; - ("{", format!("{}{}}}{}", semicolon, indent_str, comma)) - } else { - ("", String::from(",")) + }; + + let semicolon = match context.config.version() { + Version::One => "", + _ if semicolon_for_expr(context, body) => ";", + _ => "", + }; + + ("{", format!("{}{}}}{}", semicolon, indent_str, comma)) + } + _ => ("", String::from(",")), }; let block_sep = match context.config.control_brace_style() { @@ -464,7 +460,10 @@ fn rewrite_match_body( let orig_body_shape = shape .offset_left(extra_offset(pats_str, shape) + 4) .and_then(|shape| shape.sub_width(comma.len())); - let orig_body = if forbid_same_line || !arrow_comment.is_empty() { + let orig_body = if forbid_same_line + || !arrow_comment.is_empty() + || (!is_block && context.config.match_arm_wrapping() == MatchArmWrapping::Always) + { None } else if let Some(body_shape) = orig_body_shape { let rewrite = nop_block_collapse( @@ -485,6 +484,7 @@ fn rewrite_match_body( } else { None }; + let orig_budget = orig_body_shape.map_or(0, |shape| shape.width); // Try putting body on the next line and see if it looks better. @@ -493,6 +493,7 @@ fn rewrite_match_body( format_expr(body, ExprType::Statement, context, next_line_body_shape), next_line_body_shape.width, ); + match (orig_body, next_line_body) { (Some(ref orig_str), Some(ref next_line_str)) if prefer_next_line(orig_str, next_line_str, RhsTactics::Default) => diff --git a/tests/source/configs/match_arm_wrapping/always.rs b/tests/source/configs/match_arm_wrapping/always.rs new file mode 100644 index 00000000000..ac51e4cef88 --- /dev/null +++ b/tests/source/configs/match_arm_wrapping/always.rs @@ -0,0 +1,18 @@ +// rustfmt-match_arm_wrapping: Always +// Wrap match-arms + +fn main() { + match lorem { + 1 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2 => { + println!("{}", sit) + } + 3 => panic!(), + 4 => (), + y => { + // Some comment + let ipsum = y - 1; + println!("{}", ipsum); + } + } +} diff --git a/tests/target/configs/match_arm_wrapping/always.rs b/tests/target/configs/match_arm_wrapping/always.rs new file mode 100644 index 00000000000..7f410121698 --- /dev/null +++ b/tests/target/configs/match_arm_wrapping/always.rs @@ -0,0 +1,24 @@ +// rustfmt-match_arm_wrapping: Always +// Wrap match-arms + +fn main() { + match lorem { + 1 => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + 2 => { + println!("{}", sit) + } + 3 => { + panic!() + } + 4 => { + () + } + y => { + // Some comment + let ipsum = y - 1; + println!("{}", ipsum); + } + } +} From 1ee0c72f2c2b25cb0595dc6c3c6a97c2bce914e3 Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Thu, 22 Jul 2021 01:17:06 +0530 Subject: [PATCH 5/8] feat: add Preserve option to match_arm_wrapping (#4896) --- src/config/options.rs | 7 ++++-- src/matches.rs | 7 +++++- .../configs/match_arm_wrapping/always.rs | 14 ++++++------ .../configs/match_arm_wrapping/preserve.rs | 20 +++++++++++++++++ .../configs/match_arm_wrapping/always.rs | 14 ++++++------ .../configs/match_arm_wrapping/preserve.rs | 22 +++++++++++++++++++ 6 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 tests/source/configs/match_arm_wrapping/preserve.rs create mode 100644 tests/target/configs/match_arm_wrapping/preserve.rs diff --git a/src/config/options.rs b/src/config/options.rs index 10c49977ef6..d58c8ed0f3b 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -448,8 +448,11 @@ pub enum MatchArmLeadingPipe { pub enum MatchArmWrapping { /// Follow the Style Guide Prescription Default, - /// Don't block wrap when the first line can't fit + /// Same as Default, except don't block wrap match arms when the opening line of its body + /// can't fit on the same line as the `=>`. NoBlockFirstLine, - /// Always wrap match arms + /// Always block wrap match arms Always, + /// Preserve the block wrapping on match arms + Preserve, } diff --git a/src/matches.rs b/src/matches.rs index 072c436def8..f9f2b5b0b07 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -326,6 +326,10 @@ fn flatten_arm_body<'a>( if let ast::ExprKind::Block(..) = expr.kind { flatten_arm_body(context, expr, None) } else { + if context.config.match_arm_wrapping() == MatchArmWrapping::Preserve { + return (false, body); + } + let cond_becomes_muti_line = opt_shape .and_then(|shape| rewrite_cond(context, expr, shape)) .map_or(false, |cond| cond.contains('\n')); @@ -416,7 +420,8 @@ fn rewrite_match_body( let indent_str = shape.indent.to_string_with_newline(context.config); let (body_prefix, body_suffix) = match context.config.match_arm_wrapping() { - MatchArmWrapping::Default | MatchArmWrapping::Always if !context.inside_macro() => { + MatchArmWrapping::NoBlockFirstLine => ("", String::from(",")), + _ if !context.inside_macro() => { let comma = if context.config.match_block_trailing_comma() { "," } else { diff --git a/tests/source/configs/match_arm_wrapping/always.rs b/tests/source/configs/match_arm_wrapping/always.rs index ac51e4cef88..debb491f8b1 100644 --- a/tests/source/configs/match_arm_wrapping/always.rs +++ b/tests/source/configs/match_arm_wrapping/always.rs @@ -3,16 +3,16 @@ fn main() { match lorem { - 1 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), - 2 => { + 1000 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { println!("{}", sit) } - 3 => panic!(), - 4 => (), - y => { + 3000 => panic!(), + 4000 => (), + ipsum => { // Some comment - let ipsum = y - 1; - println!("{}", ipsum); + let dolor = ipsum % 2; + println!("{}", dolor); } } } diff --git a/tests/source/configs/match_arm_wrapping/preserve.rs b/tests/source/configs/match_arm_wrapping/preserve.rs new file mode 100644 index 00000000000..c1c387b3622 --- /dev/null +++ b/tests/source/configs/match_arm_wrapping/preserve.rs @@ -0,0 +1,20 @@ +// rustfmt-match_arm_wrapping: Preserve +// Wrap match-arms + +fn main() { + match lorem { + 1000 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => { + () + } + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } + } +} diff --git a/tests/target/configs/match_arm_wrapping/always.rs b/tests/target/configs/match_arm_wrapping/always.rs index 7f410121698..571149692e9 100644 --- a/tests/target/configs/match_arm_wrapping/always.rs +++ b/tests/target/configs/match_arm_wrapping/always.rs @@ -3,22 +3,22 @@ fn main() { match lorem { - 1 => { + 1000 => { foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) } - 2 => { + 2000 => { println!("{}", sit) } - 3 => { + 3000 => { panic!() } - 4 => { + 4000 => { () } - y => { + ipsum => { // Some comment - let ipsum = y - 1; - println!("{}", ipsum); + let dolor = ipsum % 2; + println!("{}", dolor); } } } diff --git a/tests/target/configs/match_arm_wrapping/preserve.rs b/tests/target/configs/match_arm_wrapping/preserve.rs new file mode 100644 index 00000000000..75a5b1a9be3 --- /dev/null +++ b/tests/target/configs/match_arm_wrapping/preserve.rs @@ -0,0 +1,22 @@ +// rustfmt-match_arm_wrapping: Preserve +// Wrap match-arms + +fn main() { + match lorem { + 1000 => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => { + () + } + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } + } +} From db129434bbf8fec5b27f38853a190665aaf5943c Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Fri, 23 Jul 2021 03:37:58 +0530 Subject: [PATCH 6/8] feat: add FitEntireBody as new match_arm_wrapping option, and rename NoBlockFirstLine to FitFirstLine (#4896) --- src/config/config_type.rs | 2 +- src/config/mod.rs | 12 +++--------- src/config/options.rs | 5 ++++- src/matches.rs | 6 ++++-- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 5db0352107a..08134322439 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -435,7 +435,7 @@ macro_rules! create_config { self.match_arm_wrapping.2 = if self.match_arm_blocks() { MatchArmWrapping::Default } else { - MatchArmWrapping::NoBlockFirstLine + MatchArmWrapping::FitFirstLine }; } } diff --git a/src/config/mod.rs b/src/config/mod.rs index fd3c27237ca..96469bd6d56 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -745,10 +745,7 @@ make_backup = false match_arm_blocks = false "#; let config = Config::from_toml(toml, Path::new("")).unwrap(); - assert_eq!( - config.match_arm_wrapping(), - MatchArmWrapping::NoBlockFirstLine - ); + assert_eq!(config.match_arm_wrapping(), MatchArmWrapping::FitFirstLine); } #[test] @@ -786,15 +783,12 @@ make_backup = false } let toml = r#" unstable_features = true - match_arm_wrapping = "NoBlockFirstLine" + match_arm_wrapping = "FitFirstLine" "#; let mut config = Config::from_toml(toml, Path::new("")).unwrap(); config.override_value("match_arm_blocks", "false"); // no effect: the new option always takes precedence - assert_eq!( - config.match_arm_wrapping(), - MatchArmWrapping::NoBlockFirstLine - ); + assert_eq!(config.match_arm_wrapping(), MatchArmWrapping::FitFirstLine); } } diff --git a/src/config/options.rs b/src/config/options.rs index d58c8ed0f3b..ca80834c19a 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -450,9 +450,12 @@ pub enum MatchArmWrapping { Default, /// Same as Default, except don't block wrap match arms when the opening line of its body /// can't fit on the same line as the `=>`. - NoBlockFirstLine, + FitFirstLine, /// Always block wrap match arms Always, /// Preserve the block wrapping on match arms Preserve, + /// Same as Default, except wrap the match arm if the entire body cannot fit on the same line + /// as the `=>`. + FitEntireBody, } diff --git a/src/matches.rs b/src/matches.rs index f9f2b5b0b07..5a994f353d1 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -420,7 +420,7 @@ fn rewrite_match_body( let indent_str = shape.indent.to_string_with_newline(context.config); let (body_prefix, body_suffix) = match context.config.match_arm_wrapping() { - MatchArmWrapping::NoBlockFirstLine => ("", String::from(",")), + MatchArmWrapping::FitFirstLine => ("", String::from(",")), _ if !context.inside_macro() => { let comma = if context.config.match_block_trailing_comma() { "," @@ -501,7 +501,9 @@ fn rewrite_match_body( match (orig_body, next_line_body) { (Some(ref orig_str), Some(ref next_line_str)) - if prefer_next_line(orig_str, next_line_str, RhsTactics::Default) => + if prefer_next_line(orig_str, next_line_str, RhsTactics::Default) + || (context.config.match_arm_wrapping() == MatchArmWrapping::FitEntireBody + && orig_str.contains('\n')) => { combine_next_line_body(next_line_str) } From 310023b0402863d3db3869a1f3fe0416c4d873b5 Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Fri, 23 Jul 2021 03:39:04 +0530 Subject: [PATCH 7/8] test: add tests for FitEntireBody, and make the tests for match_arm_wrapping consistent --- .../configs/match_arm_wrapping/always.rs | 5 ++- .../configs/match_arm_wrapping/default.rs | 14 +++++++-- .../match_arm_wrapping/fit_entire_body.rs | 21 +++++++++++++ .../match_arm_wrapping/fit_first_line.rs | 21 +++++++++++++ .../match_arm_wrapping/no_block_first_line.rs | 11 ------- .../configs/match_arm_wrapping/preserve.rs | 1 + .../configs/match_arm_wrapping/always.rs | 11 +++++++ .../configs/match_arm_wrapping/default.rs | 20 ++++++++++-- .../match_arm_wrapping/fit_entire_body.rs | 31 +++++++++++++++++++ .../match_arm_wrapping/fit_first_line.rs | 28 +++++++++++++++++ .../match_arm_wrapping/no_block_first_line.rs | 12 ------- .../configs/match_arm_wrapping/preserve.rs | 9 ++++++ 12 files changed, 156 insertions(+), 28 deletions(-) create mode 100644 tests/source/configs/match_arm_wrapping/fit_entire_body.rs create mode 100644 tests/source/configs/match_arm_wrapping/fit_first_line.rs delete mode 100644 tests/source/configs/match_arm_wrapping/no_block_first_line.rs create mode 100644 tests/target/configs/match_arm_wrapping/fit_entire_body.rs create mode 100644 tests/target/configs/match_arm_wrapping/fit_first_line.rs delete mode 100644 tests/target/configs/match_arm_wrapping/no_block_first_line.rs diff --git a/tests/source/configs/match_arm_wrapping/always.rs b/tests/source/configs/match_arm_wrapping/always.rs index debb491f8b1..318e963164f 100644 --- a/tests/source/configs/match_arm_wrapping/always.rs +++ b/tests/source/configs/match_arm_wrapping/always.rs @@ -8,7 +8,10 @@ fn main() { println!("{}", sit) } 3000 => panic!(), - 4000 => (), + 4000 => { + () + } + 5000 => this.a_very_long_function_name(foo, bar, bazz, fizz, another_argument, some_more_arguments, which_dont_fit), ipsum => { // Some comment let dolor = ipsum % 2; diff --git a/tests/source/configs/match_arm_wrapping/default.rs b/tests/source/configs/match_arm_wrapping/default.rs index 03f29d373bc..d004cc75f0e 100644 --- a/tests/source/configs/match_arm_wrapping/default.rs +++ b/tests/source/configs/match_arm_wrapping/default.rs @@ -3,9 +3,19 @@ fn main() { match lorem { - true => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), - false => { + 1000 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { println!("{}", sit) } + 3000 => panic!(), + 4000 => { + () + } + 5000 => this.a_very_long_function_name(foo, bar, bazz, fizz, another_argument, some_more_arguments, which_dont_fit), + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } } } diff --git a/tests/source/configs/match_arm_wrapping/fit_entire_body.rs b/tests/source/configs/match_arm_wrapping/fit_entire_body.rs new file mode 100644 index 00000000000..91f8da4ae22 --- /dev/null +++ b/tests/source/configs/match_arm_wrapping/fit_entire_body.rs @@ -0,0 +1,21 @@ +// rustfmt-match_arm_wrapping: FitEntireBody +// Wrap match-arms + +fn main() { + match lorem { + 1000 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => { + () + } + 5000 => this.a_very_long_function_name(foo, bar, bazz, fizz, another_argument, some_more_arguments, which_dont_fit), + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } + } +} diff --git a/tests/source/configs/match_arm_wrapping/fit_first_line.rs b/tests/source/configs/match_arm_wrapping/fit_first_line.rs new file mode 100644 index 00000000000..5142f73bdd8 --- /dev/null +++ b/tests/source/configs/match_arm_wrapping/fit_first_line.rs @@ -0,0 +1,21 @@ +// rustfmt-match_arm_wrapping: FitFirstLine +// Wrap match-arms + +fn main() { + match lorem { + 1000 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => { + () + } + 5000 => this.a_very_long_function_name(foo, bar, bazz, fizz, another_argument, some_more_arguments, which_dont_fit), + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } + } +} diff --git a/tests/source/configs/match_arm_wrapping/no_block_first_line.rs b/tests/source/configs/match_arm_wrapping/no_block_first_line.rs deleted file mode 100644 index 25383169106..00000000000 --- a/tests/source/configs/match_arm_wrapping/no_block_first_line.rs +++ /dev/null @@ -1,11 +0,0 @@ -// rustfmt-match_arm_wrapping: NoBlockFirstLine -// Wrap match-arms - -fn main() { - match lorem { - true => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), - false => { - println!("{}", sit) - } - } -} diff --git a/tests/source/configs/match_arm_wrapping/preserve.rs b/tests/source/configs/match_arm_wrapping/preserve.rs index c1c387b3622..642272c1619 100644 --- a/tests/source/configs/match_arm_wrapping/preserve.rs +++ b/tests/source/configs/match_arm_wrapping/preserve.rs @@ -11,6 +11,7 @@ fn main() { 4000 => { () } + 5000 => this.a_very_long_function_name(foo, bar, bazz, fizz, another_argument, some_more_arguments, which_dont_fit), ipsum => { // Some comment let dolor = ipsum % 2; diff --git a/tests/target/configs/match_arm_wrapping/always.rs b/tests/target/configs/match_arm_wrapping/always.rs index 571149692e9..a7c56796f0f 100644 --- a/tests/target/configs/match_arm_wrapping/always.rs +++ b/tests/target/configs/match_arm_wrapping/always.rs @@ -15,6 +15,17 @@ fn main() { 4000 => { () } + 5000 => { + this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ) + } ipsum => { // Some comment let dolor = ipsum % 2; diff --git a/tests/target/configs/match_arm_wrapping/default.rs b/tests/target/configs/match_arm_wrapping/default.rs index c1812ca1fb7..7867da97f06 100644 --- a/tests/target/configs/match_arm_wrapping/default.rs +++ b/tests/target/configs/match_arm_wrapping/default.rs @@ -3,11 +3,27 @@ fn main() { match lorem { - true => { + 1000 => { foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) } - false => { + 2000 => { println!("{}", sit) } + 3000 => panic!(), + 4000 => (), + 5000 => this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ), + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } } } diff --git a/tests/target/configs/match_arm_wrapping/fit_entire_body.rs b/tests/target/configs/match_arm_wrapping/fit_entire_body.rs new file mode 100644 index 00000000000..e9ce3944887 --- /dev/null +++ b/tests/target/configs/match_arm_wrapping/fit_entire_body.rs @@ -0,0 +1,31 @@ +// rustfmt-match_arm_wrapping: FitEntireBody +// Wrap match-arms + +fn main() { + match lorem { + 1000 => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => (), + 5000 => { + this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ) + } + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } + } +} diff --git a/tests/target/configs/match_arm_wrapping/fit_first_line.rs b/tests/target/configs/match_arm_wrapping/fit_first_line.rs new file mode 100644 index 00000000000..ceebc7371da --- /dev/null +++ b/tests/target/configs/match_arm_wrapping/fit_first_line.rs @@ -0,0 +1,28 @@ +// rustfmt-match_arm_wrapping: FitFirstLine +// Wrap match-arms + +fn main() { + match lorem { + 1000 => + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => (), + 5000 => this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ), + ipsum => { + // Some comment + let dolor = ipsum % 2; + println!("{}", dolor); + } + } +} diff --git a/tests/target/configs/match_arm_wrapping/no_block_first_line.rs b/tests/target/configs/match_arm_wrapping/no_block_first_line.rs deleted file mode 100644 index 6d6aeb50539..00000000000 --- a/tests/target/configs/match_arm_wrapping/no_block_first_line.rs +++ /dev/null @@ -1,12 +0,0 @@ -// rustfmt-match_arm_wrapping: NoBlockFirstLine -// Wrap match-arms - -fn main() { - match lorem { - true => - foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), - false => { - println!("{}", sit) - } - } -} diff --git a/tests/target/configs/match_arm_wrapping/preserve.rs b/tests/target/configs/match_arm_wrapping/preserve.rs index 75a5b1a9be3..9a5199df2cf 100644 --- a/tests/target/configs/match_arm_wrapping/preserve.rs +++ b/tests/target/configs/match_arm_wrapping/preserve.rs @@ -13,6 +13,15 @@ fn main() { 4000 => { () } + 5000 => this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ), ipsum => { // Some comment let dolor = ipsum % 2; From 3a262bd9c9ab9af5ad79d28c52cfaa3085d0a3de Mon Sep 17 00:00:00 2001 From: Ashvin Arsakularatne Date: Wed, 28 Jul 2021 01:09:25 +0530 Subject: [PATCH 8/8] doc: add configuration snippets for match_arm_wrapping options --- Configurations.md | 179 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/Configurations.md b/Configurations.md index d2e5613eba9..c480703bff8 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1604,6 +1604,185 @@ fn foo() { } ``` +## `match_arm_wrapping` + +Controls when to block wrap match arm bodies. + +- **Default value**: `"Default"` +- **Possible values**: `"Default"`, `"FitFirstLine"`, `"FitEntireBody"`, `"Always"`, `"Preserve` +- **Stable**: No (tracking issue: #4896) + +### Example + +#### Original code + +```rust +#![rustfmt::skip] + +fn main() { + match lorem { + 1000 => foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => { + () + } + 5000 => this.a_very_long_function_name(foo, bar, bazz, fizz, another_argument, some_more_arguments, which_dont_fit), + } +} +``` + +#### `"Default"` (default): + +The default block wrapping settings, as described in the Style Guide. + +```rust +fn main() { + match lorem { + 1000 => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => (), + 5000 => this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ), + } +} +``` + +#### `"FitFirstLine"`: + +Same as the default, except don't block wrap match arms when the opening line of its body can't fit on the same line as the `=>`. + +```rust +fn main() { + match lorem { + 1000 => + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x), + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => (), + 5000 => this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ), + } +} +``` + +#### `"Always"`: + +Always block wrap match arm bodies. + +```rust +fn main() { + match lorem { + 1000 => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + 2000 => { + println!("{}", sit) + } + 3000 => { + panic!() + } + 4000 => { + () + } + 5000 => { + this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ) + } + } +} +``` + +#### `"Preserve"`: + +Preserve block wrapping on match arm bodies if the developer originally had the body wrapped. + +```rust +fn main() { + match lorem { + 1000 => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => { + () + } + 5000 => this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ), + } +} +``` + +#### `"FitEntireBody"`: + +Same as default, except block wrap the match arm if the entire body cannot fit on the same line as the `=>`. + +```rust +fn main() { + match lorem { + 1000 => { + foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(x) + } + 2000 => { + println!("{}", sit) + } + 3000 => panic!(), + 4000 => (), + 5000 => { + this.a_very_long_function_name( + foo, + bar, + bazz, + fizz, + another_argument, + some_more_arguments, + which_dont_fit, + ) + } + } +} +``` + ## `match_block_trailing_comma` Put a trailing comma after a block based match arm (non-block arms are not affected)