Use of incorrect width when rewriting nested doc comment can lead to doc comment exceeding the max width

[laralove143]

My config:

unstable_features = true
error_on_line_overflow = true
error_on_unformatted = true
format_code_in_doc_comments = true
format_strings = true
hex_literal_case = "Lower"
imports_granularity = "Crate"
normalize_comments = true
reorder_impl_items = true
reorder_imports = true
group_imports = "StdExternalCrate"
reorder_modules = true
use_field_init_shorthand = true
wrap_comments = true

Version: rustfmt 1.5.1-nightly (84f0c3f7 2022-09-03)

Hard to explain with text so here's a recording:
https://user-images.githubusercontent.com/82576556/188334072-8fb46dd0-e76c-46ce-8f03-8db6a08eb096.mov

It even "fixes" the string being split with \

[ytmimi]

@laralove143 could you please post the code snippet to help us try and reproduce the issue.

[laralove143]

I really can't reproduce this, it's weird I'll just send the file which still triggers this
http.rs.zip
I don't understand what's going on but I had an almost 1:1 clone of this code and it worked fine

[calebcartwright]

@laralove143 - please keep working at it and try to identify a minimal reproducible example yourself. We really don't like have to work with arbitrary archive files, and the video in the issue description doesn't work.

[laralove143]

What do you mean by doesn't work? And here, it's barely any different than the file with a few unused items removed

pub struct Request {
    required_permissions: u8,
    method: u8,
    endpoint: String,
}

impl Request {
    /// Creates a new `Request` from the given fields
    #[must_use]
    pub const fn new(required_permissions: u8, method: u8, endpoint: String) -> Self {
        Self {
            required_permissions,
            method,
            endpoint,
        }
    }
}

struct Foo;

impl Foo {
    /// ```rust
    /// #[derive(Clone)]
    /// struct Bar;
    ///
    /// impl Bar {
    ///     fn exec(self, guild_id: u8, auto_moderation_rule_id: u8) {
    ///         Foo.request_with_params(
    ///             Request::new(
    ///                 1,
    ///                 1,
    ///                 format!("/guilds/{guild_id}/auto-moderation/rules/{auto_moderation_rule_id}"),
    ///             ),
    ///             self,
    ///         )
    ///     }
    /// }
    /// ```
    fn foo(request: Request, params: impl Clone) {}
}

[calebcartwright]

And here, it's barely any different than the file with a few unused items removed

Thank you! Inline code snippets are always significantly more useful and appreciated than media.

What do you mean by doesn't work?

The video doesn't work, doesn't play/shows an error when trying to play, doesn't provide any utility in its current form, etc. It's moot now though, the code snippet is all that matters.

Rustfmt "fixes" my code then tells me an error about the code it broke itself

For awareness, there's a decent amount of subtext that comes across in choices of wording like this in the thread. That may be unintentional, but I just wanted to make note of it so that we can keep the conversation constructive and focused on technical aspects.

The error you are seeing is due to your usage of the still unstable option error_on_line_overflow, and quoting the docs for that option:

Error if Rustfmt is unable to get all lines within max_width, except for comments and string literals. If this happens, then it is a bug in Rustfmt. You might be able to work around the bug by refactoring your code to avoid long/complex expressions, usually by extracting a local variable or using a shorter name

So yes, there's a likely bug here (perhaps two actually), and that's surfacing because you've opted to use this unstable option. Many options remain unstable because there's known (or presumed bugs), so bug reports relating to unstable options are always helpful, thank you!

For anyone that wants to have a go at digging into this, I feel like there are at least two items worth exploring:

• error_on_line_overflow is supposed to ignore long string lits and comments. The offending line here happens to be code (a macro call) within a code block in a doc comment, so there's likely a question of whether this is intentional and whether this is desirable.
• It seems to me that a standard wrapping of the call itself across three lines would've sufficed to keep long string arg within the width boundaries. I'm not sure why that's not happening, though perhaps there's a bug in that code which isn't accounting for trailing tokens, e.g. comma, paren, etc.

[laralove143]

It could also be related to format_code_in_doc_comments

Also is there a way to enable error_on_line_overflow for every line, to force the comments, docs and strings to fit into width as aell

[ytmimi]

I took a look at this, and I believe the issue is that we're not setting the correct max_width When formatting the code in doc comments. rustfmt always use the smaller of max_width and doc_comment_code_block_width. By default they're both 100, so rustfmt will rewrite the code allowing lines up to 100 characters before wrapping.

rustfmt/src/comment.rs

Lines 733 to 736 in ee2bed9

	let comment_max_width = config
	.doc_comment_code_block_width()
	.min(config.max_width());
	config.set().max_width(comment_max_width);

In this case, rustfmt is just doing what it's designed to do. The issue is that rustfmt doesn't have 100 characters of width to use. It only has 92 (max_width(100) - indent(4) - "/// "(4)).

format_strings=true doesn't have any impact on the formatting of the snippet for the same reason. The snippet as originally written would fit within 100 characters so there's no need to wrap, but once we add the indentation and the doc comment opener the snippet won't fit into 92 characters and format_strings=true can start to work.

We can force the doc comment to be written with a max width of 92 in this case by setting doc_comment_code_block_width=92, but a proper fix would involve dynamically determining the max_width based on the indentation.

using rustfmt 1.5.1-nightly (ee2bed96 2022-11-08)
running: rustfmt --config=doc_comment_code_block_width=92,format_strings=true
Input

impl Foo {
    /// ```rust
    /// #[derive(Clone)]
    /// struct Bar;
    ///
    /// impl Bar {
    ///     fn exec(self, guild_id: u8, auto_moderation_rule_id: u8) {
    ///         Foo.request_with_params(
    ///             Request::new(
    ///                 1,
    ///                 1,
    ///                 format!("/guilds/{guild_id}/auto-moderation/rules/{auto_moderation_rule_id}"),
    ///             ),
    ///             self,
    ///         )
    ///     }
    /// }
    /// ```
    fn foo(request: Request, params: impl Clone) {}
}

output

impl Foo {
    /// ```rust
    /// #[derive(Clone)]
    /// struct Bar;
    ///
    /// impl Bar {
    ///     fn exec(self, guild_id: u8, auto_moderation_rule_id: u8) {
    ///         Foo.request_with_params(
    ///             Request::new(
    ///                 1,
    ///                 1,
    ///                 format!(
    ///                     "/guilds/{guild_id}/auto-moderation/rules/\
    ///                      {auto_moderation_rule_id}"
    ///                 ),
    ///             ),
    ///             self,
    ///         )
    ///     }
    /// }
    /// ```
    fn foo(request: Request, params: impl Clone) {}
}

This probably brings up a larger question about whether doc_comment_code_block_width should have a default of 100, since in the best case there will only ever be 96 characters ( or more generally max_width minus /// characters).

[ytmimi]

Linking tracking issue for doc_comment_code_block_width (#5415) and format_code_in_doc_comments (#3348)