From e9f20b70084f4137eb22bb722c34eb42c9ce3cde Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 11:11:30 -0300 Subject: [PATCH 01/13] Move `name_matches` to a separate file --- tooling/lsp/src/name_match.rs | 61 +++++++++++++++++++++++++ tooling/lsp/src/requests/completion.rs | 62 +------------------------- 2 files changed, 62 insertions(+), 61 deletions(-) create mode 100644 tooling/lsp/src/name_match.rs diff --git a/tooling/lsp/src/name_match.rs b/tooling/lsp/src/name_match.rs new file mode 100644 index 00000000000..9357063ad12 --- /dev/null +++ b/tooling/lsp/src/name_match.rs @@ -0,0 +1,61 @@ +use convert_case::{Case, Casing}; + +/// Returns true if name matches a prefix written in code. +/// `prefix` must already be in snake case. +/// This method splits both name and prefix by underscore, +/// then checks that every part of name starts with a part of +/// prefix, in order. +/// +/// For example: +/// +/// // "merk" and "ro" match "merkle" and "root" and are in order +/// name_matches("compute_merkle_root", "merk_ro") == true +/// +/// // "ro" matches "root", but "merkle" comes before it, so no match +/// name_matches("compute_merkle_root", "ro_mer") == false +/// +/// // neither "compute" nor "merkle" nor "root" start with "oot" +/// name_matches("compute_merkle_root", "oot") == false +pub(crate) fn name_matches(name: &str, prefix: &str) -> bool { + let name = name.to_case(Case::Snake); + let name_parts: Vec<&str> = name.split('_').collect(); + + let mut last_index: i32 = -1; + for prefix_part in prefix.split('_') { + // Look past parts we already matched + let offset = if last_index >= 0 { last_index as usize + 1 } else { 0 }; + + if let Some(mut name_part_index) = + name_parts.iter().skip(offset).position(|name_part| name_part.starts_with(prefix_part)) + { + // Need to adjust the index if we skipped some segments + name_part_index += offset; + + if last_index >= name_part_index as i32 { + return false; + } + last_index = name_part_index as i32; + } else { + return false; + } + } + + true +} + +#[cfg(test)] +mod completion_name_matches_tests { + use crate::name_match::name_matches; + + #[test] + fn test_name_matches() { + assert!(name_matches("foo", "foo")); + assert!(name_matches("foo_bar", "bar")); + assert!(name_matches("FooBar", "foo")); + assert!(name_matches("FooBar", "bar")); + assert!(name_matches("FooBar", "foo_bar")); + assert!(name_matches("bar_baz", "bar_b")); + + assert!(!name_matches("foo_bar", "o_b")); + } +} diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 5eb66e8aca6..48f24c30c94 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -39,7 +39,7 @@ use noirc_frontend::{ use sort_text::underscore_sort_text; use crate::{ - LspState, requests::to_lsp_location, + LspState, name_match::name_matches, requests::to_lsp_location, trait_impl_method_stub_generator::TraitImplMethodStubGenerator, use_segment_positions::UseSegmentPositions, utils, visibility::module_def_id_is_visible, }; @@ -1935,49 +1935,6 @@ fn get_type_type_id(typ: &Type) -> Option { } } -/// Returns true if name matches a prefix written in code. -/// `prefix` must already be in snake case. -/// This method splits both name and prefix by underscore, -/// then checks that every part of name starts with a part of -/// prefix, in order. -/// -/// For example: -/// -/// // "merk" and "ro" match "merkle" and "root" and are in order -/// name_matches("compute_merkle_root", "merk_ro") == true -/// -/// // "ro" matches "root", but "merkle" comes before it, so no match -/// name_matches("compute_merkle_root", "ro_mer") == false -/// -/// // neither "compute" nor "merkle" nor "root" start with "oot" -/// name_matches("compute_merkle_root", "oot") == false -fn name_matches(name: &str, prefix: &str) -> bool { - let name = name.to_case(Case::Snake); - let name_parts: Vec<&str> = name.split('_').collect(); - - let mut last_index: i32 = -1; - for prefix_part in prefix.split('_') { - // Look past parts we already matched - let offset = if last_index >= 0 { last_index as usize + 1 } else { 0 }; - - if let Some(mut name_part_index) = - name_parts.iter().skip(offset).position(|name_part| name_part.starts_with(prefix_part)) - { - // Need to adjust the index if we skipped some segments - name_part_index += offset; - - if last_index >= name_part_index as i32 { - return false; - } - last_index = name_part_index as i32; - } else { - return false; - } - } - - true -} - fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option { match reference_id { ReferenceId::Module(module_id) => Some(ModuleDefId::ModuleId(module_id)), @@ -1992,20 +1949,3 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option None, } } - -#[cfg(test)] -mod completion_name_matches_tests { - use crate::requests::completion::name_matches; - - #[test] - fn test_name_matches() { - assert!(name_matches("foo", "foo")); - assert!(name_matches("foo_bar", "bar")); - assert!(name_matches("FooBar", "foo")); - assert!(name_matches("FooBar", "bar")); - assert!(name_matches("FooBar", "foo_bar")); - assert!(name_matches("bar_baz", "bar_b")); - - assert!(!name_matches("foo_bar", "o_b")); - } -} From 33d65984ff264d238229fc7f455567dbc216579d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 11:11:46 -0300 Subject: [PATCH 02/13] Extract `insert_all_files_under_path_into_file_manager` --- tooling/nargo/src/lib.rs | 66 ++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index e384f4ce531..3ddd9addc74 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -98,35 +98,7 @@ fn insert_all_files_for_package_into_file_manager( .parent() .unwrap_or_else(|| panic!("The entry path is expected to be a single file within a directory and so should have a parent {:?}", package.entry_path)); - for entry in WalkDir::new(entry_path_parent).sort_by_file_name() { - let Ok(entry) = entry else { - continue; - }; - - if !entry.file_type().is_file() { - continue; - } - - if entry.path().extension().is_none_or(|ext| ext != FILE_EXTENSION) { - continue; - }; - - let path = entry.into_path(); - - // Avoid reading the source if the file is already there - if file_manager.has_file(&path) { - continue; - } - - let source = if let Some(src) = overrides.get(path.as_path()) { - src.to_string() - } else { - std::fs::read_to_string(path.as_path()) - .unwrap_or_else(|_| panic!("could not read file {:?} into string", path)) - }; - - file_manager.add_file_with_source(path.as_path(), source); - } + insert_all_files_under_path_into_file_manager(file_manager, entry_path_parent, overrides); insert_all_files_for_packages_dependencies_into_file_manager( package, @@ -158,6 +130,42 @@ fn insert_all_files_for_packages_dependencies_into_file_manager( } } +pub fn insert_all_files_under_path_into_file_manager( + file_manager: &mut FileManager, + path: &std::path::Path, + overrides: &HashMap<&std::path::Path, &str>, +) { + for entry in WalkDir::new(path).sort_by_file_name() { + let Ok(entry) = entry else { + continue; + }; + + if !entry.file_type().is_file() { + continue; + } + + if entry.path().extension().is_none_or(|ext| ext != FILE_EXTENSION) { + continue; + }; + + let path = entry.into_path(); + + // Avoid reading the source if the file is already there + if file_manager.has_file(&path) { + continue; + } + + let source = if let Some(src) = overrides.get(path.as_path()) { + src.to_string() + } else { + std::fs::read_to_string(path.as_path()) + .unwrap_or_else(|_| panic!("could not read file {:?} into string", path)) + }; + + file_manager.add_file_with_source(path.as_path(), source); + } +} + pub fn parse_all(file_manager: &FileManager) -> ParsedFiles { file_manager .as_file_map() From 8b5d21737f676f77a031f524516c17b30d002989 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 11:12:00 -0300 Subject: [PATCH 03/13] Brute-force WorkspaceSymbol provider --- tooling/lsp/src/lib.rs | 5 +- tooling/lsp/src/requests/mod.rs | 11 +- tooling/lsp/src/requests/workspace_symbol.rs | 193 +++++++++++++++++++ tooling/lsp/src/types.rs | 6 +- 4 files changed, 212 insertions(+), 3 deletions(-) create mode 100644 tooling/lsp/src/requests/workspace_symbol.rs diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 784ac8cf93d..7a9d4a796c1 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -24,7 +24,7 @@ use lsp_types::{ CodeLens, request::{ CodeActionRequest, Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, - PrepareRenameRequest, References, Rename, SignatureHelpRequest, + PrepareRenameRequest, References, Rename, SignatureHelpRequest, WorkspaceSymbolRequest, }, }; use nargo::{ @@ -57,6 +57,7 @@ use requests::{ on_goto_definition_request, on_goto_type_definition_request, on_hover_request, on_initialize, on_inlay_hint_request, on_prepare_rename_request, on_references_request, on_rename_request, on_shutdown, on_signature_help_request, on_test_run_request, on_tests_request, + on_workspace_symbol_request, }; use serde_json::Value as JsonValue; use thiserror::Error; @@ -64,6 +65,7 @@ use tower::Service; mod attribute_reference_finder; mod modules; +mod name_match; mod notifications; mod requests; mod solver; @@ -169,6 +171,7 @@ impl NargoLspService { .request::(on_completion_request) .request::(on_signature_help_request) .request::(on_code_action_request) + .request::(on_workspace_symbol_request) .notification::(on_initialized) .notification::(on_did_change_configuration) .notification::(on_did_open_text_document) diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index a5ffd1155fa..f8decb293aa 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -53,6 +53,7 @@ mod rename; mod signature_help; mod test_run; mod tests; +mod workspace_symbol; pub(crate) use { code_action::on_code_action_request, code_lens_request::collect_lenses_for_package, @@ -62,7 +63,7 @@ pub(crate) use { hover::on_hover_request, inlay_hint::on_inlay_hint_request, references::on_references_request, rename::on_prepare_rename_request, rename::on_rename_request, signature_help::on_signature_help_request, test_run::on_test_run_request, - tests::on_tests_request, + tests::on_tests_request, workspace_symbol::on_workspace_symbol_request, }; /// LSP client will send initialization request after the server has started. @@ -285,6 +286,14 @@ pub(crate) fn on_initialize( }, resolve_provider: None, })), + workspace_symbol_provider: Some(lsp_types::OneOf::Right( + lsp_types::WorkspaceSymbolOptions { + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: None, + }, + resolve_provider: None, + }, + )), }, server_info: None, }) diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs new file mode 100644 index 00000000000..f8ecc969177 --- /dev/null +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -0,0 +1,193 @@ +use std::{ + collections::HashMap, + future::{self, Future}, + path::{Path, PathBuf}, +}; + +use async_lsp::ResponseError; +use convert_case::{Case, Casing}; +use fm::{FileManager, FileMap}; +use lsp_types::{SymbolKind, WorkspaceSymbol, WorkspaceSymbolParams, WorkspaceSymbolResponse}; +use nargo::{insert_all_files_under_path_into_file_manager, parse_all}; +use noirc_errors::{Location, Span}; +use noirc_frontend::ast::{ + Ident, LetStatement, ModuleDeclaration, NoirEnumeration, NoirFunction, NoirStruct, NoirTrait, + NoirTraitImpl, NoirTypeAlias, Pattern, TraitImplItemKind, TraitItem, TypeImpl, Visitor, +}; + +use crate::{LspState, name_match::name_matches}; + +use super::to_lsp_location; + +pub(crate) fn on_workspace_symbol_request( + state: &mut LspState, + params: WorkspaceSymbolParams, +) -> impl Future, ResponseError>> + use<> { + let Some(root_path) = state.root_path.clone() else { + return future::ready(Ok(None)); + }; + + let now = std::time::Instant::now(); + + let query = ¶ms.query; + let query_lowercase = query.to_lowercase(); + let query_snake_case = query.to_case(Case::Snake); + + let file_manager = setup_file_manager(state, root_path); + let parsed_files = parse_all(&file_manager); + + let mut symbols = Vec::new(); + + for (_, (parsed_module, _)) in parsed_files { + let mut gatherer = WorkspaceSymboGatherer::new(&mut symbols, file_manager.as_file_map()); + parsed_module.accept(&mut gatherer); + } + + let symbols = symbols + .into_iter() + .filter(|symbol| { + let name = &symbol.name; + let name_lowercase = name.to_lowercase(); + name_lowercase.contains(&query_lowercase) || name_matches(name, &query_snake_case) + }) + .collect::>(); + + let elapsed = now.elapsed(); + eprintln!("Elapsed: {:.2?}", elapsed); + + future::ready(Ok(Some(WorkspaceSymbolResponse::Nested(symbols)))) +} + +fn setup_file_manager(state: &mut LspState, root_path: PathBuf) -> FileManager { + let mut file_manager = FileManager::new(root_path.as_path()); + + // Source code for files we cached override those that are read from disk. + let mut overrides: HashMap<&Path, &str> = HashMap::new(); + for (path, source) in &state.input_files { + let path = path.strip_prefix("file://").unwrap(); + overrides.insert(Path::new(path), source); + } + + insert_all_files_under_path_into_file_manager(&mut file_manager, &root_path, &overrides); + + file_manager +} + +struct WorkspaceSymboGatherer<'symbols, 'files> { + symbols: &'symbols mut Vec, + files: &'files FileMap, +} + +impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { + fn new(symbols: &'symbols mut Vec, files: &'files FileMap) -> Self { + Self { symbols, files } + } + + fn to_lsp_location(&self, location: Location) -> Option { + to_lsp_location(self.files, location.file, location.span) + } + + fn push_symbol(&mut self, name: &Ident, kind: SymbolKind) { + self.push_symbol_impl(name, kind, None); + } + + fn push_contained_symbol(&mut self, name: &Ident, kind: SymbolKind, container_name: String) { + self.push_symbol_impl(name, kind, Some(container_name)); + } + + fn push_symbol_impl(&mut self, name: &Ident, kind: SymbolKind, container_name: Option) { + let Some(location) = self.to_lsp_location(name.location()) else { + return; + }; + + self.symbols.push(WorkspaceSymbol { + name: name.to_string(), + kind, + tags: None, + container_name, + location: lsp_types::OneOf::Left(location), + data: None, + }); + } +} + +impl Visitor for WorkspaceSymboGatherer<'_, '_> { + fn visit_module_declaration(&mut self, module: &ModuleDeclaration, _span: Span) { + self.push_symbol(&module.ident, SymbolKind::MODULE); + } + + fn visit_noir_function(&mut self, noir_function: &NoirFunction, _span: Span) -> bool { + self.push_symbol(noir_function.name_ident(), SymbolKind::FUNCTION); + false + } + + fn visit_noir_struct(&mut self, noir_struct: &NoirStruct, _span: Span) -> bool { + self.push_symbol(&noir_struct.name, SymbolKind::STRUCT); + false + } + + fn visit_noir_enum(&mut self, noir_enum: &NoirEnumeration, _span: Span) -> bool { + self.push_symbol(&noir_enum.name, SymbolKind::ENUM); + false + } + + fn visit_noir_type_alias(&mut self, alias: &NoirTypeAlias, _span: Span) -> bool { + self.push_symbol(&alias.name, SymbolKind::STRUCT); + false + } + + fn visit_noir_trait(&mut self, noir_trait: &NoirTrait, _: Span) -> bool { + self.push_symbol(&noir_trait.name, SymbolKind::INTERFACE); + + let container_name = noir_trait.name.to_string(); + + for item in &noir_trait.items { + match &item.item { + TraitItem::Function { name, .. } => { + self.push_contained_symbol(name, SymbolKind::FUNCTION, container_name.clone()); + } + TraitItem::Constant { .. } | TraitItem::Type { .. } => (), + } + } + + false + } + + fn visit_type_impl(&mut self, type_impl: &TypeImpl, _: Span) -> bool { + let container_name = type_impl.object_type.to_string(); + + for (method, _location) in &type_impl.methods { + let method = &method.item; + let kind = SymbolKind::FUNCTION; + self.push_contained_symbol(method.name_ident(), kind, container_name.clone()); + } + + false + } + + fn visit_noir_trait_impl(&mut self, trait_impl: &NoirTraitImpl, _: Span) -> bool { + let container_name = trait_impl.object_type.to_string(); + + for item in &trait_impl.items { + match &item.item.kind { + TraitImplItemKind::Function(noir_function) => { + let name = noir_function.name_ident(); + let kind = SymbolKind::FUNCTION; + self.push_contained_symbol(name, kind, container_name.clone()); + } + TraitImplItemKind::Constant(..) | TraitImplItemKind::Type { .. } => (), + } + } + + false + } + + fn visit_global(&mut self, global: &LetStatement, _span: Span) -> bool { + let Pattern::Identifier(name) = &global.pattern else { + return false; + }; + + self.push_symbol(name, SymbolKind::CONSTANT); + false + } +} diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index 21d274a3776..f5c51000e8e 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -1,7 +1,7 @@ use lsp_types::{ CodeActionOptions, CompletionOptions, DeclarationCapability, DefinitionOptions, DocumentSymbolOptions, HoverOptions, InlayHintOptions, OneOf, ReferencesOptions, RenameOptions, - SignatureHelpOptions, TypeDefinitionProviderCapability, + SignatureHelpOptions, TypeDefinitionProviderCapability, WorkspaceSymbolOptions, }; use noirc_frontend::graph::CrateName; use serde::{Deserialize, Serialize}; @@ -156,6 +156,10 @@ pub(crate) struct ServerCapabilities { /// The server provides code action support. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) code_action_provider: Option>, + + /// The server provides workspace symbol support. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) workspace_symbol_provider: Option>, } #[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] From bd0011e2310e0f8259e88d2e9a1f3025137e70ef Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 13:48:14 -0300 Subject: [PATCH 04/13] Cache workspace symbols --- Cargo.lock | 1 + tooling/lsp/Cargo.toml | 1 + tooling/lsp/src/lib.rs | 10 ++- tooling/lsp/src/notifications/mod.rs | 2 + tooling/lsp/src/requests/workspace_symbol.rs | 76 ++++++++++++-------- tooling/nargo/src/lib.rs | 2 +- 6 files changed, 62 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e95f7831278..fe33c95bf03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3381,6 +3381,7 @@ dependencies = [ "thiserror", "tokio", "tower", + "walkdir", "wasm-bindgen", ] diff --git a/tooling/lsp/Cargo.toml b/tooling/lsp/Cargo.toml index d0b67f53c24..bba75e46e97 100644 --- a/tooling/lsp/Cargo.toml +++ b/tooling/lsp/Cargo.toml @@ -34,6 +34,7 @@ fxhash.workspace = true iter-extended.workspace = true convert_case = "0.6.0" num-bigint.workspace = true +walkdir = "2.5.0" [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies] wasm-bindgen.workspace = true diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 7a9d4a796c1..2cb31b33065 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -21,7 +21,7 @@ use async_lsp::{ use fm::{FileManager, codespan_files as files}; use fxhash::FxHashSet; use lsp_types::{ - CodeLens, + CodeLens, WorkspaceSymbol, request::{ CodeActionRequest, Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest, References, Rename, SignatureHelpRequest, WorkspaceSymbolRequest, @@ -102,6 +102,7 @@ pub struct LspState { cached_parsed_files: HashMap))>, workspace_cache: HashMap, package_cache: HashMap, + workspace_symbol_cache: HashMap>, options: LspInitializationOptions, // Tracks files that currently have errors, by package root. @@ -134,11 +135,18 @@ impl LspState { cached_parsed_files: HashMap::new(), workspace_cache: HashMap::new(), package_cache: HashMap::new(), + workspace_symbol_cache: HashMap::new(), open_documents_count: 0, options: Default::default(), files_with_errors: HashMap::new(), } } + + fn clear_workspace_symbol_cache(&mut self, uri: &Url) { + if let Ok(path) = uri.to_file_path() { + self.workspace_symbol_cache.remove(&path); + } + } } pub struct NargoLspService { diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index b7ba8cd4761..5e682a68afa 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -62,6 +62,7 @@ pub(super) fn on_did_change_text_document( ) -> ControlFlow> { let text = params.content_changes.into_iter().next().unwrap().text; state.input_files.insert(params.text_document.uri.to_string(), text.clone()); + state.clear_workspace_symbol_cache(¶ms.text_document.uri); let document_uri = params.text_document.uri; let output_diagnostics = false; @@ -78,6 +79,7 @@ pub(super) fn on_did_close_text_document( ) -> ControlFlow> { state.input_files.remove(¶ms.text_document.uri.to_string()); state.cached_lenses.remove(¶ms.text_document.uri.to_string()); + state.clear_workspace_symbol_cache(¶ms.text_document.uri); state.open_documents_count -= 1; diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index f8ecc969177..c86f91d4460 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -1,19 +1,20 @@ use std::{ collections::HashMap, future::{self, Future}, - path::{Path, PathBuf}, + path::Path, }; use async_lsp::ResponseError; use convert_case::{Case, Casing}; -use fm::{FileManager, FileMap}; +use fm::{FILE_EXTENSION, FileManager, FileMap}; use lsp_types::{SymbolKind, WorkspaceSymbol, WorkspaceSymbolParams, WorkspaceSymbolResponse}; -use nargo::{insert_all_files_under_path_into_file_manager, parse_all}; +use nargo::parse_all; use noirc_errors::{Location, Span}; use noirc_frontend::ast::{ Ident, LetStatement, ModuleDeclaration, NoirEnumeration, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Pattern, TraitImplItemKind, TraitItem, TypeImpl, Visitor, }; +use walkdir::WalkDir; use crate::{LspState, name_match::name_matches}; @@ -27,23 +28,60 @@ pub(crate) fn on_workspace_symbol_request( return future::ready(Ok(None)); }; - let now = std::time::Instant::now(); - let query = ¶ms.query; let query_lowercase = query.to_lowercase(); let query_snake_case = query.to_case(Case::Snake); - let file_manager = setup_file_manager(state, root_path); - let parsed_files = parse_all(&file_manager); + // Source code for files we cached override those that are read from disk. + let mut overrides: HashMap<&Path, &str> = HashMap::new(); + for (path, source) in &state.input_files { + let path = path.strip_prefix("file://").unwrap(); + overrides.insert(Path::new(path), source); + } - let mut symbols = Vec::new(); + let mut file_manager = FileManager::new(root_path.as_path()); + let mut all_symbols = Vec::new(); - for (_, (parsed_module, _)) in parsed_files { + for entry in WalkDir::new(root_path).sort_by_file_name() { + let Ok(entry) = entry else { + continue; + }; + + if !entry.file_type().is_file() { + continue; + } + + if entry.path().extension().is_none_or(|ext| ext != FILE_EXTENSION) { + continue; + }; + + let path = entry.into_path(); + if let Some(symbols) = state.workspace_symbol_cache.get(&path) { + all_symbols.extend(symbols.clone()); + } else { + let source = if let Some(src) = overrides.get(path.as_path()) { + src.to_string() + } else { + std::fs::read_to_string(path.as_path()) + .unwrap_or_else(|_| panic!("could not read file {:?} into string", path)) + }; + + file_manager.add_file_with_source(path.as_path(), source); + } + } + + let parsed_files = parse_all(&file_manager); + + for (file_id, (parsed_module, _)) in parsed_files { + let path = file_manager.path(file_id).unwrap().to_path_buf(); + let mut symbols = Vec::new(); let mut gatherer = WorkspaceSymboGatherer::new(&mut symbols, file_manager.as_file_map()); parsed_module.accept(&mut gatherer); + state.workspace_symbol_cache.insert(path, gatherer.symbols.clone()); + all_symbols.extend(std::mem::take(gatherer.symbols)); } - let symbols = symbols + let symbols = all_symbols .into_iter() .filter(|symbol| { let name = &symbol.name; @@ -52,27 +90,9 @@ pub(crate) fn on_workspace_symbol_request( }) .collect::>(); - let elapsed = now.elapsed(); - eprintln!("Elapsed: {:.2?}", elapsed); - future::ready(Ok(Some(WorkspaceSymbolResponse::Nested(symbols)))) } -fn setup_file_manager(state: &mut LspState, root_path: PathBuf) -> FileManager { - let mut file_manager = FileManager::new(root_path.as_path()); - - // Source code for files we cached override those that are read from disk. - let mut overrides: HashMap<&Path, &str> = HashMap::new(); - for (path, source) in &state.input_files { - let path = path.strip_prefix("file://").unwrap(); - overrides.insert(Path::new(path), source); - } - - insert_all_files_under_path_into_file_manager(&mut file_manager, &root_path, &overrides); - - file_manager -} - struct WorkspaceSymboGatherer<'symbols, 'files> { symbols: &'symbols mut Vec, files: &'files FileMap, diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 3ddd9addc74..d6f8744e5ba 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -130,7 +130,7 @@ fn insert_all_files_for_packages_dependencies_into_file_manager( } } -pub fn insert_all_files_under_path_into_file_manager( +fn insert_all_files_under_path_into_file_manager( file_manager: &mut FileManager, path: &std::path::Path, overrides: &HashMap<&std::path::Path, &str>, From 7756646cd2655c8a1a3782fe338830cc13501be2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 14:15:20 -0300 Subject: [PATCH 05/13] Optimize symbol filtering --- tooling/lsp/src/lib.rs | 9 +++++- tooling/lsp/src/name_match.rs | 5 ++++ tooling/lsp/src/requests/workspace_symbol.rs | 31 +++++++++++++------- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 2cb31b33065..5a449fadbb9 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -59,6 +59,7 @@ use requests::{ on_shutdown, on_signature_help_request, on_test_run_request, on_tests_request, on_workspace_symbol_request, }; +use serde::{Deserialize, Serialize}; use serde_json::Value as JsonValue; use thiserror::Error; use tower::Service; @@ -102,7 +103,7 @@ pub struct LspState { cached_parsed_files: HashMap))>, workspace_cache: HashMap, package_cache: HashMap, - workspace_symbol_cache: HashMap>, + workspace_symbol_cache: HashMap>, options: LspInitializationOptions, // Tracks files that currently have errors, by package root. @@ -121,6 +122,12 @@ struct PackageCacheData { usage_tracker: UsageTracker, } +#[derive(Clone, Serialize, Deserialize)] +struct CachedWorkspaceSymbol { + symbol: WorkspaceSymbol, + name_in_snake_case: String, +} + impl LspState { fn new( client: &ClientSocket, diff --git a/tooling/lsp/src/name_match.rs b/tooling/lsp/src/name_match.rs index 9357063ad12..9a19fd8df8c 100644 --- a/tooling/lsp/src/name_match.rs +++ b/tooling/lsp/src/name_match.rs @@ -18,6 +18,11 @@ use convert_case::{Case, Casing}; /// name_matches("compute_merkle_root", "oot") == false pub(crate) fn name_matches(name: &str, prefix: &str) -> bool { let name = name.to_case(Case::Snake); + snake_name_matches(&name, prefix) +} + +/// Like `name_matches` but assumes `name` is already in snake-case. +pub(crate) fn snake_name_matches(name: &str, prefix: &str) -> bool { let name_parts: Vec<&str> = name.split('_').collect(); let mut last_index: i32 = -1; diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index c86f91d4460..76cf2994954 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -16,7 +16,7 @@ use noirc_frontend::ast::{ }; use walkdir::WalkDir; -use crate::{LspState, name_match::name_matches}; +use crate::{CachedWorkspaceSymbol, LspState, name_match::snake_name_matches}; use super::to_lsp_location; @@ -39,9 +39,14 @@ pub(crate) fn on_workspace_symbol_request( overrides.insert(Path::new(path), source); } - let mut file_manager = FileManager::new(root_path.as_path()); let mut all_symbols = Vec::new(); + // Here we traverse all Noir files in the workspace, parsing those for which we don't know + // their symbols. This isn't the best way to implement WorkspaceSymbol but it's actually pretty + // fast, simple, and works very well. An alternative could be to process packages independently, + // cache their symbols, and re-cache them or invalidate them on changes, but it's a much more complex + // code that might yield marginal performance improvements. + let mut file_manager = FileManager::new(root_path.as_path()); for entry in WalkDir::new(root_path).sort_by_file_name() { let Ok(entry) = entry else { continue; @@ -83,10 +88,12 @@ pub(crate) fn on_workspace_symbol_request( let symbols = all_symbols .into_iter() - .filter(|symbol| { - let name = &symbol.name; + .filter_map(|symbol| { + let name = &symbol.symbol.name; let name_lowercase = name.to_lowercase(); - name_lowercase.contains(&query_lowercase) || name_matches(name, &query_snake_case) + (name_lowercase.contains(&query_lowercase) + || snake_name_matches(&symbol.name_in_snake_case, &query_snake_case)) + .then_some(symbol.symbol) }) .collect::>(); @@ -94,12 +101,12 @@ pub(crate) fn on_workspace_symbol_request( } struct WorkspaceSymboGatherer<'symbols, 'files> { - symbols: &'symbols mut Vec, + symbols: &'symbols mut Vec, files: &'files FileMap, } impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { - fn new(symbols: &'symbols mut Vec, files: &'files FileMap) -> Self { + fn new(symbols: &'symbols mut Vec, files: &'files FileMap) -> Self { Self { symbols, files } } @@ -120,14 +127,18 @@ impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { return; }; - self.symbols.push(WorkspaceSymbol { - name: name.to_string(), + let name = name.to_string(); + let symbol = WorkspaceSymbol { + name: name.clone(), kind, tags: None, container_name, location: lsp_types::OneOf::Left(location), data: None, - }); + }; + let cached_symbol = + CachedWorkspaceSymbol { symbol, name_in_snake_case: name.to_case(Case::Snake) }; + self.symbols.push(cached_symbol); } } From f76d8e8936593ff6e0198f3699ac6782c9581fc5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 14:15:41 -0300 Subject: [PATCH 06/13] Tests, and fix for modules --- tooling/lsp/src/requests/workspace_symbol.rs | 80 ++++++++++++++++++-- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index 76cf2994954..500af6b86f7 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -10,9 +10,12 @@ use fm::{FILE_EXTENSION, FileManager, FileMap}; use lsp_types::{SymbolKind, WorkspaceSymbol, WorkspaceSymbolParams, WorkspaceSymbolResponse}; use nargo::parse_all; use noirc_errors::{Location, Span}; -use noirc_frontend::ast::{ - Ident, LetStatement, ModuleDeclaration, NoirEnumeration, NoirFunction, NoirStruct, NoirTrait, - NoirTraitImpl, NoirTypeAlias, Pattern, TraitImplItemKind, TraitItem, TypeImpl, Visitor, +use noirc_frontend::{ + ast::{ + Ident, LetStatement, NoirEnumeration, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, + NoirTypeAlias, Pattern, TraitImplItemKind, TraitItem, TypeImpl, Visitor, + }, + parser::ParsedSubModule, }; use walkdir::WalkDir; @@ -143,8 +146,9 @@ impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { } impl Visitor for WorkspaceSymboGatherer<'_, '_> { - fn visit_module_declaration(&mut self, module: &ModuleDeclaration, _span: Span) { - self.push_symbol(&module.ident, SymbolKind::MODULE); + fn visit_parsed_submodule(&mut self, submodule: &ParsedSubModule, _: Span) -> bool { + self.push_symbol(&submodule.name, SymbolKind::MODULE); + true } fn visit_noir_function(&mut self, noir_function: &NoirFunction, _span: Span) -> bool { @@ -222,3 +226,69 @@ impl Visitor for WorkspaceSymboGatherer<'_, '_> { false } } + +#[cfg(test)] +mod tests { + use lsp_types::{ + PartialResultParams, SymbolKind, WorkDoneProgressParams, WorkspaceSymbolParams, + WorkspaceSymbolResponse, + }; + use tokio::test; + + use crate::{on_workspace_symbol_request, test_utils}; + + #[test] + async fn test_workspace_symbol() { + let (mut state, _) = test_utils::init_lsp_server("document_symbol").await; + + let response = on_workspace_symbol_request( + &mut state, + WorkspaceSymbolParams { + query: String::new(), + partial_result_params: PartialResultParams::default(), + work_done_progress_params: WorkDoneProgressParams::default(), + }, + ) + .await + .expect("Could not execute on_document_symbol_request") + .unwrap(); + + let WorkspaceSymbolResponse::Nested(symbols) = response else { + panic!("Expected Nested response, got {:?}", response); + }; + + assert_eq!(symbols.len(), 8); + + assert_eq!(&symbols[0].name, "foo"); + assert_eq!(symbols[0].kind, SymbolKind::FUNCTION); + assert!(symbols[0].container_name.is_none()); + + assert_eq!(&symbols[1].name, "SomeStruct"); + assert_eq!(symbols[1].kind, SymbolKind::STRUCT); + assert!(symbols[1].container_name.is_none()); + + assert_eq!(&symbols[2].name, "new"); + assert_eq!(symbols[2].kind, SymbolKind::FUNCTION); + assert_eq!(symbols[2].container_name.as_ref().unwrap(), "SomeStruct"); + + assert_eq!(&symbols[3].name, "SomeTrait"); + assert_eq!(symbols[3].kind, SymbolKind::INTERFACE); + assert!(symbols[3].container_name.is_none()); + + assert_eq!(&symbols[4].name, "some_method"); + assert_eq!(symbols[4].kind, SymbolKind::FUNCTION); + assert_eq!(symbols[4].container_name.as_ref().unwrap(), "SomeTrait"); + + assert_eq!(&symbols[5].name, "some_method"); + assert_eq!(symbols[5].kind, SymbolKind::FUNCTION); + assert_eq!(symbols[5].container_name.as_ref().unwrap(), "SomeStruct"); + + assert_eq!(&symbols[6].name, "submodule"); + assert_eq!(symbols[6].kind, SymbolKind::MODULE); + assert!(symbols[6].container_name.is_none()); + + assert_eq!(&symbols[7].name, "SOME_GLOBAL"); + assert_eq!(symbols[7].kind, SymbolKind::CONSTANT); + assert!(symbols[7].container_name.is_none()); + } +} From 7f098756be7193ed7dc40e81fa3b9912fa4262b0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 14:22:01 -0300 Subject: [PATCH 07/13] Tests, and fix for modules --- tooling/lsp/src/requests/workspace_symbol.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index 500af6b86f7..0dd2ba6566c 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -78,8 +78,9 @@ pub(crate) fn on_workspace_symbol_request( } } + // Here we parse all files for which we don't know their symbols yet, + // figure out the symbols and store them in the cache. let parsed_files = parse_all(&file_manager); - for (file_id, (parsed_module, _)) in parsed_files { let path = file_manager.path(file_id).unwrap().to_path_buf(); let mut symbols = Vec::new(); @@ -89,6 +90,8 @@ pub(crate) fn on_workspace_symbol_request( all_symbols.extend(std::mem::take(gatherer.symbols)); } + // Finally, we filter the symbols based on the query + // (Note: we could filter them as we gather them above, but doing this in one go is simpler) let symbols = all_symbols .into_iter() .filter_map(|symbol| { From 7f44c91f6cb27528b39c5144aef33ff018928ed7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 7 Apr 2025 15:16:35 -0300 Subject: [PATCH 08/13] Use fuzzy searching here --- Cargo.lock | 10 ++++++ tooling/lsp/Cargo.toml | 1 + tooling/lsp/src/lib.rs | 9 +---- tooling/lsp/src/name_match.rs | 5 --- tooling/lsp/src/requests/workspace_symbol.rs | 35 ++++++-------------- 5 files changed, 23 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe33c95bf03..00484469e49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1916,6 +1916,15 @@ dependencies = [ "slab", ] +[[package]] +name = "fuzzy-matcher" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54614a3312934d066701a80f20f15fa3b56d67ac7722b39eea5b4c9dd1d66c94" +dependencies = [ + "thread_local", +] + [[package]] name = "fxhash" version = "0.2.1" @@ -3364,6 +3373,7 @@ dependencies = [ "codespan-lsp", "convert_case", "fm", + "fuzzy-matcher", "fxhash", "iter-extended", "lsp-types 0.94.1", diff --git a/tooling/lsp/Cargo.toml b/tooling/lsp/Cargo.toml index bba75e46e97..f0e48d9ae71 100644 --- a/tooling/lsp/Cargo.toml +++ b/tooling/lsp/Cargo.toml @@ -35,6 +35,7 @@ iter-extended.workspace = true convert_case = "0.6.0" num-bigint.workspace = true walkdir = "2.5.0" +fuzzy-matcher = "0.3.7" [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies] wasm-bindgen.workspace = true diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 5a449fadbb9..2cb31b33065 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -59,7 +59,6 @@ use requests::{ on_shutdown, on_signature_help_request, on_test_run_request, on_tests_request, on_workspace_symbol_request, }; -use serde::{Deserialize, Serialize}; use serde_json::Value as JsonValue; use thiserror::Error; use tower::Service; @@ -103,7 +102,7 @@ pub struct LspState { cached_parsed_files: HashMap))>, workspace_cache: HashMap, package_cache: HashMap, - workspace_symbol_cache: HashMap>, + workspace_symbol_cache: HashMap>, options: LspInitializationOptions, // Tracks files that currently have errors, by package root. @@ -122,12 +121,6 @@ struct PackageCacheData { usage_tracker: UsageTracker, } -#[derive(Clone, Serialize, Deserialize)] -struct CachedWorkspaceSymbol { - symbol: WorkspaceSymbol, - name_in_snake_case: String, -} - impl LspState { fn new( client: &ClientSocket, diff --git a/tooling/lsp/src/name_match.rs b/tooling/lsp/src/name_match.rs index 9a19fd8df8c..9357063ad12 100644 --- a/tooling/lsp/src/name_match.rs +++ b/tooling/lsp/src/name_match.rs @@ -18,11 +18,6 @@ use convert_case::{Case, Casing}; /// name_matches("compute_merkle_root", "oot") == false pub(crate) fn name_matches(name: &str, prefix: &str) -> bool { let name = name.to_case(Case::Snake); - snake_name_matches(&name, prefix) -} - -/// Like `name_matches` but assumes `name` is already in snake-case. -pub(crate) fn snake_name_matches(name: &str, prefix: &str) -> bool { let name_parts: Vec<&str> = name.split('_').collect(); let mut last_index: i32 = -1; diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index 0dd2ba6566c..4623cd9b382 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -5,8 +5,8 @@ use std::{ }; use async_lsp::ResponseError; -use convert_case::{Case, Casing}; use fm::{FILE_EXTENSION, FileManager, FileMap}; +use fuzzy_matcher::{FuzzyMatcher, skim::SkimMatcherV2}; use lsp_types::{SymbolKind, WorkspaceSymbol, WorkspaceSymbolParams, WorkspaceSymbolResponse}; use nargo::parse_all; use noirc_errors::{Location, Span}; @@ -19,7 +19,7 @@ use noirc_frontend::{ }; use walkdir::WalkDir; -use crate::{CachedWorkspaceSymbol, LspState, name_match::snake_name_matches}; +use crate::LspState; use super::to_lsp_location; @@ -32,8 +32,6 @@ pub(crate) fn on_workspace_symbol_request( }; let query = ¶ms.query; - let query_lowercase = query.to_lowercase(); - let query_snake_case = query.to_case(Case::Snake); // Source code for files we cached override those that are read from disk. let mut overrides: HashMap<&Path, &str> = HashMap::new(); @@ -90,29 +88,25 @@ pub(crate) fn on_workspace_symbol_request( all_symbols.extend(std::mem::take(gatherer.symbols)); } + let matcher = SkimMatcherV2::default(); + // Finally, we filter the symbols based on the query // (Note: we could filter them as we gather them above, but doing this in one go is simpler) let symbols = all_symbols .into_iter() - .filter_map(|symbol| { - let name = &symbol.symbol.name; - let name_lowercase = name.to_lowercase(); - (name_lowercase.contains(&query_lowercase) - || snake_name_matches(&symbol.name_in_snake_case, &query_snake_case)) - .then_some(symbol.symbol) - }) + .filter(|symbol| matcher.fuzzy_match(&symbol.name, query).is_some()) .collect::>(); future::ready(Ok(Some(WorkspaceSymbolResponse::Nested(symbols)))) } struct WorkspaceSymboGatherer<'symbols, 'files> { - symbols: &'symbols mut Vec, + symbols: &'symbols mut Vec, files: &'files FileMap, } impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { - fn new(symbols: &'symbols mut Vec, files: &'files FileMap) -> Self { + fn new(symbols: &'symbols mut Vec, files: &'files FileMap) -> Self { Self { symbols, files } } @@ -134,17 +128,10 @@ impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { }; let name = name.to_string(); - let symbol = WorkspaceSymbol { - name: name.clone(), - kind, - tags: None, - container_name, - location: lsp_types::OneOf::Left(location), - data: None, - }; - let cached_symbol = - CachedWorkspaceSymbol { symbol, name_in_snake_case: name.to_case(Case::Snake) }; - self.symbols.push(cached_symbol); + let location = lsp_types::OneOf::Left(location); + let symbol = + WorkspaceSymbol { name, kind, tags: None, container_name, location, data: None }; + self.symbols.push(symbol); } } From fc8317ab97aaeb99a3597789605037fa901b14a4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 8 Apr 2025 09:03:33 -0300 Subject: [PATCH 09/13] A better and faster way --- Cargo.lock | 1 - tooling/lsp/Cargo.toml | 1 - tooling/lsp/src/lib.rs | 27 ++++-- tooling/lsp/src/requests/workspace_symbol.rs | 88 +++++++++----------- tooling/nargo/src/lib.rs | 10 +-- 5 files changed, 63 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00484469e49..120176cfd49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3391,7 +3391,6 @@ dependencies = [ "thiserror", "tokio", "tower", - "walkdir", "wasm-bindgen", ] diff --git a/tooling/lsp/Cargo.toml b/tooling/lsp/Cargo.toml index f0e48d9ae71..77f2b427867 100644 --- a/tooling/lsp/Cargo.toml +++ b/tooling/lsp/Cargo.toml @@ -34,7 +34,6 @@ fxhash.workspace = true iter-extended.workspace = true convert_case = "0.6.0" num-bigint.workspace = true -walkdir = "2.5.0" fuzzy-matcher = "0.3.7" [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies] diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 2cb31b33065..ac31e773683 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -102,7 +102,9 @@ pub struct LspState { cached_parsed_files: HashMap))>, workspace_cache: HashMap, package_cache: HashMap, + workspace_symbol_cache_initialized: bool, workspace_symbol_cache: HashMap>, + workspace_symbol_paths_to_process: HashSet, options: LspInitializationOptions, // Tracks files that currently have errors, by package root. @@ -135,7 +137,9 @@ impl LspState { cached_parsed_files: HashMap::new(), workspace_cache: HashMap::new(), package_cache: HashMap::new(), + workspace_symbol_cache_initialized: false, workspace_symbol_cache: HashMap::new(), + workspace_symbol_paths_to_process: HashSet::new(), open_documents_count: 0, options: Default::default(), files_with_errors: HashMap::new(), @@ -143,8 +147,13 @@ impl LspState { } fn clear_workspace_symbol_cache(&mut self, uri: &Url) { + if !self.workspace_symbol_cache_initialized { + return; + } + if let Ok(path) = uri.to_file_path() { self.workspace_symbol_cache.remove(&path); + self.workspace_symbol_paths_to_process.insert(path.clone()); } } } @@ -440,13 +449,7 @@ pub fn insert_all_files_for_workspace_into_file_manager( workspace: &Workspace, file_manager: &mut FileManager, ) { - // Source code for files we cached override those that are read from disk. - let mut overrides: HashMap<&Path, &str> = HashMap::new(); - for (path, source) in &state.input_files { - let path = path.strip_prefix("file://").unwrap(); - overrides.insert(Path::new(path), source); - } - + let overrides = source_code_overrides(&state.input_files); nargo::insert_all_files_for_workspace_into_file_manager_with_overrides( workspace, file_manager, @@ -454,6 +457,16 @@ pub fn insert_all_files_for_workspace_into_file_manager( ); } +// Source code for files we cached override those that are read from disk. +pub fn source_code_overrides(input_files: &HashMap) -> HashMap { + let mut overrides: HashMap = HashMap::new(); + for (path, source) in input_files { + let path = path.strip_prefix("file://").unwrap(); + overrides.insert(PathBuf::from_str(path).unwrap(), source); + } + overrides +} + #[test] fn prepare_package_from_source_string() { let source = r#" diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index 4623cd9b382..e5347e8c7f9 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -1,14 +1,10 @@ -use std::{ - collections::HashMap, - future::{self, Future}, - path::Path, -}; +use std::future::{self, Future}; use async_lsp::ResponseError; -use fm::{FILE_EXTENSION, FileManager, FileMap}; +use fm::{FileManager, FileMap}; use fuzzy_matcher::{FuzzyMatcher, skim::SkimMatcherV2}; use lsp_types::{SymbolKind, WorkspaceSymbol, WorkspaceSymbolParams, WorkspaceSymbolResponse}; -use nargo::parse_all; +use nargo::{insert_all_files_under_path_into_file_manager, parse_all}; use noirc_errors::{Location, Span}; use noirc_frontend::{ ast::{ @@ -17,9 +13,8 @@ use noirc_frontend::{ }, parser::ParsedSubModule, }; -use walkdir::WalkDir; -use crate::LspState; +use crate::{LspState, source_code_overrides}; use super::to_lsp_location; @@ -31,52 +26,38 @@ pub(crate) fn on_workspace_symbol_request( return future::ready(Ok(None)); }; - let query = ¶ms.query; - - // Source code for files we cached override those that are read from disk. - let mut overrides: HashMap<&Path, &str> = HashMap::new(); - for (path, source) in &state.input_files { - let path = path.strip_prefix("file://").unwrap(); - overrides.insert(Path::new(path), source); - } + let overrides = source_code_overrides(&state.input_files); - let mut all_symbols = Vec::new(); - - // Here we traverse all Noir files in the workspace, parsing those for which we don't know - // their symbols. This isn't the best way to implement WorkspaceSymbol but it's actually pretty - // fast, simple, and works very well. An alternative could be to process packages independently, - // cache their symbols, and re-cache them or invalidate them on changes, but it's a much more complex - // code that might yield marginal performance improvements. + // Prepare a FileManager for files we'll parse, to extract symbols from let mut file_manager = FileManager::new(root_path.as_path()); - for entry in WalkDir::new(root_path).sort_by_file_name() { - let Ok(entry) = entry else { - continue; - }; - if !entry.file_type().is_file() { - continue; - } + // If the cache is not initialized yet, put all files in the workspace in the FileManager + if !state.workspace_symbol_cache_initialized { + insert_all_files_under_path_into_file_manager(&mut file_manager, &root_path, &overrides); + state.workspace_symbol_cache_initialized = true; + } - if entry.path().extension().is_none_or(|ext| ext != FILE_EXTENSION) { + // Then add files that we need to re-process + for path in std::mem::take(&mut state.workspace_symbol_paths_to_process) { + if !path.exists() { continue; - }; + } - let path = entry.into_path(); - if let Some(symbols) = state.workspace_symbol_cache.get(&path) { - all_symbols.extend(symbols.clone()); + if let Some(source) = overrides.get(path.as_path()) { + file_manager.add_file_with_source(path.as_path(), source.to_string()); } else { - let source = if let Some(src) = overrides.get(path.as_path()) { - src.to_string() - } else { - std::fs::read_to_string(path.as_path()) - .unwrap_or_else(|_| panic!("could not read file {:?} into string", path)) - }; - + let source = std::fs::read_to_string(path.as_path()) + .unwrap_or_else(|_| panic!("could not read file {:?} into string", path)); file_manager.add_file_with_source(path.as_path(), source); } } - // Here we parse all files for which we don't know their symbols yet, + // Note: what happens if a file is deleted? We don't get notifications when a file is deleted + // so we might return symbols for a file that doesn't exist. However, VSCode seems to notice + // the file doesn't exist and simply doesn't show the symbol. What if the file is re-created? + // In that case we do get a notification so we'll reprocess that file. + + // Parse all files for which we don't know their symbols yet, // figure out the symbols and store them in the cache. let parsed_files = parse_all(&file_manager); for (file_id, (parsed_module, _)) in parsed_files { @@ -85,16 +66,23 @@ pub(crate) fn on_workspace_symbol_request( let mut gatherer = WorkspaceSymboGatherer::new(&mut symbols, file_manager.as_file_map()); parsed_module.accept(&mut gatherer); state.workspace_symbol_cache.insert(path, gatherer.symbols.clone()); - all_symbols.extend(std::mem::take(gatherer.symbols)); } - let matcher = SkimMatcherV2::default(); - // Finally, we filter the symbols based on the query // (Note: we could filter them as we gather them above, but doing this in one go is simpler) - let symbols = all_symbols - .into_iter() - .filter(|symbol| matcher.fuzzy_match(&symbol.name, query).is_some()) + let matcher = SkimMatcherV2::default(); + let symbols = state + .workspace_symbol_cache + .values() + .flat_map(|symbols| { + symbols.iter().filter_map(|symbol| { + if matcher.fuzzy_match(&symbol.name, ¶ms.query).is_some() { + Some(symbol.clone()) + } else { + None + } + }) + }) .collect::>(); future::ready(Ok(Some(WorkspaceSymbolResponse::Nested(symbols)))) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index d6f8744e5ba..e57202e39fd 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -63,7 +63,7 @@ pub fn insert_all_files_for_workspace_into_file_manager( pub fn insert_all_files_for_workspace_into_file_manager_with_overrides( workspace: &workspace::Workspace, file_manager: &mut FileManager, - overrides: &HashMap<&std::path::Path, &str>, + overrides: &HashMap, ) { let mut processed_entry_paths = HashSet::new(); for package in workspace.clone().into_iter() { @@ -84,7 +84,7 @@ pub fn insert_all_files_for_workspace_into_file_manager_with_overrides( fn insert_all_files_for_package_into_file_manager( package: &Package, file_manager: &mut FileManager, - overrides: &HashMap<&std::path::Path, &str>, + overrides: &HashMap, processed_entry_paths: &mut HashSet, ) { if processed_entry_paths.contains(&package.entry_path) { @@ -113,7 +113,7 @@ fn insert_all_files_for_package_into_file_manager( fn insert_all_files_for_packages_dependencies_into_file_manager( package: &Package, file_manager: &mut FileManager, - overrides: &HashMap<&std::path::Path, &str>, + overrides: &HashMap, processed_entry_paths: &mut HashSet, ) { for (_, dep) in package.dependencies.iter() { @@ -130,10 +130,10 @@ fn insert_all_files_for_packages_dependencies_into_file_manager( } } -fn insert_all_files_under_path_into_file_manager( +pub fn insert_all_files_under_path_into_file_manager( file_manager: &mut FileManager, path: &std::path::Path, - overrides: &HashMap<&std::path::Path, &str>, + overrides: &HashMap, ) { for entry in WalkDir::new(path).sort_by_file_name() { let Ok(entry) = entry else { From fe1d84812f4a00244f7b48234e7000278dfa5eab Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 8 Apr 2025 09:14:38 -0300 Subject: [PATCH 10/13] Extract `WorkspaceSymbolCache` --- tooling/lsp/src/lib.rs | 25 +++-------- tooling/lsp/src/notifications/mod.rs | 4 +- tooling/lsp/src/requests/mod.rs | 2 + tooling/lsp/src/requests/workspace_symbol.rs | 44 ++++++++++++++++---- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index ac31e773683..524362f0dd3 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -21,7 +21,7 @@ use async_lsp::{ use fm::{FileManager, codespan_files as files}; use fxhash::FxHashSet; use lsp_types::{ - CodeLens, WorkspaceSymbol, + CodeLens, request::{ CodeActionRequest, Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest, References, Rename, SignatureHelpRequest, WorkspaceSymbolRequest, @@ -52,8 +52,8 @@ use notifications::{ on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized, }; use requests::{ - LspInitializationOptions, on_code_action_request, on_code_lens_request, on_completion_request, - on_document_symbol_request, on_formatting, on_goto_declaration_request, + LspInitializationOptions, WorkspaceSymbolCache, on_code_action_request, on_code_lens_request, + on_completion_request, on_document_symbol_request, on_formatting, on_goto_declaration_request, on_goto_definition_request, on_goto_type_definition_request, on_hover_request, on_initialize, on_inlay_hint_request, on_prepare_rename_request, on_references_request, on_rename_request, on_shutdown, on_signature_help_request, on_test_run_request, on_tests_request, @@ -102,9 +102,7 @@ pub struct LspState { cached_parsed_files: HashMap))>, workspace_cache: HashMap, package_cache: HashMap, - workspace_symbol_cache_initialized: bool, - workspace_symbol_cache: HashMap>, - workspace_symbol_paths_to_process: HashSet, + workspace_symbol_cache: WorkspaceSymbolCache, options: LspInitializationOptions, // Tracks files that currently have errors, by package root. @@ -137,25 +135,12 @@ impl LspState { cached_parsed_files: HashMap::new(), workspace_cache: HashMap::new(), package_cache: HashMap::new(), - workspace_symbol_cache_initialized: false, - workspace_symbol_cache: HashMap::new(), - workspace_symbol_paths_to_process: HashSet::new(), + workspace_symbol_cache: WorkspaceSymbolCache::default(), open_documents_count: 0, options: Default::default(), files_with_errors: HashMap::new(), } } - - fn clear_workspace_symbol_cache(&mut self, uri: &Url) { - if !self.workspace_symbol_cache_initialized { - return; - } - - if let Ok(path) = uri.to_file_path() { - self.workspace_symbol_cache.remove(&path); - self.workspace_symbol_paths_to_process.insert(path.clone()); - } - } } pub struct NargoLspService { diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 5e682a68afa..510e088121d 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -62,7 +62,7 @@ pub(super) fn on_did_change_text_document( ) -> ControlFlow> { let text = params.content_changes.into_iter().next().unwrap().text; state.input_files.insert(params.text_document.uri.to_string(), text.clone()); - state.clear_workspace_symbol_cache(¶ms.text_document.uri); + state.workspace_symbol_cache.reprocess_url(¶ms.text_document.uri); let document_uri = params.text_document.uri; let output_diagnostics = false; @@ -79,7 +79,7 @@ pub(super) fn on_did_close_text_document( ) -> ControlFlow> { state.input_files.remove(¶ms.text_document.uri.to_string()); state.cached_lenses.remove(¶ms.text_document.uri.to_string()); - state.clear_workspace_symbol_cache(¶ms.text_document.uri); + state.workspace_symbol_cache.reprocess_url(¶ms.text_document.uri); state.open_documents_count -= 1; diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index f8decb293aa..852d2cc8edb 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -30,6 +30,8 @@ use crate::{ types::{InitializeResult, NargoCapability, NargoTestsOptions, ServerCapabilities}, }; +pub(crate) use workspace_symbol::WorkspaceSymbolCache; + // Handlers // The handlers for `request` are not `async` because it compiles down to lifetimes that can't be added to // the router. To return a future that fits the trait, it is easiest wrap your implementations in an `async {}` diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index e5347e8c7f9..a5befbd59bf 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -1,9 +1,13 @@ -use std::future::{self, Future}; +use std::{ + collections::{HashMap, HashSet}, + future::{self, Future}, + path::PathBuf, +}; use async_lsp::ResponseError; use fm::{FileManager, FileMap}; use fuzzy_matcher::{FuzzyMatcher, skim::SkimMatcherV2}; -use lsp_types::{SymbolKind, WorkspaceSymbol, WorkspaceSymbolParams, WorkspaceSymbolResponse}; +use lsp_types::{SymbolKind, Url, WorkspaceSymbol, WorkspaceSymbolParams, WorkspaceSymbolResponse}; use nargo::{insert_all_files_under_path_into_file_manager, parse_all}; use noirc_errors::{Location, Span}; use noirc_frontend::{ @@ -31,14 +35,16 @@ pub(crate) fn on_workspace_symbol_request( // Prepare a FileManager for files we'll parse, to extract symbols from let mut file_manager = FileManager::new(root_path.as_path()); + let cache = &mut state.workspace_symbol_cache; + // If the cache is not initialized yet, put all files in the workspace in the FileManager - if !state.workspace_symbol_cache_initialized { + if !cache.initialized { insert_all_files_under_path_into_file_manager(&mut file_manager, &root_path, &overrides); - state.workspace_symbol_cache_initialized = true; + cache.initialized = true; } // Then add files that we need to re-process - for path in std::mem::take(&mut state.workspace_symbol_paths_to_process) { + for path in std::mem::take(&mut cache.paths_to_reprocess) { if !path.exists() { continue; } @@ -65,14 +71,14 @@ pub(crate) fn on_workspace_symbol_request( let mut symbols = Vec::new(); let mut gatherer = WorkspaceSymboGatherer::new(&mut symbols, file_manager.as_file_map()); parsed_module.accept(&mut gatherer); - state.workspace_symbol_cache.insert(path, gatherer.symbols.clone()); + cache.symbols_per_path.insert(path, gatherer.symbols.clone()); } // Finally, we filter the symbols based on the query // (Note: we could filter them as we gather them above, but doing this in one go is simpler) let matcher = SkimMatcherV2::default(); - let symbols = state - .workspace_symbol_cache + let symbols = cache + .symbols_per_path .values() .flat_map(|symbols| { symbols.iter().filter_map(|symbol| { @@ -88,6 +94,28 @@ pub(crate) fn on_workspace_symbol_request( future::ready(Ok(Some(WorkspaceSymbolResponse::Nested(symbols)))) } +#[derive(Default)] +pub(crate) struct WorkspaceSymbolCache { + initialized: bool, + symbols_per_path: HashMap>, + /// Whenever a file changes we'll add it to this set. Then, when workspace symbols + /// are requested we'll reprocess these files in search for symbols. + paths_to_reprocess: HashSet, +} + +impl WorkspaceSymbolCache { + pub(crate) fn reprocess_url(&mut self, uri: &Url) { + if !self.initialized { + return; + } + + if let Ok(path) = uri.to_file_path() { + self.symbols_per_path.remove(&path); + self.paths_to_reprocess.insert(path.clone()); + } + } +} + struct WorkspaceSymboGatherer<'symbols, 'files> { symbols: &'symbols mut Vec, files: &'files FileMap, From 89b751af04c4f59850f967a893f5c4645eb0ad55 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 8 Apr 2025 09:17:03 -0300 Subject: [PATCH 11/13] Revert "Move `name_matches` to a separate file" This reverts commit e9f20b70084f4137eb22bb722c34eb42c9ce3cde. --- tooling/lsp/src/name_match.rs | 61 ------------------------- tooling/lsp/src/requests/completion.rs | 62 +++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 62 deletions(-) delete mode 100644 tooling/lsp/src/name_match.rs diff --git a/tooling/lsp/src/name_match.rs b/tooling/lsp/src/name_match.rs deleted file mode 100644 index 9357063ad12..00000000000 --- a/tooling/lsp/src/name_match.rs +++ /dev/null @@ -1,61 +0,0 @@ -use convert_case::{Case, Casing}; - -/// Returns true if name matches a prefix written in code. -/// `prefix` must already be in snake case. -/// This method splits both name and prefix by underscore, -/// then checks that every part of name starts with a part of -/// prefix, in order. -/// -/// For example: -/// -/// // "merk" and "ro" match "merkle" and "root" and are in order -/// name_matches("compute_merkle_root", "merk_ro") == true -/// -/// // "ro" matches "root", but "merkle" comes before it, so no match -/// name_matches("compute_merkle_root", "ro_mer") == false -/// -/// // neither "compute" nor "merkle" nor "root" start with "oot" -/// name_matches("compute_merkle_root", "oot") == false -pub(crate) fn name_matches(name: &str, prefix: &str) -> bool { - let name = name.to_case(Case::Snake); - let name_parts: Vec<&str> = name.split('_').collect(); - - let mut last_index: i32 = -1; - for prefix_part in prefix.split('_') { - // Look past parts we already matched - let offset = if last_index >= 0 { last_index as usize + 1 } else { 0 }; - - if let Some(mut name_part_index) = - name_parts.iter().skip(offset).position(|name_part| name_part.starts_with(prefix_part)) - { - // Need to adjust the index if we skipped some segments - name_part_index += offset; - - if last_index >= name_part_index as i32 { - return false; - } - last_index = name_part_index as i32; - } else { - return false; - } - } - - true -} - -#[cfg(test)] -mod completion_name_matches_tests { - use crate::name_match::name_matches; - - #[test] - fn test_name_matches() { - assert!(name_matches("foo", "foo")); - assert!(name_matches("foo_bar", "bar")); - assert!(name_matches("FooBar", "foo")); - assert!(name_matches("FooBar", "bar")); - assert!(name_matches("FooBar", "foo_bar")); - assert!(name_matches("bar_baz", "bar_b")); - - assert!(!name_matches("foo_bar", "o_b")); - } -} diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 48f24c30c94..5eb66e8aca6 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -39,7 +39,7 @@ use noirc_frontend::{ use sort_text::underscore_sort_text; use crate::{ - LspState, name_match::name_matches, requests::to_lsp_location, + LspState, requests::to_lsp_location, trait_impl_method_stub_generator::TraitImplMethodStubGenerator, use_segment_positions::UseSegmentPositions, utils, visibility::module_def_id_is_visible, }; @@ -1935,6 +1935,49 @@ fn get_type_type_id(typ: &Type) -> Option { } } +/// Returns true if name matches a prefix written in code. +/// `prefix` must already be in snake case. +/// This method splits both name and prefix by underscore, +/// then checks that every part of name starts with a part of +/// prefix, in order. +/// +/// For example: +/// +/// // "merk" and "ro" match "merkle" and "root" and are in order +/// name_matches("compute_merkle_root", "merk_ro") == true +/// +/// // "ro" matches "root", but "merkle" comes before it, so no match +/// name_matches("compute_merkle_root", "ro_mer") == false +/// +/// // neither "compute" nor "merkle" nor "root" start with "oot" +/// name_matches("compute_merkle_root", "oot") == false +fn name_matches(name: &str, prefix: &str) -> bool { + let name = name.to_case(Case::Snake); + let name_parts: Vec<&str> = name.split('_').collect(); + + let mut last_index: i32 = -1; + for prefix_part in prefix.split('_') { + // Look past parts we already matched + let offset = if last_index >= 0 { last_index as usize + 1 } else { 0 }; + + if let Some(mut name_part_index) = + name_parts.iter().skip(offset).position(|name_part| name_part.starts_with(prefix_part)) + { + // Need to adjust the index if we skipped some segments + name_part_index += offset; + + if last_index >= name_part_index as i32 { + return false; + } + last_index = name_part_index as i32; + } else { + return false; + } + } + + true +} + fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option { match reference_id { ReferenceId::Module(module_id) => Some(ModuleDefId::ModuleId(module_id)), @@ -1949,3 +1992,20 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option None, } } + +#[cfg(test)] +mod completion_name_matches_tests { + use crate::requests::completion::name_matches; + + #[test] + fn test_name_matches() { + assert!(name_matches("foo", "foo")); + assert!(name_matches("foo_bar", "bar")); + assert!(name_matches("FooBar", "foo")); + assert!(name_matches("FooBar", "bar")); + assert!(name_matches("FooBar", "foo_bar")); + assert!(name_matches("bar_baz", "bar_b")); + + assert!(!name_matches("foo_bar", "o_b")); + } +} From b7b36f0c66fdbd47645452807240ac43e60c96db Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 8 Apr 2025 09:26:45 -0300 Subject: [PATCH 12/13] Fix --- tooling/lsp/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 524362f0dd3..a73dbaa339d 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -65,7 +65,6 @@ use tower::Service; mod attribute_reference_finder; mod modules; -mod name_match; mod notifications; mod requests; mod solver; From 431cbc3ac02009758d4253c756df68aaa2522d20 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 8 Apr 2025 09:56:24 -0300 Subject: [PATCH 13/13] Apply suggestions from code review --- tooling/lsp/src/notifications/mod.rs | 4 ++-- tooling/lsp/src/requests/workspace_symbol.rs | 19 +++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 510e088121d..2a9f3ef0d00 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -62,7 +62,7 @@ pub(super) fn on_did_change_text_document( ) -> ControlFlow> { let text = params.content_changes.into_iter().next().unwrap().text; state.input_files.insert(params.text_document.uri.to_string(), text.clone()); - state.workspace_symbol_cache.reprocess_url(¶ms.text_document.uri); + state.workspace_symbol_cache.reprocess_uri(¶ms.text_document.uri); let document_uri = params.text_document.uri; let output_diagnostics = false; @@ -79,7 +79,7 @@ pub(super) fn on_did_close_text_document( ) -> ControlFlow> { state.input_files.remove(¶ms.text_document.uri.to_string()); state.cached_lenses.remove(¶ms.text_document.uri.to_string()); - state.workspace_symbol_cache.reprocess_url(¶ms.text_document.uri); + state.workspace_symbol_cache.reprocess_uri(¶ms.text_document.uri); state.open_documents_count -= 1; diff --git a/tooling/lsp/src/requests/workspace_symbol.rs b/tooling/lsp/src/requests/workspace_symbol.rs index a5befbd59bf..1402ffae233 100644 --- a/tooling/lsp/src/requests/workspace_symbol.rs +++ b/tooling/lsp/src/requests/workspace_symbol.rs @@ -68,10 +68,9 @@ pub(crate) fn on_workspace_symbol_request( let parsed_files = parse_all(&file_manager); for (file_id, (parsed_module, _)) in parsed_files { let path = file_manager.path(file_id).unwrap().to_path_buf(); - let mut symbols = Vec::new(); - let mut gatherer = WorkspaceSymboGatherer::new(&mut symbols, file_manager.as_file_map()); + let mut gatherer = WorkspaceSymbolGatherer::new(file_manager.as_file_map()); parsed_module.accept(&mut gatherer); - cache.symbols_per_path.insert(path, gatherer.symbols.clone()); + cache.symbols_per_path.insert(path, gatherer.symbols); } // Finally, we filter the symbols based on the query @@ -104,7 +103,7 @@ pub(crate) struct WorkspaceSymbolCache { } impl WorkspaceSymbolCache { - pub(crate) fn reprocess_url(&mut self, uri: &Url) { + pub(crate) fn reprocess_uri(&mut self, uri: &Url) { if !self.initialized { return; } @@ -116,14 +115,14 @@ impl WorkspaceSymbolCache { } } -struct WorkspaceSymboGatherer<'symbols, 'files> { - symbols: &'symbols mut Vec, +struct WorkspaceSymbolGatherer<'files> { + symbols: Vec, files: &'files FileMap, } -impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { - fn new(symbols: &'symbols mut Vec, files: &'files FileMap) -> Self { - Self { symbols, files } +impl<'files> WorkspaceSymbolGatherer<'files> { + fn new(files: &'files FileMap) -> Self { + Self { symbols: Vec::new(), files } } fn to_lsp_location(&self, location: Location) -> Option { @@ -151,7 +150,7 @@ impl<'symbols, 'files> WorkspaceSymboGatherer<'symbols, 'files> { } } -impl Visitor for WorkspaceSymboGatherer<'_, '_> { +impl Visitor for WorkspaceSymbolGatherer<'_> { fn visit_parsed_submodule(&mut self, submodule: &ParsedSubModule, _: Span) -> bool { self.push_symbol(&submodule.name, SymbolKind::MODULE); true