wrap_comments wraps too early

[benediktsatalia]

I have a very simple example where using wrap_comments lead to the doc line being wrapped even if it is exactly 80 characters.

/**
 * A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A
 */
fn main() {}

I played around with comment_width and it wrapped this line even with comment_width = 83 but stopped wrapping with 84 so somehow the wrapping is off by 4 here.

Also interesting if I do the same with 100 characters and comment_width = 100 it also wraps and if I increase the comment_width it still wraps always, regardless how high I set comment_width.

[ytmimi]

Thanks for the report!

Confirming I can reproduce this with rustfmt 1.6.0-nightly (e480739e 2023-08-17). I'll try to set aside some time next week to investigate what's going on here.

[ytmimi]

Linking the tracking issue for wrap_comments #3347

[ytmimi]

TLDR;

The logic for setting the max_width in CommentRewrite::new is off for multi-line block comments, but it should be accurate for line comments e.g. // comment or /// doc comment since there is no closer.

rustfmt/src/comment.rs

Lines 614 to 617 in f89cd3c

	let max_width = shape
	.width
	.checked_sub(closer.len() + opener.len())
	.unwrap_or(1);

Deep Dive

Here's what my investigation turned up. Let's start at rewrite_initial_doc_comments.

rustfmt/src/attr.rs

Lines 216 to 243 in f89cd3c

	fn rewrite_initial_doc_comments(
	context: &RewriteContext<'_>,
	attrs: &[ast::Attribute],
	shape: Shape,
	) -> Option<(usize, Option<String>)> {
	if attrs.is_empty() {
	return Some((0, None));
	}
	// Rewrite doc comments
	let sugared_docs = take_while_with_pred(context, attrs, |a| a.is_doc_comment());
	if !sugared_docs.is_empty() {
	let snippet = sugared_docs
	.iter()
	.map(|a| context.snippet(a.span))
	.collect::<Vec<_>>()
	.join("\n");
	return Some((
	sugared_docs.len(),
	Some(rewrite_doc_comment(
	&snippet,
	shape.comment(context.config),
	context.config,
	)?),
	));
	}
	Some((0, None))
	}

On L234-L238 above we call rewrite_doc_comment. The most important thing to note about the call is that we're deriving the shape based on the value of comment_width

Following the control flow from rewrite_doc_comment eventually leads to callingrewrite_comment_inner.

The first thing rewrite_comment_inner does is initialize a new CommentRewrite, and as mentioned above, CommentRewrite::new sets an incorrect max_width for multi-line block comments.

rustfmt/src/comment.rs

Lines 614 to 617 in f89cd3c

	let max_width = shape
	.width
	.checked_sub(closer.len() + opener.len())
	.unwrap_or(1);

In the default case, here's what everything is:

comment_width=80
shape.width=80
opener="/** " so opener.len()=4
closer=" */" so closer.len()=3

All of that means that max_width=73 and sure enough, if you check the length of "A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A" after wrapping and excluding the leading " * " is 73 characters long.

Thoughts on how to fix this

Deriving the max_width from the opener, closer, and shape makes sense to me for single line block comments as long as we don't need to wrap. If the comment is multi-lined or it's a single line string that exceeds the max_width then we know we'll need to wrap. As soon as we know we need to wrap then:

• only the first line needs a max_width of shape.width - opener.len()
• All lines before the last line could technically have a max_width of shape.width - CommentStyle::line_start().len()
• The last line probably needs a max_width of shape.width - line_start.len() - closer.len() just in case the */ is at the end of some text, but best case the last line only contains a */ so that shouldn't matter unless we're in an extremely nested context.

Comment rewriting happens line by line a loop so it should be fairly easy to modify the max_width before calling rewriter.handle_line using the approach outlined above:

rustfmt/src/comment.rs

Lines 942 to 946 in f89cd3c

	for (i, (line, has_leading_whitespace)) in lines.enumerate() {
	if rewriter.handle_line(orig, i, line, has_leading_whitespace, is_doc_comment) {
	break;
	}
	}

[ytmimi]

I don't think a fix for this would need to be version gated given that wrap_comments is an unstable config. Hopefully that's plenty of info for someone to come along and open a PR!