Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Doc comments on use declarations are limited to 5 characters less than max_width #5914

Open
taylordotfish opened this issue Sep 17, 2023 · 6 comments
Open

Doc comments on use declarations are limited to 5 characters less than max_width #5914

taylordotfish opened this issue Sep 17, 2023 · 6 comments
Labels
a-comments only-with-option requires a non-default option value to reproduce

Comments

@taylordotfish
Copy link

If wrap_comments is true, doc comments on use declarations are wrapped to 5 characters less than max_width (or comment_width, if it is less than max_width - 5). This makes it impossible to wrap such comments to the same value as max_width.

Example:

test.rs:

/// 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 a a a a a a
/// aa 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 a a a a a
pub use u32;

/// 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 a a a a a a
/// aa 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 a a a a a
pub struct X;

After running rustfmt --config wrap_comments=true,comment_width=79,max_width=79 test.rs:

/// 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 a a a a a a aa 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 a a a a a
pub use u32;

/// 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 a a a a a a aa 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 a a a a a
pub struct X;

The comment on the use declaration is wrapped to 74 characters, exactly 5 less than 79. This five-character difference persists across different values of comment_width and max_width, except if comment_width is more than 5 characters less than max_width, both doc comments will be limited to that value.
@taylordotfish
Copy link
Author

This issue presents similarly to #5890, but I don't know if the cause is similar—I would expect both doc comments to have the same internal shape.

@ytmimi
Copy link
Contributor

ytmimi commented Sep 20, 2023 •
edited
Loading

Thanks for reaching out. #5890 only applies to multi-line block comments so it's unrelated.

The issue here is that we're passing the nested_shape to UseTree::rewrite_top_level, and that shape gets used to rewrite both the import and the attributes.

rustfmt/src/reorder.rs

Lines 126 to 141 in da7f678

// 4 = "use ", 1 = ";"
let nested_shape = shape.offset_left(4)?.sub_width(1)?;
let item_vec: Vec<_> = regrouped_items
.into_iter()
.filter(|use_group| !use_group.is_empty())
.map(|use_group| {
let item_vec: Vec<_> = use_group
.into_iter()
.map(|use_tree| ListItem {
item: use_tree.rewrite_top_level(context, nested_shape),
..use_tree.list_item.unwrap_or_else(ListItem::empty)
})
.collect();
wrap_reorderable_items(context, &item_vec, nested_shape)
})
.collect::<Option<Vec<_>>>()?;

I can think of two possible approaches to fixes this:

  1. Move the nested_shape derivation into UseTree::rewrite_top_level (which might also obviate the need for Use the same shape to rewrite import when reordering them or not #5307)
  2. Pass both the nested_shape for rewriting imports and the original shape for rewriting attributes to UseTree::rewrite_top_level

@ytmimi ytmimi added a-comments only-with-option requires a non-default option value to reproduce labels Sep 20, 2023
@ytmimi
Copy link
Contributor

ytmimi commented Sep 20, 2023

linking the tracking issue for wrap_comments (#3347)

@taylordotfish
Copy link
Author

Is this bug related then?

/* 50 characters:
0        1         2         3         4         5
12345678901234567890123456789012345678901234567890
*/
use aaaaaaaaa::{bbbbbbbbb, ccccccccc, ddddddddd};

After rustfmt --config max_width=50:

/* 50 characters:
0        1         2         3         4         5
12345678901234567890123456789012345678901234567890
*/
use aaaaaaaaa::{
    bbbbbbbbb, ccccccccc, ddddddddd,
};

The use declaration is only 49 characters but gets wrapped anyway. This also happens with the default max_width of 100 (use declarations > 98 characters get wrapped).

@ytmimi
Copy link
Contributor

ytmimi commented Sep 22, 2023

Is this bug related then?

@taylordotfish It's possible, though my guess is that they're not related since your original issue describes doc comments on imports wrapping too early when using warp_comments=true, while this second case is about rustfmt wrapping nested use declarations too early when close to the max_width boundary.

I very much appreciate you taking the time to check other edge cases 🙏🏼. Probably best to create a new issue so we can track that separately.

@taylordotfish
Copy link
Author

@ytmimi Sure, created #5927.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-comments only-with-option requires a non-default option value to reproduce
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ytmimi @taylordotfish