From 99f9475bef7ff3f85ace2c840fbb5981da3eabec Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 8 May 2022 17:23:04 -0400 Subject: [PATCH 1/2] Add `fn_call_layout` configuration option Closes 5218 The `fn_call_layout` was added to give users more control over how function calls are formatted. This change will only impact function calls, and will not have any affect on method calls. The values are identical to those for `fn_args_layout`, which is a stable option that controls how function declarations are laid out. --- Configurations.md | 69 ++++++++ src/attr.rs | 1 + src/chains.rs | 2 +- src/config/mod.rs | 3 + src/expr.rs | 15 +- src/items.rs | 1 + src/macros.rs | 1 + src/overflow.rs | 37 ++++- src/patterns.rs | 1 + src/types.rs | 1 + .../configs/fn_call_layout/compressed.rs | 67 ++++++++ tests/source/configs/fn_call_layout/tall.rs | 67 ++++++++ .../source/configs/fn_call_layout/vertical.rs | 67 ++++++++ .../configs/fn_call_layout/compressed.rs | 79 +++++++++ tests/target/configs/fn_call_layout/tall.rs | 135 ++++++++++++++++ .../target/configs/fn_call_layout/vertical.rs | 151 ++++++++++++++++++ 16 files changed, 693 insertions(+), 4 deletions(-) create mode 100644 tests/source/configs/fn_call_layout/compressed.rs create mode 100644 tests/source/configs/fn_call_layout/tall.rs create mode 100644 tests/source/configs/fn_call_layout/vertical.rs create mode 100644 tests/target/configs/fn_call_layout/compressed.rs create mode 100644 tests/target/configs/fn_call_layout/tall.rs create mode 100644 tests/target/configs/fn_call_layout/vertical.rs diff --git a/Configurations.md b/Configurations.md index e066553dc63..110c1805a18 100644 --- a/Configurations.md +++ b/Configurations.md @@ -756,6 +756,75 @@ trait Lorem { See also [`fn_params_layout`](#fn_params_layout) +## `fn_call_layout` + +Control the layout of arguments in function calls + +- **Default value**: `"Tall"` +- **Possible values**: `"Compressed"`, `"Tall"`, `"Vertical"` +- **Stable**: No (tracking issue: N/A) + +#### `"Tall"` (default): + +```rust +fn main() { + lorem(ipsum, dolor, sit, amet); + ipsum( + dolor, + sit, + amet, + consectetur, + adipiscing, + elit, + vivamus, + ipsum, + orci, + rhoncus, + vel, + imperdiet, + ); +} +``` + +#### `"Compressed"`: + +```rust +fn main() { + lorem(ipsum, dolor, sit, amet); + ipsum( + dolor, sit, amet, consectetur, adipiscing, elit, vivamus, ipsum, orci, rhoncus, vel, + imperdiet, + ); +} +``` + +#### `"Vertical"`: + +```rust +fn main() { + lorem( + ipsum, + dolor, + sit, + amet, + ); + ipsum( + dolor, + sit, + amet, + consectetur, + adipiscing, + elit, + vivamus, + ipsum, + orci, + rhoncus, + vel, + imperdiet, + ); +} +``` + ## `fn_call_width` Maximum width of the args of a function call before falling back to vertical formatting. diff --git a/src/attr.rs b/src/attr.rs index 41ba9a847e6..4d20acfcb18 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -309,6 +309,7 @@ impl Rewrite for ast::MetaItem { } else { SeparatorTactic::Never }), + None, )? } ast::MetaItemKind::NameValue(ref literal) => { diff --git a/src/chains.rs b/src/chains.rs index 5bccb22db4c..7ba8e097f0a 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -253,7 +253,7 @@ impl ChainItem { format!("::<{}>", type_list.join(", ")) }; let callee_str = format!(".{}{}", rewrite_ident(context, method_name), type_str); - rewrite_call(context, &callee_str, &args[1..], span, shape) + rewrite_call(context, &callee_str, &args[1..], None, span, shape) } } diff --git a/src/config/mod.rs b/src/config/mod.rs index 5492519f389..695d440325c 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -127,6 +127,8 @@ create_config! { "(deprecated: use fn_params_layout instead)"; fn_params_layout: Density, Density::Tall, true, "Control the layout of parameters in function signatures."; + fn_call_layout: Density, Density::Tall, false, + "Control the layout of arguments in a function call"; brace_style: BraceStyle, BraceStyle::SameLineWhere, false, "Brace style for items"; control_brace_style: ControlBraceStyle, ControlBraceStyle::AlwaysSameLine, false, "Brace style for control flow constructs"; @@ -654,6 +656,7 @@ match_arm_blocks = true match_arm_leading_pipes = "Never" force_multiline_blocks = false fn_params_layout = "Tall" +fn_call_layout = "Tall" brace_style = "SameLineWhere" control_brace_style = "AlwaysSameLine" trailing_semicolon = true diff --git a/src/expr.rs b/src/expr.rs index 13d068d0c2d..b1e54a89667 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -12,7 +12,7 @@ use crate::comment::{ combine_strs_with_missing_comments, contains_comment, recover_comment_removed, rewrite_comment, rewrite_missing_comment, CharClasses, FindUncommented, }; -use crate::config::lists::*; +use crate::config::{lists::*, Density}; use crate::config::{Config, ControlBraceStyle, HexLiteralCase, IndentStyle, Version}; use crate::lists::{ definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape, @@ -89,7 +89,15 @@ pub(crate) fn format_expr( ast::ExprKind::Call(ref callee, ref args) => { let inner_span = mk_sp(callee.span.hi(), expr.span.hi()); let callee_str = callee.rewrite(context, shape)?; - rewrite_call(context, &callee_str, args, inner_span, shape) + let force_list_tactic = Some(context.config.fn_call_layout()); + rewrite_call( + context, + &callee_str, + args, + force_list_tactic, + inner_span, + shape, + ) } ast::ExprKind::Paren(ref subexpr) => rewrite_paren(context, subexpr, shape, expr.span), ast::ExprKind::Binary(op, ref lhs, ref rhs) => { @@ -1264,6 +1272,7 @@ pub(crate) fn rewrite_call( context: &RewriteContext<'_>, callee: &str, args: &[ptr::P], + force_list_tactic: Option, span: Span, shape: Shape, ) -> Option { @@ -1275,6 +1284,7 @@ pub(crate) fn rewrite_call( span, context.config.fn_call_width(), choose_separator_tactic(context, span), + force_list_tactic, ) } @@ -1812,6 +1822,7 @@ pub(crate) fn rewrite_tuple<'a, T: 'a + IntoOverflowableItem<'a>>( span, context.config.fn_call_width(), force_tactic, + None, ) } else { rewrite_tuple_in_visual_indent_style(context, items, span, shape, is_singleton_tuple) diff --git a/src/items.rs b/src/items.rs index a76e88b77d4..921adf3f5e2 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1488,6 +1488,7 @@ fn format_tuple_struct( mk_sp(lo, span.hi()), context.config.fn_call_width(), None, + None, )?; } diff --git a/src/macros.rs b/src/macros.rs index e78ef1f8066..81377663fb8 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -279,6 +279,7 @@ fn rewrite_macro_inner( } else { Some(SeparatorTactic::Never) }, + None, ) .map(|rw| match position { MacroPosition::Item => format!("{};", rw), diff --git a/src/overflow.rs b/src/overflow.rs index 6bf8cd0c70b..f307dfd93c5 100644 --- a/src/overflow.rs +++ b/src/overflow.rs @@ -8,8 +8,8 @@ use rustc_ast::{ast, ptr}; use rustc_span::Span; use crate::closures; -use crate::config::lists::*; use crate::config::Version; +use crate::config::{lists::*, Density}; use crate::expr::{ can_be_overflowed_expr, is_every_expr_simple, is_method_call, is_nested_call, is_simple_expr, rewrite_cond, @@ -252,6 +252,7 @@ pub(crate) fn rewrite_with_parens<'a, T: 'a + IntoOverflowableItem<'a>>( span: Span, item_max_width: usize, force_separator_tactic: Option, + force_list_tactic: Option, ) -> Option { Context::new( context, @@ -263,6 +264,7 @@ pub(crate) fn rewrite_with_parens<'a, T: 'a + IntoOverflowableItem<'a>>( ")", item_max_width, force_separator_tactic, + force_list_tactic, None, ) .rewrite(shape) @@ -286,6 +288,7 @@ pub(crate) fn rewrite_with_angle_brackets<'a, T: 'a + IntoOverflowableItem<'a>>( context.config.max_width(), None, None, + None, ) .rewrite(shape) } @@ -314,6 +317,7 @@ pub(crate) fn rewrite_with_square_brackets<'a, T: 'a + IntoOverflowableItem<'a>> rhs, context.config.array_width(), force_separator_tactic, + None, Some(("[", "]")), ) .rewrite(shape) @@ -331,6 +335,7 @@ struct Context<'a> { item_max_width: usize, one_line_width: usize, force_separator_tactic: Option, + force_list_tactic: Option, custom_delims: Option<(&'a str, &'a str)>, } @@ -345,6 +350,7 @@ impl<'a> Context<'a> { suffix: &'static str, item_max_width: usize, force_separator_tactic: Option, + force_list_tactic: Option, custom_delims: Option<(&'a str, &'a str)>, ) -> Context<'a> { let used_width = extra_offset(ident, shape); @@ -369,6 +375,7 @@ impl<'a> Context<'a> { item_max_width, one_line_width, force_separator_tactic, + force_list_tactic, custom_delims, } } @@ -584,6 +591,34 @@ impl<'a> Context<'a> { _ => (), } + // we only care if the any element but the last has a sigle line comment + let any_but_last_contains_line_comment = list_items + .iter() + .rev() + .skip(1) + .any(|item| item.has_single_line_comment()); + + match self.force_list_tactic { + Some(Density::Tall) + if tactic == DefinitiveListTactic::Mixed && any_but_last_contains_line_comment => + { + // If we determined a `Mixed` layout, but we configured tall then force + // the tactic to be vertical only if any of the items contain single line comments. + // Otherwise, the tacitc was properly set above. + tactic = DefinitiveListTactic::Vertical + } + Some(Density::Compressed) if tactic != DefinitiveListTactic::Horizontal => { + // Only force a mixed layout if we haven't already decided on going horizontal + tactic = DefinitiveListTactic::Mixed + } + // If we need to force a `Vertical` layout, we should only do so if there are + // at least 2 items for us to format. Otherwise, use the tactic already determined. + Some(Density::Vertical) if self.items.len() > 1 => { + tactic = DefinitiveListTactic::Vertical; + } + _ => {} + }; + tactic } diff --git a/src/patterns.rs b/src/patterns.rs index 9b74b35f314..f323646c336 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -494,6 +494,7 @@ fn rewrite_tuple_pat( } else { None }, + None, ) } diff --git a/src/types.rs b/src/types.rs index 64a201e45dd..0f980323373 100644 --- a/src/types.rs +++ b/src/types.rs @@ -844,6 +844,7 @@ impl Rewrite for ast::Ty { context, "typeof", &[anon_const.value.clone()], + None, self.span, shape, ), diff --git a/tests/source/configs/fn_call_layout/compressed.rs b/tests/source/configs/fn_call_layout/compressed.rs new file mode 100644 index 00000000000..393cb82bcb3 --- /dev/null +++ b/tests/source/configs/fn_call_layout/compressed.rs @@ -0,0 +1,67 @@ +// rustfmt-fn_call_layout:Compressed + +fn main() { + empty_args(); + single_arg(ipsum); + two_args(ipsum, dolor); + + lorem(ipsum, dolor, sit, amet); + lorem(ipsum, // some inine comment + dolor, sit, amet); + lorem(ipsum, /* some inine comment */ + dolor, sit, amet); + ipsum(dolor, sit, amet, consectetur, adipiscing, elit, vivamus, ipsum, orci, rhoncus, vel, imperdiet); + + // issue 2010 + let a = i8x32::new( + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32, + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32); + + // issue 4146 + return_monitor_err( + e, + channel_state, + chan, + order, + commitment_update.is_some(), + revoke_and_ack.is_some(), + ); + + + // other examples with more complex args + more_complex_args( + |a, b, c| {if a == 998765390 {- b * 3 } else {c} }, + std::ops::Range { start: 3, end: 5 }, + std::i8::MAX, String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), "another argument" + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, eeeeeeeee)))); + + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))))))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc(ipsum(), dolor(sit::amet(consectetur, adipiscing), elit(vivamus::ipsum::orci(rhoncus())))); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit(vivamus::ipsum::orci(rhoncus()))))))); +} diff --git a/tests/source/configs/fn_call_layout/tall.rs b/tests/source/configs/fn_call_layout/tall.rs new file mode 100644 index 00000000000..53077fcb59f --- /dev/null +++ b/tests/source/configs/fn_call_layout/tall.rs @@ -0,0 +1,67 @@ +// rustfmt-fn_call_layout:Tall + +fn main() { + empty_args(); + single_arg(ipsum); + two_args(ipsum, dolor); + + lorem(ipsum, dolor, sit, amet); + lorem(ipsum, // some inine comment + dolor, sit, amet); + lorem(ipsum, /* some inine comment */ + dolor, sit, amet); + ipsum(dolor, sit, amet, consectetur, adipiscing, elit, vivamus, ipsum, orci, rhoncus, vel, imperdiet); + + // issue 2010 + let a = i8x32::new( + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32, + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32); + + // issue 4146 + return_monitor_err( + e, + channel_state, + chan, + order, + commitment_update.is_some(), + revoke_and_ack.is_some(), + ); + + + // other examples with more complex args + more_complex_args( + |a, b, c| {if a == 998765390 {- b * 3 } else {c} }, + std::ops::Range { start: 3, end: 5 }, + std::i8::MAX, String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), "another argument" + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, eeeeeeeee)))); + + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))))))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc(ipsum(), dolor(sit::amet(consectetur, adipiscing), elit(vivamus::ipsum::orci(rhoncus())))); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit(vivamus::ipsum::orci(rhoncus()))))))); +} diff --git a/tests/source/configs/fn_call_layout/vertical.rs b/tests/source/configs/fn_call_layout/vertical.rs new file mode 100644 index 00000000000..4fb79d20708 --- /dev/null +++ b/tests/source/configs/fn_call_layout/vertical.rs @@ -0,0 +1,67 @@ +// rustfmt-fn_call_layout:Vertical + +fn main() { + empty_args(); + single_arg(ipsum); + two_args(ipsum, dolor); + + lorem(ipsum, dolor, sit, amet); + lorem(ipsum, // some inine comment + dolor, sit, amet); + lorem(ipsum, /* some inine comment */ + dolor, sit, amet); + ipsum(dolor, sit, amet, consectetur, adipiscing, elit, vivamus, ipsum, orci, rhoncus, vel, imperdiet); + + // issue 2010 + let a = i8x32::new( + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32, + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32); + + // issue 4146 + return_monitor_err( + e, + channel_state, + chan, + order, + commitment_update.is_some(), + revoke_and_ack.is_some(), + ); + + + // other examples with more complex args + more_complex_args( + |a, b, c| {if a == 998765390 {- b * 3 } else {c} }, + std::ops::Range { start: 3, end: 5 }, + std::i8::MAX, String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), "another argument" + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, eeeeeeeee)))); + + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))))))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc(ipsum(), dolor(sit::amet(consectetur, adipiscing), elit(vivamus::ipsum::orci(rhoncus())))); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit(vivamus::ipsum::orci(rhoncus()))))))); +} diff --git a/tests/target/configs/fn_call_layout/compressed.rs b/tests/target/configs/fn_call_layout/compressed.rs new file mode 100644 index 00000000000..ebeab299ae5 --- /dev/null +++ b/tests/target/configs/fn_call_layout/compressed.rs @@ -0,0 +1,79 @@ +// rustfmt-fn_call_layout:Compressed + +fn main() { + empty_args(); + single_arg(ipsum); + two_args(ipsum, dolor); + + lorem(ipsum, dolor, sit, amet); + lorem( + ipsum, // some inine comment + dolor, sit, amet, + ); + lorem(ipsum /* some inine comment */, dolor, sit, amet); + ipsum( + dolor, sit, amet, consectetur, adipiscing, elit, vivamus, ipsum, orci, rhoncus, vel, + imperdiet, + ); + + // issue 2010 + let a = i8x32::new( + 0, 1, -1, 2, -2, 3, -3, 4, -4, 5, -5, std::i8::MAX, std::i8::MIN + 1, 100, -100, -32, 0, 1, + -1, 2, -2, 3, -3, 4, -4, 5, -5, std::i8::MAX, std::i8::MIN + 1, 100, -100, -32, + ); + + // issue 4146 + return_monitor_err( + e, channel_state, chan, order, commitment_update.is_some(), revoke_and_ack.is_some(), + ); + + // other examples with more complex args + more_complex_args( + |a, b, c| { + if a == 998765390 { + -b * 3 + } else { + c + } + }, + std::ops::Range { start: 3, end: 5 }, std::i8::MAX, + String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), + "another argument", + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, eeeeeeeee, + )))); + + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, + adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))), + )))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc( + ipsum(), + dolor( + sit::amet(consectetur, adipiscing), elit(vivamus::ipsum::orci(rhoncus())), + ), + ); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit( + vivamus::ipsum::orci(rhoncus()), + )))))); +} diff --git a/tests/target/configs/fn_call_layout/tall.rs b/tests/target/configs/fn_call_layout/tall.rs new file mode 100644 index 00000000000..82212d0d421 --- /dev/null +++ b/tests/target/configs/fn_call_layout/tall.rs @@ -0,0 +1,135 @@ +// rustfmt-fn_call_layout:Tall + +fn main() { + empty_args(); + single_arg(ipsum); + two_args(ipsum, dolor); + + lorem(ipsum, dolor, sit, amet); + lorem( + ipsum, // some inine comment + dolor, + sit, + amet, + ); + lorem(ipsum /* some inine comment */, dolor, sit, amet); + ipsum( + dolor, + sit, + amet, + consectetur, + adipiscing, + elit, + vivamus, + ipsum, + orci, + rhoncus, + vel, + imperdiet, + ); + + // issue 2010 + let a = i8x32::new( + 0, + 1, + -1, + 2, + -2, + 3, + -3, + 4, + -4, + 5, + -5, + std::i8::MAX, + std::i8::MIN + 1, + 100, + -100, + -32, + 0, + 1, + -1, + 2, + -2, + 3, + -3, + 4, + -4, + 5, + -5, + std::i8::MAX, + std::i8::MIN + 1, + 100, + -100, + -32, + ); + + // issue 4146 + return_monitor_err( + e, + channel_state, + chan, + order, + commitment_update.is_some(), + revoke_and_ack.is_some(), + ); + + // other examples with more complex args + more_complex_args( + |a, b, c| { + if a == 998765390 { + -b * 3 + } else { + c + } + }, + std::ops::Range { start: 3, end: 5 }, + std::i8::MAX, + String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), + "another argument", + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, + bbbbbbbbbbbb, + ccccccccccccc, + ddddddddddddd, + eeeeeeeee, + )))); + + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, + bbbbbbbbbbbb, + ccccccccccccc, + ddddddddddddd, + adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))), + )))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc( + ipsum(), + dolor( + sit::amet(consectetur, adipiscing), + elit(vivamus::ipsum::orci(rhoncus())), + ), + ); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit( + vivamus::ipsum::orci(rhoncus()), + )))))); +} diff --git a/tests/target/configs/fn_call_layout/vertical.rs b/tests/target/configs/fn_call_layout/vertical.rs new file mode 100644 index 00000000000..83d5f42cf65 --- /dev/null +++ b/tests/target/configs/fn_call_layout/vertical.rs @@ -0,0 +1,151 @@ +// rustfmt-fn_call_layout:Vertical + +fn main() { + empty_args(); + single_arg(ipsum); + two_args( + ipsum, + dolor, + ); + + lorem( + ipsum, + dolor, + sit, + amet, + ); + lorem( + ipsum, // some inine comment + dolor, + sit, + amet, + ); + lorem( + ipsum, /* some inine comment */ + dolor, + sit, + amet, + ); + ipsum( + dolor, + sit, + amet, + consectetur, + adipiscing, + elit, + vivamus, + ipsum, + orci, + rhoncus, + vel, + imperdiet, + ); + + // issue 2010 + let a = i8x32::new( + 0, + 1, + -1, + 2, + -2, + 3, + -3, + 4, + -4, + 5, + -5, + std::i8::MAX, + std::i8::MIN + 1, + 100, + -100, + -32, + 0, + 1, + -1, + 2, + -2, + 3, + -3, + 4, + -4, + 5, + -5, + std::i8::MAX, + std::i8::MIN + 1, + 100, + -100, + -32, + ); + + // issue 4146 + return_monitor_err( + e, + channel_state, + chan, + order, + commitment_update.is_some(), + revoke_and_ack.is_some(), + ); + + // other examples with more complex args + more_complex_args( + |a, b, c| { + if a == 998765390 { + -b * 3 + } else { + c + } + }, + std::ops::Range { start: 3, end: 5 }, + std::i8::MAX, + String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), + "another argument", + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, + bbbbbbbbbbbb, + ccccccccccccc, + ddddddddddddd, + eeeeeeeee, + )))); + + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, + bbbbbbbbbbbb, + ccccccccccccc, + ddddddddddddd, + adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))), + )))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc( + ipsum(), + dolor( + sit::amet( + consectetur, + adipiscing, + ), + elit(vivamus::ipsum::orci(rhoncus())), + ), + ); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit( + vivamus::ipsum::orci(rhoncus()), + )))))); +} From e5f4bc01a2a2d910646efdb160fddd8b89571f64 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Thu, 28 Jul 2022 17:41:19 -0400 Subject: [PATCH 2/2] Add new config option `OverflowDensity` Through testing it was found that `Tall` as the default value for `fn_call_layout` was not backwards compatable and would lead to breaking formatting changes. Instead of version gating the new behavior, which would also lead to unexpected formatting changes for users who already set `version=Two`, it was decided that a new option should be add to encapsulate rustfmt's previous function argument behavior. In the context of `fn_call_layout` this new option has all the same variants as the `Density` option used for `fn_params_layout`. --- Configurations.md | 28 +++- src/config/mod.rs | 4 +- src/config/options.rs | 16 ++ src/expr.rs | 4 +- src/overflow.rs | 16 +- .../configs/fn_call_layout/compressed.rs | 6 + tests/source/configs/fn_call_layout/foo.rs | 73 +++++++++ tests/source/configs/fn_call_layout/tall.rs | 6 + .../source/configs/fn_call_layout/vertical.rs | 6 + .../configs/fn_call_layout/compressed.rs | 7 + tests/target/configs/fn_call_layout/foo.rs | 140 ++++++++++++++++++ tests/target/configs/fn_call_layout/tall.rs | 11 ++ .../target/configs/fn_call_layout/vertical.rs | 11 ++ 13 files changed, 314 insertions(+), 14 deletions(-) create mode 100644 tests/source/configs/fn_call_layout/foo.rs create mode 100644 tests/target/configs/fn_call_layout/foo.rs diff --git a/Configurations.md b/Configurations.md index 110c1805a18..b9ec113eded 100644 --- a/Configurations.md +++ b/Configurations.md @@ -760,11 +760,33 @@ See also [`fn_params_layout`](#fn_params_layout) Control the layout of arguments in function calls -- **Default value**: `"Tall"` -- **Possible values**: `"Compressed"`, `"Tall"`, `"Vertical"` +- **Default value**: `"Foo"` +- **Possible values**: `"Foo"` `"Compressed"`, `"Tall"`, `"Vertical"` - **Stable**: No (tracking issue: N/A) -#### `"Tall"` (default): +#### `"Foo"` (default): + +```rust +fn main() { + lorem(ipsum, dolor, sit, amet); + ipsum( + dolor, + sit, + amet, + consectetur, + adipiscing, + elit, + vivamus, + ipsum, + orci, + rhoncus, + vel, + imperdiet, + ); +} +``` + +#### `"Tall"`: ```rust fn main() { diff --git a/src/config/mod.rs b/src/config/mod.rs index 695d440325c..fceb0f2b0c6 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -127,7 +127,7 @@ create_config! { "(deprecated: use fn_params_layout instead)"; fn_params_layout: Density, Density::Tall, true, "Control the layout of parameters in function signatures."; - fn_call_layout: Density, Density::Tall, false, + fn_call_layout: OverflowDensity, OverflowDensity::Foo, false, "Control the layout of arguments in a function call"; brace_style: BraceStyle, BraceStyle::SameLineWhere, false, "Brace style for items"; control_brace_style: ControlBraceStyle, ControlBraceStyle::AlwaysSameLine, false, @@ -656,7 +656,7 @@ match_arm_blocks = true match_arm_leading_pipes = "Never" force_multiline_blocks = false fn_params_layout = "Tall" -fn_call_layout = "Tall" +fn_call_layout = "Foo" brace_style = "SameLineWhere" control_brace_style = "AlwaysSameLine" trailing_semicolon = true diff --git a/src/config/options.rs b/src/config/options.rs index 257a17b2703..33967010252 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -69,6 +69,22 @@ pub enum Density { Vertical, } +#[config_type] +/// allows users to specify a density for list-like items. +/// Currently only supports function arguments. +pub enum OverflowDensity { + /// To prevent breaking formatting changes, this option follows the default behavior for + /// list-like items that can overflow. + Foo, + /// Fit as much on one line as possible before wrapping to the next line. + Compressed, + /// Items are placed horizontally as long as there is sufficient space and there aren't + /// any line comments that would force a Vertical layout. + Tall, + /// Place every item on a separate line. There must be at least two items in the list. + Vertical, +} + #[config_type] /// Spacing around type combinators. pub enum TypeDensity { diff --git a/src/expr.rs b/src/expr.rs index b1e54a89667..7972476f37a 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -12,7 +12,7 @@ use crate::comment::{ combine_strs_with_missing_comments, contains_comment, recover_comment_removed, rewrite_comment, rewrite_missing_comment, CharClasses, FindUncommented, }; -use crate::config::{lists::*, Density}; +use crate::config::{lists::*, OverflowDensity}; use crate::config::{Config, ControlBraceStyle, HexLiteralCase, IndentStyle, Version}; use crate::lists::{ definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape, @@ -1272,7 +1272,7 @@ pub(crate) fn rewrite_call( context: &RewriteContext<'_>, callee: &str, args: &[ptr::P], - force_list_tactic: Option, + force_list_tactic: Option, span: Span, shape: Shape, ) -> Option { diff --git a/src/overflow.rs b/src/overflow.rs index f307dfd93c5..3f383af02aa 100644 --- a/src/overflow.rs +++ b/src/overflow.rs @@ -9,7 +9,7 @@ use rustc_span::Span; use crate::closures; use crate::config::Version; -use crate::config::{lists::*, Density}; +use crate::config::{lists::*, OverflowDensity}; use crate::expr::{ can_be_overflowed_expr, is_every_expr_simple, is_method_call, is_nested_call, is_simple_expr, rewrite_cond, @@ -252,7 +252,7 @@ pub(crate) fn rewrite_with_parens<'a, T: 'a + IntoOverflowableItem<'a>>( span: Span, item_max_width: usize, force_separator_tactic: Option, - force_list_tactic: Option, + force_list_tactic: Option, ) -> Option { Context::new( context, @@ -335,7 +335,7 @@ struct Context<'a> { item_max_width: usize, one_line_width: usize, force_separator_tactic: Option, - force_list_tactic: Option, + force_list_tactic: Option, custom_delims: Option<(&'a str, &'a str)>, } @@ -350,7 +350,7 @@ impl<'a> Context<'a> { suffix: &'static str, item_max_width: usize, force_separator_tactic: Option, - force_list_tactic: Option, + force_list_tactic: Option, custom_delims: Option<(&'a str, &'a str)>, ) -> Context<'a> { let used_width = extra_offset(ident, shape); @@ -599,7 +599,7 @@ impl<'a> Context<'a> { .any(|item| item.has_single_line_comment()); match self.force_list_tactic { - Some(Density::Tall) + Some(OverflowDensity::Tall) if tactic == DefinitiveListTactic::Mixed && any_but_last_contains_line_comment => { // If we determined a `Mixed` layout, but we configured tall then force @@ -607,15 +607,17 @@ impl<'a> Context<'a> { // Otherwise, the tacitc was properly set above. tactic = DefinitiveListTactic::Vertical } - Some(Density::Compressed) if tactic != DefinitiveListTactic::Horizontal => { + Some(OverflowDensity::Compressed) if tactic != DefinitiveListTactic::Horizontal => { // Only force a mixed layout if we haven't already decided on going horizontal tactic = DefinitiveListTactic::Mixed } // If we need to force a `Vertical` layout, we should only do so if there are // at least 2 items for us to format. Otherwise, use the tactic already determined. - Some(Density::Vertical) if self.items.len() > 1 => { + Some(OverflowDensity::Vertical) if self.items.len() > 1 => { tactic = DefinitiveListTactic::Vertical; } + // Default behavior for calls to match rustfmts pre `fn_call_layout` formatting. + Some(OverflowDensity::Foo) => {} _ => {} }; diff --git a/tests/source/configs/fn_call_layout/compressed.rs b/tests/source/configs/fn_call_layout/compressed.rs index 393cb82bcb3..f461cf0cb3d 100644 --- a/tests/source/configs/fn_call_layout/compressed.rs +++ b/tests/source/configs/fn_call_layout/compressed.rs @@ -33,6 +33,12 @@ fn main() { revoke_and_ack.is_some(), ); + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, e, from_ty, to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, DUMMY_SP, + ); // other examples with more complex args more_complex_args( diff --git a/tests/source/configs/fn_call_layout/foo.rs b/tests/source/configs/fn_call_layout/foo.rs new file mode 100644 index 00000000000..5ed6bc62b5a --- /dev/null +++ b/tests/source/configs/fn_call_layout/foo.rs @@ -0,0 +1,73 @@ +// rustfmt-fn_call_layout: Foo + +fn main() { + empty_args(); + single_arg(ipsum); + two_args(ipsum, dolor); + + lorem(ipsum, dolor, sit, amet); + lorem(ipsum, // some inine comment + dolor, sit, amet); + lorem(ipsum, /* some inine comment */ + dolor, sit, amet); + ipsum(dolor, sit, amet, consectetur, adipiscing, elit, vivamus, ipsum, orci, rhoncus, vel, imperdiet); + + // issue 2010 + let a = i8x32::new( + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32, + 0, 1, -1, 2, + -2, 3, -3, 4, + -4, 5, -5, std::i8::MAX, + std::i8::MIN + 1, 100, -100, -32); + + // issue 4146 + return_monitor_err( + e, + channel_state, + chan, + order, + commitment_update.is_some(), + revoke_and_ack.is_some(), + ); + + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, e, from_ty, to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, DUMMY_SP, + ); + + // other examples with more complex args + more_complex_args( + |a, b, c| {if a == 998765390 {- b * 3 } else {c} }, + std::ops::Range { start: 3, end: 5 }, + std::i8::MAX, String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), "another argument" + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, eeeeeeeee)))); + + ipsum(dolor(sit::amet(consectetur(aaaaaaaaaaaaaa, bbbbbbbbbbbb, ccccccccccccc, ddddddddddddd, adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))))))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc(ipsum(), dolor(sit::amet(consectetur, adipiscing), elit(vivamus::ipsum::orci(rhoncus())))); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit(vivamus::ipsum::orci(rhoncus()))))))); +} diff --git a/tests/source/configs/fn_call_layout/tall.rs b/tests/source/configs/fn_call_layout/tall.rs index 53077fcb59f..a0691d8c4de 100644 --- a/tests/source/configs/fn_call_layout/tall.rs +++ b/tests/source/configs/fn_call_layout/tall.rs @@ -33,6 +33,12 @@ fn main() { revoke_and_ack.is_some(), ); + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, e, from_ty, to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, DUMMY_SP, + ); // other examples with more complex args more_complex_args( diff --git a/tests/source/configs/fn_call_layout/vertical.rs b/tests/source/configs/fn_call_layout/vertical.rs index 4fb79d20708..5026a5d9a10 100644 --- a/tests/source/configs/fn_call_layout/vertical.rs +++ b/tests/source/configs/fn_call_layout/vertical.rs @@ -33,6 +33,12 @@ fn main() { revoke_and_ack.is_some(), ); + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, e, from_ty, to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, DUMMY_SP, + ); // other examples with more complex args more_complex_args( diff --git a/tests/target/configs/fn_call_layout/compressed.rs b/tests/target/configs/fn_call_layout/compressed.rs index ebeab299ae5..807a80b7c98 100644 --- a/tests/target/configs/fn_call_layout/compressed.rs +++ b/tests/target/configs/fn_call_layout/compressed.rs @@ -27,6 +27,13 @@ fn main() { e, channel_state, chan, order, commitment_update.is_some(), revoke_and_ack.is_some(), ); + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, e, from_ty, to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, DUMMY_SP, + ); + // other examples with more complex args more_complex_args( |a, b, c| { diff --git a/tests/target/configs/fn_call_layout/foo.rs b/tests/target/configs/fn_call_layout/foo.rs new file mode 100644 index 00000000000..276a0bcf4fc --- /dev/null +++ b/tests/target/configs/fn_call_layout/foo.rs @@ -0,0 +1,140 @@ +// rustfmt-fn_call_layout: Foo + +fn main() { + empty_args(); + single_arg(ipsum); + two_args(ipsum, dolor); + + lorem(ipsum, dolor, sit, amet); + lorem( + ipsum, // some inine comment + dolor, sit, amet, + ); + lorem(ipsum /* some inine comment */, dolor, sit, amet); + ipsum( + dolor, + sit, + amet, + consectetur, + adipiscing, + elit, + vivamus, + ipsum, + orci, + rhoncus, + vel, + imperdiet, + ); + + // issue 2010 + let a = i8x32::new( + 0, + 1, + -1, + 2, + -2, + 3, + -3, + 4, + -4, + 5, + -5, + std::i8::MAX, + std::i8::MIN + 1, + 100, + -100, + -32, + 0, + 1, + -1, + 2, + -2, + 3, + -3, + 4, + -4, + 5, + -5, + std::i8::MAX, + std::i8::MIN + 1, + 100, + -100, + -32, + ); + + // issue 4146 + return_monitor_err( + e, + channel_state, + chan, + order, + commitment_update.is_some(), + revoke_and_ack.is_some(), + ); + + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, e, from_ty, to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, DUMMY_SP, + ); + + // other examples with more complex args + more_complex_args( + |a, b, c| { + if a == 998765390 { + -b * 3 + } else { + c + } + }, + std::ops::Range { start: 3, end: 5 }, + std::i8::MAX, + String::from(" hello world!!").as_bytes(), + thread::Builder::new() + .name("thread1".to_string()) + .spawn(move || { + use std::sync::Arc; + + let mut values = Arc::<[u32]>::new_uninit_slice(3); + + // Deferred initialization: + let data = Arc::get_mut(&mut values).unwrap(); + data[0].write(1); + data[1].write(2); + data[2].write(3); + + let values = unsafe { values.assume_init() }; + }), + "another argument", + ); + + // nested calls + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, + bbbbbbbbbbbb, + ccccccccccccc, + ddddddddddddd, + eeeeeeeee, + )))); + + ipsum(dolor(sit::amet(consectetur( + aaaaaaaaaaaaaa, + bbbbbbbbbbbb, + ccccccccccccc, + ddddddddddddd, + adipiscing(elit(|| ipsum(dolor(sit::amet(consectetur()))))), + )))); + + aaaaaaaaaaaaaaaaaa::bbbbbbbbbbbbbb::cccccccccc( + ipsum(), + dolor( + sit::amet(consectetur, adipiscing), + elit(vivamus::ipsum::orci(rhoncus())), + ), + ); + + ipsum(dolor(sit::amet(consectetur(adipiscing(elit( + vivamus::ipsum::orci(rhoncus()), + )))))); +} diff --git a/tests/target/configs/fn_call_layout/tall.rs b/tests/target/configs/fn_call_layout/tall.rs index 82212d0d421..9dfc6b51bdb 100644 --- a/tests/target/configs/fn_call_layout/tall.rs +++ b/tests/target/configs/fn_call_layout/tall.rs @@ -74,6 +74,17 @@ fn main() { revoke_and_ack.is_some(), ); + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, + e, + from_ty, + to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, + DUMMY_SP, + ); + // other examples with more complex args more_complex_args( |a, b, c| { diff --git a/tests/target/configs/fn_call_layout/vertical.rs b/tests/target/configs/fn_call_layout/vertical.rs index 83d5f42cf65..6c2a9836fd5 100644 --- a/tests/target/configs/fn_call_layout/vertical.rs +++ b/tests/target/configs/fn_call_layout/vertical.rs @@ -87,6 +87,17 @@ fn main() { revoke_and_ack.is_some(), ); + // line comment in the middle of the args + CastCheck::new( + &fn_ctxt, + e, + from_ty, + to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, + DUMMY_SP, + ); + // other examples with more complex args more_complex_args( |a, b, c| {