-
Notifications
You must be signed in to change notification settings - Fork 898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Odd formatting of type def with many generics #5892
Comments
Thanks for reaching out. Confirming I can reproduce this with output type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
CCCCCCCCCCCCCCCCC,
DDDDDDDDDDDDDDDDD,
EEEEEEEEEEEEEEEEE,
FFFFFFFFFFFFFFFFF,
GGGGGGGGGGGGGGGGG,
HHHHHHHHHHHHHHHHH,
IIIIIIIIIIIIIIIII,
>; output type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
CCCCCCCCCCCCCCCCC,
DDDDDDDDDDDDDDDDD,
EEEEEEEEEEEEEEEEE,
FFFFFFFFFFFFFFFFF,
GGGGGGGGGGGGGGGGG,
HHHHHHHHHHHHHHHHH,
IIIIIIIIIIIIIIIII,
>; |
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 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 😌 |
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! |
@rustbot claim |
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,
>; |
@johnhuichen I don't think we need to move the bound definition to the next line. My guess is that this is a It would be great if you could figure out what the difference is between the |
I found that when rewriting The problem is that to rewrite the generics inside angle brackets, the program calls It seems to me that I have limited options:
// 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)
} |
@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. |
@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. |
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 |
@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 |
I'm seeing this behavior in rustfmt 1.6.0-nightly (c469197b 2023-08-22):
It's completely unclear to me what the expected behavior is at this point, but judging by #5577 (specifically this: #5577 (comment)), this is not what you want.
The text was updated successfully, but these errors were encountered: