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

Rustfmt ate a single line comment in a long list of trait bounds #3669

Open
Lokathor opened this issue Jul 4, 2019 · 6 comments
Open

Rustfmt ate a single line comment in a long list of trait bounds #3669

Lokathor opened this issue Jul 4, 2019 · 6 comments
Labels
a-binop a-comments bug Panic, non-idempotency, invalid code, etc.

Comments

@Lokathor
Copy link

Lokathor commented Jul 4, 2019

I ran a rustfmt cleanup as a commit on a repo, found this in the diff.

Lokathor/randomize@103992e#diff-b316dc9cf67a100913b39ef102078235L48

Particularly:

-    + Into<u128> // Note(Evrey): Because Rust is drunk.
+    + Into<u128>

Now, in this particular case the comment was just a joke, but it could have been very important, and it might have even applied to that particular line in the chain of bounds, so the comment should stay in, and it should stay with that exact bound in the bound list.

@topecongiro topecongiro added a-comments bug Panic, non-idempotency, invalid code, etc. labels Jul 5, 2019
@nagisa
Copy link
Member

nagisa commented Aug 1, 2019

It will eat as many comments as you have, only leaving one if it appeared after, I think, the last bound.

@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

I believe the issue is that we don't handle comments between binary operators

@danjl1100
Copy link

danjl1100 commented Jun 11, 2023

I ran into this issue, and saw the link in the original post isn't working, so adding a playground link of what I'm seeing.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c43f042fc662356658bc4a662a1dc566

 trait Foo {}

-trait Bar<T> //: Foo
+trait Bar<T>
 where
     T: Copy,
 {
 }

The comment is deleted by rustfmt (1.5.2-nightly (2023-06-09 43062c4)) on the playground.

@calebcartwright
Copy link
Member

@fee1-dead - flagging this as another one for you in case it's something you'd be interested in, if/when you have bandwidth

I will also link a few issues/PRs that likely bare some relevance/overlap: #4666, #5059 and #2055

@connortsui20
Copy link

Is this a good first issue to try?

I'd like to try and take a stab at this. It seems like a lot of people have attempted this in the past, but none of them have been merged? I'll have to get more familiar with the code base, but I'm willing to learn, and I can base things off of previous attempts.

@ytmimi
Copy link
Contributor

ytmimi commented Nov 25, 2024

@connortsui20 Thanks for showing interest in this one. No, I wouldn't classify this as a good first issue, but if you're feeling motivated to tackle this and run into issue you can reach out on Zulip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-binop a-comments bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants