chore(LSP): simplify code lens request handling

compiler/noirc_frontend/src/hir/mod.rs

@@ -9,6 +9,7 @@ use crate::ast::UnresolvedGenerics;
 use crate::debug::DebugInstrumenter;
 use crate::elaborator::UnstableFeature;
 use crate::graph::{CrateGraph, CrateId};
+use crate::hir::def_map::DefMaps;
 use crate::hir_def::function::FuncMeta;
 use crate::node_interner::{FuncId, NodeInterner, TypeId};
 use crate::parser::ParserError;
@@ -141,17 +142,7 @@ impl Context<'_, '_> {
     }
 
     pub fn fully_qualified_function_name(&self, crate_id: &CrateId, id: &FuncId) -> String {
-        let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already");
-
-        let name = self.def_interner.function_name(id);
-
-        let module_id = self.def_interner.function_module(*id);
-        let module = self.module(module_id);
-
-        let parent =
-            def_map.get_module_path_with_separator(module_id.local_id, module.parent, "::");
-
-        if parent.is_empty() { name.into() } else { format!("{parent}::{name}") }
+        fully_qualified_function_name(*crate_id, *id, &self.def_interner, &self.def_maps)
     }
 
     /// Returns a fully-qualified path to the given [TypeId] from the given [CrateId]. This function also
@@ -185,27 +176,12 @@ impl Context<'_, '_> {
         crate_id: &CrateId,
         pattern: &FunctionNameMatch,
     ) -> Vec<(String, TestFunction)> {
-        let interner = &self.def_interner;
-        let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already");
-
-        def_map
-            .get_all_test_functions(interner)
-            .filter_map(|test_function| {
-                let fully_qualified_name =
-                    self.fully_qualified_function_name(crate_id, &test_function.id);
-                match &pattern {
-                    FunctionNameMatch::Anything => Some((fully_qualified_name, test_function)),
-                    FunctionNameMatch::Exact(patterns) => patterns
-                        .iter()
-                        .any(|pattern| &fully_qualified_name == pattern)
-                        .then_some((fully_qualified_name, test_function)),
-                    FunctionNameMatch::Contains(patterns) => patterns
-                        .iter()
-                        .any(|pattern| fully_qualified_name.contains(pattern))
-                        .then_some((fully_qualified_name, test_function)),
-                }
-            })
-            .collect()
+        get_all_test_functions_in_crate_matching(
+            *crate_id,
+            pattern,
+            &self.def_interner,
+            &self.def_maps,
+        )
     }
 
     /// Returns a list of all functions in the current crate marked with `#[fuzz]`
@@ -303,3 +279,49 @@ impl Context<'_, '_> {
         self.interpreter_output = Some(output);
     }
 }
+
+pub fn get_all_test_functions_in_crate_matching(
+    crate_id: CrateId,
+    pattern: &FunctionNameMatch,
+    interner: &NodeInterner,
+    def_maps: &DefMaps,
+) -> Vec<(String, TestFunction)> {
+    let def_map = def_maps.get(&crate_id).expect("The local crate should be analyzed already");
+
+    def_map
+        .get_all_test_functions(interner)
+        .filter_map(|test_function| {
+            let fully_qualified_name =
+                fully_qualified_function_name(crate_id, test_function.id, interner, def_maps);
+            match &pattern {
+                FunctionNameMatch::Anything => Some((fully_qualified_name, test_function)),
+                FunctionNameMatch::Exact(patterns) => patterns
+                    .iter()
+                    .any(|pattern| &fully_qualified_name == pattern)
+                    .then_some((fully_qualified_name, test_function)),
+                FunctionNameMatch::Contains(patterns) => patterns
+                    .iter()
+                    .any(|pattern| fully_qualified_name.contains(pattern))
+                    .then_some((fully_qualified_name, test_function)),
+            }
+        })
+        .collect()
+}
+
+fn fully_qualified_function_name(
+    crate_id: CrateId,
+    id: FuncId,
+    interner: &NodeInterner,
+    def_maps: &DefMaps,
+) -> String {
+    let def_map = def_maps.get(&crate_id).expect("The local crate should be analyzed already");
+
+    let name = interner.function_name(&id);
+
+    let module_id = interner.function_module(id);
+    let module = module_id.module(def_maps);
+
+    let parent = def_map.get_module_path_with_separator(module_id.local_id, module.parent, "::");
+
+    if parent.is_empty() { name.into() } else { format!("{parent}::{name}") }
+}

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

@@ -12,7 +12,9 @@ use crate::usage_tracker::UsageTracker;
 use std::collections::BTreeMap;
 
 use crate::ast::{Ident, ItemVisibility, Path, PathKind, PathSegment};
-use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData, ModuleDefId, ModuleId, PerNs};
+use crate::hir::def_map::{
+    CrateDefMap, DefMaps, LocalModuleId, ModuleData, ModuleDefId, ModuleId, PerNs,
+};
 
 use super::errors::ResolverError;
 use super::visibility::item_in_module_is_visible;
