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`.

Author: ytmimi

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() {

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

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 {

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<ast::Expr>],
-    force_list_tactic: Option<Density>,
+    force_list_tactic: Option<OverflowDensity>,
     span: Span,
     shape: Shape,
 ) -> Option<String> {

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<SeparatorTactic>,
-    force_list_tactic: Option<Density>,
+    force_list_tactic: Option<OverflowDensity>,
 ) -> Option<String> {
     Context::new(
         context,
@@ -335,7 +335,7 @@ struct Context<'a> {
     item_max_width: usize,
     one_line_width: usize,
     force_separator_tactic: Option<SeparatorTactic>,
-    force_list_tactic: Option<Density>,
+    force_list_tactic: Option<OverflowDensity>,
     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<SeparatorTactic>,
-        force_list_tactic: Option<Density>,
+        force_list_tactic: Option<OverflowDensity>,
         custom_delims: Option<(&'a str, &'a str)>,
     ) -> Context<'a> {
         let used_width = extra_offset(ident, shape);
@@ -599,23 +599,25 @@ 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
                 // 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 => {
+            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) => {}
             _ => {}
         };
 

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(

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())))))));
+}

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(

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(

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| {