diff --git a/.noir-sync-commit b/.noir-sync-commit index f0dcc120514..c59540a2d12 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -5598059576c6cbc72474aff4b18bc5e4bb9f08e1 +e3cdebe515e4dc4ee6e16e01bd8af25135939798 diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/bubble_up_constrains.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/bubble_up_constrains.rs deleted file mode 100644 index 0409f0e6a49..00000000000 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/bubble_up_constrains.rs +++ /dev/null @@ -1,152 +0,0 @@ -use std::collections::HashMap; - -use crate::ssa::{ - ir::instruction::{Instruction, InstructionId}, - ssa_gen::Ssa, -}; - -impl Ssa { - /// A simple SSA pass to go through each instruction and move every `Instruction::Constrain` to immediately - /// after when all of its inputs are available. - #[tracing::instrument(level = "trace", skip(self))] - pub(crate) fn bubble_up_constrains(mut self) -> Ssa { - for function in self.functions.values_mut() { - for block in function.reachable_blocks() { - let instructions = function.dfg[block].take_instructions(); - let mut filtered_instructions = Vec::with_capacity(instructions.len()); - - // Multiple constrains can bubble up to sit under a single instruction. We want to maintain the ordering of these constraints, - // so we need to keep track of how many constraints are attached to a given instruction. - // Some assertions don't operate on instruction results, so we use Option so we also track the None case - let mut inserted_at_instruction: HashMap, usize> = - HashMap::with_capacity(instructions.len()); - - let dfg = &function.dfg; - for instruction in instructions { - let (lhs, rhs) = match dfg[instruction] { - Instruction::Constrain(lhs, rhs, ..) => (lhs, rhs), - _ => { - filtered_instructions.push(instruction); - continue; - } - }; - - let last_instruction_that_creates_inputs = filtered_instructions - .iter() - .rev() - .position(|&instruction_id| { - let results = dfg.instruction_results(instruction_id).to_vec(); - results.contains(&lhs) || results.contains(&rhs) - }) - // We iterate through the previous instructions in reverse order so the index is from the - // back of the vector - .map(|reversed_index| filtered_instructions.len() - reversed_index - 1); - - let insertion_index = last_instruction_that_creates_inputs - .map(|index| { - // We want to insert just after the last instruction that creates the inputs - index + 1 - }) - // If it doesn't depend from the previous instructions, then we insert at the start - .unwrap_or_default(); - - let already_inserted_for_this_instruction = inserted_at_instruction - .entry( - last_instruction_that_creates_inputs - .map(|index| filtered_instructions[index]), - ) - .or_default(); - - filtered_instructions.insert( - insertion_index + *already_inserted_for_this_instruction, - instruction, - ); - - *already_inserted_for_this_instruction += 1; - } - - *function.dfg[block].instructions_mut() = filtered_instructions; - } - } - self - } -} - -#[cfg(test)] -mod test { - use crate::ssa::{ - function_builder::FunctionBuilder, - ir::{ - instruction::{Binary, BinaryOp, Instruction}, - map::Id, - types::Type, - }, - }; - - #[test] - fn check_bubble_up_constrains() { - // fn main f0 { - // b0(v0: Field): - // v1 = add v0, Field 1 - // v2 = add v1, Field 1 - // constrain v0 == Field 1 'With message' - // constrain v2 == Field 3 - // constrain v0 == Field 1 - // constrain v1 == Field 2 - // constrain v1 == Field 2 'With message' - // } - // - let main_id = Id::test_new(0); - - // Compiling main - let mut builder = FunctionBuilder::new("main".into(), main_id); - let v0 = builder.add_parameter(Type::field()); - - let one = builder.field_constant(1u128); - let two = builder.field_constant(2u128); - let three = builder.field_constant(3u128); - - let v1 = builder.insert_binary(v0, BinaryOp::Add, one); - let v2 = builder.insert_binary(v1, BinaryOp::Add, one); - builder.insert_constrain(v0, one, Some("With message".to_string().into())); - builder.insert_constrain(v2, three, None); - builder.insert_constrain(v0, one, None); - builder.insert_constrain(v1, two, None); - builder.insert_constrain(v1, two, Some("With message".to_string().into())); - builder.terminate_with_return(vec![]); - - let ssa = builder.finish(); - - // Expected output: - // - // fn main f0 { - // b0(v0: Field): - // constrain v0 == Field 1 'With message' - // constrain v0 == Field 1 - // v1 = add v0, Field 1 - // constrain v1 == Field 2 - // constrain v1 == Field 2 'With message' - // v2 = add v1, Field 1 - // constrain v2 == Field 3 - // } - // - let ssa = ssa.bubble_up_constrains(); - let main = ssa.main(); - let block = &main.dfg[main.entry_block()]; - assert_eq!(block.instructions().len(), 7); - - let expected_instructions = vec![ - Instruction::Constrain(v0, one, Some("With message".to_string().into())), - Instruction::Constrain(v0, one, None), - Instruction::Binary(Binary { lhs: v0, rhs: one, operator: BinaryOp::Add }), - Instruction::Constrain(v1, two, None), - Instruction::Constrain(v1, two, Some("With message".to_string().into())), - Instruction::Binary(Binary { lhs: v1, rhs: one, operator: BinaryOp::Add }), - Instruction::Constrain(v2, three, None), - ]; - - for (index, instruction) in block.instructions().iter().enumerate() { - assert_eq!(&main.dfg[*instruction], &expected_instructions[index]); - } - } -} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs index 095413d7b9a..7356998ceb8 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -191,6 +191,7 @@ impl Context { self.used_values.insert(value_id); } Value::Array { array, .. } => { + self.used_values.insert(value_id); for elem in array { self.mark_used_instruction_results(dfg, *elem); } @@ -198,6 +199,9 @@ impl Context { Value::Param { .. } => { self.used_values.insert(value_id); } + Value::NumericConstant { .. } => { + self.used_values.insert(value_id); + } _ => { // Does not comprise of any instruction results } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs index bd9d0baff97..f3dbd58fa69 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -6,7 +6,6 @@ mod array_set; mod as_slice_length; mod assert_constant; -mod bubble_up_constrains; mod constant_folding; mod defunctionalize; mod die; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs index fe9153c2cf4..94e81b19582 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs @@ -460,6 +460,7 @@ impl UnresolvedTypeExpression { ExpressionKind::AsTraitPath(path) => { Ok(UnresolvedTypeExpression::AsTraitPath(Box::new(path))) } + ExpressionKind::Parenthesized(expr) => Self::from_expr_helper(*expr), _ => Err(expr), } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs index 49568d42038..299c42b85c6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs +++ b/noir/noir-repo/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}")?; } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs index fb116d34e8a..80442d29398 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs +++ b/noir/noir-repo/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); } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 6265d0e65f2..ab045c52169 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/noir/noir-repo/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)| { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index 8c00fcde2ef..75178df319d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/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> { self.doc_comments.get(&id) } + + pub fn unused_items( + &self, + ) -> &std::collections::HashMap> { + self.usage_tracker.unused_items() + } } impl Methods { diff --git a/noir/noir-repo/noir_stdlib/src/hash/keccak.nr b/noir/noir-repo/noir_stdlib/src/hash/keccak.nr index c3321b62693..96e53429ac5 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/keccak.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/keccak.nr @@ -35,32 +35,19 @@ pub(crate) fn keccak256(input: [u8; N], message_size: u32) -> [u8; 3 block_bytes[message_size] = 1; block_bytes[real_blocks_bytes - 1] = 0x80; - // keccak lanes interpret memory as little-endian integers, - // means we need to swap our byte ordering let num_limbs = max_blocks * LIMBS_PER_BLOCK; //max_blocks_length / WORD_SIZE; - for i in 0..num_limbs { - let mut temp = [0; WORD_SIZE]; - let word_size_times_i = WORD_SIZE * i; - for j in 0..WORD_SIZE { - temp[j] = block_bytes[word_size_times_i+j]; - } - for j in 0..WORD_SIZE { - block_bytes[word_size_times_i + j] = temp[7 - j]; - } - } - let mut sliced_buffer = Vec::new(); // populate a vector of 64-bit limbs from our byte array for i in 0..num_limbs { - let word_size_times_i = i * WORD_SIZE; - let ws_times_i_plus_7 = word_size_times_i + 7; + let limb_start = WORD_SIZE * i; + let mut sliced = 0; - if (word_size_times_i + WORD_SIZE > max_blocks_length) { - let slice_size = max_blocks_length - word_size_times_i; + if (limb_start + WORD_SIZE > max_blocks_length) { + let slice_size = max_blocks_length - limb_start; let byte_shift = (WORD_SIZE - slice_size) * 8; let mut v = 1; for k in 0..slice_size { - sliced += v * (block_bytes[ws_times_i_plus_7-k] as Field); + sliced += v * (block_bytes[limb_start+k] as Field); v *= 256; } let w = 1 << (byte_shift as u8); @@ -68,7 +55,7 @@ pub(crate) fn keccak256(input: [u8; N], message_size: u32) -> [u8; 3 } else { let mut v = 1; for k in 0..WORD_SIZE { - sliced += v * (block_bytes[ws_times_i_plus_7-k] as Field); + sliced += v * (block_bytes[limb_start+k] as Field); v *= 256; } } @@ -156,4 +143,3 @@ mod tests { assert_eq(keccak256(input, 13), result); } } - diff --git a/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/Nargo.toml new file mode 100644 index 00000000000..e56e9643f40 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "parenthesized_expression_in_array_length" +type = "bin" +authors = [""] +compiler_version = ">=0.32.0" + +[dependencies] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/src/main.nr new file mode 100644 index 00000000000..b596d331e7f --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/src/main.nr @@ -0,0 +1,6 @@ +global N = 100; +global BLOCK_SIZE = 10; + +fn main() { + let _: [Field; 110] = [0; ((N + BLOCK_SIZE) * BLOCK_SIZE) / BLOCK_SIZE]; +} diff --git a/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/Nargo.toml b/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/Nargo.toml new file mode 100644 index 00000000000..2df127c83e8 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "brillig_constant_reference_regression" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/Prover.toml b/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/Prover.toml new file mode 100644 index 00000000000..8fbe88fbb98 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/Prover.toml @@ -0,0 +1 @@ +sorted_index = ["1", "0"] diff --git a/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/src/main.nr new file mode 100644 index 00000000000..c975d20a3b6 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/brillig_constant_reference_regression/src/main.nr @@ -0,0 +1,16 @@ +unconstrained fn main(sorted_index: [u32; 2]) { + let original = [ + 55, + 11 + ]; + + let mut sorted = original; // Stores the constant "original" into the sorted reference + + for i in 0..2 { + let index = sorted_index[i]; + let value = original[index]; + sorted[i] = value; // On first iteration, we should not mutate the original constant array, RC should be > 1 + } + + assert_eq(sorted[1], 55); +} diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_action.rs b/noir/noir-repo/tooling/lsp/src/requests/code_action.rs index f5cfabe5141..64eccab8947 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/code_action.rs +++ b/noir/noir-repo/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, ¶ms.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, /// 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, + /// Text edits for the "Remove all unused imports" code action + unused_imports_text_edits: Vec, + code_actions: Vec, } 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, krate: CrateId, def_maps: &'a BTreeMap, 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,20 +133,30 @@ 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) -> 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), @@ -150,7 +164,7 @@ impl<'a> CodeActionFinder<'a> { 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); } diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_action/remove_unused_import.rs b/noir/noir-repo/tooling/lsp/src/requests/code_action/remove_unused_import.rs new file mode 100644 index 00000000000..c660dd57e47 --- /dev/null +++ b/noir/noir-repo/tooling/lsp/src/requests/code_action/remove_unused_import.rs @@ -0,0 +1,314 @@ +use std::collections::HashMap; + +use lsp_types::TextEdit; +use noirc_errors::Span; +use noirc_frontend::{ + ast::{Ident, ItemVisibility, UseTree, UseTreeKind}, + parser::{Item, ItemKind}, + usage_tracker::UnusedItem, + ParsedModule, +}; + +use crate::byte_span_to_range; + +use super::CodeActionFinder; + +impl<'a> CodeActionFinder<'a> { + pub(super) fn remove_unused_import( + &mut self, + use_tree: &UseTree, + visibility: ItemVisibility, + span: Span, + ) { + if !self.includes_span(span) { + return; + } + + let Some(unused_items) = self.interner.unused_items().get(&self.module_id) else { + return; + }; + + if unused_items.is_empty() { + return; + } + + if has_unused_import(use_tree, unused_items) { + let byte_span = span.start() as usize..span.end() as usize; + let Some(range) = byte_span_to_range(self.files, self.file, byte_span) else { + return; + }; + + let (use_tree, removed_count) = use_tree_without_unused_import(use_tree, unused_items); + let (title, new_text) = match use_tree { + Some(use_tree) => ( + if removed_count == 1 { + "Remove unused import".to_string() + } else { + "Remove unused imports".to_string() + }, + use_tree_to_string(use_tree, visibility, self.nesting), + ), + None => ("Remove the whole `use` item".to_string(), "".to_string()), + }; + + let text_edit = TextEdit { range, new_text }; + self.unused_imports_text_edits.push(text_edit.clone()); + + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } + } +} + +fn has_unused_import(use_tree: &UseTree, unused_items: &HashMap) -> bool { + match &use_tree.kind { + UseTreeKind::Path(name, alias) => { + let ident = alias.as_ref().unwrap_or(name); + unused_items.contains_key(ident) + } + UseTreeKind::List(use_trees) => { + use_trees.iter().any(|use_tree| has_unused_import(use_tree, unused_items)) + } + } +} + +/// Returns a new `UseTree` with all the unused imports removed, and the number of removed imports. +fn use_tree_without_unused_import( + use_tree: &UseTree, + unused_items: &HashMap, +) -> (Option, usize) { + match &use_tree.kind { + UseTreeKind::Path(name, alias) => { + let ident = alias.as_ref().unwrap_or(name); + if unused_items.contains_key(ident) { + (None, 1) + } else { + (Some(use_tree.clone()), 0) + } + } + UseTreeKind::List(use_trees) => { + let mut new_use_trees: Vec = Vec::new(); + let mut total_count = 0; + + for use_tree in use_trees { + let (new_use_tree, count) = use_tree_without_unused_import(use_tree, unused_items); + if let Some(new_use_tree) = new_use_tree { + new_use_trees.push(new_use_tree); + } + total_count += count; + } + + let new_use_tree = if new_use_trees.is_empty() { + None + } else if new_use_trees.len() == 1 { + let new_use_tree = new_use_trees.remove(0); + + let mut prefix = use_tree.prefix.clone(); + prefix.segments.extend(new_use_tree.prefix.segments); + + Some(UseTree { prefix, kind: new_use_tree.kind }) + } else { + Some(UseTree { + prefix: use_tree.prefix.clone(), + kind: UseTreeKind::List(new_use_trees), + }) + }; + + (new_use_tree, total_count) + } + } +} + +fn use_tree_to_string(use_tree: UseTree, visibility: ItemVisibility, nesting: usize) -> String { + // We are going to use the formatter to format the use tree + let source = if visibility == ItemVisibility::Private { + format!("use {};", &use_tree) + } else { + format!("{} use {};", visibility, &use_tree) + }; + let parsed_module = ParsedModule { + items: vec![Item { + kind: ItemKind::Import(use_tree, visibility), + span: Span::from(0..source.len() as u32), + doc_comments: Vec::new(), + }], + inner_doc_comments: Vec::new(), + }; + + // Adjust the max width according to the current nesting + let mut config = nargo_fmt::Config::default(); + config.max_width -= nesting * 4; + + let string = nargo_fmt::format(&source, parsed_module, &config); + + let string = if nesting > 0 && string.contains('\n') { + // If the import is nested in a module, we just formatted it without indents so we need to add them. + let indent = " ".repeat(nesting * 4); + string.lines().map(|line| format!("{}{}", indent, line)).collect::>().join("\n") + } else { + string + }; + string.trim().to_string() +} + +#[cfg(test)] +mod tests { + use tokio::test; + + use crate::requests::code_action::tests::assert_code_action; + + #[test] + async fn test_removes_entire_unused_import_at_top_level() { + let title = "Remove the whole `use` item"; + + let src = r#" + mod moo { + fn foo() {} + } + use moo::fo>||||| Visitor for NodeFinder<'a> { self.includes_span(item.span) } - fn visit_import(&mut self, use_tree: &UseTree, _visibility: ItemVisibility) -> bool { + fn visit_import( + &mut self, + use_tree: &UseTree, + _span: Span, + _visibility: ItemVisibility, + ) -> bool { let mut prefixes = Vec::new(); self.find_in_use_tree(use_tree, &mut prefixes); false