feat: LSP hover (noir-lang#5491)

# Description ## Problem Resolves noir-lang#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 <[email protected]>
manastech · Jul 12, 2024 · 010c835 · 010c835
1 parent f459257
commit 010c835
Show file tree

Hide file tree

Showing 25 changed files with 986 additions and 137 deletions.
diff --git 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
@@ -720,13 +720,20 @@ 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,
             location,
             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
@@ -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<ReferenceId> = 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
@@ -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
@@ -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<ReferenceId> = Vec::new();
+                let mut references: Vec<Option<ReferenceId>> = 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
@@ -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<NoirTypeAlias>,
+        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
@@ -81,7 +81,7 @@ pub fn resolve_import(
     crate_id: CrateId,
     import_directive: &ImportDirective,
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<ReferenceId>>,
+    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
 ) -> Result<ResolvedImport, PathResolutionError> {
     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<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<ReferenceId>>,
+    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
 ) -> 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<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<ReferenceId>>,
+    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
 ) -> 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<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<ReferenceId>>,
+    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
 ) -> NamespaceResolutionResult {
     let def_map = &def_maps[&krate];
     let mut current_mod_id = ModuleId { krate, local_id: starting_mod };
@@ -247,22 +247,22 @@ 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
             }
             ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"),
             // 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<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<ReferenceId>>,
+    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
     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
@@ -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<CrateId, CrateDefMap>,
         path: Path,
-        path_references: &mut Option<&mut Vec<ReferenceId>>,
+        path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
     ) -> PathResolutionResult;
 
     fn local_module_id(&self) -> LocalModuleId;
@@ -38,7 +39,7 @@ impl PathResolver for StandardPathResolver {
         &self,
         def_maps: &BTreeMap<CrateId, CrateDefMap>,
         path: Path,
-        path_references: &mut Option<&mut Vec<ReferenceId>>,
+        path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
     ) -> PathResolutionResult {
         resolve_path(def_maps, self.module_id, path, path_references)
     }
@@ -58,7 +59,7 @@ pub fn resolve_path(
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
     module_id: ModuleId,
     path: Path,
-    path_references: &mut Option<&mut Vec<ReferenceId>>,
+    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
 ) -> 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
@@ -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(),