fix(import_granularity): do not merge aliases

[sivizius]

Fixes #6027.

Related: #4991.

[sivizius]

use {
    super::harriblex,
    super::harriblex::{
        Iraquant,
        Mediviction
    },
};

should be merged to:

use super::harriblex::{self, Iraquant, Mediviction};

and not:

use super::{
    harriblex,
    harriblex::{Iraquant, Mediviction},
};

But my change neither introduced nor fixed this.
I will try to fix this as well, but this is technically a different issue and thus topic of a separate PR, right?

[ytmimi]

use {
    super::harriblex,
    super::harriblex::{
        Iraquant,
        Mediviction
    },
};

should be merged to:

use super::harriblex::{self, Iraquant, Mediviction};

and not:

use super::{
    harriblex,
    harriblex::{Iraquant, Mediviction},
};

But my change neither introduced nor fixed this. I will try to fix this as well, but this is technically a different issue and thus topic of a separate PR, right?

What you've highlighted is not an issue. I know there's been prior discussion on the topic, but I couldn't find the exact link I was thinking about. I was able to find #3750, which mentions the issue. Ultimately, use super::harriblex; and use super::harriblex::{self}; are semantically different, and we don't want to make any formatting changes that would break those semantics.

[sivizius]

I rebased it, is there anything else left to do?

[ytmimi]

I still need to take another look at this one. After I understood what the max_depth changes were doing I questioned whether or not a.equal_except_alias(b) was the right comparison to make in the take_while closure. Did you investigate whether or not it's possible to create a new function that takes the aliases into account when comparing a and b?

[ytmimi]

@sivizius I see that you recently force pushed. Did you have a chance to consider any alternative approaches to this instead of using max_depth? The main issue I can see with this approach is that it retains more duplicates than the current behavior:

Passing the following input to rustfmt via stdin (note that all use statements are duplicated) when running the command cargo run --bin rustfmt -- --config=imports_granularity=One:

pub use foo::x;
pub use foo::x as x2;
pub use foo::y;
use bar::a;
use bar::b;
use bar::b::f;
use bar::b::f as f2;
pub use foo::x;
pub use foo::x as x2;
pub use foo::y;
use bar::a;
use bar::b;
use bar::b::f;
use bar::b::f as f2;

Your PR produces the following:

use bar::{
    a, a,
    b::{self, self, f, f as f2, f, f as f2},
};
pub use foo::{x, x as x2, x, x as x2, y, y};

And the current behavior on the latest commit 8cb2820, produces this:

use bar::{
    a,
    b::{self, self, f},
};
pub use foo::{x, x as x2, y};

Also, the conversion from bar::b -> bar::{self}, in the above input seems incorrect to me. What's strange is that if we only have bar::b imports we don't convert bar:b -> bar::{self} and we don't merge aliases. This behavior is consistent for your PR and 8cb2820:

input

use bar::b;
use bar::b::f;
use bar::b::f as f2;

output

use bar::{
    b,
    b::{f, f as f2},
};

[sivizius]

@sivizius I see that you recently force pushed. Did you have a chance to consider any alternative approaches to this instead of using max_depth?

Not yet, but I started to work on this PR again and will take a look. I just rebased and pushed an up-to-date state yesterday. I had to setup a development-environment again, etc.

Ultimately, use super::harriblex; and use super::harriblex::{self}; are semantically different.

I recently tried the current nightly rustfmt (master, not this one) and apparently, use {foo, foo::bar}; was merged to use foo::{self, bar};? Was there a change?

[ytmimi]

I recently tried the current nightly rustfmt (master, not this one) and apparently, use {foo, foo::bar}; was merged to use foo::{self, bar};? Was there a change?

Not that I'm aware of, at least not a change that would have intentionally produced that result. For what it's worth, I can also reproduce that behavior when running rustfmt from source (cargo run --bin rustfmt -- --config=imports_granularity=One).

That said, this seems like a separate issue from merging aliases, and can probably be addressed in it's own PR.