fix panic on failure to format generics in enum

[ding-young]

fixes #5738

Description

Above issue reports panic when formatting complex enum with generic parameters. This is because rustfmt simply calls unwrap() on Rewrite (RewriteResult in future), the return value from format_generics. It is better not to panic on this sort of rewrite failure. ~Therefore, this pr restores original snippet when we fail to format generics in enum instead of calling unwrap().~

• This pr returns None early and then leave entire enum unformatted instead of calling unwrap().
• visit_enum is refactored with format_enum that returns Rewrite. Later pr will replace this rewrite with RewriteResult

TODO

• [x] Revisit test case
• Any other solutions ? Calling existing functions provided in visitor.rs?
• check whether version gate is required (originally it would've panic, so I'm not sure if this is a formatting change)
• [x] check other related issues like #6378

[ytmimi]

@ding-young thanks for picking this one up! I wonder if now would be a good time to refactor visit_enum, and introduce a format_enum function, which we can return a RewriteResult from, similar to how visit_static and visit_struct are implemented. Then if we fail to format the generics we can just return early with an error and leave the enum unformatted.

Let me know what you think?

[ytmimi]

Can we also add this smaller reproducible case from the original issue:

enum Node where P::<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S<S>>>>>>>>>>>>>>>>>>>>>>>>:  {
    Cons,
}

[ytmimi]

@ding-young there were also some linked issues like https://github.com/rust-lang/rustfmt/issues/6137, https://github.com/rust-lang/rustfmt/issues/6318, https://github.com/rust-lang/rustfmt/issues/6378. Would be good to check these issues out as well. https://github.com/rust-lang/rustfmt/issues/6378 doesn't seem to have a minimal test case so feel free to ignore that one.

[ding-young]

Thank you for listing all the related issues! I included only the cases that originally lead to a rustfmt crash.

[ding-young]

About refactoring visit_enum, I think introducing format_enum would make it easier to propagate errors later, as we wouldn't need to visit all the push_str and push_rewrite calls scattered throughout. I personally feel it may be better for users to have their enum variants formatted even if generics aren’t, and vice versa, while listing all rewrite errors if possible. Anyway, I think we can leave that for later consideration.

I still need to take a closer look to ensure the original behavior is preserved, so I’ll leave it as a draft, but it would be great if you could briefly check whether I’m refactoring in the right direction.

[ytmimi]

@ding-young thanks for looking into this refactor. I won't have time to review this PR this weekend, but I'll try to set aside some time for a review next week.

[TheLostLambda]

Is this PR still the primary PR for fixing this bug? Or #6630? It would seem I'm another person who has hit the same issue! This PR fixes Rustfmt for me, but without it I can't format the project! Let me know if there is anything I can do to help move things forward!

[ding-young]

@avrabe Hi! I've added your test case on #6630. Haven't run it on your repository, though. Would you take a look?

@TheLostLambda I lost some contexts on my pr, but maybe this pr is the primary fix, I guess. To push this change pr forward, we’ll need to find someone who has permission to merge it. I guess the core maintainers have been busy lately.

[ding-young]

Would you mind reviewing this? @camsteffen @jieyouxu