From 010c835e4ebfdf49ea4e9326abafcdeb587153b6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 12 Jul 2024 10:54:06 -0300 Subject: [PATCH] feat: LSP hover (#5491) # Description ## Problem Resolves #1575 ## Summary Implements LSP hover for: - modules - structs - struct members - traits - global variables - functions - aliases - local variables The popup content is similar to that of Rust Analyzer except that there's no link to go to that reference. The reason is that it seems the LSP type to do that has an "actions" property but I can't find that in the lsp-types library we are using. That said, once this PR is merged doing that should be relatively easy, and it might be better to keep PRs smaller. There's one case where the hover doesn't work: if you have `let x: i32 = Default::default();`, hovering over `default` won't show the impl function. It's doable, but doing it requires more changes in this PR and at one point I wanted to stop :-) This PR also includes several fixes that I found in "find-all-references" and "rename", but because hover and those are all kind of related, and testing things for hover is simpler, I decided to add tests for those things and fix them. Here it's in action: https://github.com/noir-lang/noir/assets/209371/bb64d368-2cf7-450c-99f3-e2aba52ca4e3 ## Additional Context None. ## 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. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .../src/elaborator/expressions.rs | 9 +- compiler/noirc_frontend/src/elaborator/mod.rs | 12 +- .../noirc_frontend/src/elaborator/scope.rs | 16 +- .../noirc_frontend/src/elaborator/types.rs | 18 +- .../src/hir/def_collector/dc_crate.rs | 27 +- .../src/hir/def_collector/dc_mod.rs | 29 +- .../src/hir/resolution/import.rs | 21 +- .../src/hir/resolution/path_resolver.rs | 9 +- .../src/hir/resolution/resolver.rs | 1 + .../noirc_frontend/src/hir/type_check/mod.rs | 3 +- .../noirc_frontend/src/hir_def/function.rs | 5 +- compiler/noirc_frontend/src/locations.rs | 49 +- compiler/noirc_frontend/src/node_interner.rs | 32 +- tooling/lsp/src/lib.rs | 5 +- tooling/lsp/src/requests/goto_declaration.rs | 6 +- tooling/lsp/src/requests/goto_definition.rs | 13 +- tooling/lsp/src/requests/hover.rs | 641 ++++++++++++++++++ tooling/lsp/src/requests/mod.rs | 35 +- tooling/lsp/src/requests/references.rs | 24 +- tooling/lsp/src/requests/rename.rs | 71 +- tooling/lsp/src/types.rs | 8 +- .../test_programs/rename_function/src/main.nr | 13 + .../test_programs/workspace/one/src/lib.nr | 26 +- .../test_programs/workspace/two/src/lib.nr | 48 ++ .../test_programs/workspace/two/src/other.nr | 2 + 25 files changed, 986 insertions(+), 137 deletions(-) create mode 100644 tooling/lsp/src/requests/hover.rs create mode 100644 tooling/lsp/test_programs/workspace/two/src/other.nr diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index e8098723692..72e2beea570 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -320,6 +320,7 @@ impl<'context> Elaborator<'context> { let (mut object, mut object_type) = self.elaborate_expression(method_call.object); object_type = object_type.follow_bindings(); + let method_name_span = method_call.method_name.span(); let method_name = method_call.method_name.0.contents.as_str(); match self.lookup_method(&object_type, method_name, span) { Some(method_ref) => { @@ -385,6 +386,9 @@ impl<'context> Elaborator<'context> { self.interner.push_expr_type(function_id, func_type.clone()); + self.interner + .add_function_reference(func_id, Location::new(method_name_span, self.file)); + // Type check the new call now that it has been changed from a method call // to a function call. This way we avoid duplicating code. let typ = self.type_check_call(&function_call, func_type, function_args, span); @@ -399,7 +403,8 @@ impl<'context> Elaborator<'context> { constructor: ConstructorExpression, ) -> (HirExpression, Type) { let span = constructor.type_name.span(); - let is_self_type = constructor.type_name.last_segment().is_self_type_name(); + let last_segment = constructor.type_name.last_segment(); + let is_self_type = last_segment.is_self_type_name(); let (r#type, struct_generics) = if let Some(struct_id) = constructor.struct_type { let typ = self.interner.get_struct(struct_id); @@ -430,7 +435,7 @@ impl<'context> Elaborator<'context> { }); let struct_id = struct_type.borrow().id; - let reference_location = Location::new(span, self.file); + let reference_location = Location::new(last_segment.span(), self.file); self.interner.add_struct_reference(struct_id, reference_location, is_self_type); (expr, Type::Struct(struct_type, generics)) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index f736ad76970..f0d3e3cae1f 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -720,6 +720,12 @@ impl<'context> Elaborator<'context> { let statements = std::mem::take(&mut func.def.body.statements); let body = BlockExpression { statements }; + let struct_id = if let Some(Type::Struct(struct_type, _)) = &self.self_type { + Some(struct_type.borrow().id) + } else { + None + }; + let meta = FuncMeta { name: name_ident, kind: func.kind, @@ -727,6 +733,7 @@ impl<'context> Elaborator<'context> { typ, direct_generics, all_generics: self.generics.clone(), + struct_id, trait_impl: self.current_trait_impl, parameters: parameters.into(), parameter_idents, @@ -1232,7 +1239,7 @@ impl<'context> Elaborator<'context> { for field_index in 0..fields_len { self.interner - .add_definition_location(ReferenceId::StructMember(type_id, field_index)); + .add_definition_location(ReferenceId::StructMember(type_id, field_index), None); } self.run_comptime_attributes_on_struct(attributes, type_id, span, &mut generated_items); @@ -1426,7 +1433,8 @@ impl<'context> Elaborator<'context> { self.elaborate_comptime_global(global_id); } - self.interner.add_definition_location(ReferenceId::Global(global_id)); + self.interner + .add_definition_location(ReferenceId::Global(global_id), Some(self.module_id())); self.local_module = old_module; self.file = old_file; diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index af6fc0e5d5e..e01c7e997d4 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -6,7 +6,6 @@ use crate::hir::resolution::path_resolver::{PathResolver, StandardPathResolver}; use crate::hir::resolution::resolver::SELF_TYPE_NAME; use crate::hir::scope::{Scope as GenericScope, ScopeTree as GenericScopeTree}; use crate::macros_api::Ident; -use crate::node_interner::ReferenceId; use crate::{ hir::{ def_map::{ModuleDefId, TryFromModuleDefId}, @@ -48,17 +47,30 @@ impl<'context> Elaborator<'context> { let path_resolution; if self.interner.track_references { - let mut references: Vec = Vec::new(); + let last_segment = path.last_segment(); + let location = Location::new(last_segment.span(), self.file); + let is_self_type_name = last_segment.is_self_type_name(); + + let mut references: Vec<_> = Vec::new(); path_resolution = resolver.resolve(self.def_maps, path.clone(), &mut Some(&mut references))?; for (referenced, ident) in references.iter().zip(path.segments) { + let Some(referenced) = referenced else { + continue; + }; self.interner.add_reference( *referenced, Location::new(ident.span(), self.file), ident.is_self_type_name(), ); } + + self.interner.add_module_def_id_reference( + path_resolution.module_def_id, + location, + is_self_type_name, + ); } else { path_resolution = resolver.resolve(self.def_maps, path, &mut None)?; } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 9134fc851b9..35ff421ed32 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -57,12 +57,16 @@ impl<'context> Elaborator<'context> { use crate::ast::UnresolvedTypeData::*; let span = typ.span; - let (is_self_type_name, is_synthetic) = if let Named(ref named_path, _, synthetic) = typ.typ - { - (named_path.last_segment().is_self_type_name(), synthetic) - } else { - (false, false) - }; + let (named_path_span, is_self_type_name, is_synthetic) = + if let Named(ref named_path, _, synthetic) = typ.typ { + ( + Some(named_path.last_segment().span()), + named_path.last_segment().is_self_type_name(), + synthetic, + ) + } else { + (None, false, false) + }; let resolved_type = match typ.typ { FieldElement => Type::FieldElement, @@ -153,7 +157,7 @@ impl<'context> Elaborator<'context> { }; if let Some(unresolved_span) = typ.span { - let location = Location::new(unresolved_span, self.file); + let location = Location::new(named_path_span.unwrap_or(unresolved_span), self.file); match resolved_type { Type::Struct(ref struct_type, _) => { diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 8b926c8107f..f42819dab1f 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -347,7 +347,7 @@ impl DefCollector { for collected_import in std::mem::take(&mut def_collector.imports) { let module_id = collected_import.module_id; let resolved_import = if context.def_interner.track_references { - let mut references: Vec = Vec::new(); + let mut references: Vec> = Vec::new(); let resolved_import = resolve_import( crate_id, &collected_import, @@ -359,6 +359,9 @@ impl DefCollector { let file_id = current_def_map.file_id(module_id); for (referenced, ident) in references.iter().zip(&collected_import.path.segments) { + let Some(referenced) = referenced else { + continue; + }; context.def_interner.add_reference( *referenced, Location::new(ident.span(), file_id), @@ -552,27 +555,7 @@ fn add_import_reference( } let location = Location::new(name.span(), file_id); - - match def_id { - crate::macros_api::ModuleDefId::ModuleId(module_id) => { - interner.add_module_reference(module_id, location); - } - crate::macros_api::ModuleDefId::FunctionId(func_id) => { - interner.add_function_reference(func_id, location); - } - crate::macros_api::ModuleDefId::TypeId(struct_id) => { - interner.add_struct_reference(struct_id, location, false); - } - crate::macros_api::ModuleDefId::TraitId(trait_id) => { - interner.add_trait_reference(trait_id, location, false); - } - crate::macros_api::ModuleDefId::TypeAliasId(type_alias_id) => { - interner.add_alias_reference(type_alias_id, location); - } - crate::macros_api::ModuleDefId::GlobalId(global_id) => { - interner.add_global_reference(global_id, location); - } - }; + interner.add_module_def_id_reference(def_id, location, false); } fn inject_prelude( 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 bcd24ca8ed3..1f30b4388b6 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::ReferenceId; +use crate::node_interner::{ModuleAttributes, ReferenceId}; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, @@ -90,7 +90,7 @@ pub fn collect_defs( errors.extend(collector.collect_structs(context, ast.types, crate_id)); - errors.extend(collector.collect_type_aliases(context, ast.type_aliases)); + errors.extend(collector.collect_type_aliases(context, ast.type_aliases, crate_id)); errors.extend(collector.collect_functions(context, ast.functions, crate_id)); @@ -318,7 +318,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)); + context.def_interner.add_definition_location( + ReferenceId::Struct(id), + Some(ModuleId { krate, local_id: self.module_id }), + ); } definition_errors } @@ -329,6 +332,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, type_aliases: Vec, + krate: CrateId, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; for type_alias in type_aliases { @@ -365,7 +369,10 @@ 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)); + context.def_interner.add_definition_location( + ReferenceId::Alias(type_alias_id), + Some(ModuleId { krate, local_id: self.module_id }), + ); } errors } @@ -532,7 +539,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)); + context.def_interner.add_definition_location( + ReferenceId::Trait(trait_id), + Some(ModuleId { krate, local_id: self.module_id }), + ); self.def_collector.items.traits.insert(trait_id, unresolved); } @@ -720,7 +730,14 @@ impl<'a> ModCollector<'a> { return Err(err); } - context.def_interner.add_module_location(mod_id, mod_location); + context.def_interner.add_module_attributes( + mod_id, + ModuleAttributes { + name: mod_name.0.contents.clone(), + location: mod_location, + parent: self.module_id, + }, + ); } Ok(mod_id) diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 710c12a91bf..e0c87e2d9e4 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -81,7 +81,7 @@ pub fn resolve_import( crate_id: CrateId, import_directive: &ImportDirective, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> Result { let module_scope = import_directive.module_id; let NamespaceResolution { @@ -126,7 +126,7 @@ fn resolve_path_to_ns( crate_id: CrateId, importing_crate: CrateId, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> NamespaceResolutionResult { let import_path = &import_directive.path.segments; let def_map = &def_maps[&crate_id]; @@ -196,7 +196,7 @@ fn resolve_path_from_crate_root( import_path: &[Ident], def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> NamespaceResolutionResult { resolve_name_in_module( crate_id, @@ -214,7 +214,7 @@ fn resolve_name_in_module( import_path: &[Ident], starting_mod: LocalModuleId, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> NamespaceResolutionResult { let def_map = &def_maps[&krate]; let mut current_mod_id = ModuleId { krate, local_id: starting_mod }; @@ -247,7 +247,7 @@ fn resolve_name_in_module( current_mod_id = match typ { ModuleDefId::ModuleId(id) => { if let Some(path_references) = path_references { - path_references.push(ReferenceId::Module(id)); + path_references.push(Some(ReferenceId::Module(id))); } id } @@ -255,14 +255,14 @@ fn resolve_name_in_module( // TODO: If impls are ever implemented, types can be used in a path ModuleDefId::TypeId(id) => { if let Some(path_references) = path_references { - path_references.push(ReferenceId::Struct(id)); + path_references.push(Some(ReferenceId::Struct(id))); } id.module_id() } ModuleDefId::TypeAliasId(_) => panic!("type aliases cannot be used in type namespace"), ModuleDefId::TraitId(id) => { if let Some(path_references) = path_references { - path_references.push(ReferenceId::Trait(id)); + path_references.push(Some(ReferenceId::Trait(id))); } id.0 } @@ -309,7 +309,7 @@ fn resolve_external_dep( current_def_map: &CrateDefMap, directive: &ImportDirective, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, importing_crate: CrateId, ) -> NamespaceResolutionResult { // Use extern_prelude to get the dep @@ -327,6 +327,11 @@ fn resolve_external_dep( // See `singleton_import.nr` test case for a check that such cases are handled elsewhere. let path_without_crate_name = &path[1..]; + // Given that we skipped the first segment, record that it doesn't refer to any module or type. + if let Some(path_references) = path_references { + path_references.push(None); + } + let path = Path { segments: path_without_crate_name.to_vec(), kind: PathKind::Plain, diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index c3dc76b635f..7cd44a84018 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -9,12 +9,13 @@ use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; pub trait PathResolver { /// Resolve the given path returning the resolved ModuleDefId. /// If `path_references` is `Some`, a `ReferenceId` for each segment in `path` - /// will be resolved and pushed. + /// will be resolved and pushed (some entries will be None if they don't refer to + /// a module or type). fn resolve( &self, def_maps: &BTreeMap, path: Path, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult; fn local_module_id(&self) -> LocalModuleId; @@ -38,7 +39,7 @@ impl PathResolver for StandardPathResolver { &self, def_maps: &BTreeMap, path: Path, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult { resolve_path(def_maps, self.module_id, path, path_references) } @@ -58,7 +59,7 @@ pub fn resolve_path( def_maps: &BTreeMap, module_id: ModuleId, path: Path, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult { // lets package up the path into an ImportDirective and resolve it using that let import = diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 856a769c9dd..55cb2145ba8 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1105,6 +1105,7 @@ impl<'a> Resolver<'a> { location, typ, direct_generics, + struct_id: None, trait_impl: self.current_trait_impl, parameters: parameters.into(), return_type: func.def.return_type.clone(), diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 1a70bade863..57125e1e2dd 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -570,6 +570,7 @@ pub mod test { .into(), return_visibility: Visibility::Private, has_body: true, + struct_id: None, trait_impl: None, return_type: FunctionReturnType::Default(Span::default()), trait_constraints: Vec::new(), @@ -696,7 +697,7 @@ pub mod test { &self, _def_maps: &BTreeMap, path: Path, - _path_references: &mut Option<&mut Vec>, + _path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult { // Not here that foo::bar and hello::foo::bar would fetch the same thing let name = path.segments.last().unwrap(); diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index fa8bb55abee..e666f996231 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -6,7 +6,7 @@ use super::stmt::HirPattern; use super::traits::TraitConstraint; use crate::ast::{FunctionKind, FunctionReturnType, Visibility}; use crate::graph::CrateId; -use crate::macros_api::BlockExpression; +use crate::macros_api::{BlockExpression, StructId}; use crate::node_interner::{ExprId, NodeInterner, TraitImplId}; use crate::{ResolvedGeneric, Type}; @@ -126,6 +126,9 @@ pub struct FuncMeta { pub trait_constraints: Vec, + /// The struct this function belongs to, if any + pub struct_id: Option, + /// The trait impl this function belongs to, if any pub trait_impl: Option, diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 5ca8c1493eb..0ba74e22781 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -4,7 +4,7 @@ use rangemap::RangeMap; use rustc_hash::FxHashMap; use crate::{ - hir::def_map::ModuleId, + hir::def_map::{ModuleDefId, ModuleId}, macros_api::{NodeInterner, StructId}, node_interner::{DefinitionId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId}, }; @@ -17,7 +17,7 @@ pub(crate) struct LocationIndices { impl LocationIndices { pub(crate) fn add_location(&mut self, location: Location, node_index: PetGraphIndex) { - // Some location spans are empty: maybe they are from ficticious nodes? + // Some location spans are empty: maybe they are from fictitious nodes? if location.span.start() == location.span.end() { return; } @@ -35,7 +35,7 @@ impl LocationIndices { impl NodeInterner { pub fn reference_location(&self, reference: ReferenceId) -> Location { match reference { - ReferenceId::Module(id) => self.module_location(&id), + ReferenceId::Module(id) => self.module_attributes(&id).location, ReferenceId::Function(id) => self.function_modifiers(&id).name_location, ReferenceId::Struct(id) => { let struct_type = self.get_struct(id); @@ -62,6 +62,38 @@ impl NodeInterner { } } + pub fn reference_module(&self, reference: ReferenceId) -> Option<&ModuleId> { + self.reference_modules.get(&reference) + } + + pub(crate) fn add_module_def_id_reference( + &mut self, + def_id: ModuleDefId, + location: Location, + is_self_type: bool, + ) { + match def_id { + ModuleDefId::ModuleId(module_id) => { + self.add_module_reference(module_id, location); + } + ModuleDefId::FunctionId(func_id) => { + self.add_function_reference(func_id, location); + } + ModuleDefId::TypeId(struct_id) => { + self.add_struct_reference(struct_id, location, is_self_type); + } + ModuleDefId::TraitId(trait_id) => { + self.add_trait_reference(trait_id, location, is_self_type); + } + ModuleDefId::TypeAliasId(type_alias_id) => { + self.add_alias_reference(type_alias_id, location); + } + ModuleDefId::GlobalId(global_id) => { + self.add_global_reference(global_id, location); + } + }; + } + pub(crate) fn add_module_reference(&mut self, id: ModuleId, location: Location) { self.add_reference(ReferenceId::Module(id), location, false); } @@ -129,7 +161,11 @@ impl NodeInterner { self.location_indices.add_location(reference_location, reference_index); } - pub(crate) fn add_definition_location(&mut self, referenced: ReferenceId) { + pub(crate) fn add_definition_location( + &mut self, + referenced: ReferenceId, + module_id: Option, + ) { if !self.track_references { return; } @@ -137,6 +173,9 @@ impl NodeInterner { let referenced_index = self.get_or_insert_reference(referenced); let referenced_location = self.reference_location(referenced); self.location_indices.add_location(referenced_location, referenced_index); + if let Some(module_id) = module_id { + self.reference_modules.insert(referenced, module_id); + } } #[tracing::instrument(skip(self), ret)] @@ -168,7 +207,7 @@ impl NodeInterner { // Starting at the given location, find the node referenced by it. Then, gather // all locations that reference that node, and return all of them - // (the references and optionally the referenced node if `include_referencedd` is true). + // (the references and optionally the referenced node if `include_referenced` is true). // If `include_self_type_name` is true, references where "Self" is written are returned, // otherwise they are not. // Returns `None` if the location is not known to this interner. diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index a009a42df53..e534f1bab29 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -44,6 +44,13 @@ use crate::{Shared, TypeAlias, TypeBindings, TypeVariable, TypeVariableId, TypeV /// This is needed to stop recursing for cases such as `impl Foo for T where T: Eq` const IMPL_SEARCH_RECURSION_LIMIT: u32 = 10; +#[derive(Debug)] +pub struct ModuleAttributes { + pub name: String, + pub location: Location, + pub parent: LocalModuleId, +} + type StructAttributes = Vec; /// The node interner is the central storage location of all nodes in Noir's Hir (the @@ -68,7 +75,7 @@ pub struct NodeInterner { function_modules: HashMap, // The location of each module - module_locations: HashMap, + module_attributes: HashMap, /// This graph tracks dependencies between different global definitions. /// This is used to ensure the absence of dependency cycles for globals and types. @@ -219,6 +226,10 @@ pub struct NodeInterner { /// Store the location of the references in the graph pub(crate) location_indices: LocationIndices, + + // The module where each reference is + // (ReferenceId::Reference and ReferenceId::Local aren't included here) + pub(crate) reference_modules: HashMap, } /// A dependency in the dependency graph may be a type or a definition. @@ -553,7 +564,7 @@ impl Default for NodeInterner { function_definition_ids: HashMap::new(), function_modifiers: HashMap::new(), function_modules: HashMap::new(), - module_locations: HashMap::new(), + module_attributes: HashMap::new(), func_id_to_trait: HashMap::new(), dependency_graph: petgraph::graph::DiGraph::new(), dependency_graph_indices: HashMap::new(), @@ -586,6 +597,7 @@ impl Default for NodeInterner { location_indices: LocationIndices::default(), reference_graph: petgraph::graph::DiGraph::new(), reference_graph_indices: HashMap::new(), + reference_modules: HashMap::new(), }; // An empty block expression is used often, we add this into the `node` on startup @@ -854,7 +866,7 @@ impl NodeInterner { self.definitions.push(DefinitionInfo { name, mutable, comptime, kind, location }); if is_local { - self.add_definition_location(ReferenceId::Local(id)); + self.add_definition_location(ReferenceId::Local(id), None); } id @@ -892,7 +904,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)); + self.add_definition_location(ReferenceId::Function(id), Some(module)); definition_id } @@ -993,12 +1005,16 @@ impl NodeInterner { &self.struct_attributes[struct_id] } - pub fn add_module_location(&mut self, module_id: ModuleId, location: Location) { - self.module_locations.insert(module_id, location); + pub fn add_module_attributes(&mut self, module_id: ModuleId, attributes: ModuleAttributes) { + self.module_attributes.insert(module_id, attributes); + } + + pub fn module_attributes(&self, module_id: &ModuleId) -> &ModuleAttributes { + &self.module_attributes[module_id] } - pub fn module_location(&self, module_id: &ModuleId) -> Location { - self.module_locations[module_id] + pub fn try_module_attributes(&self, module_id: &ModuleId) -> Option<&ModuleAttributes> { + self.module_attributes.get(module_id) } pub fn global_attributes(&self, global_id: &GlobalId) -> &[SecondaryAttribute] { diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index e91e0fb3325..28ba5425fe7 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -21,7 +21,7 @@ use async_lsp::{ use fm::{codespan_files as files, FileManager}; use fxhash::FxHashSet; use lsp_types::{ - request::{PrepareRenameRequest, References, Rename}, + request::{HoverRequest, PrepareRenameRequest, References, Rename}, CodeLens, }; use nargo::{ @@ -46,7 +46,7 @@ use notifications::{ }; use requests::{ on_code_lens_request, on_formatting, on_goto_declaration_request, on_goto_definition_request, - on_goto_type_definition_request, on_initialize, on_prepare_rename_request, + on_goto_type_definition_request, on_hover_request, on_initialize, on_prepare_rename_request, on_profile_run_request, on_references_request, on_rename_request, on_shutdown, on_test_run_request, on_tests_request, }; @@ -129,6 +129,7 @@ impl NargoLspService { .request::(on_references_request) .request::(on_prepare_rename_request) .request::(on_rename_request) + .request::(on_hover_request) .notification::(on_initialized) .notification::(on_did_change_configuration) .notification::(on_did_open_text_document) diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 1f87f2c87d4..bd0f0afb827 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -20,10 +20,10 @@ fn on_goto_definition_inner( state: &mut LspState, params: GotoDeclarationParams, ) -> Result { - process_request(state, params.text_document_position_params, |location, interner, files, _| { - interner.get_declaration_location_from(location).and_then(|found_location| { + process_request(state, params.text_document_position_params, |args| { + args.interner.get_declaration_location_from(args.location).and_then(|found_location| { let file_id = found_location.file; - let definition_position = to_lsp_location(files, file_id, found_location.span)?; + let definition_position = to_lsp_location(args.files, file_id, found_location.span)?; let response = GotoDeclarationResponse::from(definition_position).to_owned(); Some(response) }) diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 27d3503a2fd..72fac676a72 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -29,15 +29,16 @@ fn on_goto_definition_inner( params: GotoDefinitionParams, return_type_location_instead: bool, ) -> Result { - process_request(state, params.text_document_position_params, |location, interner, files, _| { - interner.get_definition_location_from(location, return_type_location_instead).and_then( - |found_location| { + process_request(state, params.text_document_position_params, |args| { + args.interner + .get_definition_location_from(args.location, return_type_location_instead) + .and_then(|found_location| { let file_id = found_location.file; - let definition_position = to_lsp_location(files, file_id, found_location.span)?; + let definition_position = + to_lsp_location(args.files, file_id, found_location.span)?; let response = GotoDefinitionResponse::from(definition_position).to_owned(); Some(response) - }, - ) + }) }) } diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs new file mode 100644 index 00000000000..161fd20f555 --- /dev/null +++ b/tooling/lsp/src/requests/hover.rs @@ -0,0 +1,641 @@ +use std::future::{self, Future}; + +use async_lsp::ResponseError; +use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; +use noirc_frontend::{ + ast::Visibility, + graph::CrateId, + hir::def_map::ModuleId, + hir_def::stmt::HirPattern, + macros_api::{NodeInterner, StructId}, + node_interner::{ + DefinitionId, DefinitionKind, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, + }, + Generics, Type, +}; + +use crate::LspState; + +use super::{process_request, to_lsp_location, ProcessRequestCallbackArgs}; + +pub(crate) fn on_hover_request( + state: &mut LspState, + params: HoverParams, +) -> impl Future, ResponseError>> { + let result = process_request(state, params.text_document_position_params, |args| { + args.interner.reference_at_location(args.location).map(|reference| { + let location = args.interner.reference_location(reference); + let lsp_location = to_lsp_location(args.files, location.file, location.span); + Hover { + range: lsp_location.map(|location| location.range), + contents: HoverContents::Markup(MarkupContent { + kind: MarkupKind::Markdown, + value: format_reference(reference, &args), + }), + } + }) + }); + + future::ready(result) +} + +fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) -> String { + match reference { + ReferenceId::Module(id) => format_module(id, args), + ReferenceId::Struct(id) => format_struct(id, args), + ReferenceId::StructMember(id, field_index) => format_struct_member(id, field_index, args), + ReferenceId::Trait(id) => format_trait(id, args), + ReferenceId::Global(id) => format_global(id, args), + ReferenceId::Function(id) => format_function(id, args), + ReferenceId::Alias(id) => format_alias(id, args), + ReferenceId::Local(id) => format_local(id, args), + ReferenceId::Reference(location, _) => { + format_reference(args.interner.find_referenced(location).unwrap(), args) + } + } +} +fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> String { + let module_attributes = args.interner.module_attributes(&id); + + let mut string = String::new(); + if format_parent_module_from_module_id( + &ModuleId { krate: id.krate, local_id: module_attributes.parent }, + args, + &mut string, + ) { + string.push('\n'); + } + string.push_str(" "); + string.push_str("mod "); + string.push_str(&module_attributes.name); + string +} + +fn format_struct(id: StructId, args: &ProcessRequestCallbackArgs) -> String { + let struct_type = args.interner.get_struct(id); + let struct_type = struct_type.borrow(); + + let mut string = String::new(); + if format_parent_module(ReferenceId::Struct(id), args, &mut string) { + string.push('\n'); + } + string.push_str(" "); + string.push_str("struct "); + string.push_str(&struct_type.name.0.contents); + format_generics(&struct_type.generics, &mut string); + string.push_str(" {\n"); + for (field_name, field_type) in struct_type.get_fields_as_written() { + string.push_str(" "); + string.push_str(&field_name); + string.push_str(": "); + string.push_str(&format!("{}", field_type)); + string.push_str(",\n"); + } + string.push_str(" }"); + string +} + +fn format_struct_member( + id: StructId, + field_index: usize, + args: &ProcessRequestCallbackArgs, +) -> String { + let struct_type = args.interner.get_struct(id); + let struct_type = struct_type.borrow(); + let (field_name, field_type) = struct_type.field_at(field_index); + + let mut string = String::new(); + if format_parent_module(ReferenceId::Struct(id), args, &mut string) { + string.push_str("::"); + } + string.push_str(&struct_type.name.0.contents); + string.push('\n'); + string.push_str(" "); + string.push_str(&field_name.0.contents); + string.push_str(": "); + string.push_str(&format!("{}", field_type)); + string +} + +fn format_trait(id: TraitId, args: &ProcessRequestCallbackArgs) -> String { + let a_trait = args.interner.get_trait(id); + + let mut string = String::new(); + if format_parent_module(ReferenceId::Trait(id), args, &mut string) { + string.push('\n'); + } + string.push_str(" "); + string.push_str("trait "); + string.push_str(&a_trait.name.0.contents); + format_generics(&a_trait.generics, &mut string); + string +} + +fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { + let global_info = args.interner.get_global(id); + let definition_id = global_info.definition_id; + let typ = args.interner.definition_type(definition_id); + + let mut string = String::new(); + if format_parent_module(ReferenceId::Global(id), args, &mut string) { + string.push('\n'); + } + string.push_str(" "); + string.push_str("global "); + string.push_str(&global_info.ident.0.contents); + string.push_str(": "); + string.push_str(&format!("{}", typ)); + string +} + +fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { + let func_meta = args.interner.function_meta(&id); + let func_name_definition_id = args.interner.definition(func_meta.name.id); + + let mut string = String::new(); + let formatted_parent_module = + format_parent_module(ReferenceId::Function(id), args, &mut string); + let formatted_parent_struct = if let Some(struct_id) = func_meta.struct_id { + let struct_type = args.interner.get_struct(struct_id); + let struct_type = struct_type.borrow(); + if formatted_parent_module { + string.push_str("::"); + } + string.push_str(&struct_type.name.0.contents); + true + } else { + false + }; + if formatted_parent_module || formatted_parent_struct { + string.push('\n'); + } + string.push_str(" "); + string.push_str("fn "); + string.push_str(&func_name_definition_id.name); + format_generics(&func_meta.direct_generics, &mut string); + string.push('('); + let parameters = &func_meta.parameters; + for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { + format_pattern(pattern, args.interner, &mut string); + if !pattern_is_self(pattern, args.interner) { + string.push_str(": "); + if matches!(visibility, Visibility::Public) { + string.push_str("pub "); + } + string.push_str(&format!("{}", typ)); + } + if index != parameters.len() - 1 { + string.push_str(", "); + } + } + + string.push(')'); + + let return_type = func_meta.return_type(); + match return_type { + Type::Unit => (), + _ => { + string.push_str(" -> "); + string.push_str(&format!("{}", return_type)); + } + } + + string +} + +fn format_alias(id: TypeAliasId, args: &ProcessRequestCallbackArgs) -> String { + let type_alias = args.interner.get_type_alias(id); + let type_alias = type_alias.borrow(); + + let mut string = String::new(); + format_parent_module(ReferenceId::Alias(id), args, &mut string); + string.push('\n'); + string.push_str(" "); + string.push_str("type "); + string.push_str(&type_alias.name.0.contents); + string.push_str(" = "); + string.push_str(&format!("{}", &type_alias.typ)); + string +} + +fn format_local(id: DefinitionId, args: &ProcessRequestCallbackArgs) -> String { + let definition_info = args.interner.definition(id); + let DefinitionKind::Local(expr_id) = definition_info.kind else { + panic!("Expected a local reference to reference a local definition") + }; + let typ = args.interner.definition_type(id); + + let mut string = String::new(); + string.push_str(" "); + if definition_info.comptime { + string.push_str("comptime "); + } + if expr_id.is_some() { + string.push_str("let "); + } + if definition_info.mutable { + if expr_id.is_none() { + string.push_str("let "); + } + string.push_str("mut "); + } + string.push_str(&definition_info.name); + if !matches!(typ, Type::Error) { + string.push_str(": "); + string.push_str(&format!("{}", typ)); + } + string +} + +fn format_generics(generics: &Generics, string: &mut String) { + if generics.is_empty() { + return; + } + + string.push('<'); + for (index, generic) in generics.iter().enumerate() { + string.push_str(&generic.name); + if index != generics.len() - 1 { + string.push_str(", "); + } + } + string.push('>'); +} +fn format_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut String) { + match pattern { + HirPattern::Identifier(ident) => { + let definition = interner.definition(ident.id); + string.push_str(&definition.name); + } + HirPattern::Mutable(pattern, _) => { + string.push_str("mut "); + format_pattern(pattern, interner, string); + } + HirPattern::Tuple(..) | HirPattern::Struct(..) => { + string.push('_'); + } + } +} + +fn pattern_is_self(pattern: &HirPattern, interner: &NodeInterner) -> bool { + match pattern { + HirPattern::Identifier(ident) => { + let definition = interner.definition(ident.id); + definition.name == "self" + } + HirPattern::Mutable(pattern, _) => pattern_is_self(pattern, interner), + HirPattern::Tuple(..) | HirPattern::Struct(..) => false, + } +} + +fn format_parent_module( + referenced: ReferenceId, + args: &ProcessRequestCallbackArgs, + string: &mut String, +) -> bool { + let Some(module) = args.interner.reference_module(referenced) else { + return false; + }; + + format_parent_module_from_module_id(module, args, string) +} + +fn format_parent_module_from_module_id( + module: &ModuleId, + args: &ProcessRequestCallbackArgs, + string: &mut String, +) -> bool { + let crate_id = module.krate; + let crate_name = match crate_id { + CrateId::Root(_) => Some(args.root_crate_name.clone()), + CrateId::Crate(_) => args + .root_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(" "); + string.push_str(&crate_name); + true + } else { + false + }; + + let Some(module_attributes) = args.interner.try_module_attributes(module) else { + return wrote_crate; + }; + + 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("::"); + } + + string.push_str(&module_attributes.name); + + true +} + +#[cfg(test)] +mod hover_tests { + use crate::test_utils; + + use super::*; + use lsp_types::{ + Position, TextDocumentIdentifier, TextDocumentPositionParams, Url, WorkDoneProgressParams, + }; + use tokio::test; + + async fn assert_hover(directory: &str, file: &str, position: Position, expected_text: &str) { + let (mut state, noir_text_document) = test_utils::init_lsp_server(directory).await; + + // noir_text_document is always `src/main.nr` in the workspace directory, so let's go to the workspace dir + let noir_text_document = noir_text_document.to_file_path().unwrap(); + let workspace_dir = noir_text_document.parent().unwrap().parent().unwrap(); + + let file_uri = Url::from_file_path(workspace_dir.join(file)).unwrap(); + + let hover = on_hover_request( + &mut state, + HoverParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: file_uri }, + position, + }, + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + }, + ) + .await + .expect("Could not execute hover") + .unwrap(); + + let HoverContents::Markup(markup) = hover.contents else { + panic!("Expected hover contents to be Markup"); + }; + + assert_eq!(markup.value, expected_text); + } + + #[test] + async fn hover_on_module() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 6, character: 9 }, + r#" one + mod subone"#, + ) + .await; + } + + #[test] + async fn hover_on_struct() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 9, character: 20 }, + r#" one::subone + struct SubOneStruct { + some_field: i32, + some_other_field: Field, + }"#, + ) + .await; + } + + #[test] + async fn hover_on_generic_struct() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 46, character: 17 }, + r#" one::subone + struct GenericStruct { + }"#, + ) + .await; + } + + #[test] + async fn hover_on_struct_member() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 9, character: 35 }, + r#" one::subone::SubOneStruct + some_field: i32"#, + ) + .await; + } + + #[test] + async fn hover_on_trait() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 12, character: 17 }, + r#" one::subone + trait SomeTrait"#, + ) + .await; + } + + #[test] + async fn hover_on_global() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 15, character: 25 }, + r#" one::subone + global some_global: Field"#, + ) + .await; + } + + #[test] + async fn hover_on_function() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 3, character: 4 }, + r#" one + fn function_one()"#, + ) + .await; + } + + #[test] + async fn hover_on_local_function() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 2, character: 7 }, + r#" two + fn function_two()"#, + ) + .await; + } + + #[test] + async fn hover_on_struct_method() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 20, character: 6 }, + r#" one::subone::SubOneStruct + fn foo(self, x: i32, y: i32) -> Field"#, + ) + .await; + } + + #[test] + async fn hover_on_local_var() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 25, character: 12 }, + " let regular_var: Field", + ) + .await; + } + + #[test] + async fn hover_on_local_mut_var() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 27, character: 4 }, + " let mut mutable_var: Field", + ) + .await; + } + + #[test] + async fn hover_on_parameter() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 31, character: 12 }, + " some_param: i32", + ) + .await; + } + + #[test] + async fn hover_on_alias() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 34, character: 17 }, + r#" one::subone + type SomeAlias = i32"#, + ) + .await; + } + + #[test] + async fn hover_on_trait_on_call() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 39, character: 17 }, + r#" std::default + trait Default"#, + ) + .await; + } + + #[test] + async fn hover_on_std_module_in_use() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 36, character: 9 }, + r#" std + mod default"#, + ) + .await; + } + + #[test] + async fn hover_on_crate_module_in_call() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 15, character: 17 }, + r#" one + mod subone"#, + ) + .await; + } + + #[test] + async fn hover_on_module_without_crate_or_std_prefix() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 43, character: 4 }, + r#" two + mod other"#, + ) + .await; + } + + #[test] + async fn hover_on_module_with_crate_prefix() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 44, character: 11 }, + r#" two + mod other"#, + ) + .await; + } + + #[test] + async fn hover_on_module_on_struct_constructor() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 19, character: 12 }, + r#" one + mod subone"#, + ) + .await; + } + + #[test] + async fn hover_on_type_inside_generic_arguments() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 51, character: 30 }, + r#" one::subone + struct SubOneStruct { + some_field: i32, + some_other_field: Field, + }"#, + ) + .await; + } +} diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 9ece797a57a..e029750e589 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -14,7 +14,7 @@ use lsp_types::{ use nargo::insert_all_files_for_workspace_into_file_manager; use nargo_fmt::Config; use noirc_driver::file_manager_with_stdlib; -use noirc_frontend::macros_api::NodeInterner; +use noirc_frontend::{graph::Dependency, macros_api::NodeInterner}; use serde::{Deserialize, Serialize}; use crate::{ @@ -35,6 +35,7 @@ use crate::{ mod code_lens_request; mod goto_declaration; mod goto_definition; +mod hover; mod profile_run; mod references; mod rename; @@ -44,9 +45,10 @@ mod tests; pub(crate) use { code_lens_request::collect_lenses_for_package, code_lens_request::on_code_lens_request, goto_declaration::on_goto_declaration_request, goto_definition::on_goto_definition_request, - goto_definition::on_goto_type_definition_request, profile_run::on_profile_run_request, - references::on_references_request, rename::on_prepare_rename_request, - rename::on_rename_request, test_run::on_test_run_request, tests::on_tests_request, + goto_definition::on_goto_type_definition_request, hover::on_hover_request, + profile_run::on_profile_run_request, references::on_references_request, + rename::on_prepare_rename_request, rename::on_rename_request, test_run::on_test_run_request, + tests::on_tests_request, }; /// LSP client will send initialization request after the server has started. @@ -127,6 +129,11 @@ pub(crate) fn on_initialize( work_done_progress: None, }, })), + hover_provider: Some(lsp_types::OneOf::Right(lsp_types::HoverOptions { + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: None, + }, + })), }, server_info: None, }) @@ -264,13 +271,22 @@ pub(crate) fn on_shutdown( async { Ok(()) } } +pub(crate) struct ProcessRequestCallbackArgs<'a> { + location: noirc_errors::Location, + files: &'a FileMap, + interner: &'a NodeInterner, + interners: &'a HashMap, + root_crate_name: String, + root_crate_dependencies: &'a Vec, +} + pub(crate) fn process_request( state: &mut LspState, text_document_position_params: TextDocumentPositionParams, callback: F, ) -> Result where - F: FnOnce(noirc_errors::Location, &NodeInterner, &FileMap, &HashMap) -> T, + F: FnOnce(ProcessRequestCallbackArgs) -> T, { let file_path = text_document_position_params.text_document.uri.to_file_path().map_err(|_| { @@ -309,7 +325,14 @@ where &text_document_position_params.position, )?; - Ok(callback(location, interner, files, &state.cached_definitions)) + Ok(callback(ProcessRequestCallbackArgs { + location, + files, + interner, + interners: &state.cached_definitions, + root_crate_name: package.name.to_string(), + root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies, + })) } pub(crate) fn find_all_references_in_workspace( location: noirc_errors::Location, diff --git a/tooling/lsp/src/requests/references.rs b/tooling/lsp/src/requests/references.rs index 6c872656c28..badea8921b2 100644 --- a/tooling/lsp/src/requests/references.rs +++ b/tooling/lsp/src/requests/references.rs @@ -12,20 +12,16 @@ pub(crate) fn on_references_request( params: ReferenceParams, ) -> impl Future>, ResponseError>> { let include_declaration = params.context.include_declaration; - let result = process_request( - state, - params.text_document_position, - |location, interner, files, cached_interners| { - find_all_references_in_workspace( - location, - interner, - cached_interners, - files, - include_declaration, - true, - ) - }, - ); + let result = process_request(state, params.text_document_position, |args| { + find_all_references_in_workspace( + args.location, + args.interner, + args.interners, + args.files, + include_declaration, + true, + ) + }); future::ready(result) } diff --git a/tooling/lsp/src/requests/rename.rs b/tooling/lsp/src/requests/rename.rs index 245c2ed0882..84956681167 100644 --- a/tooling/lsp/src/requests/rename.rs +++ b/tooling/lsp/src/requests/rename.rs @@ -17,8 +17,8 @@ pub(crate) fn on_prepare_rename_request( state: &mut LspState, params: TextDocumentPositionParams, ) -> impl Future, ResponseError>> { - let result = process_request(state, params, |location, interner, _, _| { - let reference_id = interner.reference_at_location(location); + let result = process_request(state, params, |args| { + let reference_id = args.interner.reference_at_location(args.location); let rename_possible = match reference_id { // Rename shouldn't be possible when triggered on top of "Self" Some(ReferenceId::Reference(_, true /* is self type name */)) => false, @@ -34,40 +34,36 @@ pub(crate) fn on_rename_request( state: &mut LspState, params: RenameParams, ) -> impl Future, ResponseError>> { - let result = process_request( - state, - params.text_document_position, - |location, interner, files, cached_interners| { - let rename_changes = find_all_references_in_workspace( - location, - interner, - cached_interners, - files, - true, - false, - ) - .map(|locations| { - let rs = locations.iter().fold( - HashMap::new(), - |mut acc: HashMap>, location| { - let edit = - TextEdit { range: location.range, new_text: params.new_name.clone() }; - acc.entry(location.uri.clone()).or_default().push(edit); - acc - }, - ); - rs - }); - - let response = WorkspaceEdit { - changes: rename_changes, - document_changes: None, - change_annotations: None, - }; + let result = process_request(state, params.text_document_position, |args| { + let rename_changes = find_all_references_in_workspace( + args.location, + args.interner, + args.interners, + args.files, + true, + false, + ) + .map(|locations| { + let rs = locations.iter().fold( + HashMap::new(), + |mut acc: HashMap>, location| { + let edit = + TextEdit { range: location.range, new_text: params.new_name.clone() }; + acc.entry(location.uri.clone()).or_default().push(edit); + acc + }, + ); + rs + }); + + let response = WorkspaceEdit { + changes: rename_changes, + document_changes: None, + change_annotations: None, + }; - Some(response) - }, - ); + Some(response) + }); future::ready(result) } @@ -174,6 +170,11 @@ mod rename_tests { check_rename_succeeds("rename_function_use", "some_function").await; } + #[test] + async fn test_rename_method() { + check_rename_succeeds("rename_function", "some_method").await; + } + #[test] async fn test_rename_struct() { check_rename_succeeds("rename_struct", "Foo").await; diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index 57eb2dd3618..991773ce94f 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -1,7 +1,7 @@ use fm::FileId; use lsp_types::{ - DeclarationCapability, DefinitionOptions, OneOf, ReferencesOptions, RenameOptions, - TypeDefinitionProviderCapability, + DeclarationCapability, DefinitionOptions, HoverOptions, OneOf, ReferencesOptions, + RenameOptions, TypeDefinitionProviderCapability, }; use noirc_driver::DebugFile; use noirc_errors::{debug_info::OpCodesCount, Location}; @@ -144,6 +144,10 @@ pub(crate) struct ServerCapabilities { /// The server provides references support. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) references_provider: Option>, + + /// The server provides hover support. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) hover_provider: Option>, } #[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] diff --git a/tooling/lsp/test_programs/rename_function/src/main.nr b/tooling/lsp/test_programs/rename_function/src/main.nr index 7a70084276e..e77b50c0b26 100644 --- a/tooling/lsp/test_programs/rename_function/src/main.nr +++ b/tooling/lsp/test_programs/rename_function/src/main.nr @@ -25,3 +25,16 @@ use foo::some_other_function as bar; fn x() { bar(); } + +struct SomeStruct { + +} + +impl SomeStruct { + fn some_method(self) {} +} + +fn y() { + let some_struct = SomeStruct {}; + some_struct.some_method(); +} diff --git a/tooling/lsp/test_programs/workspace/one/src/lib.nr b/tooling/lsp/test_programs/workspace/one/src/lib.nr index d4f660e35fb..61f282fa2a7 100644 --- a/tooling/lsp/test_programs/workspace/one/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/one/src/lib.nr @@ -1 +1,25 @@ -pub fn function_one() {} +pub fn function_one() {} + +mod subone { + struct SubOneStruct { + some_field: i32, + some_other_field: Field, + } + + impl SubOneStruct { + fn foo(self, x: i32, y: i32) -> Field { + 0 + } + } + + trait SomeTrait { + } + + global some_global = 2; + + type SomeAlias = i32; + + struct GenericStruct { + + } +} diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index adf7079b4c9..3f0f0f117b7 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -3,3 +3,51 @@ use one::function_one; pub fn function_two() { function_one() } + +use one::subone; + +fn use_struct() { + let _ = subone::SubOneStruct { some_field: 0, some_other_field: 2 }; +} + +use one::subone::SomeTrait; + +fn use_global() { + let _ = one::subone::some_global; +} + +fn use_struct_method() { + let s = subone::SubOneStruct { some_field: 0, some_other_field: 2 }; + s.foo(0, 1); +} + +fn use_local_var() { + let regular_var = 0; + let _ = regular_var; + let mut mutable_var = 0; + mutable_var = 1; +} + +fn use_parameter(some_param: i32) { + let _ = some_param; +} + +use one::subone::SomeAlias; + +use std::default::Default; + +fn use_impl_method() { + let _: i32 = Default::default(); +} + +mod other; +use other::another_function; +use crate::other::other_function; + +use one::subone::GenericStruct; + +use std::collections::bounded_vec::BoundedVec; + +fn instantiate_generic() { + let x: BoundedVec = BoundedVec::new(); +} diff --git a/tooling/lsp/test_programs/workspace/two/src/other.nr b/tooling/lsp/test_programs/workspace/two/src/other.nr new file mode 100644 index 00000000000..4d2fffcee80 --- /dev/null +++ b/tooling/lsp/test_programs/workspace/two/src/other.nr @@ -0,0 +1,2 @@ +fn other_function() {} +fn another_function() {}