Gives up on chains if any line is too long.

[ehuss]

If there is a method call chain, and just one call exceeds max_width, it seems like rustfmt gives up and doesn't format anything.

Example:

fn f() {
    foo("This text is under the max_width limit, and shouldn't cause any problems on its own.").long("But this line is extra long, and doesn't fit within 100 max_width. 1234567890123456789").baz().collect().unwrap();
}

No format changes will be applied to this. I'm also a bit surprised that error_on_line_overflow does not complain.

I would expect it to wrap each call chain element, even if one of them is too long, such as:

fn f() {
    foo("This text is under the max_width limit, and shouldn't cause any problems on its own.")
        .long("But this line is extra long, and doesn't fit within 100 max_width. 1234567890123456789")
        .baz()
        .collect()
        .unwrap();
}

All default settings.
rustfmt 1.4.8-nightly (afb1ee1 2019-09-08)

[calebcartwright]

Yeah, AFAICT that's because rustfmt is expecting to be able to rewrite each of the individual ChainItems to be able to format the Chain. When it attempts to rewrite that long string literal in the Chain it fails due to the length which results in the entire original chain being left as-is.

rustfmt/src/chains.rs

Lines 419 to 444 in a15e97f

	fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
	debug!("rewrite chain {:?} {:?}", self, shape);
	let mut formatter = match context.config.indent_style() {
	IndentStyle::Block => {
	Box::new(ChainFormatterBlock::new(self)) as Box<dyn ChainFormatter>
	}
	IndentStyle::Visual => {
	Box::new(ChainFormatterVisual::new(self)) as Box<dyn ChainFormatter>
	}
	};
	formatter.format_root(&self.parent, context, shape)?;
	if let Some(result) = formatter.pure_root() {
	return wrap_str(result, context.config.max_width(), shape);
	}
	// Decide how to layout the rest of the chain.
	let child_shape = formatter.child_shape(context, shape)?;
	formatter.format_children(context, child_shape)?;
	formatter.format_last_child(context, shape, child_shape)?;
	let result = formatter.join_rewrites(context, child_shape)?;
	wrap_str(result, context.config.max_width(), shape)
	}

I think I see a way we could support still wrapping chain elements like your expected output, though it would mean rustfmt emitting a line that exceeds the max_width (although the original line does too, and the wrapped version is more human friendly IMO even with the one longer line).

@topecongiro @scampi - any concerns with that? if not, then I'll take a look at implementing something for consideration.

[scampi]

@calebcartwright sounds good to me!

[agate-pris]

I think this issue is duplicate of #2970 .

[NilsIrl]

Using format_strings pretty much fixed it for me as strings get broken up and rustfmt can keep the line length below max_width.

[joshtriplett]

I just ran into this as well. My code:

fn main() {
    clap::App::new("foo")
        .subcommand(
            SubCommand::with_name("bar")
                .about("some text here")
                .setting(AppSettings::ArgRequiredElseHelp)
                .arg(
                    Arg::from_usage("-l, --latest 'some very long help text goes here [default: default version]'")
                                               .conflicts_with(  "template_version"  )
                )
        )
        .get_matches();
}

rustfmt seems to completely give up after the line with the long help text, and doesn't even try to reindent the .conflicts_with, or fix the spacing in its argument, or add a trailing comma in either of the two places that should have trailing commas.

I'd expect rustfmt to do best-effort on the long line, and then continue afterward.

[calebcartwright]

Doing something about this is still on my to-do list, though I do think it's worth expanding a bit on what folks have in mind when we say "best effort".

It seems this is most often encountered with a single long string, but will also note there are plenty of other scenarios that can run into this where IMO the expected behavior starts to get a little tricky. Consider for example a chain whose parent starts in a heavily indented position, and the chain element that exceeds the max width value is a closure param that itself has a sizeable body with additional nested indentation.

Would users still want rustfmt to format that closure anyway even though it blows way past the max width value? Are we changing the meaning/honoring of max width with a caveat of "except within chains"? Should rustfmt do this by default or should users have to explicitly opt-in to allowing rustfmt to exceed the max width when there are chains involved?

[joshtriplett]

@calebcartwright In general, I'd expect rustfmt to indent everything to the correct level, and try to wrap appropriately at line-width, but whether it's successful or not, it should continue on to wrap the rest, yes.

It's not "except within chains", it's "when possible, without violating more important constraints like properly indenting". If you have an indent-width of 4, and (say) 15 levels of indentation and a 50-character function name, you cannot format that without exceeding 100 characters, and that shouldn't stop you from continuing on to format more of the line. And if you need to wrap the function parameters, those should still get indented 16 levels, even if one of them is a long string.

[calebcartwright]

more important constraints like properly indenting

What defines the relative importance of one constraint vs. another though? Is there a consensus on which constraints can be violated? Does the style guide have a prescription? The only thing I'm familiar with (and I know you're far more familiar with the style guide than I @joshtriplett 😄) is https://github.com/rust-dev-tools/fmt-rfcs/blob/7416e12356a55aae959e9629b58eb7c7c3c86f6f/guide/guide.md#indentation-and-line-width

It's not that we can't technically make this change, but in these types of scenarios where rustfmt can't satisfy all the constraints it bails and defers to the programmer to format, or refactor, as needed. This is usually quite manageable, and often accompanied with advice like refactor your code to avoid long/complex expressions, usually by extracting a local variable or using a shorter name

If you have an indent-width of 4, and (say) 15 levels of indentation and a 50-character function name, you cannot format that without exceeding 100 characters

Precisely and agreed, I was just trying to keep the example in the context of chains given the issue.

There's also portions of the user base that do want to strictly enforce the width limits in their code and would leverage options like error_on_line_overflow to ensure that any violations of the width constraints were indeed raised, and any changes to the behavior would have to continue to honor that (an implementation detail that I'm just noting for future reference).

