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.

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

wrap_comments sometimes introduces trailing whitespace into rustdoc #5421

Open
mqudsi opened this issue Jun 29, 2022 · 4 comments · May be fixed by #5288
Open

wrap_comments sometimes introduces trailing whitespace into rustdoc #5421

mqudsi opened this issue Jun 29, 2022 · 4 comments · May be fixed by #5288
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release a-comments e-trailing whitespace error[internal]: left behind trailing whitespace only-with-option requires a non-default option value to reproduce

Comments

@mqudsi
Copy link

mqudsi commented Jun 29, 2022

This is related to (but I believe separate from) #5420

Whatever situation that rustfmt finds itself in after failing to detect a cfg_attr-declared code block, rustfmt then randomly introduces trailing whitespace (which afaik should never happen for regular code, regular comments, regular rustdoc, rustdoc tests/examples).

Input, processed with wrap_comments = true:

//! This is a rustdoc comment that goes past the default allowed width of eighty characters. rustfmt will re-wrap this comment when run with `wrap_comments`
//! enabled.
//!
//! The preceding line does not have any whitespace and is a total of three backslashes followed by a new line (`\n`). This does not change after a
//! `rustfmt` run.
//!
//! Some examples of supported mathematical operations:
#![cfg_attr(not(feature = "std"), doc = "```ignore")]
#![cfg_attr(feature = "std", doc = "```")]
//! use size::Size;
//!
//! // No trailing whitespace is introduced on the line above
//! let s1 = Size::from_mib(13) / 2;
//! assert_eq!(s1, Size::from_mib(6.5_f32));
//!
//! // Nor is any trailing whitespace introduced on the line above or the line below, even though this comment exceeds the maximum supported width
//! let s3 = Size::from_mib(12) - Size::from_mib(14.2_f64);
//! assert_eq!(s3, Size::from_kib(-2252.8));
//! ```
//!
//! In this section of the module documentation, `rustfmt` will introduce a trailing whitespace on the line above after
//! it is executed with `wrap_comments = true`.
//!
//! `rustfmt` does not introduce trailing whitespace on the line above this comment, even though it too is rewrapped from its long length.

Output:

//! This is a rustdoc comment that goes past the default allowed width of eighty
//! characters. rustfmt will re-wrap this comment when run with `wrap_comments`
//! enabled.
//!
//! The preceding line does not have any whitespace and is a total of three
//! backslashes followed by a new line (`\n`). This does not change after a
//! `rustfmt` run.
//!
//! Some examples of supported mathematical operations:
#![cfg_attr(not(feature = "std"), doc = "```ignore")]
#![cfg_attr(feature = "std", doc = "```")]
//! use size::Size;
//!
//! // No trailing whitespace is introduced on the line above
//! let s1 = Size::from_mib(13) / 2;
//! assert_eq!(s1, Size::from_mib(6.5_f32));
//!
//! // Nor is any trailing whitespace introduced on the line above or the line
//! below, even though this comment exceeds the maximum supported width let s3 =
//! Size::from_mib(12) - Size::from_mib(14.2_f64); assert_eq!(s3,
//! Size::from_kib(-2252.8)); ```
//! 
//! In this section of the module documentation, `rustfmt` will introduce a trailing whitespace on the line above after
//! it is executed with `wrap_comments = true`.
//!
//! `rustfmt` does not introduce trailing whitespace on the line above this comment, even though it too is rewrapped from its long length.

It doesn't actually show on GitHub, but the second to last instance of an empty //! line has trailing whitespace after formatting that was not there before formatting.

This is the diff between the two with their spaces replaced with an interpunct for visibility into what's happening:

1c1,2
< //!·This is a rustdoc comment that goes past the default allowed width of eighty characters. rustfmt will re-wrap this comment when run with `wrap_comments`
---
> //!·This is a rustdoc comment that goes past the default allowed width of eighty
> //!·characters. rustfmt will re-wrap this comment when run with `wrap_comments`
4c5,6
< //!·The preceding line does not have any whitespace and is a total of three backslashes followed by a new line (`\n`). This does not change after a
---
> //!·The preceding line does not have any whitespace and is a total of three
> //!·backslashes followed by a new line (`\n`). This does not change after a
16,20c18,22
< //!·// Nor is any trailing whitespace introduced on the line above or the line below, even though this comment exceeds the maximum supported width
< //!·let s3 = Size::from_mib(12) - Size::from_mib(14.2_f64);
< //!·assert_eq!(s3, Size::from_kib(-2252.8));
< //!·```
< //!
---
> //!·// Nor is any trailing whitespace introduced on the line above or the line
> //!·below, even though this comment exceeds the maximum supported width let s3 =
> //!·Size::from_mib(12) - Size::from_mib(14.2_f64); assert_eq!(s3,
> //!·Size::from_kib(-2252.8)); ```
> //!·
@ytmimi
Copy link
Contributor

ytmimi commented Jun 30, 2022

I believe we also need format_code_in_doc_comments=true to trigger this

@ytmimi ytmimi added a-comments only-with-option requires a non-default option value to reproduce labels Jun 30, 2022
@mqudsi
Copy link
Author

mqudsi commented Jun 30, 2022

No, I didn’t have that option set in .rustfmt.toml, unless it is implicitly set somehow.

@ytmimi
Copy link
Contributor

ytmimi commented Jun 30, 2022

Apologies. You're correct. You don't need format_code_in_doc_comments=true to trigger this.

Just ran rustfmt --check --config=wrap_comments=true and got

error[internal]: left behind trailing whitespace
  --> <stdin>:22:22:4
   |
22 | //! 
   |    ^
   |
   = note: set `error_on_unformatted = false` to suppress the warning against comments or string literals

error[internal]: line formatted, but exceeded maximum width (maximum: 100 (see `max_width` option), found: 119)
  --> <stdin>:23:23:101
   |
23 | //! In this section of the module documentation, `rustfmt` will introduce a trailing whitespace on the line above after
   |                                                                                                     ^^^^^^^^^^^^^^^^^^^
   |
   = note: set `error_on_unformatted = false` to suppress the warning against comments or string literals

error[internal]: line formatted, but exceeded maximum width (maximum: 100 (see `max_width` option), found: 138)
  --> <stdin>:26:26:101
   |
26 | //! `rustfmt` does not introduce trailing whitespace on the line above this comment, even though it too is rewrapped from its long length.
   |                                                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: set `error_on_unformatted = false` to suppress the warning against comments or string literals

warning: rustfmt has failed to format. See previous 3 errors.

@ytmimi ytmimi added the e-trailing whitespace error[internal]: left behind trailing whitespace label Jul 22, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Jul 26, 2022

I think the trailing whitespace issue would be resolved when we backport #4304 in #5288

@ytmimi ytmimi added the 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release label Jul 26, 2022
@ytmimi ytmimi linked a pull request Jul 27, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release a-comments e-trailing whitespace error[internal]: left behind trailing whitespace only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants