feat: (LSP) remove unused imports (#6129)

# Description

## Problem

Part of #1579

## Summary

This implements another commonly used feature: "remove unused import/s".

This is when the entire `use` item is removed:


![lsp-remove-unused-imports-1](https://github.com/user-attachments/assets/7b10d429-5e31-4f05-bbc2-ba87e7ccf9bf)

This is when part of the `use` item is removed:


![lsp-remove-unused-imports-2](https://github.com/user-attachments/assets/01a819a6-1790-4701-8820-b0d6188c5497)

This is when "Remove all the unused imports" is chosen with a selection
range:


![lsp-remove-unused-imports-3](https://github.com/user-attachments/assets/9797d88d-960e-4da5-8d11-913431741381)

## Additional Context

It works exactly the same as Rust Analyzer:
1. You get a "Remove unused import", "Remove unused imports" or "Remove
the entire `use` item" on each `use` item that has unused import(s)
2. You also get a "Remove all the unused imports" code action. This will
remove all unused imports in the current selection.

I need to clarify 2 because it's something I learned while coding this:
a code action is suggested in the selection you have (the selection is
the cursor in case there's no selection, though VSCode would sometimes
use the current line's error span as the selection). That's why if you
have the cursor on an import and choose "Remove all the unused imports"
it just removes the imports in the current `use` item. You can get
VSCode to actually remove **all** the unused imports if you select all
the `use` items, then ask for code actions, then choose "Remove all the
unused imports". To be honest this is kind of annoying, but it kind of
also makes sense. We _could_ make "Remove all the unused imports" remove
all of them regardless of what's the selection (or, well, if at least
one selection has unused items) but we could do that as a follow-up.

---

Other things to know:
1. This works by creating a new `UseTree` from the original `UseTree`
but with imports removed, then converting that to a string (formatting
it with `nargo fmt`). What happens if there were comments in the middle
of the import? They are lost! But... this is also what Rust Analyzer
does. I guess worse case scenario is you revert that code action and
manually remove things (comments in these positions are very unlikely to
happen).
2. I didn't test the "Remove all the unused imports" action because
right now our tests don't support a selection range. But given that the
logic for that is relatively simple (just get all the "remove unused
import" edits and present that as a new code action) I thought it was
okay not to test it (I did try it out, though). We could eventually add
tests for this, though.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

compiler/noirc_frontend/src/ast/statement.rs

@@ -330,9 +330,13 @@ impl Display for UseTree {
 
         match &self.kind {
             UseTreeKind::Path(name, alias) => {
+                if !(self.prefix.segments.is_empty() && self.prefix.kind == PathKind::Plain) {
+                    write!(f, "::")?;
+                }
+
                 write!(f, "{name}")?;
 
-                while let Some(alias) = alias {
+                if let Some(alias) = alias {
                     write!(f, " as {alias}")?;
                 }
 

compiler/noirc_frontend/src/ast/visitor.rs

@@ -271,7 +271,7 @@ pub trait Visitor {
         true
     }
 
-    fn visit_import(&mut self, _: &UseTree, _visibility: ItemVisibility) -> bool {
+    fn visit_import(&mut self, _: &UseTree, _: Span, _visibility: ItemVisibility) -> bool {
         true
     }
 
@@ -506,7 +506,7 @@ impl Item {
             }
             ItemKind::Trait(noir_trait) => noir_trait.accept(self.span, visitor),
             ItemKind::Import(use_tree, visibility) => {
-                if visitor.visit_import(use_tree, *visibility) {
+                if visitor.visit_import(use_tree, self.span, *visibility) {
                     use_tree.accept(visitor);
                 }
             }

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -499,7 +499,7 @@ impl DefCollector {
         crate_id: CrateId,
         errors: &mut Vec<(CompilationError, FileId)>,
     ) {
-        let unused_imports = context.def_interner.usage_tracker.unused_items().iter();
+        let unused_imports = context.def_interner.unused_items().iter();
         let unused_imports = unused_imports.filter(|(module_id, _)| module_id.krate == crate_id);
 
         errors.extend(unused_imports.flat_map(|(module_id, usage_tracker)| {

compiler/noirc_frontend/src/node_interner.rs

@@ -28,6 +28,7 @@ use crate::hir::type_check::generics::TraitGenerics;
 use crate::hir_def::traits::NamedType;
 use crate::macros_api::ModuleDefId;
 use crate::macros_api::UnaryOp;
+use crate::usage_tracker::UnusedItem;
 use crate::usage_tracker::UsageTracker;
 use crate::QuotedType;
 
@@ -2249,6 +2250,12 @@ impl NodeInterner {
     pub fn doc_comments(&self, id: ReferenceId) -> Option<&Vec<String>> {
         self.doc_comments.get(&id)
     }
+
+    pub fn unused_items(
+        &self,
+    ) -> &std::collections::HashMap<ModuleId, std::collections::HashMap<Ident, UnusedItem>> {
+        self.usage_tracker.unused_items()
+    }
 }
 
 impl Methods {

tooling/lsp/src/requests/code_action.rs

@@ -1,6 +1,7 @@
 use std::{
     collections::{BTreeMap, HashMap},
     future::{self, Future},
+    ops::Range,
 };
 
 use async_lsp::ResponseError;
@@ -11,7 +12,7 @@ use lsp_types::{
 };
 use noirc_errors::Span;
 use noirc_frontend::{
-    ast::{ConstructorExpression, NoirTraitImpl, Path, Visitor},
+    ast::{ConstructorExpression, ItemVisibility, NoirTraitImpl, Path, UseTree, Visitor},
     graph::CrateId,
     hir::def_map::{CrateDefMap, LocalModuleId, ModuleId},
     macros_api::NodeInterner,
@@ -28,7 +29,7 @@ use super::{process_request, to_lsp_location};
 mod fill_struct_fields;
 mod implement_missing_members;
 mod import_or_qualify;
-#[cfg(test)]
+mod remove_unused_import;
 mod tests;
 
 pub(crate) fn on_code_action_request(
@@ -43,7 +44,7 @@ pub(crate) fn on_code_action_request(
     let result = process_request(state, text_document_position_params, |args| {
         let path = PathString::from_path(uri.to_file_path().unwrap());
         args.files.get_file_id(&path).and_then(|file_id| {
-            utils::position_to_byte_index(args.files, file_id, &position).and_then(|byte_index| {
+            utils::range_to_byte_span(args.files, file_id, &params.range).and_then(|byte_range| {
                 let file = args.files.get_file(file_id).unwrap();
                 let source = file.source();
                 let (parsed_module, _errors) = noirc_frontend::parse_program(source);
@@ -53,7 +54,7 @@ pub(crate) fn on_code_action_request(
                     args.files,
                     file_id,
                     source,
-                    byte_index,
+                    byte_range,
                     args.crate_id,
                     args.def_maps,
                     args.interner,
@@ -71,7 +72,7 @@ struct CodeActionFinder<'a> {
     file: FileId,
     source: &'a str,
     lines: Vec<&'a str>,
-    byte_index: usize,
+    byte_range: Range<usize>,
     /// The module ID in scope. This might change as we traverse the AST
     /// if we are analyzing something inside an inline module declaration.
     module_id: ModuleId,
@@ -81,7 +82,9 @@ struct CodeActionFinder<'a> {
     nesting: usize,
     /// The line where an auto_import must be inserted
     auto_import_line: usize,
-    code_actions: Vec<CodeActionOrCommand>,
+    /// Text edits for the "Remove all unused imports" code action
+    unused_imports_text_edits: Vec<TextEdit>,
+    code_actions: Vec<CodeAction>,
 }
 
 impl<'a> CodeActionFinder<'a> {
@@ -91,7 +94,7 @@ impl<'a> CodeActionFinder<'a> {
         files: &'a FileMap,
         file: FileId,
         source: &'a str,
-        byte_index: usize,
+        byte_range: Range<usize>,
         krate: CrateId,
         def_maps: &'a BTreeMap<CrateId, CrateDefMap>,
         interner: &'a NodeInterner,
@@ -112,12 +115,13 @@ impl<'a> CodeActionFinder<'a> {
             file,
             source,
             lines: source.lines().collect(),
-            byte_index,
+            byte_range,
             module_id,
             def_maps,
             interner,
             nesting: 0,
             auto_import_line: 0,
+            unused_imports_text_edits: vec![],
             code_actions: vec![],
         }
     }
@@ -129,28 +133,38 @@ impl<'a> CodeActionFinder<'a> {
             return None;
         }
 
+        // We also suggest a single "Remove all the unused imports" code action that combines all of the
+        // "Remove unused imports" (similar to Rust Analyzer)
+        if self.unused_imports_text_edits.len() > 1 {
+            let text_edits = std::mem::take(&mut self.unused_imports_text_edits);
+            let code_action = self.new_quick_fix_multiple_edits(
+                "Remove all the unused imports".to_string(),
+                text_edits,
+            );
+            self.code_actions.push(code_action);
+        }
+
         let mut code_actions = std::mem::take(&mut self.code_actions);
-        code_actions.sort_by_key(|code_action| {
-            let CodeActionOrCommand::CodeAction(code_action) = code_action else {
-                panic!("We only gather code actions, never commands");
-            };
-            code_action.title.clone()
-        });
-
-        Some(code_actions)
+        code_actions.sort_by_key(|code_action| code_action.title.clone());
+
+        Some(code_actions.into_iter().map(CodeActionOrCommand::CodeAction).collect())
     }
 
-    fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeActionOrCommand {
+    fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeAction {
+        self.new_quick_fix_multiple_edits(title, vec![text_edit])
+    }
+
+    fn new_quick_fix_multiple_edits(&self, title: String, text_edits: Vec<TextEdit>) -> CodeAction {
         let mut changes = HashMap::new();
-        changes.insert(self.uri.clone(), vec![text_edit]);
+        changes.insert(self.uri.clone(), text_edits);
 
         let workspace_edit = WorkspaceEdit {
             changes: Some(changes),
             document_changes: None,
             change_annotations: None,
         };
 
-        CodeActionOrCommand::CodeAction(CodeAction {
+        CodeAction {
             title,
             kind: Some(CodeActionKind::QUICKFIX),
             diagnostics: None,
@@ -159,11 +173,12 @@ impl<'a> CodeActionFinder<'a> {
             is_preferred: None,
             disabled: None,
             data: None,
-        })
+        }
     }
 
     fn includes_span(&self, span: Span) -> bool {
-        span.start() as usize <= self.byte_index && self.byte_index <= span.end() as usize
+        let byte_range_span = Span::from(self.byte_range.start as u32..self.byte_range.end as u32);
+        span.intersects(&byte_range_span)
     }
 }
 
@@ -207,6 +222,12 @@ impl<'a> Visitor for CodeActionFinder<'a> {
         false
     }
 
+    fn visit_import(&mut self, use_tree: &UseTree, span: Span, visibility: ItemVisibility) -> bool {
+        self.remove_unused_import(use_tree, visibility, span);
+
+        true
+    }
+
     fn visit_path(&mut self, path: &Path) {
         self.import_or_qualify(path);
     }