feat: LSP hover and go-to-definition for crates (#5786)

# Description

## Problem

Resolves #5787

## Summary

This is a minor thing, but hover and go-to-definition didn't work in
path segments that referred to crates. For completeness, and for
features we could build later on (for example making auto-import insert
new stuff inside existing paths if possible) I thought it would be nice
to have this working.


![lsp-crate](https://github.com/user-attachments/assets/be6d4308-6387-45c4-aac8-66459d15b746)

## Additional Context

While doing it some code got simplified: the part of a path that
referred to a crate didn't refer to anything (None) and that was the
only case where an Option was needed. Now that we capture all references
that None is gone.

I know we are kind of capturing module attributes twice, one in the def
maps and another in NodeInterner, but otherwise getting a Location from
a ReferenceId would also need def maps to be passed to NodeInterner,
which is a bit less convenient. In any case this information was tracked
like that already, it's just that it was missing for modules that were
crate roots.

Note that the information is still missing for the root module of the
root crate, so that case is handled separately in a couple of places.
Capturing this is doable, but I didn't know where to get the root crate
name from in `CrateDefMap::collect_defs`.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

compiler/noirc_frontend/src/elaborator/scope.rs

@@ -83,9 +83,6 @@ impl<'context> Elaborator<'context> {
                 resolver.resolve(self.def_maps, path.clone(), &mut Some(&mut references))?;
 
             for (referenced, segment) in references.iter().zip(path.segments) {
-                let Some(referenced) = referenced else {
-                    continue;
-                };
                 self.interner.add_reference(
                     *referenced,
                     Location::new(segment.ident.span(), self.file),

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -14,7 +14,8 @@ use crate::hir::Context;
 
 use crate::macros_api::{MacroError, MacroProcessor};
 use crate::node_interner::{
-    FuncId, GlobalId, NodeInterner, ReferenceId, StructId, TraitId, TraitImplId, TypeAliasId,
+    FuncId, GlobalId, ModuleAttributes, NodeInterner, ReferenceId, StructId, TraitId, TraitImplId,
+    TypeAliasId,
 };
 
 use crate::ast::{
@@ -269,11 +270,17 @@ impl DefCollector {
                 macro_processors,
             ));
 
-            let dep_def_root =
-                context.def_map(&dep.crate_id).expect("ice: def map was just created").root;
+            let dep_def_map =
+                context.def_map(&dep.crate_id).expect("ice: def map was just created");
+
+            let dep_def_root = dep_def_map.root;
             let module_id = ModuleId { krate: dep.crate_id, local_id: dep_def_root };
             // Add this crate as a dependency by linking it's root module
             def_map.extern_prelude.insert(dep.as_name(), module_id);
+
+            let location = dep_def_map[dep_def_root].location;
+            let attriutes = ModuleAttributes { name: dep.as_name(), location, parent: None };
+            context.def_interner.add_module_attributes(module_id, attriutes);
         }
 
         // At this point, all dependencies are resolved and type checked.
@@ -309,7 +316,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.lsp_mode {
-                let mut references: Vec<Option<ReferenceId>> = Vec::new();
+                let mut references: Vec<ReferenceId> = Vec::new();
                 let resolved_import = resolve_import(
                     crate_id,
                     &collected_import,
@@ -322,9 +329,6 @@ impl DefCollector {
 
                 for (referenced, segment) in references.iter().zip(&collected_import.path.segments)
                 {
-                    let Some(referenced) = referenced else {
-                        continue;
-                    };
                     context.def_interner.add_reference(
                         *referenced,
                         Location::new(segment.ident.span(), file_id),

compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -771,7 +771,7 @@ impl<'a> ModCollector<'a> {
                 ModuleAttributes {
                     name: mod_name.0.contents.clone(),
                     location: mod_location,
-                    parent: self.module_id,
+                    parent: Some(self.module_id),
                 },
             );
 

compiler/noirc_frontend/src/hir/resolution/import.rs

@@ -86,7 +86,7 @@ pub fn resolve_import(
     crate_id: CrateId,
     import_directive: &ImportDirective,
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+    path_references: &mut Option<&mut Vec<ReferenceId>>,
 ) -> Result<ResolvedImport, PathResolutionError> {
     let module_scope = import_directive.module_id;
     let NamespaceResolution {
@@ -131,7 +131,7 @@ fn resolve_path_to_ns(
     crate_id: CrateId,
     importing_crate: CrateId,
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+    path_references: &mut Option<&mut Vec<ReferenceId>>,
 ) -> NamespaceResolutionResult {
     let import_path = &import_directive.path.segments;
     let def_map = &def_maps[&crate_id];
@@ -221,7 +221,7 @@ fn resolve_path_from_crate_root(
 
     import_path: &[PathSegment],
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+    path_references: &mut Option<&mut Vec<ReferenceId>>,
 ) -> NamespaceResolutionResult {
     resolve_name_in_module(
         crate_id,
@@ -239,7 +239,7 @@ fn resolve_name_in_module(
     import_path: &[PathSegment],
     starting_mod: LocalModuleId,
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+    path_references: &mut Option<&mut Vec<ReferenceId>>,
 ) -> NamespaceResolutionResult {
     let def_map = &def_maps[&krate];
     let mut current_mod_id = ModuleId { krate, local_id: starting_mod };
@@ -275,22 +275,22 @@ fn resolve_name_in_module(
         current_mod_id = match typ {
             ModuleDefId::ModuleId(id) => {
                 if let Some(path_references) = path_references {
-                    path_references.push(Some(ReferenceId::Module(id)));
+                    path_references.push(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(Some(ReferenceId::Struct(id)));
+                    path_references.push(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(Some(ReferenceId::Trait(id)));
+                    path_references.push(ReferenceId::Trait(id));
                 }
                 id.0
             }
@@ -337,7 +337,7 @@ fn resolve_external_dep(
     current_def_map: &CrateDefMap,
     directive: &ImportDirective,
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
-    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+    path_references: &mut Option<&mut Vec<ReferenceId>>,
     importing_crate: CrateId,
 ) -> NamespaceResolutionResult {
     // Use extern_prelude to get the dep
@@ -355,9 +355,8 @@ 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);
+        path_references.push(ReferenceId::Module(*dep_module));
     }
 
     let path = Path {

compiler/noirc_frontend/src/hir/resolution/path_resolver.rs

@@ -15,7 +15,7 @@ pub trait PathResolver {
         &self,
         def_maps: &BTreeMap<CrateId, CrateDefMap>,
         path: Path,
-        path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+        path_references: &mut Option<&mut Vec<ReferenceId>>,
     ) -> PathResolutionResult;
 
     fn local_module_id(&self) -> LocalModuleId;
@@ -39,7 +39,7 @@ impl PathResolver for StandardPathResolver {
         &self,
         def_maps: &BTreeMap<CrateId, CrateDefMap>,
         path: Path,
-        path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+        path_references: &mut Option<&mut Vec<ReferenceId>>,
     ) -> PathResolutionResult {
         resolve_path(def_maps, self.module_id, path, path_references)
     }
@@ -59,7 +59,7 @@ pub fn resolve_path(
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
     module_id: ModuleId,
     path: Path,
-    path_references: &mut Option<&mut Vec<Option<ReferenceId>>>,
+    path_references: &mut Option<&mut Vec<ReferenceId>>,
 ) -> PathResolutionResult {
     // lets package up the path into an ImportDirective and resolve it using that
     let import =

compiler/noirc_frontend/src/node_interner.rs

@@ -49,7 +49,7 @@ const IMPL_SEARCH_RECURSION_LIMIT: u32 = 10;
 pub struct ModuleAttributes {
     pub name: String,
     pub location: Location,
-    pub parent: LocalModuleId,
+    pub parent: Option<LocalModuleId>,
 }
 
 type StructAttributes = Vec<SecondaryAttribute>;
@@ -1035,7 +1035,7 @@ impl NodeInterner {
     }
 
     pub fn try_module_parent(&self, module_id: &ModuleId) -> Option<LocalModuleId> {
-        self.try_module_attributes(module_id).map(|attrs| attrs.parent)
+        self.try_module_attributes(module_id).and_then(|attrs| attrs.parent)
     }
 
     pub fn global_attributes(&self, global_id: &GlobalId) -> &[SecondaryAttribute] {

tooling/lsp/src/requests/completion/auto_import.rs

@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
 use lsp_types::{Position, Range, TextEdit};
 use noirc_frontend::{
     ast::ItemVisibility,
-    graph::{CrateId, Dependency},
+    graph::CrateId,
     hir::def_map::{CrateDefMap, ModuleId},
     macros_api::{ModuleDefId, NodeInterner},
     node_interner::ReferenceId,
@@ -47,7 +47,6 @@ impl<'a> NodeFinder<'a> {
                         &self.module_id,
                         current_module_parent_id,
                         self.interner,
-                        self.dependencies,
                     );
                 } else {
                     let Some(parent_module) = get_parent_module(self.interner, *module_def_id)
@@ -74,7 +73,6 @@ impl<'a> NodeFinder<'a> {
                         &self.module_id,
                         current_module_parent_id,
                         self.interner,
-                        self.dependencies,
                     );
                 }
 
@@ -149,7 +147,6 @@ fn module_id_path(
     current_module_id: &ModuleId,
     current_module_parent_id: Option<ModuleId>,
     interner: &NodeInterner,
-    dependencies: &[Dependency],
 ) -> String {
     if Some(target_module_id) == current_module_parent_id {
         return "super".to_string();
@@ -163,8 +160,12 @@ fn module_id_path(
 
         let mut current_attributes = module_attributes;
         loop {
+            let Some(parent_local_id) = current_attributes.parent else {
+                break;
+            };
+
             let parent_module_id =
-                &ModuleId { krate: target_module_id.krate, local_id: current_attributes.parent };
+                &ModuleId { krate: target_module_id.krate, local_id: parent_local_id };
 
             if current_module_id == parent_module_id {
                 is_relative = true;
@@ -186,24 +187,13 @@ fn module_id_path(
         }
     }
 
-    let crate_id = target_module_id.krate;
-    let crate_name = if is_relative {
-        None
-    } else {
-        match crate_id {
-            CrateId::Root(_) => Some("crate".to_string()),
-            CrateId::Stdlib(_) => Some("std".to_string()),
-            CrateId::Crate(_) => dependencies
-                .iter()
-                .find(|dep| dep.crate_id == crate_id)
-                .map(|dep| dep.name.to_string()),
-            CrateId::Dummy => unreachable!("ICE: A dummy CrateId should not be accessible"),
+    if !is_relative {
+        // We don't record module attriubtes for the root module,
+        // so we handle that case separately
+        if let CrateId::Root(_) = target_module_id.krate {
+            segments.push("crate");
         }
-    };
-
-    if let Some(crate_name) = &crate_name {
-        segments.push(crate_name);
-    };
+    }
 
     segments.reverse();
     segments.join("::")

tooling/lsp/src/requests/completion/tests.rs

@@ -1638,4 +1638,22 @@ mod completion_tests {
             })
         );
     }
+
+    #[test]
+    async fn test_auto_import_from_std() {
+        let src = r#"
+            fn main() {
+                compute_merkle_roo>|<
+            }
+        "#;
+        let items = get_completions(src).await;
+        assert_eq!(items.len(), 1);
+
+        let item = &items[0];
+        assert_eq!(item.label, "compute_merkle_root(…)");
+        assert_eq!(
+            item.label_details.as_ref().unwrap().detail,
+            Some("(use std::merkle::compute_merkle_root)".to_string()),
+        );
+    }
 }

tooling/lsp/src/requests/goto_definition.rs

@@ -235,4 +235,18 @@ mod goto_definition_tests {
         )
         .await;
     }
+
+    #[test]
+    async fn goto_crate() {
+        expect_goto(
+            "go_to_definition",
+            Position { line: 29, character: 6 }, // "dependency" in "use dependency::something"
+            "dependency/src/lib.nr",
+            Range {
+                start: Position { line: 0, character: 0 },
+                end: Position { line: 0, character: 0 },
+            },
+        )
+        .await;
+    }
 }