Skip to content

fix: trim trailing whitespace from lhs when rhs wraps to new line#6881

Open
jlizen wants to merge 4 commits intorust-lang:mainfrom
jlizen:fix/trailing-whitespace-tuple-struct-with-vis
Open

fix: trim trailing whitespace from lhs when rhs wraps to new line#6881
jlizen wants to merge 4 commits intorust-lang:mainfrom
jlizen:fix/trailing-whitespace-tuple-struct-with-vis

Conversation

@jlizen
Copy link
Copy Markdown

@jlizen jlizen commented May 1, 2026

Fixes #6880, #5703 , #5525, #5997

rewrite_assign_rhs_with joins lhs + rhs. When the rhs wraps to a new line, trailing whitespace on the lhs (from format_visibility returning e.g. "pub(crate) ") gets left behind. The fix trims trailing whitespace from lhs when rhs starts with \n. Same fix applied to rewrite_assign_rhs_with_comments.

Every other caller passes an lhs ending in = or :, so the trim is a no-op outside the tuple struct field case.

Regression test covers all three parenthesized visibility qualifiers (pub(crate), pub(super), pub(in path)) on tuple struct fields with long types, plus pub and named struct fields as controls. Previously these tests failed before the fix due to trailing white space. (I couldn't figure out a good way to have an initial atomic commit showing the failure that still passed the test harness).

@ytmimi
Copy link
Copy Markdown
Contributor

ytmimi commented May 2, 2026

Previously we tried to address this in #5708, which took a more targeted approach and tried to update all the visibility formatting call sites instead of just conditionally checking the output of rewrite_assign_rhs_expr. Personally, I think that's how we should address this so please take a look at that PR for reference. You can keep the scope of the PR focused on tuple struct visibility if you'd like. Also, I first thought we'd have to gate this fix, but now I think we can consider this as a bugfix.

@ytmimi ytmimi self-assigned this May 2, 2026
@ytmimi
Copy link
Copy Markdown
Contributor

ytmimi commented May 2, 2026

@rustbot author

@rustbot rustbot removed the S-waiting-on-review Status: awaiting review from the assignee but also interested parties. label May 2, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 2, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. label May 2, 2026
@jlizen
Copy link
Copy Markdown
Author

jlizen commented May 2, 2026

Thanks for the backlinks! Sorry to miss the other issue... I tried to search x.x

It looks like there was a consensus on approach in 5708. Let me see how bad it is to freshen that up.

I do agree that this feels like a bugfix rather than something cutting across a version/>=2027 style boundary... As a user, I see rustfmt adding in trailing spaces itself, which I can't control and are clearly incorrect. We just need to be careful not to fix it in a way that has unwanted side effects.

Glad to add in a gate though if needed.

format_visibility returned e.g. "pub(crate) " with a trailing space.
Every call site either relied on this space as a separator or (in
format_header) explicitly trimmed it off.

Remove the trailing space from format_visibility and add it explicitly
at each call site. No change in formatted output.

This fixes the root cause of rust-lang#6880: tuple struct field prefixes no
longer carry a trailing space into rewrite_assign_rhs, so wrapping
the type to the next line can no longer leave trailing whitespace.
@jlizen jlizen force-pushed the fix/trailing-whitespace-tuple-struct-with-vis branch from fea9a7c to 33ad64a Compare May 3, 2026 18:02
jlizen added 3 commits May 3, 2026 18:16
Covers trailing whitespace on tuple struct fields with parenthesized
visibility qualifiers (pub(crate), pub(super), pub(in path)) when the
type wraps to the next line. Also includes pub with wrapping, named
struct fields, and a short tuple field as controls.

Fixes rust-lang#5703
Fixes rust-lang#6880
Covers the double space between pub and extern "C" in tuple struct
fields when arguments wrap to multiple lines.

Fixes rust-lang#5525
Covers the double space between pub and type in a newtype when an
attribute is followed by a comment.

Fixes rust-lang#5997
@jlizen jlizen force-pushed the fix/trailing-whitespace-tuple-struct-with-vis branch from 33ad64a to 0742c12 Compare May 3, 2026 18:19
@jlizen
Copy link
Copy Markdown
Author

jlizen commented May 3, 2026

I switched over to the approach from #5708 (removing the trailing space from format_visibility and adding it explicitly at each call site). It is basically just a port of the older PR rebased against main, except with no style edition gate. I think that's the right choice since this is a bugfix, but happy to add one if preferred.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. labels May 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trailing whitespace when wrapping long tuple struct fields with parenthesized visibility (pub(crate), pub(super))

3 participants