Skip to content

organize imports action #5131

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
0x8f701 opened this issue Jun 29, 2020 · 20 comments
Open

organize imports action #5131

0x8f701 opened this issue Jun 29, 2020 · 20 comments
Labels
A-assists E-hard E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now

Comments

@0x8f701
Copy link

0x8f701 commented Jun 29, 2020

It'll be good to have this feature so that we can remove unused imports

EDIT:

instructions

@matklad matklad added the E-hard label Jul 11, 2020
@matklad
Copy link
Member

matklad commented Jul 11, 2020

cc #3301

@lnicola lnicola added A-assists S-actionable Someone could pick this issue up and work on it right now labels Jan 18, 2021
@TonalidadeHidrica
Copy link
Contributor

Actually, there are several operations I expect for "organize import" feature:

  • Remove all redundant imports, and remove curly brackets if unneeded
  • Merge all imports as much as possible, or split all imports, one identifier for one line
  • Sort (and maybe insert appropriate blank lines between groups) the imports (the first of which can be already done by cargo fmt)

@branpk
Copy link

branpk commented Mar 31, 2021

It would also be nice if local imports could be canonicalized, so that it consistently uses relative or absolute paths (or has a rule for determining which to use).

@dnut
Copy link

dnut commented Jun 18, 2021

I would follow the approach intellij uses, with imports grouped by standard library, external, and local, and alphabetized within each group.

@aloucks
Copy link
Contributor

aloucks commented Jan 16, 2022

I like the suggestions here (merging and sorting), but removal of unused imports is by far the what I'm missing the most.

@ocheret
Copy link

ocheret commented Apr 2, 2023

Is there any ongoing work on this issue?

@Ciel-MC
Copy link

Ciel-MC commented Apr 27, 2023

There is already diagnostic info and quick fix for unused imports, what is stopping this from happening?

@Veykril
Copy link
Member

Veykril commented Apr 27, 2023

