Comments handling for BinOp and Lists and for Multiline

[davidBar-On]

This PR was indented to be for BinOp comments handling (a replacement to the previous try - #4417). However, because the changes touch general used functions, other features are affected, mainly general handling of Multiline comments and handling of Lists comments.

Some test cases files are added to test the changes. In addition, some changes were done to existing test cases so that the expected formatting output will be according to the changes done.

A main change is adding combine_strs_with_comments() which was split from combine_strs_with_missing_comments(), and allow general handling of string-comments combining, not just span-comments. In addition some parameters were added to allow more flexibility with the comments indentation. The multiline comments handling changes were mainly done in light_rewrite_comment().

As many changes are include in this PR, I understand that it may be difficult to test them to allow merging with the mainline. However, at least this PR can be used for to decide about the desired formatting of different cases. Once the output is agreed, it may be possible to merge subsets of these changes.

The main guidelines used for the comments indentations are:

1. Comments that start a new line in the original code will also start new line in the formatted code. E.g. the following code format should not change:

let x =
    /* Comment starts a new line */ 5;

2. Comment that ends a line in the original code will also end a line in the formatted code. Following code will always be in a new line. E.g. the following code format should not change:

let x = /* Comment ends a line */
    5;

3. Comments are trimmed and original prefix/suffix white spaces are ignore (except for line start and end for the above formatting).

4. Multiline comments are appended to a line or starts new line based on the width of the longest line in the comment (and not the first line width).

5. Comments at the end of a List items (Array, function parameters, match items, etc.), are aligned to the "Block" of items they belong to - consecutive items that include post-line one-line comments (not multiline). E.g. formatting output example:

	struct s {
		a: Bool,     // comment 1
		bbb: Binary, // comment 2
		ccccccc: bool,
		dddddddd: Binary, /* comment 3 */
		e: bool,          // comment 4
	}

6. For comments that follow each other, the first line of the first comment is formatted and indented as the code. The internal (non-first) comment lines indentation is:

   • If all the comments trimmed-left lines start with "//" or "/*", or '*' that follow at least one whitespace (i.e. " *"), the trimmed lines are indented as the first line, except that a ' ' is added before the '*' to the " *" lines.
   • Otherwise, the multiline internal lies are not formatted and are written as is (assuming the comment is a commented code).

   Formatting output example:

	fn main() {
        x = 6; /* Assignment comment 1st line
                * Assignment comment 2nd line */
        /* Code commented out:
		let y = 8;
             let z = 9;
                  let u = 20; */
	}

[calebcartwright]

Thank you for the PR @davidBar-On.

To be fully transparent, this is unlikely to be thoroughly reviewed and considered for merging any time soon. We've got limited bandwidth on the rustfmt team at the moment, and for the foreseeable future this PR would require too much of that bandwidth for something that's primarily adding binop comment support. If you believe supporting binop comments is going to require such a widespread impact on formatting then I feel the best tactical path forward for the time being is likely going to be maintaining the status quo.

I mentioned a couple things in #4417 that I want to reiterate here:

• Suggestions for major/significant changes to current formatting would likely be best discussed in an issue first. Users are much more likely to be able to discover discussions and weigh in on them on the issue tracker
• Please pay careful attention to how the markdown snippets are rendered in the GitHub UI where they are reviewed to ensure that the spacing and indentation are what you intended.

That being said, as it relates to the guidelines you enumerated in the PR description I do see several issues:

1. **Comments that start a new** line in the original code will also start new line in the formatted code.  E.g. the following code format should not change:

let x =
    /* Comment starts a new line */ 5;

2. **Comment that ends a line** in the original code will also end a line in the formatted code.  Following code will always be in a new line.  E.g. the following code format should not change:

let x = /* Comment ends a line */
    5;

Context matters here, which is why rustfmt handles comments differently depending on the context. For example, consider a function call with exaggerated line breaks for illustrative purposes:

foo(a, b,




           /* c */


            d, e);

4. **Multiline comments** are appended to a line or starts new line based on the width of the **longest** line in the comment (and not the first line width).

This is not a complete/sufficient characterization of the formatting rules. Other factors like position/width limits and potentially indent style are also relevant considerations.

5. **Comments at the end of a List items** (Array, function parameters, match items, etc.), are aligned to the "Block" of items they belong to - consecutive items that include post-line one-line comments (not multiline). E.g. formatting output example:

	struct s {
		a: Bool,     // comment 1
		bbb: Binary, // comment 2
		ccccccc: bool,
		dddddddd: Binary, /* comment 3 */
		e: bool,          // comment 4
	}

This is an incorrect articulation of expected/target behavior, but technically a correct description of current behavior due to a known bug that incorrectly defaults to visual alignment for these comments (#4108).

We want to fix the bug, not codify the buggy behavior as correct.

6. For **comments that follow each other**, the first line of the first comment is formatted and indented as the code.  The internal (non-first) comment lines indentation is:
   
   * If **all** the comments trimmed-left lines start with "`//`" or "`/*`", or '`*`' that follow at least one whitespace (i.e. "` *`"), the trimmed lines are indented as the first line, except that a ' ' is added before the '`*`' to the "` *`" lines.
   * Otherwise, the multiline internal lies are not formatted and are written as is (assuming the comment is a commented code).
   
   Formatting output example:

	fn main() {
        x = 6; /* Assignment comment 1st line
                * Assignment comment 2nd line */
        /* Code commented out:
		let y = 8;
             let z = 9;
                  let u = 20; */
	}

This formatting (indentation in particular) is explicitly incorrect. It's another case where I'm not sure if this is what you are actually suggesting, or if somehow what you've posted is out of sync with what you intended to suggest.

Finally, but quite important, is rustfmt's stability guarantee. Updated versions of rustfmt are not supposed to modify existing rustfmt-ed code, with exceptions in a few cases such as iterative formatting improvements for new syntax and rustfmt bug fixes. We cannot arbitrarily change comment handling rules such that users that rustup update ... && cargo fmt -- --check see failures/formatting changes

[davidBar-On]

@calebcartwright,

To be fully transparent, this is unlikely to be thoroughly reviewed and considered for merging any time soon. We've got limited bandwidth on the rustfmt team at the moment, and for the foreseeable future this PR would require too much of that bandwidth for something that's primarily adding binop comment support.

I fully understand. When submitting the PR I thought it may be useful for testing comments formatting approaches, but now I understand this is not realistic (except maybe for myself). Taking this into account, thanks for still taking the time for answering.

Suggestions for major/significant changes to current formatting would likely be best discussed in an issue first. Users are much more likely to be able to discover discussions and weigh in on them on the issue tracker

O.k. I will dig through the changes I did and see if and what specific concerns may be raised as separate issues.

Please pay careful attention to how the markdown snippets are rendered in the GitHub UI where they are reviewed to ensure that the spacing and indentation are what you intended.

This is strange. I assume you refer to **Comments that start a new** for example. However I see that as bold Comments that start a new text, so I am not sure what the problem is (I am using Firefox under Windows-10). (I already learned to carefully read GitHub messages in the "Preview" tab before sending them).

This formatting (indentation in particular) is explicitly incorrect. It's another case where I'm not sure if this is what you are actually suggesting, or if somehow what you've posted is out of sync with what you intended to suggest.

The code shown (repeated below) is what I suggested. Since the code starting let y = 8; ... is inside a comment, it is not formatted:

fn main() {
    x = 6; /* Assignment comment */
    /* Code commented out:
                    let y = 8;
             let z = 9;
                  let u = 20;
    */
}

Finally, but quite important, is rustfmt's stability guarantee. Updated versions of rustfmt are not supposed to modify existing rustfmt-ed code ....

I understand. Since I don't have enough knowledge to know what may be the impact of a modification on the rustfmt users, I will make sure to first discuss any suggestion.

[calebcartwright]

The code shown (repeated below) is what I suggested. Since the code starting let y = 8; ... is inside a comment, it is not formatted:

I'd be quite surprised if GH's rendering of the snippet I referenced from the issue description varies by browser/OS, but for reference here's what I see in Firefox on Linux. Note that none of the block members are indented

[davidBar-On]

Ok, My mistake. This is also what I see. Sorry for that. I through you are referring to the commented code. For some reason I didn't notice that the fn main() { is indented as the inner block. (In my last response, when I repeated a modified version of that code, the indentation was already o.k.,)

[calebcartwright]

Ok, My mistake. This is also what I see. Sorry for that.

No worries! It can happen pretty easily both when typing directly in GitHub as well as copy/paste. In a lot of projects the formatting of a snippet in GH doesn't really matter all that much, but obviously in a case like rustfmt every single space (or lackthereof) carries significant meaning 😄

[davidBar-On]

Closing the PR per the discussion. I will check current rustfmt output for the target test files added in this PR. In cases where output is different than what I expected I will open a new issue to discuss.