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

rustfmt breaks a /* ... */ comment into /* ... //, with no terminating */ #6339

Open
ssbr opened this issue Sep 20, 2024 · 4 comments · May be fixed by #6358
Open

rustfmt breaks a /* ... */ comment into /* ... //, with no terminating */ #6339

ssbr opened this issue Sep 20, 2024 · 4 comments · May be fixed by #6358
Labels
a-comments bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce

Comments

@ssbr
Copy link

ssbr commented Sep 20, 2024

With version = "Two" and wrap_comments = true, rustfmt can break a /* ... */ comment by changing one of its lines to start with //. The error does not occur without version "Two" or without wrap_comments. Maybe worth noting, we have both of those options on at work.

(Sorry if this is a duplicate bug, couldn't find anything by searching for is:issue state:open "wrap_comments = true" "version = \"Two\"")

Reproduction steps:

cargo new test-crate
cd test-crate
echo 'version = "Two"
wrap_comments = true' > rustfmt.toml
echo 'use rustc_middle::ty::{self, Region, Ty, TyCtxt}; /* xx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
                                                   * xxxxxxxxxxxxx */

fn main() {}
' > src/main.rs
cargo +nightly fmt # or rustfmt +nightly src/main.rs
cat src/main.rs

On my computer, I get:

use rustc_middle::ty::{self, Region, Ty, TyCtxt}; /* xx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
// xxxxxxxxxxxxx

fn main() {}

Which is not valid syntax.

rustfmt version:

$ rustfmt +nightly --version
rustfmt 1.7.1-nightly (506f22b 2024-09-19)
@ytmimi
Copy link
Contributor

ytmimi commented Sep 21, 2024

Confirming I can reproduce this using rustfmt 1.8.0-nightly (6157568 2024-09-20), style_edition=20241 and wrap_comments=true. There have been similar issues reported before and I'll try to dig through the backlog later to find any that seem relevant here.

Input

use rustc_middle::ty::{self, Region, Ty, TyCtxt}; /* xx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
                                                   * xxxxxxxxxxxxx */

Output

use rustc_middle::ty::{self, Region, Ty, TyCtxt}; /* xx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
// xxxxxxxxxxxxx

Footnotes

  1. style_edition replaces version in newer rustfmt releases ↩

@ytmimi ytmimi added bug Panic, non-idempotency, invalid code, etc. a-comments only-with-option requires a non-default option value to reproduce labels Sep 21, 2024
@hlopko
Copy link

hlopko commented Sep 26, 2024

Thanks @ytmimi!

@dqkqd
Copy link

dqkqd commented Oct 5, 2024

Is it ok for me to work on this issue? @ytmimi

I think the bug occurred because we try to rewrite_comment on other_lines, which is a part of a multi line block comment.

rustfmt/src/missed_spans.rs

Lines 285 to 289 in 5f48fe9

let other_lines = &subslice[offset + 1..];
let comment_str =
rewrite_comment(other_lines, false, comment_shape, self.config)
.unwrap_or_else(|_| String::from(other_lines));
self.push_str(&comment_str);

I suppose a correct fix would be not separating comment lines in case of multi line block comment.
However, it will wrap comment in this case below as well.

Input

use Foo; /* x x x x x x
          * x x 
          * */

Output

use Foo; /* x x x x
          * x x x x
          * */

@ytmimi
Copy link
Contributor

ytmimi commented Oct 6, 2024

@dqkqd if you're interested in taking a look at this that would be great!

dqkqd added a commit to dqkqd/rustfmt that referenced this issue Oct 7, 2024
@dqkqd
Wrap multi lined trailing block comments (rust-lang#6339)
206b944
@dqkqd dqkqd linked a pull request Oct 7, 2024 that will close this issue
Open
dqkqd added a commit to dqkqd/rustfmt that referenced this issue Oct 10, 2024
@dqkqd
Wrap multi lined trailing block comments (rust-lang#6339)
7d9cbef
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 bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Wrap multi lined trailing block comments (#6339) dqkqd/rustfmt
4 participants
@ssbr @hlopko @dqkqd @ytmimi