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

Deduplication doesn't occur with `imports_granularity = "Item" #4725

Closed
djmarcin opened this issue Feb 27, 2021 · 7 comments
Closed

Deduplication doesn't occur with `imports_granularity = "Item" #4725

djmarcin opened this issue Feb 27, 2021 · 7 comments
Labels
1x-backport:completed good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted only-with-option requires a non-default option value to reproduce poor-formatting

Comments

@djmarcin
Copy link

Describe the bug

Duplicate imports are not removed with imports_granularity = "Item" turned on.

To Reproduce

test.rs

use std::env;
use std::env;

fn main() {
  println!("{:?}", env::var("TEST"));
}

.rustfmt.toml

unstable_features = true
edition = "2018"
imports_granularity = "Item"

Run rustfmt test.rs

Expected behavior

Duplicate use std::env; should be removed. It is not.

Meta

  • rustfmt version: rustfmt 1.4.36-nightly (7de6968 2021-02-07)
  • From where did you install rustfmt?: asdf install with .tool-versions rust nightly-2021-02-17
  • How do you run rustfmt: rustfmt, in VSCode
@djmarcin djmarcin added the bug Panic, non-idempotency, invalid code, etc. label Feb 27, 2021
@calebcartwright calebcartwright added only-with-option requires a non-default option value to reproduce poor-formatting and removed bug Panic, non-idempotency, invalid code, etc. labels Feb 27, 2021
@calebcartwright
Copy link
Member

Thanks for opening this! We'd actually just observed this recently as well (#4716 (comment)) so good to have an issue to track 👍

@calebcartwright
Copy link
Member

For anyone interested in working on this, would suggest starting here given the current code structure. Note that the trees are not yet sorted at this point, as sorting conditionally happens a bit later on depending on the user specified value for the reorder_imports option. We need to dedupe regardless of the value of reorder_imports

pub(crate) fn flatten_use_trees(use_trees: Vec<UseTree>) -> Vec<UseTree> {
use_trees
.into_iter()
.flat_map(UseTree::flatten)
.map(|mut tree| {
// If a path ends in `::self`, rewrite it to `::{self}`.
if let Some(UseSegment::Slf(..)) = tree.path.last() {
let self_segment = tree.path.pop().unwrap();
tree.path.push(UseSegment::List(vec![UseTree::from_path(
vec![self_segment],
DUMMY_SP,
)]));
}
tree
})
.collect()
}

@calebcartwright calebcartwright added good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted labels Feb 27, 2021
@zaki-yama
Copy link

@calebcartwright
Hello! May I try this?

@calebcartwright
Copy link
Member

@calebcartwright
Hello! May I try this?

Sure! However, I think @davidBar-On may have beaten you to it in #4737. Will leave it to the two of you to sort who wants to work on it.

@davidBar-On
Copy link
Contributor

@zaki-yama, I suggest to see if my approach in PR #4737 to resolve the issue is accepted. If not, I have no problem that you will take over. (Usually I am not trying to resolve new help wanted assigned issues, but this one is somewhat related to #4681 which I submitted a PR for.)

@zaki-yama
Copy link

@calebcartwright @davidBar-On
Thank you for replying.
Sure! Since I'm new to try the issue in this repository, I'll see and learn from #4737.
Also, I'll see other help wanted or good first issue labeled issues.

@calebcartwright calebcartwright added 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release and removed good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted labels Mar 16, 2021
@calebcartwright
Copy link
Member

Resolved via 6b415bd, backport/release pending

@calebcartwright calebcartwright added good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted labels Mar 16, 2021
@calebcartwright calebcartwright added 1x-backport:completed and removed 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release labels Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1x-backport:completed good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted only-with-option requires a non-default option value to reproduce poor-formatting
Projects
None yet
Development

No branches or pull requests

4 participants