Imports are not formatted when long import lines are present

[ciphergoth]

On some inputs containing long lines, rustfmt makes no changes, either to those lines or any other imports. Using shorter identifiers fixes the problem.

To Reproduce

Run rustup run nightly rustfmt --verbose --config imports_granularity=Crate --emit stdout on this input file:

use foo::bar;
use foo::{quuz, baz};
use abaadfsasdfdsfdfas::aasdffjsioejr::abc::sdsdf::sdfsdfsdf::sdfsdfdsf::{
    asdfasdefasdasdfsdfdfasdf::asdfasdasedfafasdfasdf,
};

The input file will be returned unchanged.

Expected behavior

Delete the final s from the initial abaadfsasdfdsfdfas and the correct behavior is seen; the output is

use abaadfsasdfdsfdfa::aasdffjsioejr::abc::sdsdf::sdfsdfsdf::sdfsdfdsf::asdfasdefasdasdfsdfdfasdf::asdfasdasedfafasdfasdf;
use foo::{bar, baz, quuz};

I can reproduce the same behavior with merge_imports=true on stable.

Meta

• rustfmt version: rustup run nightly rustfmt --version returns rustfmt 1.4.36-nightly (7de6968 2021-02-07).
• From where did you install rustfmt?: rustup
• How do you run rustfmt: rustfmt

[ciphergoth]

Done some more testing, and no option is needed to trigger this issue. If the first character of the last name in a format would go over the end of the line, then imports are not formatted. Try this with rustfmt --emit stdout test.rs

use z123::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::asdf;
use b;
use a;

It is output unchanged. However, bump up the line width by one, and it correctly sorts the imports into alphabetical order: rustfmt --config max_width=101 --emit stdout test.rs:

use a;
use b;
use z123::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::asdf;

[ciphergoth]

Hey, can I get the only-with-option tag removed from this issue, now we know that it can be reproduced with no options? Thanks!

[calebcartwright]

Hey, can I get the only-with-option tag removed from this issue, now we know that it can be reproduced with no options? Thanks!

Sure, although worth noting that the labels are really just for categorization, and this one primarily as a reminder when adding tests; the presence/absence of the only-with-option label has no impact on priority, etc.

Another thing to keep in mind is that it was a conscious decision in many cases to have rustfmt bail and revert to the original formatting in cases where it's impossible to format within the max_width constraints. I know that's not ideal in many cases and there's been some active discussions about changing that behavior, but it's not a minor change/quick fix.

[ciphergoth]

Ah, fair, yep I'd assumed it was a prioritization thing.

For my example, one way to format such an import within max_width would be something like:

use a;
use b;
use z123::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::z1234567::{
    z1234567::asdf,
};

Do you think that would be acceptable? If so, how difficult do you think it might be for someone new to the project to make this change? Right now the imports of my project are a bit of a mess and I'd love to automate cleaning them up, but we're blocked on that by this issue.

[calebcartwright]

Do you think that would be acceptable? If so, how difficult do you think it might be for someone new to the project to make this change? Right now the imports of my project are a bit of a mess and I'd love to automate cleaning them up, but we're blocked on that by this issue.

Deepest apologies @ciphergoth, this must've slipped through a crack in the notifications inbox.

I think we'd have to version gate any change, but I do think your proposal may align closely enough the the Style Guide prescriptions for imports to be viable.

I'm not sure I can quantify the difficulty as I'm not sure what the root cause is. It seems to only popup in the presence of a tree, so I'd probably suggest starting somewhere around the location linked below. I suppose there's a chance it could be a simple fix, but imports can start to get complicated quickly so I also wouldn't be surprised if it ends up being involved and complex

rustfmt/src/imports.rs

Lines 967 to 983 in 5fa2727

	impl Rewrite for UseTree {
	// This does NOT format attributes and visibility or add a trailing `;`.
	fn rewrite(&self, context: &RewriteContext<'_>, mut shape: Shape) -> Option<String> {
	let mut result = String::with_capacity(256);
	let mut iter = self.path.iter().peekable();
	while let Some(segment) = iter.next() {
	let segment_str = segment.rewrite(context, shape)?;
	result.push_str(&segment_str);
	if iter.peek().is_some() {
	result.push_str("::");
	// 2 = "::"
	shape = shape.offset_left(2 + segment_str.len())?;
	}
	}
	Some(result)
	}
	}