Fix issue with extra semicolon when import comment precedes semicolon

[davidBar-On]

Fix an issue identified in #4737 evaluation, where an extra semicolon was added for import with comment preceding the ;. For example, with imports granularity Crate/Module/Item the following code:

use crate::bar::self /* bar 1 */;
use crate::bar::self;

was formatted as:

use crate::bar; /* bar 1 */;

The fix is extending handling that was done by extract_post_comment() to , after the comment, to be for the separator parameter instead of the constant ,.

The test files added include some additional tests for imports with comments, as a basis for tests for other imports comments issues. This is because 4 files are needed to test imports by granularity (Crate/Module/Item and default), so it seems that it is better to have all the imports comments related PRs tests per granularity in one file.

[calebcartwright]

Good spot! My guess is that this is just a long-standing bug from back in the day when commas were presumed to be the only separator and it was missed on some refactoring to allow custom separators for greater reuse. I'm also trying to think through any edge cases that are worth validating.

IIRC there's very few (maybe only 2?) cases where we itemize a list with a non-comma , separator: with the reorderable items and macro def branches being the 2 that come to mind, so could you also add a test case file that has a similar comment placement before the trailing semicolon on the end of a branch?

I think something as simple as this might suffice:

macro_rules! foo {
    () => {
        // something
    } /* asdf */  ;
    ($a: ident : $b: ty) => { $a(42): $b; } /* asdf */  ;
    ($a: ident $b: ident $c: ident) => { $a=$b+$c; } //evil comments
;
}

[davidBar-On]

could you also add a test case file that has a similar comment placement before the trailing semicolon on the end of a branch?

Currently, such comments are not included in the formatted output, with or without this PR change. Based on debug prints, I believe that this is because MacroBranch rewrite is not including comments between the end of self.whole_span and the end of self.span.

Should I include the test case file to make sure nothing was broken with this PR change? (The target file will be updated if and when such comments will be added to the output.)

Should I try to add such macro comments in another PR or it is not worth trying to handle such comments?

[calebcartwright]

I believe that this is because MacroBranch rewrite is not including comments between the end of self.whole_span and the end of self.span.

Ah okay fair enough

Should I include the test case file to make sure nothing was broken with this PR change? (The target file will be updated if and when such comments will be added to the output.)

No not now, as the best we could do would be a test that validates what would technically be bad/buggy behavior (the dropping of comments). If and when it ever becomes necessary to handle comments there (which would be a sad day) then we can incorporate tests at that time.

Should I try to add such macro comments in another PR or it is not worth trying to handle such comments?

No, I don't want to try to deal with this unless and until someone comes forward with a good use case on why they need to have comments there. It might be worthwhile to drop an inline comment somewhere relevant in the macro formatting code, but I wouldn't want to do anything more than that at this time.

[calebcartwright]

Thanks!