feat: (LSP) remove unused imports

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 @@
         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 @@
             }
             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);
                 }
             }
@@ -1291,8 +1291,8 @@
             UnresolvedTypeData::Unspecified => visitor.visit_unspecified_type(self.span),
             UnresolvedTypeData::Quoted(typ) => visitor.visit_quoted_type(typ, self.span),
             UnresolvedTypeData::FieldElement => visitor.visit_field_element_type(self.span),
             UnresolvedTypeData::Integer(signdness, size) => {
                 visitor.visit_integer_type(*signdness, *size, self.span);
             }
             UnresolvedTypeData::Bool => visitor.visit_bool_type(self.span),
             UnresolvedTypeData::Unit => visitor.visit_unit_type(self.span),

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_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;
 
@@ -221,12 +222,12 @@
     interned_statement_kinds: noirc_arena::Arena<StatementKind>,
 
     // Interned `UnresolvedTypeData`s during comptime code.
     interned_unresolved_type_datas: noirc_arena::Arena<UnresolvedTypeData>,
 
     // Interned `Pattern`s during comptime code.
     interned_patterns: noirc_arena::Arena<Pattern>,
 
     /// Determins whether to run in LSP mode. In LSP mode references are tracked.
     pub(crate) lsp_mode: bool,
 
     /// Store the location of the references in the graph.
@@ -665,7 +666,7 @@
             quoted_types: Default::default(),
             interned_expression_kinds: Default::default(),
             interned_statement_kinds: Default::default(),
             interned_unresolved_type_datas: Default::default(),
             interned_patterns: Default::default(),
             lsp_mode: false,
             location_indices: LocationIndices::default(),
@@ -2144,11 +2145,11 @@
         &mut self,
         typ: UnresolvedTypeData,
     ) -> InternedUnresolvedTypeData {
         InternedUnresolvedTypeData(self.interned_unresolved_type_datas.insert(typ))
     }
 
     pub fn get_unresolved_type_data(&self, id: InternedUnresolvedTypeData) -> &UnresolvedTypeData {
         &self.interned_unresolved_type_datas[id.0]
     }
 
     /// Returns the type of an operator (which is always a function), along with its return type.
@@ -2249,6 +2250,12 @@
     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 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 @@
 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 @@
     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 @@
                     args.files,
                     file_id,
                     source,
-                    byte_index,
+                    byte_range,
                     args.crate_id,
                     args.def_maps,
                     args.interner,
@@ -71,7 +72,7 @@
     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 @@
     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 @@
         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 @@
             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,41 +133,52 @@
             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,
             edit: Some(workspace_edit),
             command: None,
             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 @@
         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);
     }