Rustfmt removes all indentation from the where clause here, making it inconsistent with the function header

[jyn514]

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f049e90069a93ab8471f51e0a4f27db5

mod inner {
    fn foo() -> String
    where { // running rustfmt removes all whitespace at the start of the line
        String::new()
    }
}

cc @davidBar-On

Originally posted by @jyn514 in #4689 (comment)

[ytmimi]

I've tracked the issue down to lines 2421-2446 in items::rewrite_fn_base. rustfmt is trying to recover comments between the return type and function body, but instead of finding comments it finds "\n____where " (underscores used to represent spaces).

rustfmt/src/items.rs

Lines 2421 to 2446 in c4416f2

	// Comment between return type and the end of the decl.
	let snippet_lo = fd.output.span().hi();
	if where_clause.predicates.is_empty() {
	let snippet_hi = span.hi();
	let snippet = context.snippet(mk_sp(snippet_lo, snippet_hi));
	// Try to preserve the layout of the original snippet.
	let original_starts_with_newline = snippet
	.find(|c| c != ' ')
	.map_or(false, |i| starts_with_newline(&snippet[i..]));
	let original_ends_with_newline = snippet
	.rfind(|c| c != ' ')
	.map_or(false, |i| snippet[i..].ends_with('\n'));
	let snippet = snippet.trim();
	if !snippet.is_empty() {
	result.push(if original_starts_with_newline {
	'\n'
	} else {
	' '
	});
	result.push_str(snippet);
	if original_ends_with_newline {
	force_new_line_for_brace = true;
	}
	}
	}
	}

And when we get to line 2434 the trimmed snippet isn't empty, so rustfmt appends "\nwhere" assuming that it just appended a comment to the result.

I think this problem could be solved by using the has_where_token attribute of the ast::WhereClause to adjust the span of the snippet to start before the where tokens if they were parsed.

pub struct [WhereClause](https://doc.rust-lang.org/beta/nightly-rustc/src/rustc_ast/ast.rs.html#444-452) {
    /// `true` if we ate a `where` token: this can happen
    /// if we parsed no predicates (e.g. `struct Foo where {}`).
    /// This allows us to accurately pretty-print
    /// in `nt_to_tokenstream`
    pub has_where_token: [bool](https://doc.rust-lang.org/beta/std/primitive.bool.html),
    pub predicates: Vec<[WherePredicate](https://doc.rust-lang.org/beta/nightly-rustc/src/rustc_ast/ast.rs.html#456-463)>,
    pub span: [Span](https://doc.rust-lang.org/beta/nightly-rustc/rustc_span/span_encoding/struct.Span.html),
}

Here's what that change might look like:

let snippet = if !where_clause.has_where_token {
    let snippet_hi = span.hi();
    context.snippet(mk_sp(snippet_lo, snippet_hi))
} else {
    let snippet_hi = where_clause.span.lo();
    context.snippet(mk_sp(snippet_lo, snippet_hi))
};

The proposed solution would remove the empty where clause and produce the following formatting:

mod inner {
    fn foo() -> String {
        // running rustfmt removes all whitespace at the start of the line
        String::new()
    }
}

[jyn514]

I like that a lot :) yeah I didn't even realize I had an empty where clause at first, it seems fine to remove.

[davidBar-On]

Three enhancements may be considered to the change suggested by ytmimi:

1st* proposed enhancement:

Fix the original indentation issue. As the where was handled as a comment, replacing it with a comment results the following output:

mod inner {
    fn foo() -> String
/* comment */ {
        // running rustfmt removes all whitespace at the start of the line
        String::new()
    }
}

This can probably be fixed by replacing the '\n' in line 2436 by indent.to_string_with_newline(context.config) (and change result.push to result.push_str).

2nd proposed enhancement:

Removing the empty where also removes a comment after it (as the where was handled as a comment). E.g. with that change:

mod inner {
    fn foo() -> String where /* post-comment */ {}
}

is formatted as:

mod inner {
    fn foo() -> String {}
}

This can be fixed by adding similar code as in lines 24421-2445 - after these lines, starting with:

if where_clause.has_where_token {
    let snippet = context.snippet(mk_sp(where_clause.span.hi(), span.hi()));
    ...

3rd proposed enhancement:

This problem is an existing rustfmt issue and is not related to the change. However it is related issue and probably should also be fixed as part of this change. The issue is that current code of handling the case where "there are neither where-clause nor return type" in lines 2478-2493 is also not considering the case of empty where. Therefore post where comments are removed:

mod inner {
    fn foo() where /* post-comment */ {}
}

is formatted as:

mod inner {
    fn foo() {}
}

The following replacement of the code in lines 2478-2493 seem to properly handle the pre and post empty where comments, in case there is no return type (although I hope that there is a nicer way to write this code ...):

        if let ast::FnRetTy::Default(ret_span) = fd.output {
            let (span_lo, span_hi) = if where_clause.has_where_token {
                (params_span.hi(), where_clause.span.lo())
            } else {
                (params_span.hi(), ret_span.hi())
            };
            match recover_missing_comment_in_span(
                mk_sp(span_lo, span_hi),
                shape,
                context,
                last_line_width(&result),
            ) {
                Some(ref missing_comment) if !missing_comment.is_empty() => {
                    result.push_str(missing_comment);
                    force_new_line_for_brace = true;
                }
                _ => (),
            }
            if where_clause.has_where_token {
                match recover_missing_comment_in_span(
                    mk_sp(where_clause.span.hi(), span.hi()),
                    shape,
                    context,
                    last_line_width(&result),
                ) {
                    Some(ref missing_comment) if !missing_comment.is_empty() => {
                        result.push_str(missing_comment);
                        force_new_line_for_brace = true;
                    }
                    _ => (),
                }
            }
        }

[ytmimi]

@davidBar-On thanks for the thorough explanation, and for pointing out some cases that I missed! If you're interested in opening up a PR for this that would be great, otherwise I think there's plenty of good insights listed above that someone else could use to come with a fix

[davidBar-On]

I will open a PR, as I already have a draft implementation of the changes (this is how I found the other issues).

[davidBar-On]

Submitted PR #5411 with a proposed fix.