I also think that if max width were to be changed to more of a soft constraint then we'd need to explicitly convey that rustfmt will format your code within the limit, unless it really can't, in which case it will format out indentations indefinitely as needed.

[nazar-pc]

I very much agree with Josh here, but I think even bigger issue is that there are no visible errors/warnings that formatting is not possible and why.
It just silently gives up, leaving user guessing why this particular part of the code is not formatted correctly.

[calebcartwright]

@nazar-pc try running with --config error_on_line_overflow=true which exists for precisely this purpose (make sure you're on nightly), it's just disabled by default. If you'd like to make a case for that being enabled by default then #3391 would be the place to do so

example output with that enabled:

error: line formatted, but exceeded maximum width (maximum: 100 (see `max_width` option), found: 115)
 --> <stdin>:8:8:101
  |
8 |                     Arg::from_usage("-l, --latest 'some very long help text goes here [default: default version]'")
  |                                                                                                     ^^^^^^^^^^^^^^^
  |

warning: rustfmt has failed to format. See previous 1 errors.

[MonliH]

Another bug caused by this issue, that makes my editor's formatter, which uses rustfmt, anrgy (I'd imagine many more like this, it's happened to me in match statements as well):

Input:

fn main() {
    (
        foo, 
        "this is a long line that breaks rustfmt, let's make it even longer foo bar foo bar foo bar"
    )
}

Note the trailing space after foo,

Output:

error[internal]: left behind trailing whitespace
 --> /playground/src/main.rs:3:3:12
  |
3 |         foo, 
  |             ^
  |

warning: rustfmt has failed to format. See previous 1 errors.

Playground Link

[calebcartwright]

@MonliH - that's not really a separate bug or strictly related to this issue. Your original snippet has a trailing space which is the root cause (rustfmt isn't inserting a random trailing space), rustfmt is just leaving your original snippet as is and highlighting the fact that it did not remove the trailing spaces.

[Hugo-Trentesaux]

You do not need strings to get this bug:

// this does not format
let offenders = current_validators
    .into_iter()
    .enumerate()
    .filter_map(|(_, id)| 
    <<Runtime as pallet_im_online::Config>::ValidatorSet as ValidatorSetWithIdentification<
        sp_runtime::AccountId32>>::IdentificationOf::convert(id.clone()).map(|full_id| (id, full_id)))
    .collect::<Vec<IdentificationTuple<Runtime>>>();

// this type alias prevents the formatting bug
type XXX =
    <<Runtime as pallet_im_online::Config>::ValidatorSet as ValidatorSetWithIdentification<
        sp_runtime::AccountId32,
    >>::IdentificationOf;

// this does format
let offenders = current_validators
    .into_iter()
    .enumerate()
    .filter_map(|(_, id)| XXX::convert(id.clone()).map(|full_id| (id, full_id)))
    .collect::<Vec<IdentificationTuple<Runtime>>>();

And the giving up propagates to the whole block.

[Timarrr]

I would like an option to have max_width be the width of the line not including the indentation, or a switch, like relative_max_width

[quantenzitrone]

How has this bug been open for almost 5 years without a fix?
"Just giving up without any error" is unacceptable behavior for a formatter.

[matthiaskrgr]

Why don't you fix it yourself? 🤔

[quantenzitrone]

i don't have the required programming experience and the rust skills yet

[cyqsimon]

i don't have the required programming experience and the rust skills yet

So be respectful when you speak. Don't act like a self-entitled git (in the traditional sense of the word).

[orium]

Another example where a closure is part of the chain:

fn main() {
    let f = foo("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
        .bar(|| {
foo( 233545   );



             if   let Some   ( x)   = foo(3) {
   }
    });
}

[Lorak-mmk]

Do I understand correctly that if I have too long string literal then rustfmt will just ignore the whole function without any warning / error, and there is nothing that stable toolchain users can do about it because format_strings and https://rust-lang.github.io/rustfmt/?version=master&search=overflow#error_on_line_overflow are all unstable? And this situation is going on for 5 years now? Great...

[tylerlaprade]

Here's my suggestion:

Rustfmt should treat this case the same as Prettier, Biome, Black, Ruff, and I assume many other auto-formatters which I'm not familiar with. Namely, treat the max width as a target but not a strict maximum. They try their best to get lines to fit under that length, but if they can't, they can't and that's fine. They still get the line as short as possible.

[kolbma]

Here's my suggestion:

Rustfmt should treat this case the same as Prettier, Biome, Black, Ruff, and I assume many other auto-formatters which I'm not familiar with. Namely, treat the max width as a target but not a strict maximum. They try their best to get lines to fit under that length, but if they can't, they can't and that's fine.

It could write a warning to stderr, with a config option disable_warning_on_ignored_line.
Both existing options, next to only existing in nightly, have the problem, that you still have the problem with code styling.
If you set a longer max line length it restyles all your code.
If you set the error on overflow, it errors and you have to fiddle around to get the line shorter.
Which is also not so easy viewable, because IDE inserts virtually the types and makes lines virtually longer they are.

And overall I don't understand why it ignores the other lines of code block following the long line.
This looks like a bug and should have been fixed separately to introducing options as a workaround.

[theoparis]

Do I understand correctly that if I have too long string literal then rustfmt will just ignore the whole function without any warning / error, and there is nothing that stable toolchain users can do about it because format_strings and https://rust-lang.github.io/rustfmt/?version=master&search=overflow#error_on_line_overflow are all unstable? And this situation is going on for 5 years now? Great...

I tried using the following rustfmt.toml and nightly rust and it still doesn't seem to be formatting any of my code... I guess this still needs to be fixed in nightly rust as well.

format_strings = true
unstable_features = true