fix: wrap function without a body exceeding 100 characters (#6539)

[lukewilson2002]

From Yacin Tmimi:

The issue is that we're not accounting for cases where we don't have a body, or where the body is on the next line.

Hopefully these changes suggested by @ytmimi and the test case I provided fix the original problem. Let me know if more tests are needed!

Closes #6539

Running the new tests without the proposed changes to src/items.rs:

Mismatch at tests/source/issue-6539/issue_example.rs:1:
 // rustfmt-style_edition: 2027
 
 pub trait Trait {
-    fn a_one_hundred_column_fn_decl_no_body(&self, aaa: f64, b: f64, c: f64, d: f64, e: f64) -> f64;
+    fn a_one_hundred_column_fn_decl_no_body(&self, aaa: f64, b: f64, c: f64, d: f64, e: f64)
+    -> f64;
 
     fn a_one_hundred_one_column_fn_decl_no_body(
         self,

Mismatch at tests/target/issue-6539/issue_example.rs:1:
 // rustfmt-style_edition: 2027
 
 pub trait Trait {
-    fn a_one_hundred_column_fn_decl_no_body(&self, aaa: f64, b: f64, c: f64, d: f64, e: f64) -> f64;
+    fn a_one_hundred_column_fn_decl_no_body(&self, aaa: f64, b: f64, c: f64, d: f64, e: f64)
+    -> f64;
 
     fn a_one_hundred_one_column_fn_decl_no_body(
         self,

Tests are passing with these changes.

EDIT: Corrected attribution.

[ytmimi]

@lukewilson2002 I suggested those changes not @chrisduerr 😅, but I'm glad to see that you've picked them up for this PR.

In addition to the test case you've added here I'd also like to see cases that include a block body and where clause, because based on some testing that I've done those AST nodes also impact how we lay out the fn parameters. We should also test the input against different values of brace_style, And It wouldn't hurt to include test cases for where_single_line, since that impacts the FnBraceStyle as well.

As an example, using style_edition=2027, brace_style=AlwaysNextLine, and where_single_line=false (default) on the following:

Input

pub trait Trait
{
    fn a_one_hundred_column_fn_decl_no_body(&self, aaa: f64, b: f64, c: f64, d: f64, e: f64) -> f64;

    fn a_one_hundred_column_fn_decl_no_body_and_where_clause<T>(&self, aaaaa: T, bbbbbb: f64) -> f64 where T: Debug;

    fn a_one_hundred_column_fn_decl_with_body(&self, aaaa: f64, bbb: f64, ccc: f64, ddd: f64) -> f64 {}

    fn a_one_hundred_column_fn_decl_with_body_and_where_clause<T>(&self, aaaa: f64, bbb: f64) -> f64 where T: Debug {}
}

I'd expect it to produce this output:

output style_edition=2027, brace_style=AlwaysNextLine, and where_single_line=false

pub trait Trait
{
    fn a_one_hundred_column_fn_decl_no_body(&self, aaa: f64, b: f64, c: f64, d: f64, e: f64) -> f64;

    fn a_one_hundred_column_fn_decl_no_body_and_where_clause<T>(&self, aaaaa: T, bbbbbb: f64) -> f64
    where
        T: Debug;

    fn a_one_hundred_column_fn_decl_with_body(&self, aaaa: f64, bbb: f64, ccc: f64, ddd: f64) -> f64
    {
    }

    fn a_one_hundred_column_fn_decl_with_body_and_where_clause<T>(&self, aaaa: f64, bbb: f64) -> f64
    where
        T: Debug,
    {
    }
}

And using style_edition=2027, brace_style=AlwaysNextLine, and where_single_line=false on the original input, i'd expect this result:

output style_edition=2027, brace_style=AlwaysNextLine, and where_single_line=true

pub trait Trait
{
    fn a_one_hundred_column_fn_decl_no_body(&self, aaa: f64, b: f64, c: f64, d: f64, e: f64) -> f64;

    fn a_one_hundred_column_fn_decl_no_body_and_where_clause<T>(&self, aaaaa: T, bbbbbb: f64) -> f64
    where T: Debug;

    fn a_one_hundred_column_fn_decl_with_body(&self, aaaa: f64, bbb: f64, ccc: f64, ddd: f64) -> f64
    {
    }

    fn a_one_hundred_column_fn_decl_with_body_and_where_clause<T>(&self, aaaa: f64, bbb: f64) -> f64
    where T: Debug
    {
    }
}

[ytmimi]

After some additional investigation I noticed that compute_budgets_for_params seems to be off when there's a where_clause; specifically on how it updates the used_space variable. This error causes the params to wrap early.

rustfmt/src/items.rs

Lines 2905 to 2909 in ef94a72

	match fn_brace_style {
	FnBraceStyle::None => used_space += 1, // 1 = `;`
	FnBraceStyle::SameLine => used_space += 2, // 2 = `{}`
	FnBraceStyle::NextLine => (),
	}

I think the following patch resolves the compute_budgets_for_params issue, but again, I've only done some minor testing, and more might be required:

diff --git a/src/items.rs b/src/items.rs
index be01a2af..65b990be 100644
--- a/src/items.rs
+++ b/src/items.rs
@@ -2500,6 +2500,7 @@ fn rewrite_fn_base(
         ret_str_len,
         fn_brace_style,
         multi_line_ret_str,
+        where_clause,
     );
 
     debug!(
@@ -2899,6 +2900,7 @@ fn compute_budgets_for_params(
     ret_str_len: usize,
     fn_brace_style: FnBraceStyle,
     force_vertical_layout: bool,
+    where_clause: &ast::WhereClause,
 ) -> (usize, usize, Indent) {
     debug!(
         "compute_budgets_for_params {} {:?}, {}, {:?}",
@@ -2913,7 +2915,15 @@ fn compute_budgets_for_params(
         let overhead = if ret_str_len == 0 { 2 } else { 3 };
         let mut used_space = indent.width() + result.len() + ret_str_len + overhead;
         match fn_brace_style {
-            FnBraceStyle::None => used_space += 1,     // 1 = `;`
+            _ if context.config.style_edition() >= StyleEdition::Edition2027
+                && where_clause.predicates.len() > 0 =>
+            {
+                // Don't add anything to `used_space` if we have a where clause.
+                // For all `FnBraceStyle` values if we have a where cluase that can't fit
+                // on the current line it'll be written to the next line.
+                // Therefore, we don't need to account for a trailing `;` or `{}`
+            }
+            FnBraceStyle::None => used_space += 1, // 1 = `;`
             FnBraceStyle::SameLine => used_space += 2, // 2 = `{}`
             FnBraceStyle::NextLine => (),
         }

[lukewilson2002]

Thank you @ytmimi for clarifying! I don't know where the confusion came from because I was reading that issue quite a lot. Just a silly mistake I guess. 😄 I'll update the commit.

I greatly appreciate the feedback and the test cases you suggested. I will work on this right now.

[lukewilson2002]

@ytmimi I incorporated the patch, but I couldn't really figure out how to write a test that would cover it. If you have a suggestion let me know and I'd be happy to incorporate one.

The test files contain the same functions, except that default.rs contains the 101 column function that is supposed to wrap its return type to the next line. I felt it was unnecessary to include it for other tests since the options wouldn't affect it. I also included a *where_clause2 function that's intended to be long enough to get the arguments to wrap, which does affect whether the where clause gets its where_same_line 'vetoed' if the arguments wrapped. Not sure whether that should be included or not.

I can tell some of these tests are unlikely to change between the test flags, but I just assumed they should all be included so to make it easier to read.

Let me know if a test is unnecessary or if I missed anything.

[ytmimi]

@lukewilson2002 Thanks for expanding on the test cases! Let's keep the examples that you've added. I think it's still valuable to show cases where things will wrap.

To cover our based I'd also want to include test files for the the remaining combinations of brace_style and where_single_line.

brace_style	where_single_line (true)	where_single_line (false)
AlwaysNextLine	Done ✅	Done ✅
SameLineWhere	TODO	Done ✅ Technically your default.rs covers this case because it implicitly uses the defaults for these options. Let's be more explicit by setting the options with config comments. e.g. // rustfmt-brace_style: SameLineWhere and // rustfmt-where_single_line: false
PreferSameLine	TODO	TODO

Also, if you're able to expand on the test cases a little more that would be great!

The function that we'er modifying in this PR (rewrite_fn_base), gets called any time a function definition is rewritten by rustfmt, so it applies to methods and associated functions in trait definitions and also for top level functions in a file.

It would be great to include some examples of top level functions, as well as functions where the return type is formatted over multiple lines.

[lukewilson2002]

@ytmimi again thank you for your patience and very helpful suggestions. I was able to come up with a better set of function declarations for testing. And I separated the basic "off by one" original issue description tests into the idempotence.rs file for some sanity checking. And of course I adhered to the 6 different test configurations as you described above. 😃

I patched a bug where multi-lined function arguments caused the brace after the where clause to be formatted on the next line, intended for where_single_line, but it was incorrectly being applied for the PreferSameLine brace style.

This brace was being forced to the next line:
94a1c9d#diff-6c41fcfc145b01b6b2505a7b48dc8e4e1fc5fd5b24bb17dd4ad1b253fbd9acf1R114