From 1575c3148cbf36f678a84e2a04ef5de6a48f972e Mon Sep 17 00:00:00 2001 From: Koby Date: Mon, 8 Jan 2024 18:45:11 +0100 Subject: [PATCH 1/6] feat(lsp): resolve trait method declaration --- .../src/hir/resolution/traits.rs | 3 +- compiler/noirc_frontend/src/hir_def/traits.rs | 3 +- compiler/noirc_frontend/src/node_interner.rs | 18 ++++- tooling/lsp/src/lib.rs | 5 +- tooling/lsp/src/requests/goto_declaration.rs | 80 +++++++++++++++++++ tooling/lsp/src/requests/goto_definition.rs | 72 ++--------------- tooling/lsp/src/requests/mod.rs | 76 +++++++++++++++++- tooling/lsp/src/types.rs | 9 ++- 8 files changed, 187 insertions(+), 79 deletions(-) create mode 100644 tooling/lsp/src/requests/goto_declaration.rs diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 40041b0fd00..545a46fd8e4 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -159,9 +159,8 @@ fn resolve_trait_methods( functions.push(TraitFunction { name: name.clone(), typ: Type::Forall(generics, Box::new(function_type)), - span: name.span(), + location: Location::new(name.span(), unresolved_trait.file_id), default_impl, - default_impl_file_id: unresolved_trait.file_id, default_impl_module_id: unresolved_trait.module_id, }); diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs index 7bf5870e1f5..92e80510032 100644 --- a/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/compiler/noirc_frontend/src/hir_def/traits.rs @@ -12,9 +12,8 @@ use noirc_errors::{Location, Span}; pub struct TraitFunction { pub name: Ident, pub typ: Type, - pub span: Span, + pub location: Location, pub default_impl: Option>, - pub default_impl_file_id: fm::FileId, pub default_impl_module_id: crate::hir::def_map::LocalModuleId, } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 9d310b595de..ce1e3bf303c 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1143,7 +1143,6 @@ impl NodeInterner { } /// Adds a trait implementation to the list of known implementations. - #[tracing::instrument(skip(self))] pub fn add_trait_implementation( &mut self, object_type: Type, @@ -1279,6 +1278,7 @@ impl NodeInterner { self.find_location_index(location) .and_then(|index| self.resolve_location(index)) .or_else(|| self.try_resolve_trait_impl_location(location)) + .or_else(|| self.try_resolve_trait_method_declaration(location)) } /// For a given [Index] we return [Location] to which we resolved to @@ -1451,6 +1451,22 @@ impl NodeInterner { self.traits.get(&trait_impl.trait_id).map(|trait_| trait_.location) }) } + + fn try_resolve_trait_method_declaration(&self, location: Location) -> Option { + self.func_meta + .iter() + .find(|(_, func_meta)| func_meta.location.contains(&location)) + .and_then(|(func_id, _func_meta)| { + let (_, trait_id) = self.get_function_trait(func_id)?; + + self.traits + .get(&trait_id)? + .methods + .iter() + .find(|method| method.name.0.contents == self.function_name(func_id)) + .map(|method| method.location) + }) + } } impl Methods { diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 271e1e40df3..97f8b6ffd85 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -33,8 +33,8 @@ use notifications::{ on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized, }; use requests::{ - on_code_lens_request, on_formatting, on_goto_definition_request, on_initialize, - on_profile_run_request, on_shutdown, on_test_run_request, on_tests_request, + on_code_lens_request, on_formatting, on_goto_declaration_request, on_goto_definition_request, + on_initialize, on_profile_run_request, on_shutdown, on_test_run_request, on_tests_request, }; use serde_json::Value as JsonValue; use thiserror::Error; @@ -97,6 +97,7 @@ impl NargoLspService { .request::(on_test_run_request) .request::(on_profile_run_request) .request::(on_goto_definition_request) + .request::(on_goto_declaration_request) .notification::(on_initialized) .notification::(on_did_change_configuration) .notification::(on_did_open_text_document) diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs new file mode 100644 index 00000000000..ba203d02991 --- /dev/null +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -0,0 +1,80 @@ +use std::future::{self, Future}; + +use crate::resolve_workspace_for_source_path; +use crate::types::GotoDeclarationResult; +use crate::LspState; +use async_lsp::{ErrorCode, ResponseError}; + +use lsp_types::request::{GotoDeclarationParams, GotoDeclarationResponse}; + +use nargo::insert_all_files_for_workspace_into_file_manager; +use noirc_driver::file_manager_with_stdlib; + +use super::{position_to_byte_index, to_lsp_location}; + +pub(crate) fn on_goto_declaration_request( + state: &mut LspState, + params: GotoDeclarationParams, +) -> impl Future> { + let result = on_goto_definition_inner(state, params); + future::ready(result) +} + +fn on_goto_definition_inner( + _state: &mut LspState, + params: GotoDeclarationParams, +) -> Result { + let file_path = + params.text_document_position_params.text_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.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); + + 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| { + ResponseError::new( + ErrorCode::REQUEST_FAILED, + format!("Could not convert position to byte index. Error: {:?}", err), + ) + })?; + + let search_for_location = noirc_errors::Location { + file: file_id, + span: noirc_errors::Span::single_char(byte_index as u32), + }; + + let goto_definition_response = + 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: GotoDeclarationResponse = + GotoDeclarationResponse::from(definition_position).to_owned(); + Some(response) + }); + + Ok(goto_definition_response) +} diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 2ff5901ff9c..6d44761de94 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -3,12 +3,13 @@ use std::future::{self, Future}; use crate::resolve_workspace_for_source_path; use crate::{types::GotoDefinitionResult, LspState}; use async_lsp::{ErrorCode, ResponseError}; -use fm::codespan_files::Error; -use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse, Location}; -use lsp_types::{Position, Url}; + +use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse}; use nargo::insert_all_files_for_workspace_into_file_manager; use noirc_driver::file_manager_with_stdlib; +use super::{position_to_byte_index, to_lsp_location}; + pub(crate) fn on_goto_definition_request( state: &mut LspState, params: GotoDefinitionParams, @@ -76,74 +77,11 @@ fn on_goto_definition_inner( Ok(goto_definition_response) } -fn to_lsp_location<'a, F>( - files: &'a F, - file_id: F::FileId, - definition_span: noirc_errors::Span, -) -> Option -where - F: fm::codespan_files::Files<'a> + ?Sized, -{ - let range = crate::byte_span_to_range(files, file_id, definition_span.into())?; - let file_name = files.name(file_id).ok()?; - - let path = file_name.to_string(); - let uri = Url::from_file_path(path).ok()?; - - Some(Location { uri, range }) -} - -pub(crate) fn position_to_byte_index<'a, F>( - files: &'a F, - file_id: F::FileId, - position: &Position, -) -> Result -where - F: fm::codespan_files::Files<'a> + ?Sized, -{ - let source = files.source(file_id)?; - let source = source.as_ref(); - - let line_span = files.line_range(file_id, position.line as usize)?; - - let line_str = source.get(line_span.clone()); - - if let Some(line_str) = line_str { - let byte_offset = character_to_line_offset(line_str, position.character)?; - Ok(line_span.start + byte_offset) - } else { - Err(Error::InvalidCharBoundary { given: position.line as usize }) - } -} - -fn character_to_line_offset(line: &str, character: u32) -> Result { - let line_len = line.len(); - let mut character_offset = 0; - - let mut chars = line.chars(); - while let Some(ch) = chars.next() { - if character_offset == character { - let chars_off = chars.as_str().len(); - let ch_off = ch.len_utf8(); - - return Ok(line_len - chars_off - ch_off); - } - - character_offset += ch.len_utf16() as u32; - } - - // Handle positions after the last character on the line - if character_offset == character { - Ok(line_len) - } else { - Err(Error::ColumnTooLarge { given: character_offset as usize, max: line.len() }) - } -} - #[cfg(test)] mod goto_definition_tests { use async_lsp::ClientSocket; + use lsp_types::{Position, Url}; use tokio::test; use crate::solver::MockBackend; diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 2711c597bcf..64a855df648 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -2,7 +2,11 @@ use std::future::Future; use crate::types::{CodeLensOptions, InitializeParams}; use async_lsp::ResponseError; -use lsp_types::{Position, TextDocumentSyncCapability, TextDocumentSyncKind}; +use fm::codespan_files::Error; +use lsp_types::{ + DeclarationCapability, Location, Position, TextDocumentSyncCapability, TextDocumentSyncKind, + Url, +}; use nargo_fmt::Config; use serde::{Deserialize, Serialize}; @@ -22,6 +26,7 @@ use crate::{ // and params passed in. mod code_lens_request; +mod goto_declaration; mod goto_definition; mod profile_run; mod test_run; @@ -29,8 +34,8 @@ mod tests; pub(crate) use { code_lens_request::collect_lenses_for_package, code_lens_request::on_code_lens_request, - goto_definition::on_goto_definition_request, profile_run::on_profile_run_request, - test_run::on_test_run_request, tests::on_tests_request, + goto_declaration::on_goto_declaration_request, goto_definition::on_goto_definition_request, + profile_run::on_profile_run_request, test_run::on_test_run_request, tests::on_tests_request, }; /// LSP client will send initialization request after the server has started. @@ -88,6 +93,7 @@ pub(crate) fn on_initialize( document_formatting_provider: true, nargo: Some(nargo), definition_provider: Some(lsp_types::OneOf::Left(true)), + declaration_provider: Some(DeclarationCapability::Simple(true)), }, server_info: None, }) @@ -130,6 +136,70 @@ fn on_formatting_inner( } } +pub(crate) fn position_to_byte_index<'a, F>( + files: &'a F, + file_id: F::FileId, + position: &Position, +) -> Result +where + F: fm::codespan_files::Files<'a> + ?Sized, +{ + let source = files.source(file_id)?; + let source = source.as_ref(); + + let line_span = files.line_range(file_id, position.line as usize)?; + + let line_str = source.get(line_span.clone()); + + if let Some(line_str) = line_str { + let byte_offset = character_to_line_offset(line_str, position.character)?; + Ok(line_span.start + byte_offset) + } else { + Err(Error::InvalidCharBoundary { given: position.line as usize }) + } +} + +fn character_to_line_offset(line: &str, character: u32) -> Result { + let line_len = line.len(); + let mut character_offset = 0; + + let mut chars = line.chars(); + while let Some(ch) = chars.next() { + if character_offset == character { + let chars_off = chars.as_str().len(); + let ch_off = ch.len_utf8(); + + return Ok(line_len - chars_off - ch_off); + } + + character_offset += ch.len_utf16() as u32; + } + + // Handle positions after the last character on the line + if character_offset == character { + Ok(line_len) + } else { + Err(Error::ColumnTooLarge { given: character_offset as usize, max: line.len() }) + } +} + +fn to_lsp_location<'a, F>( + files: &'a F, + file_id: F::FileId, + definition_span: noirc_errors::Span, +) -> Option +where + F: fm::codespan_files::Files<'a> + ?Sized, +{ + let range = crate::byte_span_to_range(files, file_id, definition_span.into())?; + let file_name = files.name(file_id).ok()?; + + let path = file_name.to_string(); + let uri = Url::from_file_path(path).ok()?; + + Some(Location { uri, range }) +} + pub(crate) fn on_shutdown( _state: &mut LspState, _params: (), diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index b2960964e7c..8dbc51ec83c 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -1,5 +1,5 @@ use fm::FileId; -use lsp_types::{DefinitionOptions, OneOf}; +use lsp_types::{DeclarationCapability, DefinitionOptions, OneOf}; use noirc_driver::DebugFile; use noirc_errors::{debug_info::OpCodesCount, Location}; use noirc_frontend::graph::CrateName; @@ -25,7 +25,7 @@ pub(crate) mod request { // Re-providing lsp_types that we don't need to override pub(crate) use lsp_types::request::{ - CodeLensRequest as CodeLens, Formatting, GotoDefinition, Shutdown, + CodeLensRequest as CodeLens, Formatting, GotoDeclaration, GotoDefinition, Shutdown, }; #[derive(Debug)] @@ -110,6 +110,10 @@ pub(crate) struct ServerCapabilities { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) text_document_sync: Option, + /// The server provides go to declaration support. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) declaration_provider: Option, + /// The server provides goto definition support. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) definition_provider: Option>, @@ -222,3 +226,4 @@ pub(crate) struct NargoProfileRunResult { pub(crate) type CodeLensResult = Option>; pub(crate) type GotoDefinitionResult = Option; +pub(crate) type GotoDeclarationResult = Option; From 3d52dfef2cfe09711c9e4e80e9f23648ba537678 Mon Sep 17 00:00:00 2001 From: Koby Date: Tue, 9 Jan 2024 16:42:34 +0100 Subject: [PATCH 2/6] make goto decl working for method invocation --- compiler/noirc_frontend/src/node_interner.rs | 11 ++++++++++- tooling/lsp/src/requests/goto_declaration.rs | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index ce1e3bf303c..9eb04fd18d9 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -468,7 +468,6 @@ impl NodeInterner { /// The [Location] may not necessarily point to the beginning of the item /// so we check if the location's span is contained within the start or end /// of each items [Span] - #[tracing::instrument(skip(self))] pub fn find_location_index(&self, location: Location) -> Option> { let mut location_candidate: Option<(&Index, &Location)> = None; @@ -1281,6 +1280,16 @@ impl NodeInterner { .or_else(|| self.try_resolve_trait_method_declaration(location)) } + pub fn get_declaration_location_from(&self, location: Location) -> Option { + self.try_resolve_trait_method_declaration(location).or_else(|| { + self.find_location_index(location) + .and_then(|index| self.resolve_location(index)) + .and_then(|found_impl_location| { + self.try_resolve_trait_method_declaration(found_impl_location) + }) + }) + } + /// 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 diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index ba203d02991..9186393748e 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -68,7 +68,7 @@ fn on_goto_definition_inner( }; let goto_definition_response = - interner.get_definition_location_from(search_for_location).and_then(|found_location| { + interner.get_declaration_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: GotoDeclarationResponse = From 9d11718f5c480e9a1e9d2782058a0348c9c4770d Mon Sep 17 00:00:00 2001 From: Koby Date: Tue, 9 Jan 2024 16:47:34 +0100 Subject: [PATCH 3/6] chore: naming --- tooling/lsp/src/requests/goto_declaration.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 9186393748e..6e3664804f6 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -67,7 +67,7 @@ fn on_goto_definition_inner( span: noirc_errors::Span::single_char(byte_index as u32), }; - let goto_definition_response = + let goto_declaration_response = interner.get_declaration_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)?; @@ -76,5 +76,5 @@ fn on_goto_definition_inner( Some(response) }); - Ok(goto_definition_response) + Ok(goto_declaration_response) } From 9a00a75fb356e8e389d12153cd86c82de5f4684c Mon Sep 17 00:00:00 2001 From: Koby Hall <102518238+kobyhallx@users.noreply.github.com> Date: Thu, 11 Jan 2024 11:35:47 +0100 Subject: [PATCH 4/6] Update compiler/noirc_frontend/src/node_interner.rs Co-authored-by: jfecher --- compiler/noirc_frontend/src/node_interner.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index c0af68ab83e..048c60c8086 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1471,12 +1471,9 @@ impl NodeInterner { .and_then(|(func_id, _func_meta)| { let (_, trait_id) = self.get_function_trait(func_id)?; - self.traits - .get(&trait_id)? - .methods - .iter() - .find(|method| method.name.0.contents == self.function_name(func_id)) - .map(|method| method.location) + let methods = self.traits.get(&trait_id)?.methods.iter(); + let method = methods.find(|method| method.name.0.contents == self.function_name(func_id)); + method.map(|method| method.location) }) } } From d941309c9a9e9ac015e3fe39336933d488e5a601 Mon Sep 17 00:00:00 2001 From: Koby Date: Thu, 11 Jan 2024 13:05:23 +0100 Subject: [PATCH 5/6] chore: separate location resolution to own file --- compiler/noirc_frontend/src/lib.rs | 1 + compiler/noirc_frontend/src/node_interner.rs | 154 +--------------- .../noirc_frontend/src/resolve_locations.rs | 170 ++++++++++++++++++ 3 files changed, 177 insertions(+), 148 deletions(-) create mode 100644 compiler/noirc_frontend/src/resolve_locations.rs diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index 77107d3e7db..9582b80dcba 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -16,6 +16,7 @@ pub mod lexer; pub mod monomorphization; pub mod node_interner; pub mod parser; +pub mod resolve_locations; pub mod hir; pub mod hir_def; diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 048c60c8086..e30f2019f35 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -39,8 +39,8 @@ type StructAttributes = Vec; /// monomorphization - and it is not useful afterward. #[derive(Debug)] pub struct NodeInterner { - nodes: Arena, - func_meta: HashMap, + pub(crate) nodes: Arena, + pub(crate) func_meta: HashMap, function_definition_ids: HashMap, // For a given function ID, this gives the function's modifiers which includes @@ -52,7 +52,7 @@ pub struct NodeInterner { function_modules: HashMap, // Map each `Index` to it's own location - id_to_location: HashMap, + pub(crate) id_to_location: HashMap, // Maps each DefinitionId to a DefinitionInfo. definitions: Vec, @@ -85,14 +85,14 @@ pub struct NodeInterner { // Each trait definition is possibly shared across multiple type nodes. // It is also mutated through the RefCell during name resolution to append // methods from impls to the type. - traits: HashMap, + pub(crate) traits: HashMap, // Trait implementation map // For each type that implements a given Trait ( corresponding TraitId), there should be an entry here // The purpose for this hashmap is to detect duplication of trait implementations ( if any ) // // Indexed by TraitImplIds - trait_implementations: Vec>, + pub(crate) trait_implementations: Vec>, /// Trait implementations on each type. This is expected to always have the same length as /// `self.trait_implementations`. @@ -350,7 +350,7 @@ partialeq!(StmtId); /// This data structure is never accessed directly, so API wise there is no difference between using /// Multiple arenas and a single Arena #[derive(Debug, Clone)] -enum Node { +pub(crate) enum Node { Function(HirFunction), Statement(HirStatement), Expression(HirExpression), @@ -463,30 +463,6 @@ impl NodeInterner { self.id_to_location.insert(expr_id.into(), Location::new(span, file)); } - /// Scans the interner for the item which is located at that [Location] - /// - /// The [Location] may not necessarily point to the beginning of the item - /// so we check if the location's span is contained within the start or end - /// of each items [Span] - pub fn find_location_index(&self, location: Location) -> Option> { - let mut location_candidate: Option<(&Index, &Location)> = None; - - // Note: we can modify this in the future to not do a linear - // scan by storing a separate map of the spans or by sorting the locations. - for (index, interned_location) in self.id_to_location.iter() { - if interned_location.contains(&location) { - if let Some(current_location) = location_candidate { - if interned_location.span.is_smaller(¤t_location.1.span) { - location_candidate = Some((index, interned_location)); - } - } else { - location_candidate = Some((index, interned_location)); - } - } - } - location_candidate.map(|(index, _location)| *index) - } - /// Interns a HIR Function. pub fn push_fn(&mut self, func: HirFunction) -> FuncId { FuncId(self.nodes.insert(Node::Function(func))) @@ -1274,93 +1250,6 @@ 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)) - .or_else(|| self.try_resolve_trait_impl_location(location)) - .or_else(|| self.try_resolve_trait_method_declaration(location)) - } - - pub fn get_declaration_location_from(&self, location: Location) -> Option { - self.try_resolve_trait_method_declaration(location).or_else(|| { - self.find_location_index(location) - .and_then(|index| self.resolve_location(index)) - .and_then(|found_impl_location| { - self.try_resolve_trait_method_declaration(found_impl_location) - }) - }) - } - - /// 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 - fn resolve_location(&self, index: impl Into) -> Option { - let node = self.nodes.get(index.into())?; - - match node { - Node::Function(func) => self.resolve_location(func.as_expr()), - Node::Expression(expression) => self.resolve_expression_location(expression), - _ => None, - } - } - - /// Resolves the [Location] of the definition for a given [HirExpression] - /// - /// Note: current the code returns None because some expressions are not yet implemented. - fn resolve_expression_location(&self, expression: &HirExpression) -> Option { - match expression { - HirExpression::Ident(ident) => { - let definition_info = self.definition(ident.id); - match definition_info.kind { - DefinitionKind::Function(func_id) => { - Some(self.function_meta(&func_id).location) - } - DefinitionKind::Local(_local_id) => Some(definition_info.location), - _ => None, - } - } - HirExpression::Constructor(expr) => { - let struct_type = &expr.r#type.borrow(); - Some(struct_type.location) - } - HirExpression::MemberAccess(expr_member_access) => { - self.resolve_struct_member_access(expr_member_access) - } - HirExpression::Call(expr_call) => { - let func = expr_call.func; - self.resolve_location(func) - } - - _ => None, - } - } - - /// Resolves the [Location] of the definition for a given [crate::hir_def::expr::HirMemberAccess] - /// This is used to resolve the location of a struct member access. - /// For example, in the expression `foo.bar` we want to resolve the location of `bar` - /// to the location of the definition of `bar` in the struct `foo`. - fn resolve_struct_member_access( - &self, - expr_member_access: &crate::hir_def::expr::HirMemberAccess, - ) -> Option { - let expr_lhs = &expr_member_access.lhs; - let expr_rhs = &expr_member_access.rhs; - - let lhs_self_struct = match self.id_type(expr_lhs) { - Type::Struct(struct_type, _) => struct_type, - _ => return None, - }; - - let struct_type = lhs_self_struct.borrow(); - let field_names = struct_type.field_names(); - - field_names.iter().find(|field_name| field_name.0 == expr_rhs.0).map(|found_field_name| { - Location::new(found_field_name.span(), struct_type.location.file) - }) - } - /// Retrieves the trait id for a given binary operator. /// All binary operators correspond to a trait - although multiple may correspond /// to the same trait (such as `==` and `!=`). @@ -1445,37 +1334,6 @@ impl NodeInterner { pub(crate) fn ordering_type(&self) -> Type { self.ordering_type.clone().expect("Expected ordering_type to be set in the NodeInterner") } - - /// Attempts to resolve [Location] of [Trait] based on [Location] of [TraitImpl] - /// This is used by LSP to resolve the location of a trait based on the location of a trait impl. - /// - /// Example: - /// impl Foo for Bar { ... } -> trait Foo { ... } - fn try_resolve_trait_impl_location(&self, location: Location) -> Option { - self.trait_implementations - .iter() - .find(|shared_trait_impl| { - let trait_impl = shared_trait_impl.borrow(); - trait_impl.file == location.file && trait_impl.ident.span().contains(&location.span) - }) - .and_then(|shared_trait_impl| { - let trait_impl = shared_trait_impl.borrow(); - self.traits.get(&trait_impl.trait_id).map(|trait_| trait_.location) - }) - } - - fn try_resolve_trait_method_declaration(&self, location: Location) -> Option { - self.func_meta - .iter() - .find(|(_, func_meta)| func_meta.location.contains(&location)) - .and_then(|(func_id, _func_meta)| { - let (_, trait_id) = self.get_function_trait(func_id)?; - - let methods = self.traits.get(&trait_id)?.methods.iter(); - let method = methods.find(|method| method.name.0.contents == self.function_name(func_id)); - method.map(|method| method.location) - }) - } } impl Methods { diff --git a/compiler/noirc_frontend/src/resolve_locations.rs b/compiler/noirc_frontend/src/resolve_locations.rs new file mode 100644 index 00000000000..53bf0dff978 --- /dev/null +++ b/compiler/noirc_frontend/src/resolve_locations.rs @@ -0,0 +1,170 @@ +use arena::Index; +use noirc_errors::Location; + +use crate::hir_def::expr::HirExpression; +use crate::hir_def::types::Type; + +use crate::node_interner::{DefinitionKind, Node, NodeInterner}; + +impl NodeInterner { + /// Scans the interner for the item which is located at that [Location] + /// + /// The [Location] may not necessarily point to the beginning of the item + /// so we check if the location's span is contained within the start or end + /// of each items [Span] + pub fn find_location_index(&self, location: Location) -> Option> { + let mut location_candidate: Option<(&Index, &Location)> = None; + + // Note: we can modify this in the future to not do a linear + // scan by storing a separate map of the spans or by sorting the locations. + for (index, interned_location) in self.id_to_location.iter() { + if interned_location.contains(&location) { + if let Some(current_location) = location_candidate { + if interned_location.span.is_smaller(¤t_location.1.span) { + location_candidate = Some((index, interned_location)); + } + } else { + location_candidate = Some((index, interned_location)); + } + } + } + location_candidate.map(|(index, _location)| *index) + } + + /// 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)) + .or_else(|| self.try_resolve_trait_impl_location(location)) + .or_else(|| self.try_resolve_trait_method_declaration(location)) + } + + pub fn get_declaration_location_from(&self, location: Location) -> Option { + self.try_resolve_trait_method_declaration(location).or_else(|| { + self.find_location_index(location) + .and_then(|index| self.resolve_location(index)) + .and_then(|found_impl_location| { + self.try_resolve_trait_method_declaration(found_impl_location) + }) + }) + } + + /// 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 + fn resolve_location(&self, index: impl Into) -> Option { + let node = self.nodes.get(index.into())?; + + match node { + Node::Function(func) => self.resolve_location(func.as_expr()), + Node::Expression(expression) => self.resolve_expression_location(expression), + _ => None, + } + } + + /// Resolves the [Location] of the definition for a given [HirExpression] + /// + /// Note: current the code returns None because some expressions are not yet implemented. + fn resolve_expression_location(&self, expression: &HirExpression) -> Option { + match expression { + HirExpression::Ident(ident) => { + let definition_info = self.definition(ident.id); + match definition_info.kind { + DefinitionKind::Function(func_id) => { + Some(self.function_meta(&func_id).location) + } + DefinitionKind::Local(_local_id) => Some(definition_info.location), + _ => None, + } + } + HirExpression::Constructor(expr) => { + let struct_type = &expr.r#type.borrow(); + Some(struct_type.location) + } + HirExpression::MemberAccess(expr_member_access) => { + self.resolve_struct_member_access(expr_member_access) + } + HirExpression::Call(expr_call) => { + let func = expr_call.func; + self.resolve_location(func) + } + + _ => None, + } + } + + /// Resolves the [Location] of the definition for a given [crate::hir_def::expr::HirMemberAccess] + /// This is used to resolve the location of a struct member access. + /// For example, in the expression `foo.bar` we want to resolve the location of `bar` + /// to the location of the definition of `bar` in the struct `foo`. + fn resolve_struct_member_access( + &self, + expr_member_access: &crate::hir_def::expr::HirMemberAccess, + ) -> Option { + let expr_lhs = &expr_member_access.lhs; + let expr_rhs = &expr_member_access.rhs; + + let lhs_self_struct = match self.id_type(expr_lhs) { + Type::Struct(struct_type, _) => struct_type, + _ => return None, + }; + + let struct_type = lhs_self_struct.borrow(); + let field_names = struct_type.field_names(); + + field_names.iter().find(|field_name| field_name.0 == expr_rhs.0).map(|found_field_name| { + Location::new(found_field_name.span(), struct_type.location.file) + }) + } + + /// Attempts to resolve [Location] of [Trait] based on [Location] of [TraitImpl] + /// This is used by LSP to resolve the location of a trait based on the location of a trait impl. + /// + /// Example: + /// impl Foo for Bar { ... } -> trait Foo { ... } + fn try_resolve_trait_impl_location(&self, location: Location) -> Option { + self.trait_implementations + .iter() + .find(|shared_trait_impl| { + let trait_impl = shared_trait_impl.borrow(); + trait_impl.file == location.file && trait_impl.ident.span().contains(&location.span) + }) + .and_then(|shared_trait_impl| { + let trait_impl = shared_trait_impl.borrow(); + self.traits.get(&trait_impl.trait_id).map(|trait_| trait_.location) + }) + } + + /// Attempts to resolve [Location] of [Trait]'s [TraitFunction] declaration based on [Location] of [TraitFunction] call. + /// + /// This is used by LSP to resolve the location. + /// + /// ### Example: + /// ``` + /// // Noir + /// trait Fieldable { + /// fn to_field(self) -> Field; + /// // ^------------------------------| + /// } // | + /// // | + /// fn main(x: u32) { // | + /// assert(x.to_field() == 15); // | + /// // \.......................| + /// } + /// ``` + /// + fn try_resolve_trait_method_declaration(&self, location: Location) -> Option { + self.func_meta + .iter() + .find(|(_, func_meta)| func_meta.location.contains(&location)) + .and_then(|(func_id, _func_meta)| { + let (_, trait_id) = self.get_function_trait(func_id)?; + + let mut methods = self.traits.get(&trait_id)?.methods.iter(); + let method = + methods.find(|method| method.name.0.contents == self.function_name(func_id)); + method.map(|method| method.location) + }) + } +} From b3d1e92734b67d5ef24a85408b3a7d5ea5349938 Mon Sep 17 00:00:00 2001 From: Koby Date: Thu, 11 Jan 2024 13:12:43 +0100 Subject: [PATCH 6/6] chore: avoid docs clippy clash --- compiler/noirc_frontend/src/resolve_locations.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/resolve_locations.rs b/compiler/noirc_frontend/src/resolve_locations.rs index 53bf0dff978..bfacee0ef96 100644 --- a/compiler/noirc_frontend/src/resolve_locations.rs +++ b/compiler/noirc_frontend/src/resolve_locations.rs @@ -141,16 +141,15 @@ impl NodeInterner { /// This is used by LSP to resolve the location. /// /// ### Example: - /// ``` - /// // Noir + /// ```nr /// trait Fieldable { /// fn to_field(self) -> Field; - /// // ^------------------------------| - /// } // | - /// // | - /// fn main(x: u32) { // | - /// assert(x.to_field() == 15); // | - /// // \.......................| + /// ^------------------------------\ + /// } | + /// | + /// fn main_func(x: u32) { | + /// assert(x.to_field() == 15); | + /// \......................./ /// } /// ``` ///