Odd formatting of type def with many generics

[maltekliemann]

trafficstars

I'm seeing this behavior in rustfmt 1.6.0-nightly (c469197b 2023-08-22):

// Before (correct in stable)
type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
    CCCCCCCCCCCCCCCCC,
    DDDDDDDDDDDDDDDDD,
    EEEEEEEEEEEEEEEEE,
    FFFFFFFFFFFFFFFFF,
    GGGGGGGGGGGGGGGGG,
    HHHHHHHHHHHHHHHHH,
    IIIIIIIIIIIIIIIII,
>;

// After (hopefully not correct)
type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
        CCCCCCCCCCCCCCCCC,
        DDDDDDDDDDDDDDDDD,
        EEEEEEEEEEEEEEEEE,
        FFFFFFFFFFFFFFFFF,
        GGGGGGGGGGGGGGGGG,
        HHHHHHHHHHHHHHHHH,
        IIIIIIIIIIIIIIIII,
    >;

It's completely unclear to me what the expected behavior is at this point, but judging by https://github.com/rust-lang/rustfmt/issues/5577 (specifically this: https://github.com/rust-lang/rustfmt/issues/5577#issuecomment-1407489330), this is not what you want.

[ytmimi]

Thanks for reaching out. Confirming I can reproduce this with rustfmt 1.6.0-nightly (e480739e 2023-08-17).

output using version=0ne

type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
    CCCCCCCCCCCCCCCCC,
    DDDDDDDDDDDDDDDDD,
    EEEEEEEEEEEEEEEEE,
    FFFFFFFFFFFFFFFFF,
    GGGGGGGGGGGGGGGGG,
    HHHHHHHHHHHHHHHHH,
    IIIIIIIIIIIIIIIII,
>;

output using version=Two

type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
        CCCCCCCCCCCCCCCCC,
        DDDDDDDDDDDDDDDDD,
        EEEEEEEEEEEEEEEEE,
        FFFFFFFFFFFFFFFFF,
        GGGGGGGGGGGGGGGGG,
        HHHHHHHHHHHHHHHHH,
        IIIIIIIIIIIIIIIII,
    >;

[calebcartwright]

I did have a bit of a panic when I saw this report earlier today, but fortunately, the default behavior is still correct and doesn't have any bearing on stable vs. nightly

Using the nightly-only version config option with the non-default value means opting in to formatting improvements, but at the risk of bugs/formatting diffs.

This is absolutely something that needs to be fixed as it's definitely wrong in the v=2 case, but it's not something we need to madly scramble to fix and try to get backported into the 1.72 release :relieved:

[maltekliemann]

Oh, sorry, didn't mean to cause a panic by underspecifying the problem. Thanks for taking a look so quickly. :pray:

[calebcartwright]

Oh, sorry, didn't mean to cause a panic by underspecifying the problem. Thanks for taking a look so quickly. 🙏

np, thanks for the report!

[johnhuichen]

@rustbot claim

[johnhuichen]

Currently similar code but for the trait is formatted OK

trait SomeTrait:
    BBBBBBBBBBBBBBB<
        CCCCCCCCCCCCCCCCC,
        DDDDDDDDDDDDDDDDD,
        EEEEEEEEEEEEEEEEE,
        FFFFFFFFFFFFFFFFF,
        GGGGGGGGGGGGGGGGG,
        HHHHHHHHHHHHHHHHH,
        IIIIIIIIIIIIIIIII,
    > + Eq
    + PartialEq
{
}

whereas if it's a type definition

type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
        CCCCCCCCCCCCCCCCC,
        DDDDDDDDDDDDDDDDD,
        EEEEEEEEEEEEEEEEE,
        FFFFFFFFFFFFFFFFF,
        GGGGGGGGGGGGGGGGG,
        HHHHHHHHHHHHHHHHH,
        IIIIIIIIIIIIIIIII,
    >;

I think the solution is to make sure the first trait bound goes to the next line

type AAAAAAAAAAAAA: 
    BBBBBBBBBBBBBBB<
        CCCCCCCCCCCCCCCCC,
        DDDDDDDDDDDDDDDDD,
        EEEEEEEEEEEEEEEEE,
        FFFFFFFFFFFFFFFFF,
        GGGGGGGGGGGGGGGGG,
        HHHHHHHHHHHHHHHHH,
        IIIIIIIIIIIIIIIII,
    >;

[ytmimi]

@johnhuichen I don't think we need to move the bound definition to the next line. My guess is that this is a Shape related issue. Shape is the type we use within the codebase to determine indentation. Given that things format correct with v1 and don't format correctly with v2 I suspect we must be using the wrong shape for v2.

