From cdbb940a883ae32dd84c667ec06b0d155f2d7520 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 Aug 2024 16:00:24 -0300 Subject: [PATCH] feat: LSP auto-import completion (#5741) # Description ## Problem Part of https://github.com/noir-lang/noir/issues/1577 ## Summary I said I wasn't going to send any more LSP features, but I had this one in the back of my head since I started working on autocompletion. I had the idea of how to do it in my head, but I wanted to first work on the easier stuff (regular autocompletion). It turns out auto-import wasn't that hard to implement after all! ![lsp-autoimport](https://github.com/user-attachments/assets/9d2268fb-5caf-4b42-9bb3-b01f6ca40a9b) ![lsp-autoimport-nested](https://github.com/user-attachments/assets/ac4cd18a-d87d-4311-82d0-61f6d5a8dd19) ## Additional Context In this PR every new imported item is added as a full path. Ideally, like in Rust Analyzer, we'd group common use prefixes, add to existing ones, etc. We can do that! But maybe it could be part of a follow-up PR, or maybe we could do it much later. Or maybe `nargo fmt` could handle this, so the auto-import doesn't do it, but once you save the file it's done. That's why I didn't spend time on that in this PR. There's another thing I noticed while working on this PR: Rust Analyzer will offer matches that match **any** part of the name, not just the beginning. So, in Noir, if you type `merkle` it should probably offer `compute_merkle_root` as an auto-import completion, which is nice because maybe you are looking for stuff related to "merkle" but don't know the full name... but I didn't want to introduce that change in this PR (it also works for every other autocompletion). But, as always, this could be done later on. ## 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/expression.rs | 16 +- .../noirc_frontend/src/elaborator/comptime.rs | 8 + compiler/noirc_frontend/src/elaborator/mod.rs | 11 +- .../src/hir/def_collector/dc_mod.rs | 48 +-- .../src/hir/def_map/module_def.rs | 2 +- compiler/noirc_frontend/src/locations.rs | 76 ++++- compiler/noirc_frontend/src/node_interner.rs | 8 + tooling/lsp/src/requests/completion.rs | 43 ++- .../src/requests/completion/auto_import.rs | 195 ++++++++++++ .../lsp/src/requests/completion/sort_text.rs | 5 + tooling/lsp/src/requests/completion/tests.rs | 291 ++++++++++++++++-- 11 files changed, 648 insertions(+), 55 deletions(-) create mode 100644 tooling/lsp/src/requests/completion/auto_import.rs diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index a3ce069b204..1fd1e313c44 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -8,7 +8,7 @@ use crate::ast::{ use crate::hir::def_collector::errors::DefCollectorErrorKind; use crate::macros_api::StructId; use crate::node_interner::{ExprId, QuotedTypeId}; -use crate::token::{Attributes, Token, Tokens}; +use crate::token::{Attributes, FunctionAttribute, Token, Tokens}; use crate::{Kind, Type}; use acvm::{acir::AcirField, FieldElement}; use iter_extended::vecmap; @@ -478,6 +478,20 @@ pub struct FunctionDefinition { pub return_visibility: Visibility, } +impl FunctionDefinition { + pub fn is_private(&self) -> bool { + self.visibility == ItemVisibility::Private + } + + pub fn is_test(&self) -> bool { + if let Some(attribute) = &self.attributes.function { + matches!(attribute, FunctionAttribute::Test(..)) + } else { + false + } + } +} + #[derive(Debug, PartialEq, Eq, Clone)] pub struct Param { pub visibility: Visibility, diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index afa2e7fa7a8..2b78c02e53c 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -267,6 +267,14 @@ impl<'context> Elaborator<'context> { let id = self.interner.push_empty_fn(); let module = self.module_id(); self.interner.push_function(id, &function.def, module, location); + + if self.interner.is_in_lsp_mode() + && !function.def.is_test() + && !function.def.is_private() + { + self.interner.register_function(id, &function.def); + } + let functions = vec![(self.local_module, id, function)]; generated_items.functions.push(UnresolvedFunctions { file_id: self.file, diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 78241ce95a4..2e4a69c067f 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1297,6 +1297,12 @@ impl<'context> Elaborator<'context> { self.current_item = Some(DependencyId::Global(global_id)); let let_stmt = global.stmt_def; + let name = if self.interner.is_in_lsp_mode() { + Some(let_stmt.pattern.name_ident().to_string()) + } else { + None + }; + if !self.in_contract() && let_stmt.attributes.iter().any(|attr| matches!(attr, SecondaryAttribute::Abi(_))) { @@ -1319,8 +1325,9 @@ impl<'context> Elaborator<'context> { self.elaborate_comptime_global(global_id); } - self.interner - .add_definition_location(ReferenceId::Global(global_id), Some(self.module_id())); + if let Some(name) = name { + self.interner.register_global(global_id, name, self.module_id()); + } self.local_module = old_module; self.file = old_file; diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index be2afd13507..6436c2562bc 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -14,7 +14,7 @@ use crate::ast::{ TypeImpl, }; use crate::macros_api::NodeInterner; -use crate::node_interner::{ModuleAttributes, ReferenceId}; +use crate::node_interner::ModuleAttributes; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, @@ -232,6 +232,13 @@ impl<'a> ModCollector<'a> { let location = Location::new(function.span(), self.file_id); context.def_interner.push_function(func_id, &function.def, module, location); + if context.def_interner.is_in_lsp_mode() + && !function.def.is_test() + && !function.def.is_private() + { + context.def_interner.register_function(func_id, &function.def); + } + // Now link this func_id to a crate level map with the noir function and the module id // Encountering a NoirFunction, we retrieve it's module_data to get the namespace // Once we have lowered it to a HirFunction, we retrieve it's Id from the DefInterner @@ -307,8 +314,8 @@ impl<'a> ModCollector<'a> { }; // Add the struct to scope so its path can be looked up later - let result = - self.def_collector.def_map.modules[self.module_id.0].declare_struct(name, id); + let result = self.def_collector.def_map.modules[self.module_id.0] + .declare_struct(name.clone(), id); if let Err((first_def, second_def)) = result { let error = DefCollectorErrorKind::Duplicate { @@ -322,10 +329,10 @@ impl<'a> ModCollector<'a> { // And store the TypeId -> StructType mapping somewhere it is reachable self.def_collector.items.types.insert(id, unresolved); - context.def_interner.add_definition_location( - ReferenceId::Struct(id), - Some(ModuleId { krate, local_id: self.module_id }), - ); + if context.def_interner.is_in_lsp_mode() { + let parent_module_id = ModuleId { krate, local_id: self.module_id }; + context.def_interner.register_struct(id, name.to_string(), parent_module_id); + } } definition_errors } @@ -383,7 +390,7 @@ impl<'a> ModCollector<'a> { // Add the type alias to scope so its path can be looked up later let result = self.def_collector.def_map.modules[self.module_id.0] - .declare_type_alias(name, type_alias_id); + .declare_type_alias(name.clone(), type_alias_id); if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::Duplicate { @@ -396,10 +403,11 @@ impl<'a> ModCollector<'a> { self.def_collector.items.type_aliases.insert(type_alias_id, unresolved); - context.def_interner.add_definition_location( - ReferenceId::Alias(type_alias_id), - Some(ModuleId { krate, local_id: self.module_id }), - ); + if context.def_interner.is_in_lsp_mode() { + let parent_module_id = ModuleId { krate, local_id: self.module_id }; + let name = name.to_string(); + context.def_interner.register_type_alias(type_alias_id, name, parent_module_id); + } } errors } @@ -432,8 +440,8 @@ impl<'a> ModCollector<'a> { }; // Add the trait to scope so its path can be looked up later - let result = - self.def_collector.def_map.modules[self.module_id.0].declare_trait(name, trait_id); + let result = self.def_collector.def_map.modules[self.module_id.0] + .declare_trait(name.clone(), trait_id); if let Err((first_def, second_def)) = result { let error = DefCollectorErrorKind::Duplicate { @@ -567,10 +575,10 @@ impl<'a> ModCollector<'a> { }; context.def_interner.push_empty_trait(trait_id, &unresolved, resolved_generics); - context.def_interner.add_definition_location( - ReferenceId::Trait(trait_id), - Some(ModuleId { krate, local_id: self.module_id }), - ); + if context.def_interner.is_in_lsp_mode() { + let parent_module_id = ModuleId { krate, local_id: self.module_id }; + context.def_interner.register_trait(trait_id, name.to_string(), parent_module_id); + } self.def_collector.items.traits.insert(trait_id, unresolved); } @@ -766,6 +774,10 @@ impl<'a> ModCollector<'a> { parent: self.module_id, }, ); + + if context.def_interner.is_in_lsp_mode() { + context.def_interner.register_module(mod_id, mod_name.0.contents.clone()); + } } Ok(mod_id) diff --git a/compiler/noirc_frontend/src/hir/def_map/module_def.rs b/compiler/noirc_frontend/src/hir/def_map/module_def.rs index 54d092f9515..a487bda81b3 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_def.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_def.rs @@ -3,7 +3,7 @@ use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; use super::ModuleId; /// A generic ID that references either a module, function, type, interface or global -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum ModuleDefId { ModuleId(ModuleId), FunctionId(FuncId), diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index c437676b605..0ac13a58ecf 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -1,9 +1,10 @@ use fm::FileId; use noirc_errors::Location; use rangemap::RangeMap; -use rustc_hash::FxHashMap; +use rustc_hash::FxHashMap as HashMap; use crate::{ + ast::{FunctionDefinition, ItemVisibility}, hir::def_map::{ModuleDefId, ModuleId}, macros_api::{NodeInterner, StructId}, node_interner::{DefinitionId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId}, @@ -12,7 +13,7 @@ use petgraph::prelude::NodeIndex as PetGraphIndex; #[derive(Debug, Default)] pub(crate) struct LocationIndices { - map_file_to_range: FxHashMap>, + map_file_to_range: HashMap>, } impl LocationIndices { @@ -275,4 +276,75 @@ impl NodeInterner { .neighbors_directed(reference_index, petgraph::Direction::Outgoing) .next() } + + pub(crate) fn register_module(&mut self, id: ModuleId, name: String) { + self.register_name_for_auto_import(name, ModuleDefId::ModuleId(id), ItemVisibility::Public); + } + + pub(crate) fn register_global( + &mut self, + id: GlobalId, + name: String, + parent_module_id: ModuleId, + ) { + self.add_definition_location(ReferenceId::Global(id), Some(parent_module_id)); + + let visibility = ItemVisibility::Public; + self.register_name_for_auto_import(name, ModuleDefId::GlobalId(id), visibility); + } + + pub(crate) fn register_struct( + &mut self, + id: StructId, + name: String, + parent_module_id: ModuleId, + ) { + self.add_definition_location(ReferenceId::Struct(id), Some(parent_module_id)); + + let visibility = ItemVisibility::Public; + self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility); + } + + pub(crate) fn register_trait(&mut self, id: TraitId, name: String, parent_module_id: ModuleId) { + self.add_definition_location(ReferenceId::Trait(id), Some(parent_module_id)); + + self.register_name_for_auto_import(name, ModuleDefId::TraitId(id), ItemVisibility::Public); + } + + pub(crate) fn register_type_alias( + &mut self, + id: TypeAliasId, + name: String, + parent_module_id: ModuleId, + ) { + self.add_definition_location(ReferenceId::Alias(id), Some(parent_module_id)); + + let visibility = ItemVisibility::Public; + self.register_name_for_auto_import(name, ModuleDefId::TypeAliasId(id), visibility); + } + + pub(crate) fn register_function(&mut self, id: FuncId, func_def: &FunctionDefinition) { + self.register_name_for_auto_import( + func_def.name.0.contents.clone(), + ModuleDefId::FunctionId(id), + func_def.visibility, + ); + } + + fn register_name_for_auto_import( + &mut self, + name: String, + module_def_id: ModuleDefId, + visibility: ItemVisibility, + ) { + if !self.lsp_mode { + return; + } + + self.auto_import_names.entry(name).or_default().push((module_def_id, visibility)); + } + + pub fn get_auto_import_names(&self) -> &HashMap> { + &self.auto_import_names + } } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index e9d1dbb67f8..46ec2676930 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -19,6 +19,7 @@ use crate::hir::comptime; use crate::hir::def_collector::dc_crate::CompilationError; use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias}; use crate::hir::def_map::{LocalModuleId, ModuleId}; +use crate::macros_api::ModuleDefId; use crate::macros_api::UnaryOp; use crate::QuotedType; @@ -231,6 +232,11 @@ pub struct NodeInterner { // (ReferenceId::Reference and ReferenceId::Local aren't included here) pub(crate) reference_modules: HashMap, + // All names (and their definitions) that can be offered for auto_import. + // These include top-level functions, global variables and types, but excludes + // impl and trait-impl methods. + pub(crate) auto_import_names: HashMap>, + /// Each value currently in scope in the comptime interpreter. /// Each element of the Vec represents a scope with every scope together making /// up all currently visible definitions. The first scope is always the global scope. @@ -602,6 +608,7 @@ impl Default for NodeInterner { reference_graph: petgraph::graph::DiGraph::new(), reference_graph_indices: HashMap::default(), reference_modules: HashMap::default(), + auto_import_names: HashMap::default(), comptime_scopes: vec![HashMap::default()], } } @@ -910,6 +917,7 @@ impl NodeInterner { // This needs to be done after pushing the definition since it will reference the // location that was stored self.add_definition_location(ReferenceId::Function(id), Some(module)); + definition_id } diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index dd3131aba56..ad0cf0c5b9b 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -8,7 +8,7 @@ use completion_items::{ crate_completion_item, field_completion_item, simple_completion_item, struct_field_completion_item, }; -use fm::{FileId, PathString}; +use fm::{FileId, FileMap, PathString}; use kinds::{FunctionCompletionKind, FunctionKind, ModuleCompletionKind, RequestedItems}; use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse}; use noirc_errors::{Location, Span}; @@ -33,10 +33,11 @@ use noirc_frontend::{ }; use sort_text::underscore_sort_text; -use crate::{utils, LspState}; +use crate::{requests::to_lsp_location, utils, LspState}; use super::process_request; +mod auto_import; mod builtins; mod completion_items; mod kinds; @@ -65,7 +66,9 @@ pub(crate) fn on_completion_request( let (parsed_module, _errors) = noirc_frontend::parse_program(source); let mut finder = NodeFinder::new( + args.files, file_id, + source, byte_index, byte, args.crate_id, @@ -81,7 +84,9 @@ pub(crate) fn on_completion_request( } struct NodeFinder<'a> { + files: &'a FileMap, file: FileId, + lines: Vec<&'a str>, byte_index: usize, byte: Option, /// The module ID of the current file. @@ -100,11 +105,20 @@ struct NodeFinder<'a> { /// Type parameters in the current scope. These are collected when entering /// a struct, a function, etc., and cleared afterwards. type_parameters: HashSet, + /// ModuleDefIds we already suggested, so we don't offer these for auto-import. + suggested_module_def_ids: HashSet, + /// How many nested `mod` we are in deep + nesting: usize, + /// The line where an auto_import must be inserted + auto_import_line: usize, } impl<'a> NodeFinder<'a> { + #[allow(clippy::too_many_arguments)] fn new( + files: &'a FileMap, file: FileId, + source: &'a str, byte_index: usize, byte: Option, krate: CrateId, @@ -124,7 +138,9 @@ impl<'a> NodeFinder<'a> { }; let module_id = ModuleId { krate, local_id }; Self { + files, file, + lines: source.lines().collect(), byte_index, byte, root_module_id, @@ -135,6 +151,9 @@ impl<'a> NodeFinder<'a> { completion_items: Vec::new(), local_variables: HashMap::new(), type_parameters: HashSet::new(), + suggested_module_def_ids: HashSet::new(), + nesting: 0, + auto_import_line: 0, } } @@ -158,6 +177,12 @@ impl<'a> NodeFinder<'a> { } fn find_in_item(&mut self, item: &Item) { + if let ItemKind::Import(..) = &item.kind { + if let Some(lsp_location) = to_lsp_location(self.files, self.file, item.span) { + self.auto_import_line = (lsp_location.range.end.line + 1) as usize; + } + } + if !self.includes_span(item.span) { return; } @@ -180,10 +205,19 @@ impl<'a> NodeFinder<'a> { ModuleId { krate: self.module_id.krate, local_id: *child_module }; } + let old_auto_import_line = self.auto_import_line; + self.nesting += 1; + + if let Some(lsp_location) = to_lsp_location(self.files, self.file, item.span) { + self.auto_import_line = (lsp_location.range.start.line + 1) as usize; + } + self.find_in_parsed_module(&parsed_sub_module.contents); // Restore the old module before continuing self.module_id = previous_module_id; + self.nesting -= 1; + self.auto_import_line = old_auto_import_line; } ItemKind::Function(noir_function) => self.find_in_noir_function(noir_function), ItemKind::TraitImpl(noir_trait_impl) => self.find_in_noir_trait_impl(noir_trait_impl), @@ -714,6 +748,7 @@ impl<'a> NodeFinder<'a> { self.type_parameters_completion(&prefix); } } + self.complete_auto_imports(&prefix, requested_items); } } @@ -935,6 +970,7 @@ impl<'a> NodeFinder<'a> { function_kind, ) { self.completion_items.push(completion_item); + self.suggested_module_def_ids.insert(ModuleDefId::FunctionId(func_id)); } } } @@ -955,6 +991,7 @@ impl<'a> NodeFinder<'a> { function_kind, ) { self.completion_items.push(completion_item); + self.suggested_module_def_ids.insert(ModuleDefId::FunctionId(*func_id)); } } } @@ -1038,6 +1075,7 @@ impl<'a> NodeFinder<'a> { requested_items, ) { self.completion_items.push(completion_item); + self.suggested_module_def_ids.insert(module_def_id); } } @@ -1050,6 +1088,7 @@ impl<'a> NodeFinder<'a> { requested_items, ) { self.completion_items.push(completion_item); + self.suggested_module_def_ids.insert(module_def_id); } } } diff --git a/tooling/lsp/src/requests/completion/auto_import.rs b/tooling/lsp/src/requests/completion/auto_import.rs new file mode 100644 index 00000000000..c5710d30b5c --- /dev/null +++ b/tooling/lsp/src/requests/completion/auto_import.rs @@ -0,0 +1,195 @@ +use lsp_types::{Position, Range, TextEdit}; +use noirc_frontend::{ + ast::ItemVisibility, + graph::{CrateId, Dependency}, + hir::def_map::ModuleId, + macros_api::{ModuleDefId, NodeInterner}, + node_interner::ReferenceId, +}; + +use super::{ + kinds::{FunctionCompletionKind, FunctionKind, RequestedItems}, + name_matches, + sort_text::auto_import_sort_text, + NodeFinder, +}; + +impl<'a> NodeFinder<'a> { + pub(super) fn complete_auto_imports(&mut self, prefix: &str, requested_items: RequestedItems) { + for (name, entries) in self.interner.get_auto_import_names() { + if !name_matches(name, prefix) { + continue; + } + + for (module_def_id, visibility) in entries { + if self.suggested_module_def_ids.contains(module_def_id) { + continue; + } + + let Some(mut completion_item) = self.module_def_id_completion_item( + *module_def_id, + name.clone(), + FunctionCompletionKind::NameAndParameters, + FunctionKind::Any, + requested_items, + ) else { + continue; + }; + + let module_full_path; + if let ModuleDefId::ModuleId(module_id) = module_def_id { + module_full_path = module_id_path( + module_id, + &self.module_id, + self.interner, + self.dependencies, + ); + } else { + let Some(parent_module) = get_parent_module(self.interner, *module_def_id) + else { + continue; + }; + + match *visibility { + ItemVisibility::Public => (), + ItemVisibility::Private => { + // Technically this can't be reached because we don't record private items for auto-import, + // but this is here for completeness. + continue; + } + ItemVisibility::PublicCrate => { + if self.module_id.krate != parent_module.krate { + continue; + } + } + } + + module_full_path = module_id_path( + parent_module, + &self.module_id, + self.interner, + self.dependencies, + ); + } + + let full_path = if let ModuleDefId::ModuleId(..) = module_def_id { + module_full_path + } else { + format!("{}::{}", module_full_path, name) + }; + + let mut label_details = completion_item.label_details.unwrap(); + label_details.detail = Some(format!("(use {})", full_path)); + completion_item.label_details = Some(label_details); + + let line = self.auto_import_line as u32; + let character = (self.nesting * 4) as u32; + let indent = " ".repeat(self.nesting * 4); + let mut newlines = "\n"; + + // If the line we are inserting into is not an empty line, insert an extra line to make some room + if let Some(line_text) = self.lines.get(line as usize) { + if !line_text.trim().is_empty() { + newlines = "\n\n"; + } + } + + completion_item.additional_text_edits = Some(vec![TextEdit { + range: Range { + start: Position { line, character }, + end: Position { line, character }, + }, + new_text: format!("use {};{}{}", full_path, newlines, indent), + }]); + + completion_item.sort_text = Some(auto_import_sort_text()); + + self.completion_items.push(completion_item); + self.suggested_module_def_ids.insert(*module_def_id); + } + } + } +} + +fn get_parent_module(interner: &NodeInterner, module_def_id: ModuleDefId) -> Option<&ModuleId> { + let reference_id = module_def_id_to_reference_id(module_def_id); + interner.reference_module(reference_id) +} + +fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { + match module_def_id { + ModuleDefId::ModuleId(id) => ReferenceId::Module(id), + ModuleDefId::FunctionId(id) => ReferenceId::Function(id), + ModuleDefId::TypeId(id) => ReferenceId::Struct(id), + ModuleDefId::TypeAliasId(id) => ReferenceId::Alias(id), + ModuleDefId::TraitId(id) => ReferenceId::Trait(id), + ModuleDefId::GlobalId(id) => ReferenceId::Global(id), + } +} + +/// Computes the path of `module_id` relative to `current_module_id`. +/// If it's not relative, the full path is returned. +fn module_id_path( + module_id: &ModuleId, + current_module_id: &ModuleId, + interner: &NodeInterner, + dependencies: &[Dependency], +) -> String { + let mut string = String::new(); + + let crate_id = module_id.krate; + let crate_name = match crate_id { + CrateId::Root(_) => Some("crate".to_string()), + CrateId::Crate(_) => dependencies + .iter() + .find(|dep| dep.crate_id == crate_id) + .map(|dep| format!("{}", dep.name)), + CrateId::Stdlib(_) => Some("std".to_string()), + CrateId::Dummy => None, + }; + + let wrote_crate = if let Some(crate_name) = crate_name { + string.push_str(&crate_name); + true + } else { + false + }; + + let Some(module_attributes) = interner.try_module_attributes(module_id) else { + return string; + }; + + if wrote_crate { + string.push_str("::"); + } + + let mut segments = Vec::new(); + let mut current_attributes = module_attributes; + loop { + let parent_module_id = + &ModuleId { krate: module_id.krate, local_id: current_attributes.parent }; + + // If the parent module is the current module we stop because we want a relative path to the module + if current_module_id == parent_module_id { + // When the path is relative we don't want the "crate::" prefix anymore + string = string.strip_prefix("crate::").unwrap_or(&string).to_string(); + break; + } + + let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { + break; + }; + + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } + + for segment in segments.iter().rev() { + string.push_str(segment); + string.push_str("::"); + } + + string.push_str(&module_attributes.name); + + string +} diff --git a/tooling/lsp/src/requests/completion/sort_text.rs b/tooling/lsp/src/requests/completion/sort_text.rs index 4a8aebd7988..a1dd0ba5e94 100644 --- a/tooling/lsp/src/requests/completion/sort_text.rs +++ b/tooling/lsp/src/requests/completion/sort_text.rs @@ -9,6 +9,11 @@ pub(super) fn default_sort_text() -> String { "5".to_string() } +/// Sort text for auto-import items. We want these to show up after local definitions. +pub(super) fn auto_import_sort_text() -> String { + "6".to_string() +} + /// When completing something like `Foo::`, we want to show methods that take /// self after the other ones. pub(super) fn self_mismatch_sort_text() -> String { diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index ff028954f7b..825a7dd0081 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -10,7 +10,7 @@ mod completion_tests { field_completion_item, module_completion_item, simple_completion_item, snippet_completion_item, }, - sort_text::self_mismatch_sort_text, + sort_text::{auto_import_sort_text, self_mismatch_sort_text}, }, on_completion_request, }, @@ -18,9 +18,10 @@ mod completion_tests { }; use lsp_types::{ - CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse, - DidOpenTextDocumentParams, PartialResultParams, Position, TextDocumentIdentifier, - TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams, + CompletionItem, CompletionItemKind, CompletionItemLabelDetails, CompletionParams, + CompletionResponse, DidOpenTextDocumentParams, PartialResultParams, Position, Range, + TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, TextEdit, + WorkDoneProgressParams, }; use tokio::test; @@ -63,14 +64,13 @@ mod completion_tests { }, ) .await - .expect("Could not execute on_completion_request") - .unwrap(); + .expect("Could not execute on_completion_request"); - let CompletionResponse::Array(items) = response else { - panic!("Expected response to be CompletionResponse::Array"); - }; - - items + if let Some(CompletionResponse::Array(items)) = response { + items + } else { + vec![] + } } fn assert_items_match(mut items: Vec, mut expected: Vec) { @@ -97,6 +97,12 @@ mod completion_tests { assert_items_match(items, expected); } + async fn assert_completion_excluding_auto_import(src: &str, expected: Vec) { + let items = get_completions(src).await; + let items = items.into_iter().filter(|item| item.additional_text_edits.is_none()).collect(); + assert_items_match(items, expected); + } + pub(super) fn function_completion_item( label: impl Into, insert_text: impl Into, @@ -368,7 +374,7 @@ mod completion_tests { l>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "local", @@ -388,7 +394,7 @@ mod completion_tests { l>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "local", @@ -406,7 +412,7 @@ mod completion_tests { l>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "local", @@ -426,7 +432,11 @@ mod completion_tests { h>|< } "#; - assert_completion(src, vec![function_completion_item("hello()", "hello()", "fn()")]).await; + assert_completion_excluding_auto_import( + src, + vec![function_completion_item("hello()", "hello()", "fn()")], + ) + .await; } #[test] @@ -438,7 +448,7 @@ mod completion_tests { h>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![function_completion_item( "hello(…)", @@ -456,7 +466,7 @@ mod completion_tests { a>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![ snippet_completion_item( @@ -488,7 +498,7 @@ mod completion_tests { } } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "SomeStruct", @@ -511,7 +521,7 @@ mod completion_tests { } } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "SomeStruct", @@ -531,7 +541,7 @@ mod completion_tests { } } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "index", @@ -551,7 +561,7 @@ mod completion_tests { lambda(|var| v>|<) } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "var", @@ -742,7 +752,7 @@ mod completion_tests { let x = t>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "true", @@ -766,7 +776,7 @@ mod completion_tests { } } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![ simple_completion_item( @@ -794,7 +804,7 @@ mod completion_tests { } } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![ simple_completion_item( @@ -822,7 +832,7 @@ mod completion_tests { g>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "good", @@ -844,7 +854,7 @@ mod completion_tests { } } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![ simple_completion_item( @@ -870,7 +880,7 @@ mod completion_tests { g>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item( "good", @@ -888,7 +898,7 @@ mod completion_tests { context: C>|< } "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item("Context", CompletionItemKind::TYPE_PARAMETER, None)], ) @@ -954,7 +964,7 @@ mod completion_tests { let src = r#" fn foo(x: C>|<) {} "#; - assert_completion( + assert_completion_excluding_auto_import( src, vec![simple_completion_item("Context", CompletionItemKind::TYPE_PARAMETER, None)], ) @@ -1323,4 +1333,227 @@ mod completion_tests { assert_completion(src, vec![function_completion_item("one()", "one()", "fn() -> Self")]) .await; } + + #[test] + async fn test_auto_imports() { + let src = r#" + mod foo { + mod bar { + pub fn hello_world() {} + + struct Foo { + } + + impl Foo { + // This is here to make sure it's not offered for completion + fn hello_world() {} + } + } + } + + fn main() { + hel>|< + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "hello_world()"); + assert_eq!( + item.label_details, + Some(CompletionItemLabelDetails { + detail: Some("(use foo::bar::hello_world)".to_string()), + description: Some("fn()".to_string()) + }) + ); + + assert_eq!( + item.additional_text_edits, + Some(vec![TextEdit { + range: Range { + start: Position { line: 0, character: 0 }, + end: Position { line: 0, character: 0 }, + }, + new_text: "use foo::bar::hello_world;\n".to_string(), + }]) + ); + + assert_eq!(item.sort_text, Some(auto_import_sort_text())); + } + + #[test] + async fn test_auto_imports_when_in_nested_module_and_item_is_further_nested() { + let src = r#" + mod foo { + mod bar { + pub fn hello_world() {} + } + + fn foo() { + hel>|< + } + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "hello_world()"); + assert_eq!( + item.label_details, + Some(CompletionItemLabelDetails { + detail: Some("(use bar::hello_world)".to_string()), + description: Some("fn()".to_string()) + }) + ); + + assert_eq!( + item.additional_text_edits, + Some(vec![TextEdit { + range: Range { + start: Position { line: 2, character: 4 }, + end: Position { line: 2, character: 4 }, + }, + new_text: "use bar::hello_world;\n\n ".to_string(), + }]) + ); + } + + #[test] + async fn test_auto_imports_when_in_nested_module_and_item_is_not_further_nested() { + let src = r#" + mod foo { + mod bar { + pub fn hello_world() {} + } + + mod baz { + fn foo() { + hel>|< + } + } + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "hello_world()"); + assert_eq!( + item.label_details, + Some(CompletionItemLabelDetails { + detail: Some("(use crate::foo::bar::hello_world)".to_string()), + description: Some("fn()".to_string()) + }) + ); + + assert_eq!( + item.additional_text_edits, + Some(vec![TextEdit { + range: Range { + start: Position { line: 7, character: 8 }, + end: Position { line: 7, character: 8 }, + }, + new_text: "use crate::foo::bar::hello_world;\n\n ".to_string(), + }]) + ); + } + + #[test] + async fn test_auto_import_inserts_after_last_use() { + let src = r#" + mod foo { + mod bar { + pub fn hello_world() {} + } + } + + use foo::bar; + + fn main() { + hel>|< + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!( + item.additional_text_edits, + Some(vec![TextEdit { + range: Range { + start: Position { line: 8, character: 0 }, + end: Position { line: 8, character: 0 }, + }, + new_text: "use foo::bar::hello_world;\n".to_string(), + }]) + ); + } + + #[test] + async fn test_does_not_auto_import_test_functions() { + let src = r#" + mod foo { + mod bar { + #[test] + pub fn hello_world() {} + } + } + + use foo::bar; + + fn main() { + hel>|< + } + "#; + let items = get_completions(src).await; + assert!(items.is_empty()); + } + + #[test] + async fn test_does_not_auto_import_private_functions() { + let src = r#" + mod foo { + mod bar { + fn hello_world() {} + } + } + + use foo::bar; + + fn main() { + hel>|< + } + "#; + let items = get_completions(src).await; + assert!(items.is_empty()); + } + + #[test] + async fn test_auto_import_suggests_modules_too() { + let src = r#" + mod foo { + mod barbaz { + fn hello_world() {} + } + } + + fn main() { + barb>|< + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "barbaz"); + assert_eq!( + item.label_details, + Some(CompletionItemLabelDetails { + detail: Some("(use foo::barbaz)".to_string()), + description: None + }) + ); + } }