Cannot prohibit long code lines while allowing long doc lines

[detly]

I have code that contains this:

//! If that's what you have and it works, you don't need this crate at all — you
//! can just use [`CARGO_BIN_EXE_<name>`][cargo-env].
//!
//!   [cargo-env]: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates

The last line is 132 characters long. The code also contains this:

        fn get_cargo_env_or_panic(key: &str) -> OsString {
            std::env::var_os(key).unwrap_or_else(||
                panic!("The environment variable '{key}' is not set, is this running under a 'cargo test' command?")
            )
        }

The panic!() line is 117 characters long.

I would like to prohibit the overly long code line, because it can be fixed so that developers don't have to scroll the line to read it. This can be done with the following flags:

cargo fmt --all -- --check --config error_on_line_overflow=true --config error_on_unformatted=true

(Both are necessary.)

The good part is that these flags catch the line of code, which I can change to be formatted automatically. The bad part is that this flag also catches the URL in the comment, which I can't change to be formatted automatically.

To be honest, I'm not really sure what I'd expect, so here are some notes about it in no particular order:

• This seems related to Do not break long urls in comments. #506, which also mentions that Markdown should allow breaking a URL across lines. I can't find any mention of this in any Markdown flavour documentation, and I can't find out how to get Rustdoc to do it. Arguably this is an issue with Rustdoc, if such a thing should be possible but either isn't, or isn't documented.
• Also related is Gives up on chains if any line is too long. #3863.
• This is for running in CI, so using the flags and ignoring the error I don't care about won't work in that context.
• Perhaps what would fix this is more fine-grained options, but I don't know what for. Overlong comments vs overlong code? But I still want to catch overlong comments that can be reflowed. Should Rustfmt identify that it's a URL and it can't be reflowed? That seems extremely specific. Does Rustfmt even "understand" Markdown?

Versions:

• Cargo 1.62
• Rustfmt 1.4.38-stable
• Rustdoc 1.62.1

[ytmimi]

Thanks for reaching out.

As you correctly note we don't want to break URLs in comments #506. Here are a few other resolved URL issues that demonstrate scenarios where we used to incorrectly break links in comments (#3787, #4130, and #5095).

You also correctly note #3863. is preventing rustfmt from formatting the code.

Perhaps what would fix this is more fine-grained options, but I don't know what for.

I don't think we could provide an option to allow comment width to exceed the max_width, but maybe we could provide a new config like ignore_long_comment_errors to prevent rustfmt from emitting the error[internal]: line formatted, but exceeded maximum width error.

[detly]

Is there an expected way to handle this, out of curiosity?

[ytmimi]

Apologies, I'm not sure what you mean by expected. Do you mean what's the process for opening up a PR?

You can read the entire contributing guide, but the Configuration section should have all the details for adding a new configuration option.

The relevant code that determines if the formatted code contains an error is in FormatLines::new_line. By the time we reach this section we've already pushed all of our rewrites into a buffer and we're now checking to see if there are any issues with those rewrites (trailing whitespace, line overflows, etc.).

rustfmt/src/formatting.rs

Lines 551 to 560 in a7801aa

	// Check for any line width errors we couldn't correct.
	let error_kind = ErrorKind::LineOverflow(self.line_len, self.config.max_width());
	if self.line_len > self.config.max_width()
	&& !self.is_skipped_line()
	&& self.should_report_error(kind, &error_kind)
	{
	let is_string = self.current_line_contains_string_literal;
	self.push_err(error_kind, kind.is_comment(), is_string);
	}
	}

The FormatLines struct has a line_buffer, which contains the current line when new_line is called. Additionally there's a function in the comment mod called comment_style, which returns a CommentStyle that we can call is_doc_comment on to determine if the current line is a doc comment.

I think this is the best place to add some additional logic to determine if the doc comment line overflow is a hard error.

[calebcartwright]

error_on_line_overflow is used for code and doesn't bark if the offending long lines are comments or string lits
error_on_unformatted is the exact opposite, and only barks if the offending long lines are comments or string lits

At least that's how they are supposed to behave, though both options are still unstable and could have bugs that cause divergent behavior. My understanding is that the rational for including comments and lits together is that they have a massive overlap of reasons as to why they need to be long lines that can't readily be broken.

I think this is a case where as things currently stand you may want to also utilize the format_strings and/or wrap_comments, or perhaps consider not having a hard gate on error_on_unformatted and just running it periodically to get feedback that can then be assessed manually.

I'm not necessarily opposed to considering a comment vs. non-comment type of check down the road, but I don't see that happening any time soon.

[detly]

Apologies, I'm not sure what you mean by expected. Do you mean what's the process for opening up a PR?

I actually meant, how should a Rust developer follow the prescribed style here? How should they format the given comment? But the contributing information is useful too, thank you.

error_on_line_overflow is used for code and doesn't bark if the offending long lines are comments or string lits
error_on_unformatted is the exact opposite, and only barks if the offending long lines are comments or string lits

This was certainly not my experience with stable - removing either of those options caused both errors to disappear. Only both together catch them, and then they catch both.