diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 387006f7dae..302152f0829 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -1185,9 +1185,15 @@ fn name_matches(name: &str, prefix: &str) -> bool { let mut last_index: i32 = -1; for prefix_part in prefix.split('_') { - if let Some(name_part_index) = - name_parts.iter().position(|name_part| name_part.starts_with(prefix_part)) + // Look past parts we already matched + let offset = if last_index >= 0 { last_index as usize + 1 } else { 0 }; + + if let Some(mut name_part_index) = + name_parts.iter().skip(offset).position(|name_part| name_part.starts_with(prefix_part)) { + // Need to adjust the index if we skipped some segments + name_part_index += offset; + if last_index >= name_part_index as i32 { return false; } @@ -1225,6 +1231,7 @@ mod completion_name_matches_tests { assert!(name_matches("FooBar", "foo")); assert!(name_matches("FooBar", "bar")); assert!(name_matches("FooBar", "foo_bar")); + assert!(name_matches("bar_baz", "bar_b")); assert!(!name_matches("foo_bar", "o_b")); } diff --git a/tooling/lsp/src/requests/completion/auto_import.rs b/tooling/lsp/src/requests/completion/auto_import.rs index c5710d30b5c..8d7824502c1 100644 --- a/tooling/lsp/src/requests/completion/auto_import.rs +++ b/tooling/lsp/src/requests/completion/auto_import.rs @@ -1,8 +1,10 @@ +use std::collections::BTreeMap; + use lsp_types::{Position, Range, TextEdit}; use noirc_frontend::{ ast::ItemVisibility, graph::{CrateId, Dependency}, - hir::def_map::ModuleId, + hir::def_map::{CrateDefMap, ModuleId}, macros_api::{ModuleDefId, NodeInterner}, node_interner::ReferenceId, }; @@ -16,6 +18,8 @@ use super::{ impl<'a> NodeFinder<'a> { pub(super) fn complete_auto_imports(&mut self, prefix: &str, requested_items: RequestedItems) { + let current_module_parent_id = get_parent_module_id(self.def_maps, self.module_id); + for (name, entries) in self.interner.get_auto_import_names() { if !name_matches(name, prefix) { continue; @@ -39,8 +43,9 @@ impl<'a> NodeFinder<'a> { let module_full_path; if let ModuleDefId::ModuleId(module_id) = module_def_id { module_full_path = module_id_path( - module_id, + *module_id, &self.module_id, + current_module_parent_id, self.interner, self.dependencies, ); @@ -67,6 +72,7 @@ impl<'a> NodeFinder<'a> { module_full_path = module_id_path( parent_module, &self.module_id, + current_module_parent_id, self.interner, self.dependencies, ); @@ -111,9 +117,18 @@ impl<'a> NodeFinder<'a> { } } -fn get_parent_module(interner: &NodeInterner, module_def_id: ModuleDefId) -> Option<&ModuleId> { +fn get_parent_module(interner: &NodeInterner, module_def_id: ModuleDefId) -> Option { let reference_id = module_def_id_to_reference_id(module_def_id); - interner.reference_module(reference_id) + interner.reference_module(reference_id).copied() +} + +fn get_parent_module_id( + def_maps: &BTreeMap, + module_id: ModuleId, +) -> Option { + let crate_def_map = &def_maps[&module_id.krate]; + let module_data = &crate_def_map.modules()[module_id.local_id.0]; + module_data.parent.map(|parent| ModuleId { krate: module_id.krate, local_id: parent }) } fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { @@ -127,69 +142,69 @@ fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { } } -/// Computes the path of `module_id` relative to `current_module_id`. -/// If it's not relative, the full path is returned. +/// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`. +/// Returns a relative path if possible. fn module_id_path( - module_id: &ModuleId, + target_module_id: ModuleId, current_module_id: &ModuleId, + current_module_parent_id: Option, 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, - }; + if Some(target_module_id) == current_module_parent_id { + return "super".to_string(); + } - let wrote_crate = if let Some(crate_name) = crate_name { - string.push_str(&crate_name); - true - } else { - false - }; + let mut segments: Vec<&str> = Vec::new(); + let mut is_relative = false; - let Some(module_attributes) = interner.try_module_attributes(module_id) else { - return string; - }; + if let Some(module_attributes) = interner.try_module_attributes(&target_module_id) { + segments.push(&module_attributes.name); - if wrote_crate { - string.push_str("::"); - } + let mut current_attributes = module_attributes; + loop { + let parent_module_id = + &ModuleId { krate: target_module_id.krate, local_id: current_attributes.parent }; - 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; - } + if current_module_id == parent_module_id { + is_relative = true; + break; + } - let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { - break; - }; + if current_module_parent_id == Some(*parent_module_id) { + segments.push("super"); + is_relative = true; + break; + } - segments.push(&parent_attributes.name); - current_attributes = parent_attributes; - } + let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { + break; + }; - for segment in segments.iter().rev() { - string.push_str(segment); - string.push_str("::"); + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } } - string.push_str(&module_attributes.name); + let crate_id = target_module_id.krate; + let crate_name = if is_relative { + None + } else { + match crate_id { + CrateId::Root(_) => Some("crate".to_string()), + CrateId::Stdlib(_) => Some("std".to_string()), + CrateId::Crate(_) => dependencies + .iter() + .find(|dep| dep.crate_id == crate_id) + .map(|dep| dep.name.to_string()), + CrateId::Dummy => unreachable!("ICE: A dummy CrateId should not be accessible"), + } + }; + + if let Some(crate_name) = &crate_name { + segments.push(crate_name); + }; - string + segments.reverse(); + segments.join("::") } diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index ee2014ef8b8..8cf06adc027 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -1446,7 +1446,7 @@ mod completion_tests { assert_eq!( item.label_details, Some(CompletionItemLabelDetails { - detail: Some("(use crate::foo::bar::hello_world)".to_string()), + detail: Some("(use super::bar::hello_world)".to_string()), description: Some("fn()".to_string()) }) ); @@ -1458,7 +1458,7 @@ mod completion_tests { start: Position { line: 7, character: 8 }, end: Position { line: 7, character: 8 }, }, - new_text: "use crate::foo::bar::hello_world;\n\n ".to_string(), + new_text: "use super::bar::hello_world;\n\n ".to_string(), }]) ); } @@ -1613,4 +1613,29 @@ mod completion_tests { ) .await; } + + #[test] + async fn test_auto_import_with_super() { + let src = r#" + pub fn bar_baz() {} + + mod tests { + fn foo() { + bar_b>|< + } + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "bar_baz()"); + assert_eq!( + item.label_details, + Some(CompletionItemLabelDetails { + detail: Some("(use super::bar_baz)".to_string()), + description: Some("fn()".to_string()) + }) + ); + } } diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index aa97def8274..95d8b82f84f 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -319,6 +319,21 @@ fn format_parent_module_from_module_id( args: &ProcessRequestCallbackArgs, string: &mut String, ) -> bool { + let mut segments: Vec<&str> = Vec::new(); + + if let Some(module_attributes) = args.interner.try_module_attributes(module) { + segments.push(&module_attributes.name); + + let mut current_attributes = module_attributes; + while let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId { + krate: module.krate, + local_id: current_attributes.parent, + }) { + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } + } + let crate_id = module.krate; let crate_name = match crate_id { CrateId::Root(_) => Some(args.crate_name.clone()), @@ -328,44 +343,21 @@ fn format_parent_module_from_module_id( .find(|dep| dep.crate_id == crate_id) .map(|dep| format!("{}", dep.name)), CrateId::Stdlib(_) => Some("std".to_string()), - CrateId::Dummy => None, + CrateId::Dummy => unreachable!("ICE: A dummy CrateId should not be accessible"), }; - let wrote_crate = if let Some(crate_name) = crate_name { - string.push_str(" "); - string.push_str(&crate_name); - true - } else { - false - }; - - let Some(module_attributes) = args.interner.try_module_attributes(module) else { - return wrote_crate; + if let Some(crate_name) = &crate_name { + segments.push(crate_name); }; - if wrote_crate { - string.push_str("::"); - } else { - string.push_str(" "); - } - - let mut segments = Vec::new(); - let mut current_attributes = module_attributes; - while let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId { - krate: module.krate, - local_id: current_attributes.parent, - }) { - segments.push(&parent_attributes.name); - current_attributes = parent_attributes; - } - - for segment in segments.iter().rev() { - string.push_str(segment); - string.push_str("::"); + if segments.is_empty() { + return false; } - string.push_str(&module_attributes.name); + segments.reverse(); + string.push_str(" "); + string.push_str(&segments.join("::")); true }