A person willing to implement this (also not r-a itself does not yet diagnose unused imports, that's coming from the cargo checks)

@dnut
Copy link

dnut commented Apr 27, 2023

Clippy can remove unused imports:

cargo clippy --fix

There should be a way to bind this to a keyboard shortcut. Some ideas: https://stackoverflow.com/questions/52786022/shortcut-for-running-terminal-command-in-vs-code

Caveats:

  • applies to the entire workspace
  • changes a bunch of things that rust-analyzer does not change
  • does not work if you have local changes that have not been committed. The clippy maintainers seem to think this command might break your code, so use it carefully.

To deal with the last issue, my typical approach is to stage all my changes, so they are isolated from any additional changes. Then I run cargo clippy --fix --allow-staged. Then I review the unstaged changes from clippy. If it looks bad, I can revert them in isolation.

It would be ideal if rust-analyzer could clean up unused imports, but at least we have an alternative for now.

@Ciel-MC
Copy link

Ciel-MC commented Apr 27, 2023

Clippy can remove unused imports:

cargo clippy --fix

There should be a way to bind this to a keyboard shortcut. Some ideas: https://stackoverflow.com/questions/52786022/shortcut-for-running-terminal-command-in-vs-code

Caveats:

* applies to the entire workspace

* changes a bunch of things that rust-analyzer does not change

* does not work if you have local changes that have not been committed.

To deal with the last issue, my typical approach is to stage all my changes, so they are isolated from any additional changes. Then I run cargo clippy --fix --allow-staged. Then I review the unstaged changes from clippy. If it looks bad, I can revert them in isolation.

It would be ideal if rust-analyzer could clean up unused imports, but at least we have an alternative for now.

I am aware of cargo/clippy fix, but I don't want it to potentially nuke anything, but I couldn't figure out how to only fix a certain thing(unused imports), binding commands and stuff is the easiest part because... (neo)vim, lol.

@matklad
Copy link
Member

matklad commented Apr 28, 2023

Yeah, it feels like this should be doable. This is going to be a pretty large wad of code to add, but it seems to me that, at this point, we probably a reasonably precise analysis to make this worthwhile.

Some general pointers:

  • this probably wants to be an assist, there's a doc comment about what it is here
  • auto import might be a good read to learn about related machinery
  • Similarly, usage search is probably something we'll use here.

I think this feature can work in one of two ways:

  • In approach one, we record which imports do we use during analysis. So, eg, Body would get a field with the used imports, which is populated during lowering. import optimization than looks at all file's bodies (and whatever else can use imports), and computes the set of used things.
  • Alternative approach would be to work from search. If you have use foo::Bar, and Bar is a struct, and the file textually does not include the string Bar, then we can conclude that Bar is unused. If the Bar is a trait, then we should also look for mentions of trait's items.

Abstractly, I like the second approach much more, as it allows us to be significantly more lazy. For all unused imports, we can probably say that they are unused following the parse, and for used stuff it's enough to have just one confirmed used to rule out a false negative.

What makes me hesitant to try the second approach are macros. To textually find all uses, we have to expand the macros... But this is something our search infra has to deal with anyway.

So, yeah, my prior here is that building the feature on top of search is the way to go.

@matklad
Copy link
Member

matklad commented Apr 28, 2023

Ah, I realized that we already have a model of this workflow in the code, that's "remove unused param" assist:

https://github.com/rust-lang/rust-analyzer/blob/62c81d62932c458e23975191ecc124916be0b35e/crates/ide-assists/src/handlers/remove_unused_param.rs

So I think import optimization should have roughly the same skeleton, with just a tiny bit more meat on it :D

@matklad matklad added the E-has-instructions Issue has some instructions and pointers to code to get started label Apr 28, 2023
@obsgolem
Copy link
Contributor

I want to give this one a shot. If it turns out to be beyond me or I don't have the time I will give an update.

@obsgolem
Copy link
Contributor

obsgolem commented May 3, 2023

MR up!

@lem0nify

This comment was marked as spam.

bors added a commit that referenced this issue Aug 1, 2023
Added remove unused imports assist

This resolves the most important part of #5131. I needed to make a couple of cosmetic changes to the search infrastructure to do this.

A few open questions:
* Should imports that don't resolve to anything be considered unused? I figured probably not, but it would be a trivial change to make if we want it.
* Is there a cleaner way to make the edits to the use list?
* Is there a cleaner way to get the list of uses that intersect the current selection?
* Is the performance acceptable? When testing this on itself, it takes a good couple seconds to perform the assist.
* Is there a way to hide the rustc diagnostics that overlap with this functionality?
@igorakkerman
Copy link

I understand that the action is not fully completed yet. But is there a way to remove (all) unused imports at this point already (in VS Code) from any cursor position in the code and without scrolling?

The least disruptive solution I found so far is to open the problems view and to apply the assist on each problem.

@ospfranco
Copy link

CleanShot 2024-06-04 at 08 54 10

TIL you can half-way get this functionality. You just need to select the entire imports and then do + . and select remove unused imports

@nesrve
Copy link

nesrve commented Jun 19, 2024

any update on this? I'm so used to doing cmd + shift + o (which is the shortcut to the vscode "Organize Imports" command) to organize my imports in other languages like python, in rust it seems like this is already happening when you format the document but ideally I'd like to have an option to separate these actions.

@nmay231
Copy link

nmay231 commented Dec 23, 2024

@nervenes Could you clarify what you mean by "this is already happening when you format the document". I'm pressing ctrl+shift+I and still have unused imports. Yes, selecting the imports and pressing ctrl+. brings up the option, but similar to @igorakkerman 's request I want to be able to do this without scrolling to the top each and every time.

Is there a config in vscode or Cargo.toml that you need to enable to get this to work? Many thanks.

@yuhr
Copy link

yuhr commented Mar 30, 2025

How can I make “Remove all the unused imports” happen when running “Format Document”, so that I can also run it on save? Is there any option to enable it on format?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-assists E-hard E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests