Initial work on formatting headers

[fee1-dead]

This PR should be one of a series of PRs that formats headers. An item's header can include its keywords, its name, and its visibility. With generic header formatting facility, this would solve most, if not all issues with comments being deleted between keywords. In addition, this intends to replace the rewrite_* functions in utils.rs.

This PR is intentionally minimal, as I am not completely sure if this is desirable.

[calebcartwright]

Thanks for your continued efforts on improving rustfmt! I haven't looked closely at the code changes but did want to note that I think this is ultimately the type of direction we should take.

I'm not sure about the timing/how long it may take to get through this though as I'd like to see if the Style Team would want to weigh in on how certain constructs should be formatted in the presence of comments. The existing Style Guide at times has varying prescriptions depending on whether there's a comment, but in other cases simply says "don't put comments here" which leaves things a bit ambiguous for Rustfmt when it, inevitably, finds comments in such places

[ytmimi]

Also, I've recently revisited #5708, and I feel that centralizing this logic, would definitely help simplify that fix. I'm in agreement with Caleb on supporting this change!

[ytmimi]

Going to label this one as blocked for now until there's additional clarification from t-style, but I've reviewed the code and would feel comfortable merging this as is.

[fee1-dead]

Going to label this one as blocked for now until there's additional clarification from t-style, but I've reviewed the code and would feel comfortable merging this as is.

Per rust-lang/style-team#191 (comment) I think this is ready to merge?

[ytmimi]

@fee1-dead thanks for staying on top of this! I'll give this another look later this week. I don't think my thoughts have change, but I want to go over it one more time before merging.

[ytmimi]

After taking another pass at this I have two minor comments. Take a look when you get a chance.

[ytmimi]

Thanks for making those minor changes. I feel pretty good about this new header rewrite logic!

[ytmimi]

Just want to run the Diff-Check before merging.

Edit: Job passed ✅

[calebcartwright]

I've not looked at this in detail, but based on our style team discussion last week I'm not sure we should have merged this.

Is this applying formatting decision in the presence of all/certain comment types within the header? Or is it simply maintaining the existing content if comments are present?

If the former, then we need to revisit and potentially revert, as the specifics of what to do are still pending (rust-lang/rust#121762)

[ytmimi]

@calebcartwright From my review this preserves the comments that are already there, though some re-alignment of comments may be applied, and in the case of single-line block comments the header parts will be placed on one line. @fee1-dead only applied this updated logic to macro definitions, and it should be easy to revert if these changes are undesirable.

No changes:

macro_rules! // a
// b
// c
// d
my_macro {
    () => {};
}

macro_rules! /* e
f
g
h */
my_macro {
    () => {};
}

macro_rules! /* i
* j
* k
* l
*/
my_macro {
    () => {};
}

Comments are re-aligned:

input:

macro_rules!// a
  // b
    // c
 // d
my_macro {
    () => {};
}

output:

macro_rules! // a
// b
// c
// d
my_macro {
    () => {};
}

Single-line block comments cause header parts to be placed on a single line:

Input

macro_rules! /* a block comment */
my_macro {
    () => {};
}

macro_rules! /* a really really really really really really really really really really long comment */
my_macro {
    () => {};
}

Output

macro_rules! /* a block comment */ my_macro {
    () => {};
}

macro_rules! /* a really really really really really really really really really really long comment */ my_macro {
    () => {};
}

[calebcartwright]

Yes, I'd like to see this reverted until we've discussed our strategy on this front more holistically, because once we have rustfmt start doing something, we can't change it (at least not outside a style edition boundary).

I was asking about whether this was doing the same thing we do for expressions (e.g. if comment lost then use original contents), but I can see now that it's not, we're applying formatting here.

To be clear, I'm not saying the formatting we're applying is necessarily wrong, just that I think it's premature to be applying formatting logic with comments.

All that's been determined from a style perspective is (1) the style guide text should not be interpreted by formatters as a mandate to remove certain comments (which will likely necessitate updates to existing text for clarity) and (2) the style team is not going to prescribe style rules for the myriad comment types and shapes in every possible syntactical position, and as such the guide will give some level of license to formatters to make a best effort or skip formatting a construct in the some comment scenarios.

An outcome of both of those determinations is that on the rustfmt side we can now confidently update rustfmt to not drop comments (there was some ambiguity here previously), but there's still some TBDs on if/how formatting should be done if comments are present.

For example, the presence of a short, block style comment nestled within the header that doesn't change how the item would've been formatted without said comment e.g. macro_rules! /* a */ foo { ... , is something rustfmt can easily format. However, scenarios like a mutliline block style comment or a long comment that would push the ident beyond the width are significantly more complicated and nebulous (at least from my perspective), and if we're going to attempt some "best effort" formatting in those scenarios (as opposed to just maintaining the original content as-is) then we need to consider how we'll handle each of those scenarios (with tests) before we start applying such formatting

[calebcartwright]

And as a note for future discussion, when and where we do decide to make a best effort attempt at formatting in trickier comment scenarios, is that something we want to try to do consistently, if so, how consistently (e.g. consistent across the board? consistent within items,consistent within exprs, etc. but not necessarily consistent across those?)

[ytmimi]

Yes, I'd like to see this reverted until we've discussed our strategy on this front more holistically, because once we have rustfmt start doing something, we can't change it (at least not outside a style edition boundary).

Alright, I'll get a PR out a little later to revert this for now.

And as a note for future discussion, when and where we do decide to make a best effort attempt at formatting in trickier comment scenarios, is that something we want to try to do consistently, if so, how consistently (e.g. consistent across the board? consistent within items,consistent within exprs, etc. but not necessarily consistent across those?)

My initial reaction is to say that we should try to keep things consistent across the board, though that might be easier said than done. I'll definitely take some time to think more about this.

---

@fee1-dead take a look at ytmimi@3db08e1. Depending on how the comment discussion goes this might be one addition we could make to preserve the comments while still formatting whatever comes after the header.