Visual IndentStyle is broken on impl with where clause

[zonyitoo]

Test code:

struct A<T>
    where T: Send
{
    x: u32,
}

impl<T> A<T> where T: Send
{
    fn foo() {}
}

The impl block is expected to be formatted as:

impl<T> A<T>
    where T: Send
{
    fn foo() {}
}

Everything is ok for indent_style: "Block".

rustfmt version:

rustfmt 0.99.5-nightly (90692a59 2018-09-26)

[ytmimi]

Confirming I can still reproduce this on rustfmt 1.5.1-nightly (a7bf0090 2022-07-17)

Input

struct A<T>
    where T: Send
{
    x: u32,
}

impl<T> A<T> where T: Send
{
    fn foo() {}
}

output with indent_style=Visual (unchanged)

struct A<T>
    where T: Send
{
    x: u32,
}

impl<T> A<T> where T: Send
{
    fn foo() {}
}

output with indent_style=Block

struct A<T>
where
    T: Send,
{
    x: u32,
}

impl<T> A<T>
where
    T: Send,
{
    fn foo() {}
}

For anyone interested on working on this I'd check out what the style guide has to say about where clause formatting,

rewrite_where_clause has a on_new_line: bool parameter that can be used to control if we move the where clause to it's own line. It might be worth checking all the places where rewrite_where_clause is called to see if that value is being set correctly.

rustfmt/src/items.rs

Lines 2893 to 2904 in a7bf009

	fn rewrite_where_clause(
	context: &RewriteContext<'_>,
	predicates: &[ast::WherePredicate],
	where_span: Span,
	brace_style: BraceStyle,
	shape: Shape,
	on_new_line: bool,
	terminator: &str,
	span_end: Option<BytePos>,
	span_end_before_where: BytePos,
	where_clause_option: WhereClauseOption,
	) -> Option<String> {

At least in the case of format_impl it seems that it its always passed as false.

rustfmt/src/items.rs

Lines 694 to 705 in a7bf009

	let where_clause_str = rewrite_where_clause(
	context,
	&generics.where_clause.predicates,
	generics.where_clause.span,
	context.config.brace_style(),
	Shape::legacy(where_budget, offset.block_only()),
	false,
	"{",
	where_span_end,
	self_ty.span.hi(),
	option,
	)?;

[ytmimi]

linking the indent_style tracking issue: #3346

[jmj0502]

Hey! @ytmimi I would like to give a shot. Based on what was exposed on the style guide, the where clause should always start on a new line without any indentation, so the provided insight may be enough to tackle the issue (I don't really know but we'll see c:).

[ytmimi]

Awesome! as mentioned in the Issue claiming section of the rustc dev guide you can leave a comment with @rustbot claim to assign yourself to any issue you'd like to work on!

[jmj0502]

@rustbot claim

[jmj0502]

@ytmimi Thanks! ^^ I'll try to add a draft PR as soon as I can!👍

[ytmimi]

@jmj0502 no rush! I appreciate your interest in taking this on 😁. You might also be able to leverage the rewrite_where_keyword function, which is used when calling rewrite_where_clause_rfc_style, and I think it handles rewriting the where keyword with a newline and the correct amount of indentation.

rustfmt/src/items.rs

Lines 2745 to 2762 in f2c31ba

	fn rewrite_where_clause_rfc_style(
	context: &RewriteContext<'_>,
	predicates: &[ast::WherePredicate],
	where_span: Span,
	shape: Shape,
	terminator: &str,
	span_end: Option<BytePos>,
	span_end_before_where: BytePos,
	where_clause_option: WhereClauseOption,
	) -> Option<String> {
	let (where_keyword, allow_single_line) = rewrite_where_keyword(
	context,
	predicates,
	where_span,
	shape,
	span_end_before_where,
	where_clause_option,
	)?;

[jmj0502]

@ytmimi Cool! This might take a while 😅 but its ok tho'. Should I use the PR if I want to reach out for questions?

[ytmimi]

Either on the PR or here is fine! I'll get the notifications regardless of which channel you choose to use.

[jmj0502]

@ytmimi Hey! I got a question. How can I use the indent_style="Visual" while running idempotence test? I know that the indent_style config is a nightly feature and should be stablished on the rustfmt.toml file, but I don't really know how to turn it on while performing unit tests.

[ytmimi]

Great question! The details are outlined in the create test cases section of the Contribution docs. To set configs for any test in the tests/source and tests/target directory just include a comment at the top of the file in the form // rustfmt-{config}: {value}:

(example taken from tests/target/issue-3270/one.rs)

// rustfmt-version: One

pub fn main() {
    /*   let s = String::from(
            "
    hello
    world
    ",
        ); */

    assert_eq!(s, "\nhello\nworld\n");
}

You're not limited to just one. You can use these comments to set multiple configuration values for the test.

[jmj0502]

@ytmimi I think I'm done! The solution was along the lines of the information you provided on the issue description. The on_new_line parameter of the rewrite_where_clause function was set to false on the format_impl function. I also, noticed that the rewrite_where_clause_rfc_style is only used if the Block variant of the IndentStyle enum is provided on context parameter, so simply setting the on_new_line parameter to true seemed like a good idea c:.