From 9bf2dcbf166f9ffd97c369c0de3d95329c850d47 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 12 Sep 2024 16:41:47 -0300 Subject: [PATCH] feat: implement LSP code action "Implement missing members" (#6020) # Description ## Problem Part of #1579 ## Summary Adds a code action to add missing trait impl methods and types. Default methods are not includeded. Here it's working for `Add`: ![lsp-implement-missing-members-add](https://github.com/user-attachments/assets/0b3b4afc-c1bf-4c1e-9c9e-44186c7bb01b) Here for `BigField`: ![lsp-implement-missing-members-big-field](https://github.com/user-attachments/assets/22ec63b2-9fff-4824-b9c5-2aad85cc2fce) Here for a complex type in Aztec-Packages: ![lsp-implement-missing-members-aztec](https://github.com/user-attachments/assets/de822bcc-1397-456a-8175-58613ffa1f0e) ## Additional Context I found this code action in Rust very useful! It saves a lot of time, plus there's no need to copy-paste :-) ## 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. --- tooling/lsp/src/modules.rs | 71 +- tooling/lsp/src/requests/code_action.rs | 9 +- .../code_action/implement_missing_members.rs | 867 ++++++++++++++++++ .../requests/code_action/import_or_qualify.rs | 6 +- tooling/lsp/src/requests/code_action/tests.rs | 5 +- .../src/requests/completion/auto_import.rs | 6 +- tooling/lsp/src/requests/hover.rs | 39 +- 7 files changed, 954 insertions(+), 49 deletions(-) create mode 100644 tooling/lsp/src/requests/code_action/implement_missing_members.rs diff --git a/tooling/lsp/src/modules.rs b/tooling/lsp/src/modules.rs index cce7f324a2e..c8f7430c997 100644 --- a/tooling/lsp/src/modules.rs +++ b/tooling/lsp/src/modules.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use noirc_frontend::{ ast::ItemVisibility, - graph::CrateId, + graph::{CrateId, Dependency}, hir::def_map::{CrateDefMap, ModuleId}, macros_api::{ModuleDefId, NodeInterner}, node_interner::ReferenceId, @@ -41,7 +41,7 @@ pub(crate) fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> Refer /// Returns the fully-qualified path of the given `ModuleDefId` relative to `current_module_id`: /// - If `ModuleDefId` is a module, that module's path is returned /// - Otherwise, that item's parent module's path is returned -pub(crate) fn module_full_path( +pub(crate) fn relative_module_full_path( module_def_id: ModuleDefId, visibility: ItemVisibility, current_module_id: ModuleId, @@ -55,8 +55,12 @@ pub(crate) fn module_full_path( return None; } - full_path = - module_id_path(module_id, ¤t_module_id, current_module_parent_id, interner); + full_path = relative_module_id_path( + module_id, + ¤t_module_id, + current_module_parent_id, + interner, + ); } else { let Some(parent_module) = get_parent_module(interner, module_def_id) else { return None; @@ -66,15 +70,19 @@ pub(crate) fn module_full_path( return None; } - full_path = - module_id_path(parent_module, ¤t_module_id, current_module_parent_id, interner); + full_path = relative_module_id_path( + parent_module, + ¤t_module_id, + current_module_parent_id, + interner, + ); } Some(full_path) } /// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`. /// Returns a relative path if possible. -pub(crate) fn module_id_path( +pub(crate) fn relative_module_id_path( target_module_id: ModuleId, current_module_id: &ModuleId, current_module_parent_id: Option, @@ -130,3 +138,52 @@ pub(crate) fn module_id_path( segments.reverse(); segments.join("::") } + +pub(crate) fn module_full_path( + module: &ModuleId, + interner: &NodeInterner, + crate_id: CrateId, + crate_name: &str, + dependencies: &Vec, +) -> String { + let mut segments: Vec = Vec::new(); + + if let Some(module_attributes) = interner.try_module_attributes(module) { + segments.push(module_attributes.name.clone()); + + let mut current_attributes = module_attributes; + loop { + let Some(parent_local_id) = current_attributes.parent else { + break; + }; + + let Some(parent_attributes) = interner.try_module_attributes(&ModuleId { + krate: module.krate, + local_id: parent_local_id, + }) else { + break; + }; + + segments.push(parent_attributes.name.clone()); + current_attributes = parent_attributes; + } + } + + // We don't record module attributes for the root module, + // so we handle that case separately + if module.krate.is_root() { + if module.krate == crate_id { + segments.push(crate_name.to_string()); + } else { + for dep in dependencies { + if dep.crate_id == crate_id { + segments.push(dep.name.to_string()); + break; + } + } + } + }; + + segments.reverse(); + segments.join("::") +} diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index 95cdc0b88b4..f5cfabe5141 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -11,7 +11,7 @@ use lsp_types::{ }; use noirc_errors::Span; use noirc_frontend::{ - ast::{ConstructorExpression, Path, Visitor}, + ast::{ConstructorExpression, NoirTraitImpl, Path, Visitor}, graph::CrateId, hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}, macros_api::NodeInterner, @@ -26,6 +26,7 @@ use crate::{utils, LspState}; use super::{process_request, to_lsp_location}; mod fill_struct_fields; +mod implement_missing_members; mod import_or_qualify; #[cfg(test)] mod tests; @@ -219,4 +220,10 @@ impl<'a> Visitor for CodeActionFinder<'a> { true } + + fn visit_noir_trait_impl(&mut self, noir_trait_impl: &NoirTraitImpl, span: Span) -> bool { + self.implement_missing_members(noir_trait_impl, span); + + true + } } diff --git a/tooling/lsp/src/requests/code_action/implement_missing_members.rs b/tooling/lsp/src/requests/code_action/implement_missing_members.rs new file mode 100644 index 00000000000..eb1a8704131 --- /dev/null +++ b/tooling/lsp/src/requests/code_action/implement_missing_members.rs @@ -0,0 +1,867 @@ +use std::collections::{BTreeMap, HashMap}; + +use lsp_types::TextEdit; +use noirc_errors::{Location, Span}; +use noirc_frontend::{ + ast::{NoirTraitImpl, TraitImplItemKind, UnresolvedTypeData}, + graph::CrateId, + hir::{ + def_map::{CrateDefMap, ModuleId}, + type_check::generics::TraitGenerics, + }, + hir_def::{function::FuncMeta, stmt::HirPattern, traits::Trait}, + macros_api::{ModuleDefId, NodeInterner}, + node_interner::ReferenceId, + Kind, ResolvedGeneric, Type, TypeVariableKind, +}; + +use crate::{byte_span_to_range, modules::relative_module_id_path}; + +use super::CodeActionFinder; + +impl<'a> CodeActionFinder<'a> { + pub(super) fn implement_missing_members( + &mut self, + noir_trait_impl: &NoirTraitImpl, + span: Span, + ) { + if !self.includes_span(span) { + return; + } + + let location = Location::new(noir_trait_impl.trait_name.span(), self.file); + let Some(ReferenceId::Trait(trait_id)) = self.interner.find_referenced(location) else { + return; + }; + + let trait_ = self.interner.get_trait(trait_id); + + // Get all methods + let mut method_ids = trait_.method_ids.clone(); + + // Also get all associated types + let mut associated_types = HashMap::new(); + for associated_type in &trait_.associated_types { + associated_types.insert(associated_type.name.as_ref(), associated_type); + } + + // Remove the ones that already are implemented + for item in &noir_trait_impl.items { + match &item.item.kind { + TraitImplItemKind::Function(noir_function) => { + method_ids.remove(noir_function.name()); + } + TraitImplItemKind::Constant(..) => (), + TraitImplItemKind::Type { name, alias } => { + if let UnresolvedTypeData::Unspecified = alias.typ { + continue; + } + associated_types.remove(&name.0.contents); + } + } + } + + // Also remove default methods + for trait_function in &trait_.methods { + if trait_function.default_impl.is_some() { + method_ids.remove(&trait_function.name.0.contents); + } + } + + if method_ids.is_empty() && associated_types.is_empty() { + return; + } + + let bytes = self.source.as_bytes(); + let right_brace_index = span.end() as usize - 1; + + // Let's find out the indent + let mut cursor = right_brace_index - 1; + while cursor > 0 { + let c = bytes[cursor] as char; + if c == '\n' { + break; + } + if !c.is_whitespace() { + break; + } + cursor -= 1; + } + let cursor_char = bytes[cursor] as char; + + let indent = if cursor_char == '\n' { right_brace_index - cursor - 1 } else { 0 }; + let indent_string = " ".repeat(indent + 4); + + let index = cursor + 1; + + let Some(range) = byte_span_to_range(self.files, self.file, index..index) else { + return; + }; + + let mut method_ids: Vec<_> = method_ids.iter().collect(); + method_ids.sort_by_key(|(name, _)| *name); + + let mut stubs = Vec::new(); + + for (name, _) in associated_types { + stubs.push(format!("{}type {};\n", indent_string, name)); + } + + for (name, func_id) in method_ids { + let func_meta = self.interner.function_meta(func_id); + + let mut generator = MethodStubGenerator::new( + name, + func_meta, + trait_, + noir_trait_impl, + self.interner, + self.def_maps, + self.module_id, + indent + 4, + ); + let stub = generator.generate(); + stubs.push(stub); + } + + let mut new_text = stubs.join("\n"); + if cursor_char != '\n' { + new_text.insert(0, '\n'); + } + + let title = "Implement missing members".to_string(); + let text_edit = TextEdit { range, new_text }; + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } +} + +struct MethodStubGenerator<'a> { + name: &'a str, + func_meta: &'a FuncMeta, + trait_: &'a Trait, + noir_trait_impl: &'a NoirTraitImpl, + interner: &'a NodeInterner, + def_maps: &'a BTreeMap, + module_id: ModuleId, + indent: usize, + string: String, +} + +impl<'a> MethodStubGenerator<'a> { + #[allow(clippy::too_many_arguments)] + fn new( + name: &'a str, + func_meta: &'a FuncMeta, + trait_: &'a Trait, + noir_trait_impl: &'a NoirTraitImpl, + interner: &'a NodeInterner, + def_maps: &'a BTreeMap, + module_id: ModuleId, + indent: usize, + ) -> Self { + Self { + name, + func_meta, + trait_, + noir_trait_impl, + interner, + def_maps, + module_id, + indent, + string: String::new(), + } + } + + fn generate(&mut self) -> String { + let indent_string = " ".repeat(self.indent); + + self.string.push_str(&indent_string); + self.string.push_str("fn "); + self.string.push_str(self.name); + self.append_resolved_generics(&self.func_meta.direct_generics); + self.string.push('('); + for (index, (pattern, typ, _visibility)) in self.func_meta.parameters.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + if self.append_pattern(pattern) { + self.string.push_str(": "); + self.append_type(typ); + } + } + self.string.push(')'); + + let return_type = self.func_meta.return_type(); + if return_type != &Type::Unit { + self.string.push_str(" -> "); + self.append_type(return_type); + } + + if !self.func_meta.trait_constraints.is_empty() { + self.string.push_str(" where "); + for (index, constraint) in self.func_meta.trait_constraints.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + self.append_type(&constraint.typ); + self.string.push_str(": "); + let trait_ = self.interner.get_trait(constraint.trait_id); + self.string.push_str(&trait_.name.0.contents); + self.append_trait_generics(&constraint.trait_generics); + } + } + + self.string.push_str(" {\n"); + + let body_indent_string = " ".repeat(self.indent + 4); + self.string.push_str(&body_indent_string); + self.string.push_str("panic(f\"Implement "); + self.string.push_str(self.name); + self.string.push_str("\")\n"); + self.string.push_str(&indent_string); + self.string.push_str("}\n"); + std::mem::take(&mut self.string) + } + + /// Appends a pattern and returns true if this was not the self type + fn append_pattern(&mut self, pattern: &HirPattern) -> bool { + match pattern { + HirPattern::Identifier(hir_ident) => { + let definition = self.interner.definition(hir_ident.id); + self.string.push_str(&definition.name); + &definition.name != "self" + } + HirPattern::Mutable(pattern, _) => { + self.string.push_str("mut "); + self.append_pattern(pattern) + } + HirPattern::Tuple(patterns, _) => { + self.string.push('('); + for (index, pattern) in patterns.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + self.append_pattern(pattern); + } + self.string.push(')'); + true + } + HirPattern::Struct(typ, patterns, _) => { + self.append_type(typ); + self.string.push_str(" { "); + for (index, (name, _pattern)) in patterns.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + self.string.push_str(&name.0.contents); + } + self.string.push_str(" }"); + true + } + } + } + + fn append_type(&mut self, typ: &Type) { + match typ { + Type::FieldElement => self.string.push_str("Field"), + Type::Array(n, e) => { + self.string.push('['); + self.append_type(e); + self.string.push_str("; "); + self.append_type(n); + self.string.push(']'); + } + Type::Slice(typ) => { + self.string.push('['); + self.append_type(typ); + self.string.push(']'); + } + Type::Tuple(types) => { + self.string.push('('); + for (index, typ) in types.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + self.append_type(typ); + } + self.string.push(')'); + } + Type::Struct(struct_type, generics) => { + let struct_type = struct_type.borrow(); + + let current_module_data = + &self.def_maps[&self.module_id.krate].modules()[self.module_id.local_id.0]; + + // Check if the struct type is already imported/visible in this module + let per_ns = current_module_data.find_name(&struct_type.name); + if let Some((module_def_id, _, _)) = per_ns.types { + if module_def_id == ModuleDefId::TypeId(struct_type.id) { + self.string.push_str(&struct_type.name.0.contents); + self.append_generics(generics); + return; + } + } + + let module_id = struct_type.id.module_id(); + let module_data = &self.def_maps[&module_id.krate].modules()[module_id.local_id.0]; + let parent_module_local_id = module_data.parent.unwrap(); + let parent_module_id = + ModuleId { krate: module_id.krate, local_id: parent_module_local_id }; + + let current_module_parent_id = current_module_data + .parent + .map(|parent| ModuleId { krate: self.module_id.krate, local_id: parent }); + + let relative_path = relative_module_id_path( + parent_module_id, + &self.module_id, + current_module_parent_id, + self.interner, + ); + + if !relative_path.is_empty() { + self.string.push_str(&relative_path); + self.string.push_str("::"); + } + self.string.push_str(&struct_type.name.0.contents); + self.append_generics(generics); + } + Type::Alias(type_alias, generics) => { + let type_alias = type_alias.borrow(); + + let current_module_data = + &self.def_maps[&self.module_id.krate].modules()[self.module_id.local_id.0]; + + // Check if the alias type is already imported/visible in this module + let per_ns = current_module_data.find_name(&type_alias.name); + if let Some((module_def_id, _, _)) = per_ns.types { + if module_def_id == ModuleDefId::TypeAliasId(type_alias.id) { + self.string.push_str(&type_alias.name.0.contents); + self.append_generics(generics); + return; + } + } + + let parent_module_id = + self.interner.reference_module(ReferenceId::Alias(type_alias.id)).unwrap(); + + let current_module_parent_id = current_module_data + .parent + .map(|parent| ModuleId { krate: self.module_id.krate, local_id: parent }); + + let relative_path = relative_module_id_path( + *parent_module_id, + &self.module_id, + current_module_parent_id, + self.interner, + ); + + if !relative_path.is_empty() { + self.string.push_str(&relative_path); + self.string.push_str("::"); + } + self.string.push_str(&type_alias.name.0.contents); + self.append_generics(generics); + } + Type::TraitAsType(trait_id, _, trait_generics) => { + let trait_ = self.interner.get_trait(*trait_id); + + let current_module_data = + &self.def_maps[&self.module_id.krate].modules()[self.module_id.local_id.0]; + + // Check if the trait type is already imported/visible in this module + let per_ns = current_module_data.find_name(&trait_.name); + if let Some((module_def_id, _, _)) = per_ns.types { + if module_def_id == ModuleDefId::TraitId(*trait_id) { + self.string.push_str(&trait_.name.0.contents); + self.append_trait_generics(trait_generics); + return; + } + } + + let parent_module_id = + self.interner.reference_module(ReferenceId::Trait(*trait_id)).unwrap(); + + let current_module_parent_id = current_module_data + .parent + .map(|parent| ModuleId { krate: self.module_id.krate, local_id: parent }); + + let relative_path = relative_module_id_path( + *parent_module_id, + &self.module_id, + current_module_parent_id, + self.interner, + ); + + if !relative_path.is_empty() { + self.string.push_str(&relative_path); + self.string.push_str("::"); + } + self.string.push_str(&trait_.name.0.contents); + self.append_trait_generics(trait_generics); + } + Type::TypeVariable(typevar, _) => { + if typevar.id() == self.trait_.self_type_typevar.id() { + self.string.push_str("Self"); + return; + } + + let generics = &self.trait_.generics; + if let Some(index) = + generics.iter().position(|generic| generic.type_var.id() == typevar.id()) + { + if let Some(typ) = self.noir_trait_impl.trait_generics.ordered_args.get(index) { + self.string.push_str(&typ.to_string()); + return; + } + } + + for associated_type in &self.trait_.associated_types { + if typevar.id() == associated_type.type_var.id() { + self.string.push_str("Self::"); + self.string.push_str(&associated_type.name); + return; + } + } + + for generic in &self.func_meta.direct_generics { + if typevar.id() == generic.type_var.id() { + self.string.push_str(&generic.name); + return; + } + } + + self.string.push_str("error"); + } + Type::NamedGeneric(typevar, _name, _kind) => { + self.append_type(&Type::TypeVariable(typevar.clone(), TypeVariableKind::Normal)); + } + Type::Function(args, ret, env, unconstrained) => { + if *unconstrained { + self.string.push_str("unconstrained "); + } + self.string.push_str("fn"); + + if let Type::Unit = **env { + } else { + self.string.push('['); + self.append_type(env); + self.string.push(']'); + } + + self.string.push('('); + for (index, arg) in args.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + self.append_type(arg); + } + self.string.push(')'); + + if let Type::Unit = **ret { + } else { + self.string.push_str(" -> "); + self.append_type(ret); + } + } + Type::MutableReference(typ) => { + self.string.push_str("&mut "); + self.append_type(typ); + } + Type::Forall(_, _) => { + panic!("Shouldn't get a Type::Forall"); + } + Type::InfixExpr(left, op, right) => { + self.append_type(left); + self.string.push(' '); + self.string.push_str(&op.to_string()); + self.string.push(' '); + self.append_type(right); + } + Type::Constant(_) + | Type::Integer(_, _) + | Type::Bool + | Type::String(_) + | Type::FmtString(_, _) + | Type::Unit + | Type::Quoted(_) + | Type::Error => self.string.push_str(&typ.to_string()), + } + } + + fn append_generics(&mut self, generics: &[Type]) { + if generics.is_empty() { + return; + } + + self.string.push('<'); + for (index, typ) in generics.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + self.append_type(typ); + } + self.string.push('>'); + } + + fn append_trait_generics(&mut self, generics: &TraitGenerics) { + if generics.named.is_empty() && generics.ordered.is_empty() { + return; + } + + let mut index = 0; + + self.string.push('<'); + for generic in &generics.ordered { + if index > 0 { + self.string.push_str(", "); + } + self.append_type(generic); + index += 1; + } + for named_type in &generics.named { + if index > 0 { + self.string.push_str(", "); + } + self.string.push_str(&named_type.name.0.contents); + self.string.push_str(" = "); + self.append_type(&named_type.typ); + index += 1; + } + self.string.push('>'); + } + + fn append_resolved_generics(&mut self, generics: &[ResolvedGeneric]) { + if generics.is_empty() { + return; + } + + self.string.push('<'); + for (index, generic) in self.func_meta.direct_generics.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + self.append_resolved_generic(generic); + } + self.string.push('>'); + } + + fn append_resolved_generic(&mut self, generic: &ResolvedGeneric) { + match &generic.kind { + Kind::Normal => self.string.push_str(&generic.name), + Kind::Numeric(typ) => { + self.string.push_str("let "); + self.string.push_str(&generic.name); + self.string.push_str(": "); + self.append_type(typ); + } + } + } +} + +#[cfg(test)] +mod tests { + use tokio::test; + + use crate::requests::code_action::tests::assert_code_action; + + #[test] + async fn test_add_missing_impl_members_simple() { + let title = "Implement missing members"; + + let src = r#" +trait Trait { + fn foo(x: i32) -> i32; + fn bar() {} +} + +struct Foo {} + +impl Tra>| i32; + fn bar() {} +} + +struct Foo {} + +impl Trait for Foo { + fn foo(x: i32) -> i32 { + panic(f"Implement foo") + } +}"#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_add_missing_impl_members_multiple_with_self_type() { + let title = "Implement missing members"; + + let src = r#" +trait Trait { + fn bar(self) -> Self; + fn foo(x: i32) -> i32; +} + +struct Foo {} + +impl Tra>| Self; + fn foo(x: i32) -> i32; +} + +struct Foo {} + +impl Trait for Foo { + fn bar(self) -> Self { + panic(f"Implement bar") + } + + fn foo(x: i32) -> i32 { + panic(f"Implement foo") + } +}"#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_add_missing_impl_members_qualify_type() { + let title = "Implement missing members"; + + let src = r#" +mod moo { + struct Moo {} + + trait Trait { + fn foo(x: Moo); + } +} + +struct Foo {} + +use moo::Trait; + +impl Tra>|| { + fn foo(x: T) -> [T; N] where M: Bar; +} + +struct Foo {} + +impl Tra>| for Foo { +}"#; + + let expected = r#" +trait Bar {} + +trait Trait { + fn foo(x: T) -> [T; N] where M: Bar; +} + +struct Foo {} + +impl Trait<[U]> for Foo { + fn foo(x: [U]) -> [[U]; N] where M: Bar { + panic(f"Implement foo") + } +}"#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_add_missing_impl_members_associated_types() { + let title = "Implement missing members"; + + let src = r#" +trait Trait { + type Elem; + + fn foo(x: Self::Elem) -> [Self::Elem]; +} + +struct Foo {} + +impl Trait>|< for Foo { +}"#; + + let expected = r#" +trait Trait { + type Elem; + + fn foo(x: Self::Elem) -> [Self::Elem]; +} + +struct Foo {} + +impl Trait for Foo { + type Elem; + + fn foo(x: Self::Elem) -> [Self::Elem] { + panic(f"Implement foo") + } +}"#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_add_missing_impl_members_nested() { + let title = "Implement missing members"; + + let src = r#" +mod moo { + trait Trait { + fn foo(); + fn bar(); + } + + struct Foo {} + + impl Tra>|| CodeActionFinder<'a> { for (module_def_id, visibility, defining_module) in entries { let module_full_path = if let Some(defining_module) = defining_module { - module_id_path( + relative_module_id_path( *defining_module, &self.module_id, current_module_parent_id, self.interner, ) } else { - let Some(module_full_path) = module_full_path( + let Some(module_full_path) = relative_module_full_path( *module_def_id, *visibility, self.module_id, diff --git a/tooling/lsp/src/requests/code_action/tests.rs b/tooling/lsp/src/requests/code_action/tests.rs index a8c81ced2a7..6be2a1ced1a 100644 --- a/tooling/lsp/src/requests/code_action/tests.rs +++ b/tooling/lsp/src/requests/code_action/tests.rs @@ -71,7 +71,10 @@ pub(crate) async fn assert_code_action(title: &str, src: &str, expected: &str) { assert_eq!(text_edits.len(), 1); let result = apply_text_edit(&src.replace(">|<", ""), &text_edits[0]); - assert_eq!(result, expected); + if result != expected { + println!("Expected:\n```\n{}\n```\n\nGot:\n```\n{}\n```", expected, result); + assert_eq!(result, expected); + } } fn apply_text_edit(src: &str, text_edit: &TextEdit) -> String { diff --git a/tooling/lsp/src/requests/completion/auto_import.rs b/tooling/lsp/src/requests/completion/auto_import.rs index d8823794999..f9c5dab0672 100644 --- a/tooling/lsp/src/requests/completion/auto_import.rs +++ b/tooling/lsp/src/requests/completion/auto_import.rs @@ -1,7 +1,7 @@ use lsp_types::{Position, Range, TextEdit}; use noirc_frontend::macros_api::ModuleDefId; -use crate::modules::{get_parent_module_id, module_full_path, module_id_path}; +use crate::modules::{get_parent_module_id, relative_module_full_path, relative_module_id_path}; use super::{ kinds::{FunctionCompletionKind, FunctionKind, RequestedItems}, @@ -40,14 +40,14 @@ impl<'a> NodeFinder<'a> { }; let module_full_path = if let Some(defining_module) = defining_module { - module_id_path( + relative_module_id_path( *defining_module, &self.module_id, current_module_parent_id, self.interner, ) } else { - let Some(module_full_path) = module_full_path( + let Some(module_full_path) = relative_module_full_path( *module_def_id, *visibility, self.module_id, diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index dab6ddd0fc6..8e9666a624b 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -14,7 +14,7 @@ use noirc_frontend::{ Generics, Shared, StructType, Type, TypeAlias, TypeBinding, TypeVariable, }; -use crate::LspState; +use crate::{modules::module_full_path, LspState}; use super::{process_request, to_lsp_location, ProcessRequestCallbackArgs}; @@ -352,43 +352,14 @@ 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; - loop { - let Some(parent_local_id) = current_attributes.parent else { - break; - }; - - let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId { - krate: module.krate, - local_id: parent_local_id, - }) else { - break; - }; - - segments.push(&parent_attributes.name); - current_attributes = parent_attributes; - } - } - - // We don't record module attributes for the root module, - // so we handle that case separately - if module.krate.is_root() { - segments.push(&args.crate_name); - }; - - if segments.is_empty() { + let full_path = + module_full_path(module, args.interner, args.crate_id, &args.crate_name, args.dependencies); + if full_path.is_empty() { return false; } - segments.reverse(); - string.push_str(" "); - string.push_str(&segments.join("::")); + string.push_str(&full_path); true }