Incorrect formatting when reordering module declarations

[edg-l]

This:

pub mod a; // a
pub mod c; // c 
pub mod b; // b

Gets formated to:

pub mod a; // a
pub mod b;
pub mod c; // c // b

Using cargo fmt.

cargo 1.57.0 (b2e52d7ca 2021-10-21)
rustfmt 1.4.37-stable (f1edd04 2021-11-29)

found by @Patiga

[ytmimi]

Thanks for the report!

I can confirm that I can also reproduce the error on rustfmt 1.4.38-nightly (5056f4cf 2022-01-05).

This is a bit of a tricky one because comments aren't explicitly represented in the ast that rustfmt uses to reformat your code. Here's a high level overview of what's happening:

When going to rewrite and reorder the modules the Span starts from the start of the first module to the end of the last module. Because the trailing comments for mod a and mod c are within this span we find them easily, but we don't pick up on the trailing inline comment for mod b.

| start here
v
pub mod a; // a
pub mod c; // c 
pub mod b; // b
         ^
         | end here

Mentally You can think that rustfmt is grouping each mod with it's associated post comment (except for the last one because it's outside the span). Here's what those groups would look like:

pub mod a; // a | <-- end of item 1
pub mod c; // c | <-- end of item 2
pub mod b; | <--end of item 3 (remember we don't know about this comment because it's outside the span)

Then rustfmt reorders and writes the modules:

pub mod a; // a
pub mod b;
pub mod c; // c 

And lastly, rustfmt checks to see if there were any trailing comments that it might have missed and that's when it appends // b to the end of the now shuffled mod items because at that point there's no context that the trailing comment used to be associated with mod b.

pub mod a; // a
pub mod b;
pub mod c; // c // b

I hope that explanation of the issue helps! I've started looking into this, and I have a few ideas of how we might solve it, but I'll need a little more time to work out issues and fully test what I'm thinking!

[ytmimi]

In case anyone else wants to give this a go the Span I described above is located on Line 293 when calling FmtVisitor::walk_reorderable_or_regroupable_items, and what the fix will ultimately come down to is finding a reliable way to extend the span past any trailing comments.

rustfmt/src/reorder.rs

Lines 290 to 300 in 5056f4c

	if at_least_one_in_file_lines && !items.is_empty() {
	let lo = items.first().unwrap().span().lo();
	let hi = items.last().unwrap().span().hi();
	let span = mk_sp(lo, hi);
	let rw = rewrite_reorderable_or_regroupable_items(
	&self.get_context(),
	items,
	self.shape(),
	span,
	);
	self.push_rewrite(span, rw);

[calebcartwright]

Is this a duplicate of existing issues, e.g. #3720, #4027?

[ytmimi]

Yup, looks like this is a duplicate

[calebcartwright]

Going to close in that case to try to avoid dialog getting split across too many issues