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 incorrectly strips comments when removing empty where clause #4649

Open
vallentin opened this issue Jan 17, 2021 · 4 comments
Open

Comments

@vallentin
Copy link
Contributor

Empty where clauses are semantically valid Rust. However rustfmt incorrectly removes them and comments.

Before:

trait Foo {
    fn bar(&self)
    where
    //     Self: Bar
    // Some comment
    ;
}

fn foo<T>()
where
//     T: Bar,
// Some comment
{
    println!("foo");
}

After:

trait Foo {
    fn bar(&self);
}

fn foo<T>() {
    println!("foo");
}

rustfmt 1.4.25-stable (0f29ff6 2020-11-11)
I also tested it on master a97fd77

@vallentin vallentin added the bug Panic, non-idempotency, invalid code, etc. label Jan 17, 2021
@ChinYing-Li
Copy link
Contributor

Hi @calebcartwright is this a valid issue? I think I have an idea.

@calebcartwright calebcartwright added a-comments and removed bug Panic, non-idempotency, invalid code, etc. labels Mar 25, 2021
@calebcartwright
Copy link
Member

Hi @calebcartwright is this a valid issue? I think I have an idea.

@ChinYing-Li - yes we should never drop comments. I don't want to take it to the other end of the spectrum just yet though (leaving an empty where clause in the absence of comments), but we can and should certainly handle the case of comments between the empty clause and the start of the block.

@ralpha
Copy link

ralpha commented Oct 26, 2021

This issue also occurs when the where clause is not empty.
For example:

pub trait MyTrait
where
    Self: Sized,
// some comment
{

}

Once this is saved it will have saved:

pub trait MyTrait
where
    Self: Sized,
{

}

@davidBar-On
Copy link
Contributor

The original case with empty where is resolved by PR #5411.

The other reported case with non-empty where is a different issue and is probably the same issue as reported in #4672.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants