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

Group imports together with imports_granularity results in non-idempotent formatting #6195

Open
Kobzol opened this issue Jun 14, 2024 · 3 comments
Labels
a-imports `use` syntax bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce

Comments

@Kobzol
Copy link

Kobzol commented Jun 14, 2024

When group_imports = "StdExternalCrate" is combined with imports_granularity = "Module", rustfmt can format a file in a non-idempotent way, i.e. running rustfmt twice in a row results in a different formatting.

Reduced from a case in the stdlib (rust-lang/rust#126394).

Minimal repro:

# rustfmt.toml
version = "Two"
group_imports = "StdExternalCrate"
imports_granularity = "Module"
$ rustfmt --version
rustfmt 1.7.0-nightly (72fdf91 2024-06-05)
# file.rs
use a::c;
// foo
use a::b;
use a::d;

First run (rustfmt file.rs):

// foo
use a::b;
use a::{c, d};

Second run (rustfmt file.rs):

// foo
use a::{b, c, d};

Removing the group_imports or imports_granularity options removes the issue. The problem seems to be caused by the comment, without it it works fine.

@ytmimi ytmimi added bug Panic, non-idempotency, invalid code, etc. a-imports `use` syntax only-with-option requires a non-default option value to reproduce labels Jun 14, 2024
@ytmimi
Copy link
Contributor

ytmimi commented Jun 14, 2024

Linking the tracking issue for group_imports (#5083) and imports_granularity (#4991)

@nnethercote
Copy link
Contributor

I would argue that the second run shouldn't change anything -- that the comment on use a::b; should prevent it being merged with use a::{c, d};. This is how things behave with an attribute, as in this example:

use a::c;
#[cfg(windows)]
use a::b;
use a::d;

which ends up like this (even after multiple rustfmt invocations):

#[cfg(windows)]
use a::b;
use a::{c, d};

@Kobzol
Copy link
Author

Kobzol commented Jun 17, 2024

Yeah, I also like that better. In any case, any second run shouldn't change anything, that's a bug. Let's hope that it won't be occuring more often in the rustc codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-imports `use` syntax bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

No branches or pull requests

3 participants