It would be great if you could figure out what the difference is between the v1 and v2 codepaths. Let me know if you need help figuring out where in the codebase to get started.

[johnhuichen]

@ytmimi

I found that when rewriting TyAlias, it calls join_bounds_inner(src/types.rs) twice, which will update the shape to increment indent by 4.

The problem is that to rewrite the generics inside angle brackets, the program calls rewrite_with_angle_brackets (src/overflow.rs) which adds another 4 to the indent

It seems to me that I have limited options:

1. I can't modify rewrite_with_angle_brackets to not add another 4 indents because I will probably break the functionality in too many places
2. I can't avoid calling join_bounds_inner twice, because that seems important in version 2:

    // Whether to retry with a forced newline:
    //   Only if result is not already multiline and did not exceed line width,
    //   and either there is more than one item;
    //       or the single item is of type `Trait`,
    //          and any of the internal arrays contains more than one item;
    let retry_with_force_newline = match context.config.style_edition() {
        style_edition @ _ if style_edition <= StyleEdition::Edition2021 => {
            !force_newline
                && items.len() > 1
                && (result.0.contains('\n') || result.0.len() > shape.width)
        }
        _ if force_newline => false,
        _ if (!result.0.contains('\n') && result.0.len() <= shape.width) => false,
        _ if items.len() > 1 => true,
        _ => is_item_with_multi_items_array(&items[0]),
    };

    if retry_with_force_newline {
        join_bounds_inner(context, shape, items, need_indent, true)
    } else {
        Ok(result.0)
    }

[johnhuichen]

@ytmimi after some digging, I still think the problem is with inconsistent formatting. Since bounds is used by both trait and type, you can't without difficulty format traits as

trait SomeTrait:
    BBBBBBBBBBBBBBB<
        CCCCCCCCCCCCCCCCC,
        DDDDDDDDDDDDDDDDD,
        EEEEEEEEEEEEEEEEE,
        FFFFFFFFFFFFFFFFF,
        GGGGGGGGGGGGGGGGG,
        HHHHHHHHHHHHHHHHH,
        IIIIIIIIIIIIIIIII,
    >
{
}

while formatting type as

type SomeType: BBBBBBBBBBBBBBB<
    CCCCCCCCCCCCCCCCC,
    DDDDDDDDDDDDDDDDD,
    EEEEEEEEEEEEEEEEE,
    FFFFFFFFFFFFFFFFF,
>;

one of them needs to conform to the other.

[ytmimi]

@johnhuichen Thanks for continuing to look into this. To figure out how to proceed I've been looking throught the Rust Style Guide.

The trait formatting looks right to me, and the rules for formatting traits are described here.

trait SomeTrait:
    BBBBBBBBBBBBBBB<
        CCCCCCCCCCCCCCCCC,
        DDDDDDDDDDDDDDDDD,
        EEEEEEEEEEEEEEEEE,
        FFFFFFFFFFFFFFFFF,
        GGGGGGGGGGGGGGGGG,
        HHHHHHHHHHHHHHHHH,
        IIIIIIIIIIIIIIIII,
    >
{
}

The associated type formatting rules are mentioned here, but they seem a little under specified in this case and don't really tell us how to break the bound. The associated type docs mention to "Format associated types like type aliases", though I don't think the type alias docs help us out much either.

The Types and Bounds chapter of the guide describes line breaking, and tells us to break generic types following the rules for generics.

The rules for generics are here, and based on my reading I'd expect the following to also be the correct formatting.

type SomeType: BBBBBBBBBBBBBBB<
    CCCCCCCCCCCCCCCCC,
    DDDDDDDDDDDDDDDDD,
    EEEEEEEEEEEEEEEEE,
    FFFFFFFFFFFFFFFFF,
>;

That said, @rust-lang/style, I'd be curious to know if any of you have thoughts on this issue.

[calebcartwright]

I feel like there's too many different things being discussed on one issue to really be able to weigh in from a style perspective, and as such I don't think this is at a point of being ready for t-style to discuss in a meeting.

The item mentioned in the OP of this issue is a bug in the v2/2024 style edition, and one that needs to be fixed. For the rest, I'd ask for a concise report to be opened here: https://github.com/rust-lang/style-team/issues that explains what specifically is being asked/reported of the style team

[johnhuichen]

@calebcartwright bounds rewrite implementation was changed in Version 2 to fix

pub trait PrettyPrinter<'tcx>:
    Printer<
    'tcx,
    Error = fmt::Error,
    Path = Self,
    Region = Self,
    Type = Self,
    DynExistential = Self,
    Const = Self,
>
{
    //
}

But the fix broke formatting for type alias. I didn't find a straight forward way to fix type alias without breaking trait formatting. I can give it another try this weekend, but hopefully you see my dilemma