@@ -161,7 +163,7 @@ impl<'a> From<&'a PathResolutionError> for CustomDiagnostic {
 pub fn resolve_import(
     path: Path,
     importing_module: ModuleId,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
     usage_tracker: &mut UsageTracker,
     references_tracker: Option<ReferencesTracker>,
 ) -> ImportResolutionResult {
@@ -194,7 +196,7 @@ fn path_segment_to_typed_path_segment(segment: PathSegment) -> TypedPathSegment
 pub fn resolve_path_kind<'r>(
     path: TypedPath,
     importing_module: ModuleId,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
     references_tracker: Option<ReferencesTracker<'r>>,
 ) -> Result<(TypedPath, ModuleId, Option<ReferencesTracker<'r>>), PathResolutionError> {
     let mut solver =
@@ -424,7 +426,7 @@ impl<'def_maps, 'usage_tracker, 'references_tracker>
     }
 }
 
-fn get_module(def_maps: &BTreeMap<CrateId, CrateDefMap>, module: ModuleId) -> &ModuleData {
+fn get_module(def_maps: &DefMaps, module: ModuleId) -> &ModuleData {
     let message = "A crate should always be present for a given crate id";
     &def_maps.get(&module.krate).expect(message)[module.local_id]
 }

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

@@ -1,9 +1,6 @@
 use crate::Type;
-use crate::graph::CrateId;
 use crate::node_interner::{FuncId, NodeInterner, TraitId, TypeId};
 
-use std::collections::BTreeMap;
-
 use crate::ast::ItemVisibility;
 use crate::hir::def_map::{CrateDefMap, DefMaps, LocalModuleId, ModuleId};
 
@@ -17,7 +14,7 @@ use crate::hir::def_map::{CrateDefMap, DefMaps, LocalModuleId, ModuleId};
 /// }
 /// ```
 pub fn item_in_module_is_visible(
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
     current_module: ModuleId,
     target_module: ModuleId,
     visibility: ItemVisibility,
@@ -78,7 +75,7 @@ pub fn struct_member_is_visible(
     struct_id: TypeId,
     visibility: ItemVisibility,
     current_module_id: ModuleId,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
 ) -> bool {
     type_member_is_visible(struct_id.module_id(), visibility, current_module_id, def_maps)
 }
@@ -87,7 +84,7 @@ pub fn trait_member_is_visible(
     trait_id: TraitId,
     visibility: ItemVisibility,
     current_module_id: ModuleId,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
 ) -> bool {
     type_member_is_visible(trait_id.0, visibility, current_module_id, def_maps)
 }
@@ -96,7 +93,7 @@ fn type_member_is_visible(
     type_module_id: ModuleId,
     visibility: ItemVisibility,
     current_module_id: ModuleId,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
 ) -> bool {
     match visibility {
         ItemVisibility::Public => true,

compiler/noirc_frontend/src/modules.rs

@@ -1,10 +1,8 @@
-use std::collections::BTreeMap;
-
 use crate::{
     ast::{Ident, ItemVisibility},
     graph::{CrateId, Dependency},
     hir::{
-        def_map::{CrateDefMap, ModuleDefId, ModuleId},
+        def_map::{DefMaps, ModuleDefId, ModuleId},
         resolution::visibility::item_in_module_is_visible,
     },
     node_interner::{NodeInterner, Reexport, ReferenceId},
@@ -14,7 +12,7 @@ use crate::{
 pub fn get_parent_module(
     module_def_id: ModuleDefId,
     interner: &NodeInterner,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
 ) -> Option<ModuleId> {
     match module_def_id {
         ModuleDefId::ModuleId(id) => id.parent(def_maps),
@@ -56,7 +54,7 @@ pub fn relative_module_full_path(
     current_module_id: ModuleId,
     current_module_parent_id: Option<ModuleId>,
     interner: &NodeInterner,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
 ) -> Option<String> {
     let full_path;
     if let ModuleDefId::ModuleId(module_id) = module_def_id {
@@ -208,7 +206,7 @@ pub fn module_def_id_relative_path(
     defining_module: Option<ModuleId>,
     intermediate_name: &Option<Ident>,
     interner: &NodeInterner,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
 ) -> Option<String> {
     let module_path = if let Some(defining_module) = defining_module {
         relative_module_id_path(
@@ -251,7 +249,7 @@ pub fn module_def_id_is_visible(
     mut visibility: ItemVisibility,
     mut defining_module: Option<ModuleId>,
     interner: &NodeInterner,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
     dependencies: &[Dependency],
 ) -> bool {
     // First find out which module we need to check.
@@ -299,7 +297,7 @@ pub fn get_ancestor_module_reexport(
     visibility: ItemVisibility,
     current_module_id: ModuleId,
     interner: &NodeInterner,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    def_maps: &DefMaps,
     dependencies: &[Dependency],
 ) -> Option<Reexport> {
     let parent_module = get_parent_module(module_def_id, interner, def_maps)?;

tooling/lsp/src/lib.rs

@@ -12,12 +12,9 @@ use std::{
 };
 
 use acvm::{BlackBoxFunctionSolver, FieldElement};
-use async_lsp::lsp_types::{
-    CodeLens,
-    request::{
-        CodeActionRequest, Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest,
-        PrepareRenameRequest, References, Rename, SignatureHelpRequest, WorkspaceSymbolRequest,
-    },
+use async_lsp::lsp_types::request::{
+    CodeActionRequest, Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest,
+    PrepareRenameRequest, References, Rename, SignatureHelpRequest, WorkspaceSymbolRequest,
 };
 use async_lsp::{
     AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, LspService, ResponseError,
@@ -31,7 +28,7 @@ use nargo::{
     workspace::Workspace,
 };
 use nargo_toml::{PackageSelection, find_file_manifest, resolve_workspace_from_toml};
-use noirc_driver::{NOIR_ARTIFACT_VERSION_STRING, file_manager_with_stdlib, prepare_crate};
+use noirc_driver::NOIR_ARTIFACT_VERSION_STRING;
 use noirc_frontend::{
     ParsedModule,
     graph::{CrateGraph, CrateId, CrateName},
@@ -95,7 +92,6 @@ pub struct LspState {
     solver: WrapperSolver,
     open_documents_count: usize,
     input_files: HashMap<String, String>,
-    cached_lenses: HashMap<String, Vec<CodeLens>>,
     cached_parsed_files: HashMap<PathBuf, (usize, (ParsedModule, Vec<ParserError>))>,
     workspace_cache: HashMap<PathBuf, WorkspaceCacheData>,
     package_cache: HashMap<PathBuf, PackageCacheData>,
@@ -128,7 +124,6 @@ impl LspState {
             root_path: None,
             solver: WrapperSolver(Box::new(solver)),
             input_files: HashMap::new(),
-            cached_lenses: HashMap::new(),
             cached_parsed_files: HashMap::new(),
             workspace_cache: HashMap::new(),
             package_cache: HashMap::new(),
@@ -345,30 +340,6 @@ pub(crate) fn prepare_package<'file_manager, 'parsed_files>(
     (context, crate_id)
 }
 
-/// Prepares a package from a source string
-/// This is useful for situations when we don't need dependencies
-/// and just need to operate on single file.
-///
-/// Use case for this is the LSP server and code lenses
-/// which operate on single file and need to understand this file
-/// in order to offer code lenses to the user
-fn prepare_source(source: String, state: &mut LspState) -> (Context<'static, 'static>, CrateId) {
-    let root = Path::new("");
-    let file_name = Path::new("main.nr");
-    let mut file_manager = file_manager_with_stdlib(root);
-    file_manager.add_file_with_source(file_name, source).expect(
-        "Adding source buffer to file manager should never fail when file manager is empty",
-    );
-    let parsed_files = parse_diff(&file_manager, state);
-
-    let mut context = Context::new(file_manager, parsed_files);
-    context.activate_lsp_mode();
-
-    let root_crate_id = prepare_crate(&mut context, file_name);
-
-    (context, root_crate_id)
-}
-
 fn parse_diff(file_manager: &FileManager, state: &mut LspState) -> ParsedFiles {
     if state.options.enable_parsing_cache {
         let noir_file_hashes: Vec<_> = file_manager
@@ -452,22 +423,3 @@ pub fn source_code_overrides(input_files: &HashMap<String, String>) -> HashMap<P
     }
     overrides
 }
-
-#[test]
-fn prepare_package_from_source_string() {
-    let source = r#"
-    fn main() {
-        let x = 1;
-        let y = 2;
-        let z = x + y;
-    }
-    "#;
-
-    let client = ClientSocket::new_closed();
-    let mut state = LspState::new(&client, acvm::blackbox_solver::StubbedBlackBoxSolver::default());
-
-    let (mut context, crate_id) = prepare_source(source.to_string(), &mut state);
-    let _check_result = noirc_driver::check_crate(&mut context, crate_id, &Default::default());
-    let main_func_id = context.get_main_function(&crate_id);
-    assert!(main_func_id.is_some());
-}

tooling/lsp/src/notifications/mod.rs

@@ -79,7 +79,6 @@ pub(super) fn on_did_close_text_document(
     params: DidCloseTextDocumentParams,
 ) -> ControlFlow<Result<(), async_lsp::Error>> {
     state.input_files.remove(&params.text_document.uri.to_string());
-    state.cached_lenses.remove(&params.text_document.uri.to_string());
     state.workspace_symbol_cache.reprocess_uri(&params.text_document.uri);
 
     state.open_documents_count -= 1;
@@ -154,14 +153,6 @@ pub(crate) fn process_workspace_for_noir_document(
             });
         }
 
-        let collected_lenses = crate::requests::collect_lenses_for_package(
-            &context,
-            crate_id,
-            &workspace,
-            package,
-            Some(&file_path),
-        );
-        state.cached_lenses.insert(document_uri.to_string(), collected_lenses);
         state.package_cache.insert(
             package.root_dir.clone(),
             PackageCacheData {