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

Formatting with merge_imports creates invalid Rust #3321

Closed
jonasbb opened this issue Feb 5, 2019 · 5 comments
Closed

Formatting with merge_imports creates invalid Rust #3321

jonasbb opened this issue Feb 5, 2019 · 5 comments
Labels

Comments

@jonasbb
Copy link

jonasbb commented Feb 5, 2019

As noted below the before version is already not valid Rust. However, it would be nice if it could be supported because I run into such cases during refactoring or adding new code.
I use merge_imports to keep the imports looking clean and sometimes I want to merge different sets of imports. Then it can happen that duplicates arise. This is normally handled well, in that duplicates are merged, just not here.

Before rustfmt:

use std::fmt::{self, Display};
use std::fmt;

fn main() {
    println!("Hello, world!");
}

After rustfmt:

use std::fmt::{
    self, {self, Display},
};

fn main() {
    println!("Hello, world!");
}

rustc error after formatting:

error[E0252]: the name `fmt` is defined multiple times
 --> src/main.rs:2:12
  |
2 |     self, {self, Display},
  |     ----   ^^^^ `fmt` reimported here
  |     |
  |     previous import of the module `fmt` here
  |
  = note: `fmt` must be defined only once in the type namespace of this module

rustfmt.toml

merge_imports = true
use_field_init_shorthand = true

rustc + rustfmt versions

$ rustc --version
rustc 1.34.0-nightly (f6fac4225 2019-02-03)
$ rustfmt --version
rustfmt 1.0.1-nightly (be13559 2018-12-10)
@topecongiro topecongiro added bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce p-low labels Feb 6, 2019
@topecongiro
Copy link
Contributor

Note that since the input is semantically incorrect, rustfmt is not guaranteed to produce the valid Rust code.

@jonasbb
Copy link
Author

jonasbb commented Feb 6, 2019

@topecongiro Thanks for pointing out that my example is already invalid code. It was late when I posted this.
I added a small comment to my first message, why I think supporting the merging of duplicate imports is nice, but I agree that it is a low priority.

@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

merge_imports is deprecated. Running this with imports_granularity=Crate produces:

Input

use std::fmt::{self, Display};
use std::fmt;

fn main() {
    println!("Hello, world!");
}

Outupt

use std::{
    fmt,
    fmt::{self, Display},
};

fn main() {
    println!("Hello, world!");
}

Trying to compile either the Input or the Output with rustc leads to compilation issues. @calebcartwright do you think there is any actions we can take here since use a; and use a::{self}; are different?

@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

Linking the imports_granularity tracking issue just in case #4991

@ytmimi ytmimi added the a-imports `use` syntax label Jul 20, 2022
@calebcartwright calebcartwright removed bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce a-imports `use` syntax labels Aug 25, 2022
@calebcartwright
Copy link
Member

I'm going to close as there's no action to be taken. The title is misleading, as the only thing that's happening is that rustfmt is not converting invalid rust code to valid rust code by deduping an import, but that is intentional as it's not a change rustfmt can make because doing so is not always semantics preserving and can actually convert valid rust code to invalid rust code.

@calebcartwright calebcartwright closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants