diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index 25875385fd9..3683b17a27c 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -194,14 +194,6 @@ impl Context<'_> { .collect() } - /// Returns the [Location] of the definition of the given Ident found at [Span] of the given [FileId]. - /// Returns [None] when definition is not found. - pub fn get_definition_location_from(&self, location: Location) -> Option { - let interner = &self.def_interner; - - interner.find_location_index(location).and_then(|index| interner.resolve_location(index)) - } - /// Return a Vec of all `contract` declarations in the source code and the functions they contain pub fn get_all_contracts(&self, crate_id: &CrateId) -> Vec { self.def_map(crate_id) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 7c42e279916..9082df1bcd5 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1271,10 +1271,16 @@ impl NodeInterner { self.selected_trait_implementations.get(&ident_id).cloned() } + /// Returns the [Location] of the definition of the given Ident found at [Span] of the given [FileId]. + /// Returns [None] when definition is not found. + pub fn get_definition_location_from(&self, location: Location) -> Option { + self.find_location_index(location).and_then(|index| self.resolve_location(index)) + } + /// For a given [Index] we return [Location] to which we resolved to /// We currently return None for features not yet implemented /// TODO(#3659): LSP goto def should error when Ident at Location could not resolve - pub(crate) fn resolve_location(&self, index: impl Into) -> Option { + fn resolve_location(&self, index: impl Into) -> Option { let node = self.nodes.get(index.into())?; match node { diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 90e8e563cd3..271e1e40df3 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -25,6 +25,7 @@ use noirc_driver::{file_manager_with_stdlib, prepare_crate, NOIR_ARTIFACT_VERSIO use noirc_frontend::{ graph::{CrateId, CrateName}, hir::{Context, FunctionNameMatch}, + node_interner::NodeInterner, }; use notifications::{ @@ -59,8 +60,10 @@ pub struct LspState { root_path: Option, client: ClientSocket, solver: WrapperSolver, + open_documents_count: usize, input_files: HashMap, cached_lenses: HashMap>, + cached_definitions: HashMap, } impl LspState { @@ -71,6 +74,8 @@ impl LspState { solver: WrapperSolver(Box::new(solver)), input_files: HashMap::new(), cached_lenses: HashMap::new(), + cached_definitions: HashMap::new(), + open_documents_count: 0, } } } diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 6cefaa134ce..aec3bf61f4e 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -36,7 +36,16 @@ pub(super) fn on_did_open_text_document( params: DidOpenTextDocumentParams, ) -> ControlFlow> { state.input_files.insert(params.text_document.uri.to_string(), params.text_document.text); - ControlFlow::Continue(()) + + let document_uri = params.text_document.uri; + + match process_noir_document(document_uri, state) { + Ok(_) => { + state.open_documents_count += 1; + ControlFlow::Continue(()) + } + Err(err) => ControlFlow::Break(Err(err)), + } } pub(super) fn on_did_change_text_document( @@ -85,6 +94,13 @@ 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.open_documents_count -= 1; + + if state.open_documents_count == 0 { + state.cached_definitions.clear(); + } + ControlFlow::Continue(()) } @@ -92,27 +108,25 @@ pub(super) fn on_did_save_text_document( state: &mut LspState, params: DidSaveTextDocumentParams, ) -> ControlFlow> { - let file_path = match params.text_document.uri.to_file_path() { - Ok(file_path) => file_path, - Err(()) => { - return ControlFlow::Break(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "URI is not a valid file path", - ) - .into())) - } - }; + let document_uri = params.text_document.uri; - let workspace = match resolve_workspace_for_source_path(&file_path) { - Ok(value) => value, - Err(lsp_error) => { - return ControlFlow::Break(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - lsp_error.to_string(), - ) - .into())) - } - }; + match process_noir_document(document_uri, state) { + Ok(_) => ControlFlow::Continue(()), + Err(err) => ControlFlow::Break(Err(err)), + } +} + +fn process_noir_document( + document_uri: lsp_types::Url, + state: &mut LspState, +) -> Result<(), async_lsp::Error> { + let file_path = document_uri.to_file_path().map_err(|_| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path") + })?; + + let workspace = resolve_workspace_for_source_path(&file_path).map_err(|lsp_error| { + ResponseError::new(ErrorCode::REQUEST_FAILED, lsp_error.to_string()) + })?; let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); @@ -127,6 +141,8 @@ pub(super) fn on_did_save_text_document( Err(errors_and_warnings) => errors_and_warnings, }; + let package_root_dir: String = package.root_dir.as_os_str().to_string_lossy().into(); + // We don't add test headings for a package if it contains no `#[test]` functions if let Some(tests) = get_package_tests_in_crate(&context, &crate_id, &package.name) { let _ = state.client.notify::(NargoPackageTests { @@ -142,7 +158,9 @@ pub(super) fn on_did_save_text_document( package, Some(&file_path), ); - state.cached_lenses.insert(params.text_document.uri.to_string(), collected_lenses); + state.cached_lenses.insert(document_uri.to_string(), collected_lenses); + + state.cached_definitions.insert(package_root_dir, context.def_interner); let fm = &context.file_manager; let files = fm.as_file_map(); @@ -178,14 +196,13 @@ pub(super) fn on_did_save_text_document( .collect() }) .collect(); - let _ = state.client.publish_diagnostics(PublishDiagnosticsParams { - uri: params.text_document.uri, + uri: document_uri, version: None, diagnostics, }); - ControlFlow::Continue(()) + Ok(()) } pub(super) fn on_exit( diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index d2b66682d89..2ff5901ff9c 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -29,20 +29,27 @@ fn on_goto_definition_inner( let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap(); let package = workspace.members.first().unwrap(); + let package_root_path: String = package.root_dir.as_os_str().to_string_lossy().into(); + let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, package); - // We ignore the warnings and errors produced by compilation while resolving the definition - let _ = noirc_driver::check_crate(&mut context, crate_id, false, false); + let interner; + if let Some(def_interner) = _state.cached_definitions.get(&package_root_path) { + interner = def_interner; + } else { + // We ignore the warnings and errors produced by compilation while resolving the definition + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false); + interner = &context.def_interner; + } let files = context.file_manager.as_file_map(); let file_id = context.file_manager.name_to_id(file_path.clone()).ok_or(ResponseError::new( ErrorCode::REQUEST_FAILED, format!("Could not find file in file manager. File path: {:?}", file_path), ))?; - let byte_index = position_to_byte_index(files, file_id, ¶ms.text_document_position_params.position) .map_err(|err| { @@ -58,7 +65,7 @@ fn on_goto_definition_inner( }; let goto_definition_response = - context.get_definition_location_from(search_for_location).and_then(|found_location| { + interner.get_definition_location_from(search_for_location).and_then(|found_location| { let file_id = found_location.file; let definition_position = to_lsp_location(files, file_id, found_location.span)?; let response: GotoDefinitionResponse =