From bd38e4d795d1344c5ad89320b1c79d431775725d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 10 Jul 2024 17:22:35 -0300 Subject: [PATCH 01/23] feat: lsp hover --- tooling/lsp/src/lib.rs | 5 +- tooling/lsp/src/requests/hover.rs | 236 ++++++++++++++++++++++++++++++ tooling/lsp/src/requests/mod.rs | 13 +- tooling/lsp/src/types.rs | 8 +- 4 files changed, 255 insertions(+), 7 deletions(-) create mode 100644 tooling/lsp/src/requests/hover.rs diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index e91e0fb3325..28ba5425fe7 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -21,7 +21,7 @@ use async_lsp::{ use fm::{codespan_files as files, FileManager}; use fxhash::FxHashSet; use lsp_types::{ - request::{PrepareRenameRequest, References, Rename}, + request::{HoverRequest, PrepareRenameRequest, References, Rename}, CodeLens, }; use nargo::{ @@ -46,7 +46,7 @@ use notifications::{ }; use requests::{ on_code_lens_request, on_formatting, on_goto_declaration_request, on_goto_definition_request, - on_goto_type_definition_request, on_initialize, on_prepare_rename_request, + on_goto_type_definition_request, on_hover_request, on_initialize, on_prepare_rename_request, on_profile_run_request, on_references_request, on_rename_request, on_shutdown, on_test_run_request, on_tests_request, }; @@ -129,6 +129,7 @@ impl NargoLspService { .request::(on_references_request) .request::(on_prepare_rename_request) .request::(on_rename_request) + .request::(on_hover_request) .notification::(on_initialized) .notification::(on_did_change_configuration) .notification::(on_did_open_text_document) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs new file mode 100644 index 00000000000..5ac82417837 --- /dev/null +++ b/tooling/lsp/src/requests/hover.rs @@ -0,0 +1,236 @@ +use std::future::{self, Future}; + +use async_lsp::ResponseError; +use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; +use noirc_frontend::{ + ast::Visibility, + hir::def_map::ModuleId, + hir_def::stmt::HirPattern, + macros_api::{NodeInterner, StructId}, + node_interner::{ + DefinitionId, DefinitionKind, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, + }, + Generics, Type, +}; + +use crate::LspState; + +use super::{process_request, to_lsp_location}; + +pub(crate) fn on_hover_request( + state: &mut LspState, + params: HoverParams, +) -> impl Future, ResponseError>> { + let result = process_request( + state, + params.text_document_position_params, + |location, interner, files, _| { + interner.reference_at_location(location).map(|reference| { + let location = interner.reference_location(reference); + let lsp_location = to_lsp_location(files, location.file, location.span); + Hover { + range: lsp_location.map(|location| location.range), + contents: HoverContents::Markup(MarkupContent { + kind: MarkupKind::Markdown, + value: format_reference(reference, interner), + }), + } + }) + }, + ); + + future::ready(result) +} + +fn format_reference(reference: ReferenceId, interner: &NodeInterner) -> String { + match reference { + ReferenceId::Module(id) => format_module(id, interner), + ReferenceId::Struct(id) => format_struct(id, interner), + ReferenceId::StructMember(id, field_index) => { + format_struct_member(id, field_index, interner) + } + ReferenceId::Trait(id) => format_trait(id, interner), + ReferenceId::Global(id) => format_global(id, interner), + ReferenceId::Function(id) => format_function(id, interner), + ReferenceId::Alias(id) => format_alias(id, interner), + ReferenceId::Local(id) => format_local(id, &interner), + ReferenceId::Reference(location, _) => { + format_reference(interner.find_referenced(location).unwrap(), interner) + } + } +} +fn format_module(id: ModuleId, interner: &NodeInterner) -> String { + // TODO: append the module path + // TODO: find out the module name + format!(" mod {:?}", id) +} + +fn format_struct(id: StructId, interner: &NodeInterner) -> String { + let struct_type = interner.get_struct(id); + let struct_type = struct_type.borrow(); + + let mut string = String::new(); + // TODO: append the module path + string.push_str(" "); + string.push_str("struct "); + string.push_str(&struct_type.name.0.contents); + format_generics(&struct_type.generics, &mut string); + string.push_str(" {\n"); + for (field_name, field_type) in struct_type.get_fields_as_written() { + string.push_str(" "); + string.push_str(&field_name); + string.push_str(": "); + string.push_str(&format!("{}", field_type)); + string.push_str(",\n"); + } + string.push_str(" }"); + string +} + +fn format_struct_member(id: StructId, field_index: usize, interner: &NodeInterner) -> String { + let struct_type = interner.get_struct(id); + let struct_type = struct_type.borrow(); + let (field_name, field_type) = struct_type.field_at(field_index); + + let mut string = String::new(); + // TODO: append the module path + string.push_str(" "); + string.push_str(&field_name.0.contents); + string.push_str(": "); + string.push_str(&format!("{}", field_type)); + string +} + +fn format_trait(id: TraitId, interner: &NodeInterner) -> String { + let a_trait = interner.get_trait(id); + + let mut string = String::new(); + // TODO: append the module path + string.push_str(" "); + string.push_str("trait "); + string.push_str(&a_trait.name.0.contents); + format_generics(&a_trait.generics, &mut string); + string +} + +fn format_global(id: GlobalId, interner: &NodeInterner) -> String { + let global_info = interner.get_global(id); + let definition_id = global_info.definition_id; + let typ = interner.definition_type(definition_id); + + let mut string = String::new(); + // TODO: append the module path + string.push_str(" "); + string.push_str("global "); + string.push_str(&global_info.ident.0.contents); + string.push_str(": "); + string.push_str(&format!("{}", typ)); + string +} + +fn format_function(id: FuncId, interner: &NodeInterner) -> String { + let func_meta = interner.function_meta(&id); + let func_name_definition_id = interner.definition(func_meta.name.id); + + let mut string = String::new(); + // TODO: append the module path + string.push_str(" "); + string.push_str("fn "); + string.push_str(&func_name_definition_id.name); + string.push('('); + let parameters = &func_meta.parameters; + for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { + if matches!(visibility, Visibility::Public) { + string.push_str("pub "); + } + format_pattern(pattern, interner, &mut string); + string.push_str(": "); + string.push_str(&format!("{}", typ)); + if index != parameters.len() - 1 { + string.push_str(", "); + } + } + + string.push(')'); + + let return_type = func_meta.return_type(); + match return_type { + Type::Unit => (), + _ => { + string.push_str(" -> "); + string.push_str(&format!("{}", return_type)); + } + } + + string +} + +fn format_alias(id: TypeAliasId, interner: &NodeInterner) -> String { + let type_alias = interner.get_type_alias(id); + let type_alias = type_alias.borrow(); + + let mut string = String::new(); + // TODO: append the module path + string.push_str(" "); + string.push_str("type "); + string.push_str(&type_alias.name.0.contents); + string.push_str(" = "); + string.push_str(&format!("{}", &type_alias.typ)); + string +} + +fn format_local(id: DefinitionId, interner: &NodeInterner) -> String { + let definition_info = interner.definition(id); + let DefinitionKind::Local(expr_id) = definition_info.kind else { + panic!("Expected a local reference to reference a local definition") + }; + let typ = interner.definition_type(id); + + let mut string = String::new(); + string.push_str(" "); + if definition_info.comptime { + string.push_str("comptime "); + } + if expr_id.is_some() { + string.push_str("let "); + } + if definition_info.mutable { + string.push_str("mut "); + } + string.push_str(&definition_info.name); + if !matches!(typ, Type::Error) { + string.push_str(": "); + string.push_str(&format!("{}", typ)); + } + string +} + +fn format_generics(generics: &Generics, string: &mut String) { + if generics.is_empty() { + return; + } + + string.push('<'); + for (index, generic) in generics.iter().enumerate() { + string.push_str(&generic.name); + if index != generics.len() - 1 { + string.push_str(", "); + } + } + string.push('>'); +} +fn format_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut String) { + match pattern { + HirPattern::Identifier(ident) => { + let definition = interner.definition(ident.id); + string.push_str(&definition.name); + } + HirPattern::Mutable(pattern, _) => { + string.push_str("mut "); + format_pattern(pattern, interner, string); + } + HirPattern::Tuple(..) | HirPattern::Struct(..) => { + string.push('_'); + } + } +} diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 9ece797a57a..6dd7b7383ff 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -35,6 +35,7 @@ use crate::{ mod code_lens_request; mod goto_declaration; mod goto_definition; +mod hover; mod profile_run; mod references; mod rename; @@ -44,9 +45,10 @@ mod tests; pub(crate) use { code_lens_request::collect_lenses_for_package, code_lens_request::on_code_lens_request, goto_declaration::on_goto_declaration_request, goto_definition::on_goto_definition_request, - goto_definition::on_goto_type_definition_request, profile_run::on_profile_run_request, - references::on_references_request, rename::on_prepare_rename_request, - rename::on_rename_request, test_run::on_test_run_request, tests::on_tests_request, + goto_definition::on_goto_type_definition_request, hover::on_hover_request, + profile_run::on_profile_run_request, references::on_references_request, + rename::on_prepare_rename_request, rename::on_rename_request, test_run::on_test_run_request, + tests::on_tests_request, }; /// LSP client will send initialization request after the server has started. @@ -127,6 +129,11 @@ pub(crate) fn on_initialize( work_done_progress: None, }, })), + hover_provider: Some(lsp_types::OneOf::Right(lsp_types::HoverOptions { + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: None, + }, + })), }, server_info: None, }) diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index 57eb2dd3618..991773ce94f 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -1,7 +1,7 @@ use fm::FileId; use lsp_types::{ - DeclarationCapability, DefinitionOptions, OneOf, ReferencesOptions, RenameOptions, - TypeDefinitionProviderCapability, + DeclarationCapability, DefinitionOptions, HoverOptions, OneOf, ReferencesOptions, + RenameOptions, TypeDefinitionProviderCapability, }; use noirc_driver::DebugFile; use noirc_errors::{debug_info::OpCodesCount, Location}; @@ -144,6 +144,10 @@ pub(crate) struct ServerCapabilities { /// The server provides references support. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) references_provider: Option>, + + /// The server provides hover support. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) hover_provider: Option>, } #[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] From 691659bc58dbc9bb8c94bec43c734e68dd002be9 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 10 Jul 2024 17:40:56 -0300 Subject: [PATCH 02/23] Include fn generics --- tooling/lsp/src/requests/hover.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 5ac82417837..01cec25d832 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -136,6 +136,7 @@ fn format_function(id: FuncId, interner: &NodeInterner) -> String { // TODO: append the module path string.push_str(" "); string.push_str("fn "); + format_generics(&func_meta.direct_generics, &mut string); string.push_str(&func_name_definition_id.name); string.push('('); let parameters = &func_meta.parameters; From 15ff93fded79f1a080f5a761dff339a47ab667f8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 10 Jul 2024 17:49:31 -0300 Subject: [PATCH 03/23] Track module name, use it when hovering over mod --- .../src/hir/def_collector/dc_mod.rs | 7 +++++-- compiler/noirc_frontend/src/locations.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 18 ++++++++++++------ tooling/lsp/src/requests/hover.rs | 9 +++++++-- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index bcd24ca8ed3..ef996c9ba01 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -14,7 +14,7 @@ use crate::ast::{ TypeImpl, }; use crate::macros_api::NodeInterner; -use crate::node_interner::ReferenceId; +use crate::node_interner::{ModuleAttributes, ReferenceId}; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, @@ -720,7 +720,10 @@ impl<'a> ModCollector<'a> { return Err(err); } - context.def_interner.add_module_location(mod_id, mod_location); + context.def_interner.add_module_attributes( + mod_id, + ModuleAttributes { name: mod_name.0.contents.clone(), location: mod_location }, + ); } Ok(mod_id) diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 5ca8c1493eb..94f61c294d4 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -35,7 +35,7 @@ impl LocationIndices { impl NodeInterner { pub fn reference_location(&self, reference: ReferenceId) -> Location { match reference { - ReferenceId::Module(id) => self.module_location(&id), + ReferenceId::Module(id) => self.module_attributes(&id).location, ReferenceId::Function(id) => self.function_modifiers(&id).name_location, ReferenceId::Struct(id) => { let struct_type = self.get_struct(id); diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index a009a42df53..dd1494e0e43 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -44,6 +44,12 @@ use crate::{Shared, TypeAlias, TypeBindings, TypeVariable, TypeVariableId, TypeV /// This is needed to stop recursing for cases such as `impl Foo for T where T: Eq` const IMPL_SEARCH_RECURSION_LIMIT: u32 = 10; +#[derive(Debug)] +pub struct ModuleAttributes { + pub name: String, + pub location: Location, +} + type StructAttributes = Vec; /// The node interner is the central storage location of all nodes in Noir's Hir (the @@ -68,7 +74,7 @@ pub struct NodeInterner { function_modules: HashMap, // The location of each module - module_locations: HashMap, + module_attributes: HashMap, /// This graph tracks dependencies between different global definitions. /// This is used to ensure the absence of dependency cycles for globals and types. @@ -553,7 +559,7 @@ impl Default for NodeInterner { function_definition_ids: HashMap::new(), function_modifiers: HashMap::new(), function_modules: HashMap::new(), - module_locations: HashMap::new(), + module_attributes: HashMap::new(), func_id_to_trait: HashMap::new(), dependency_graph: petgraph::graph::DiGraph::new(), dependency_graph_indices: HashMap::new(), @@ -993,12 +999,12 @@ impl NodeInterner { &self.struct_attributes[struct_id] } - pub fn add_module_location(&mut self, module_id: ModuleId, location: Location) { - self.module_locations.insert(module_id, location); + pub fn add_module_attributes(&mut self, module_id: ModuleId, attributes: ModuleAttributes) { + self.module_attributes.insert(module_id, attributes); } - pub fn module_location(&self, module_id: &ModuleId) -> Location { - self.module_locations[module_id] + pub fn module_attributes(&self, module_id: &ModuleId) -> &ModuleAttributes { + &self.module_attributes[module_id] } pub fn global_attributes(&self, global_id: &GlobalId) -> &[SecondaryAttribute] { diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 01cec25d832..a60e7396380 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -60,9 +60,14 @@ fn format_reference(reference: ReferenceId, interner: &NodeInterner) -> String { } } fn format_module(id: ModuleId, interner: &NodeInterner) -> String { + let name = &interner.module_attributes(&id).name; + + let mut string = String::new(); // TODO: append the module path - // TODO: find out the module name - format!(" mod {:?}", id) + string.push_str(" "); + string.push_str("mod "); + string.push_str(name); + string } fn format_struct(id: StructId, interner: &NodeInterner) -> String { From 766968da4e4eb46617c266f1fd3c550bf0e44ccc Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 07:25:14 -0300 Subject: [PATCH 04/23] "pub" correct location --- tooling/lsp/src/requests/hover.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index a60e7396380..c414d56f5d7 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -146,11 +146,11 @@ fn format_function(id: FuncId, interner: &NodeInterner) -> String { string.push('('); let parameters = &func_meta.parameters; for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { + format_pattern(pattern, interner, &mut string); + string.push_str(": "); if matches!(visibility, Visibility::Public) { string.push_str("pub "); } - format_pattern(pattern, interner, &mut string); - string.push_str(": "); string.push_str(&format!("{}", typ)); if index != parameters.len() - 1 { string.push_str(", "); From cc1f1bfaacce2cb628f6321058868cb8cab7c48e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 08:32:14 -0300 Subject: [PATCH 05/23] Show parent module --- compiler/noirc_frontend/src/elaborator/mod.rs | 5 +- .../src/hir/def_collector/dc_mod.rs | 24 ++++-- compiler/noirc_frontend/src/locations.rs | 13 ++- compiler/noirc_frontend/src/node_interner.rs | 14 +++- tooling/lsp/src/requests/hover.rs | 79 ++++++++++++++++--- 5 files changed, 116 insertions(+), 19 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 7fa05c9612e..6aa87b63005 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1230,7 +1230,7 @@ impl<'context> Elaborator<'context> { for field_index in 0..fields_len { self.interner - .add_definition_location(ReferenceId::StructMember(type_id, field_index)); + .add_definition_location(ReferenceId::StructMember(type_id, field_index), None); } self.run_comptime_attributes_on_struct(attributes, type_id, span, &mut generated_items); @@ -1365,7 +1365,8 @@ impl<'context> Elaborator<'context> { self.elaborate_comptime_global(global_id); } - self.interner.add_definition_location(ReferenceId::Global(global_id)); + self.interner + .add_definition_location(ReferenceId::Global(global_id), Some(self.module_id())); self.local_module = old_module; self.file = old_file; diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index ef996c9ba01..4b6739d8c3a 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -90,7 +90,7 @@ pub fn collect_defs( errors.extend(collector.collect_structs(context, ast.types, crate_id)); - errors.extend(collector.collect_type_aliases(context, ast.type_aliases)); + errors.extend(collector.collect_type_aliases(context, ast.type_aliases, crate_id)); errors.extend(collector.collect_functions(context, ast.functions, crate_id)); @@ -318,7 +318,10 @@ impl<'a> ModCollector<'a> { // And store the TypeId -> StructType mapping somewhere it is reachable self.def_collector.items.types.insert(id, unresolved); - context.def_interner.add_definition_location(ReferenceId::Struct(id)); + context.def_interner.add_definition_location( + ReferenceId::Struct(id), + Some(ModuleId { krate: krate, local_id: self.module_id }), + ); } definition_errors } @@ -329,6 +332,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, type_aliases: Vec, + krate: CrateId, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; for type_alias in type_aliases { @@ -365,7 +369,10 @@ impl<'a> ModCollector<'a> { self.def_collector.items.type_aliases.insert(type_alias_id, unresolved); - context.def_interner.add_definition_location(ReferenceId::Alias(type_alias_id)); + context.def_interner.add_definition_location( + ReferenceId::Alias(type_alias_id), + Some(ModuleId { krate: krate, local_id: self.module_id }), + ); } errors } @@ -532,7 +539,10 @@ impl<'a> ModCollector<'a> { }; context.def_interner.push_empty_trait(trait_id, &unresolved, resolved_generics); - context.def_interner.add_definition_location(ReferenceId::Trait(trait_id)); + context.def_interner.add_definition_location( + ReferenceId::Trait(trait_id), + Some(ModuleId { krate: krate, local_id: self.module_id }), + ); self.def_collector.items.traits.insert(trait_id, unresolved); } @@ -722,7 +732,11 @@ impl<'a> ModCollector<'a> { context.def_interner.add_module_attributes( mod_id, - ModuleAttributes { name: mod_name.0.contents.clone(), location: mod_location }, + ModuleAttributes { + name: mod_name.0.contents.clone(), + location: mod_location, + parent: self.module_id, + }, ); } diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 94f61c294d4..a09c1b89a40 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -62,6 +62,10 @@ impl NodeInterner { } } + pub fn reference_module(&self, reference: ReferenceId) -> Option<&ModuleId> { + self.reference_modules.get(&reference) + } + pub(crate) fn add_module_reference(&mut self, id: ModuleId, location: Location) { self.add_reference(ReferenceId::Module(id), location, false); } @@ -129,7 +133,11 @@ impl NodeInterner { self.location_indices.add_location(reference_location, reference_index); } - pub(crate) fn add_definition_location(&mut self, referenced: ReferenceId) { + pub(crate) fn add_definition_location( + &mut self, + referenced: ReferenceId, + module_id: Option, + ) { if !self.track_references { return; } @@ -137,6 +145,9 @@ impl NodeInterner { let referenced_index = self.get_or_insert_reference(referenced); let referenced_location = self.reference_location(referenced); self.location_indices.add_location(referenced_location, referenced_index); + if let Some(module_id) = module_id { + self.reference_modules.insert(referenced, module_id); + } } #[tracing::instrument(skip(self), ret)] diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index dd1494e0e43..e534f1bab29 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -48,6 +48,7 @@ const IMPL_SEARCH_RECURSION_LIMIT: u32 = 10; pub struct ModuleAttributes { pub name: String, pub location: Location, + pub parent: LocalModuleId, } type StructAttributes = Vec; @@ -225,6 +226,10 @@ pub struct NodeInterner { /// Store the location of the references in the graph pub(crate) location_indices: LocationIndices, + + // The module where each reference is + // (ReferenceId::Reference and ReferenceId::Local aren't included here) + pub(crate) reference_modules: HashMap, } /// A dependency in the dependency graph may be a type or a definition. @@ -592,6 +597,7 @@ impl Default for NodeInterner { location_indices: LocationIndices::default(), reference_graph: petgraph::graph::DiGraph::new(), reference_graph_indices: HashMap::new(), + reference_modules: HashMap::new(), }; // An empty block expression is used often, we add this into the `node` on startup @@ -860,7 +866,7 @@ impl NodeInterner { self.definitions.push(DefinitionInfo { name, mutable, comptime, kind, location }); if is_local { - self.add_definition_location(ReferenceId::Local(id)); + self.add_definition_location(ReferenceId::Local(id), None); } id @@ -898,7 +904,7 @@ impl NodeInterner { // This needs to be done after pushing the definition since it will reference the // location that was stored - self.add_definition_location(ReferenceId::Function(id)); + self.add_definition_location(ReferenceId::Function(id), Some(module)); definition_id } @@ -1007,6 +1013,10 @@ impl NodeInterner { &self.module_attributes[module_id] } + pub fn try_module_attributes(&self, module_id: &ModuleId) -> Option<&ModuleAttributes> { + self.module_attributes.get(module_id) + } + pub fn global_attributes(&self, global_id: &GlobalId) -> &[SecondaryAttribute] { &self.global_attributes[global_id] } diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index c414d56f5d7..224b5cdc989 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -60,13 +60,19 @@ fn format_reference(reference: ReferenceId, interner: &NodeInterner) -> String { } } fn format_module(id: ModuleId, interner: &NodeInterner) -> String { - let name = &interner.module_attributes(&id).name; + let module_attributes = interner.module_attributes(&id); let mut string = String::new(); - // TODO: append the module path + if format_parent_module_from_module_id( + &ModuleId { krate: id.krate, local_id: module_attributes.parent }, + interner, + &mut string, + ) { + string.push_str("\n"); + } string.push_str(" "); string.push_str("mod "); - string.push_str(name); + string.push_str(&module_attributes.name); string } @@ -75,7 +81,9 @@ fn format_struct(id: StructId, interner: &NodeInterner) -> String { let struct_type = struct_type.borrow(); let mut string = String::new(); - // TODO: append the module path + if format_parent_module(ReferenceId::Struct(id), interner, &mut string) { + string.push_str("\n"); + } string.push_str(" "); string.push_str("struct "); string.push_str(&struct_type.name.0.contents); @@ -98,7 +106,11 @@ fn format_struct_member(id: StructId, field_index: usize, interner: &NodeInterne let (field_name, field_type) = struct_type.field_at(field_index); let mut string = String::new(); - // TODO: append the module path + if format_parent_module(ReferenceId::Struct(id), interner, &mut string) { + string.push_str("::"); + } + string.push_str(&struct_type.name.0.contents); + string.push_str("\n"); string.push_str(" "); string.push_str(&field_name.0.contents); string.push_str(": "); @@ -110,7 +122,9 @@ fn format_trait(id: TraitId, interner: &NodeInterner) -> String { let a_trait = interner.get_trait(id); let mut string = String::new(); - // TODO: append the module path + if format_parent_module(ReferenceId::Trait(id), interner, &mut string) { + string.push_str("\n"); + } string.push_str(" "); string.push_str("trait "); string.push_str(&a_trait.name.0.contents); @@ -124,7 +138,9 @@ fn format_global(id: GlobalId, interner: &NodeInterner) -> String { let typ = interner.definition_type(definition_id); let mut string = String::new(); - // TODO: append the module path + if format_parent_module(ReferenceId::Global(id), interner, &mut string) { + string.push_str("\n"); + } string.push_str(" "); string.push_str("global "); string.push_str(&global_info.ident.0.contents); @@ -138,7 +154,8 @@ fn format_function(id: FuncId, interner: &NodeInterner) -> String { let func_name_definition_id = interner.definition(func_meta.name.id); let mut string = String::new(); - // TODO: append the module path + format_parent_module(ReferenceId::Function(id), interner, &mut string); + string.push_str("\n"); string.push_str(" "); string.push_str("fn "); format_generics(&func_meta.direct_generics, &mut string); @@ -176,7 +193,8 @@ fn format_alias(id: TypeAliasId, interner: &NodeInterner) -> String { let type_alias = type_alias.borrow(); let mut string = String::new(); - // TODO: append the module path + format_parent_module(ReferenceId::Alias(id), interner, &mut string); + string.push_str("\n"); string.push_str(" "); string.push_str("type "); string.push_str(&type_alias.name.0.contents); @@ -240,3 +258,46 @@ fn format_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut St } } } + +fn format_parent_module( + referenced: ReferenceId, + interner: &NodeInterner, + string: &mut String, +) -> bool { + let Some(module) = interner.reference_module(referenced) else { + return false; + }; + + return format_parent_module_from_module_id(&module, interner, string); +} + +fn format_parent_module_from_module_id( + module: &ModuleId, + interner: &NodeInterner, + string: &mut String, +) -> bool { + let Some(module_attributes) = interner.try_module_attributes(module) else { + return false; + }; + + string.push_str(" "); + + let mut segments = Vec::new(); + let mut current_attributes = module_attributes; + while let Some(parent_attributes) = interner.try_module_attributes(&ModuleId { + krate: module.krate, + local_id: current_attributes.parent, + }) { + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } + + for segment in segments.iter().rev() { + string.push_str(segment); + string.push_str("::"); + } + + string.push_str(&module_attributes.name); + + true +} From 06420d04e54ec482830d11382b3009e67059bca9 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 08:42:51 -0300 Subject: [PATCH 06/23] Track method call references --- compiler/noirc_frontend/src/elaborator/expressions.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index e8098723692..d0d6a1cf7ce 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -320,6 +320,7 @@ impl<'context> Elaborator<'context> { let (mut object, mut object_type) = self.elaborate_expression(method_call.object); object_type = object_type.follow_bindings(); + let method_name_span = method_call.method_name.span(); let method_name = method_call.method_name.0.contents.as_str(); match self.lookup_method(&object_type, method_name, span) { Some(method_ref) => { @@ -385,6 +386,9 @@ impl<'context> Elaborator<'context> { self.interner.push_expr_type(function_id, func_type.clone()); + self.interner + .add_function_reference(func_id, Location::new(method_name_span, self.file)); + // Type check the new call now that it has been changed from a method call // to a function call. This way we avoid duplicating code. let typ = self.type_check_call(&function_call, func_type, function_args, span); From 0e2f57f2fc583f2e1b376717c219f3f6501cd56f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 10:31:59 -0300 Subject: [PATCH 07/23] Show the struct where a function is defined --- compiler/noirc_frontend/src/elaborator/mod.rs | 7 +++++++ .../src/hir/resolution/resolver.rs | 1 + .../noirc_frontend/src/hir/type_check/mod.rs | 1 + .../noirc_frontend/src/hir_def/function.rs | 5 ++++- tooling/lsp/src/requests/hover.rs | 18 ++++++++++++++++-- 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 6aa87b63005..376db5ad805 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -718,6 +718,12 @@ impl<'context> Elaborator<'context> { let statements = std::mem::take(&mut func.def.body.statements); let body = BlockExpression { statements }; + let struct_id = if let Some(Type::Struct(struct_type, _)) = &self.self_type { + Some(struct_type.borrow().id) + } else { + None + }; + let meta = FuncMeta { name: name_ident, kind: func.kind, @@ -725,6 +731,7 @@ impl<'context> Elaborator<'context> { typ, direct_generics, all_generics: self.generics.clone(), + struct_id: struct_id, trait_impl: self.current_trait_impl, parameters: parameters.into(), parameter_idents, diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 856a769c9dd..55cb2145ba8 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1105,6 +1105,7 @@ impl<'a> Resolver<'a> { location, typ, direct_generics, + struct_id: None, trait_impl: self.current_trait_impl, parameters: parameters.into(), return_type: func.def.return_type.clone(), diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 1a70bade863..60618e9fd0f 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -570,6 +570,7 @@ pub mod test { .into(), return_visibility: Visibility::Private, has_body: true, + struct_id: None, trait_impl: None, return_type: FunctionReturnType::Default(Span::default()), trait_constraints: Vec::new(), diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index fa8bb55abee..e666f996231 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -6,7 +6,7 @@ use super::stmt::HirPattern; use super::traits::TraitConstraint; use crate::ast::{FunctionKind, FunctionReturnType, Visibility}; use crate::graph::CrateId; -use crate::macros_api::BlockExpression; +use crate::macros_api::{BlockExpression, StructId}; use crate::node_interner::{ExprId, NodeInterner, TraitImplId}; use crate::{ResolvedGeneric, Type}; @@ -126,6 +126,9 @@ pub struct FuncMeta { pub trait_constraints: Vec, + /// The struct this function belongs to, if any + pub struct_id: Option, + /// The trait impl this function belongs to, if any pub trait_impl: Option, diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 224b5cdc989..551417b7993 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -154,8 +154,22 @@ fn format_function(id: FuncId, interner: &NodeInterner) -> String { let func_name_definition_id = interner.definition(func_meta.name.id); let mut string = String::new(); - format_parent_module(ReferenceId::Function(id), interner, &mut string); - string.push_str("\n"); + let formatted_parent_module = + format_parent_module(ReferenceId::Function(id), interner, &mut string); + let formatted_parent_struct = if let Some(struct_id) = func_meta.struct_id { + let struct_type = interner.get_struct(struct_id); + let struct_type = struct_type.borrow(); + if formatted_parent_module { + string.push_str("::"); + } + string.push_str(&struct_type.name.0.contents); + true + } else { + false + }; + if formatted_parent_module || formatted_parent_struct { + string.push_str("\n"); + } string.push_str(" "); string.push_str("fn "); format_generics(&func_meta.direct_generics, &mut string); From 4a4de8a42848b90b21b97883334764b69d958df7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 10:48:59 -0300 Subject: [PATCH 08/23] Extact ProcessRequestCallbackArgs --- tooling/lsp/src/requests/goto_declaration.rs | 6 +- tooling/lsp/src/requests/goto_definition.rs | 13 ++-- tooling/lsp/src/requests/hover.rs | 30 ++++----- tooling/lsp/src/requests/mod.rs | 16 ++++- tooling/lsp/src/requests/references.rs | 24 +++---- tooling/lsp/src/requests/rename.rs | 66 +++++++++----------- 6 files changed, 78 insertions(+), 77 deletions(-) diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 1f87f2c87d4..bd0f0afb827 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -20,10 +20,10 @@ fn on_goto_definition_inner( state: &mut LspState, params: GotoDeclarationParams, ) -> Result { - process_request(state, params.text_document_position_params, |location, interner, files, _| { - interner.get_declaration_location_from(location).and_then(|found_location| { + process_request(state, params.text_document_position_params, |args| { + args.interner.get_declaration_location_from(args.location).and_then(|found_location| { let file_id = found_location.file; - let definition_position = to_lsp_location(files, file_id, found_location.span)?; + let definition_position = to_lsp_location(args.files, file_id, found_location.span)?; let response = GotoDeclarationResponse::from(definition_position).to_owned(); Some(response) }) diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 27d3503a2fd..72fac676a72 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -29,15 +29,16 @@ fn on_goto_definition_inner( params: GotoDefinitionParams, return_type_location_instead: bool, ) -> Result { - process_request(state, params.text_document_position_params, |location, interner, files, _| { - interner.get_definition_location_from(location, return_type_location_instead).and_then( - |found_location| { + process_request(state, params.text_document_position_params, |args| { + args.interner + .get_definition_location_from(args.location, return_type_location_instead) + .and_then(|found_location| { let file_id = found_location.file; - let definition_position = to_lsp_location(files, file_id, found_location.span)?; + let definition_position = + to_lsp_location(args.files, file_id, found_location.span)?; let response = GotoDefinitionResponse::from(definition_position).to_owned(); Some(response) - }, - ) + }) }) } diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 551417b7993..583dc427433 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -21,23 +21,19 @@ pub(crate) fn on_hover_request( state: &mut LspState, params: HoverParams, ) -> impl Future, ResponseError>> { - let result = process_request( - state, - params.text_document_position_params, - |location, interner, files, _| { - interner.reference_at_location(location).map(|reference| { - let location = interner.reference_location(reference); - let lsp_location = to_lsp_location(files, location.file, location.span); - Hover { - range: lsp_location.map(|location| location.range), - contents: HoverContents::Markup(MarkupContent { - kind: MarkupKind::Markdown, - value: format_reference(reference, interner), - }), - } - }) - }, - ); + let result = process_request(state, params.text_document_position_params, |args| { + args.interner.reference_at_location(args.location).map(|reference| { + let location = args.interner.reference_location(reference); + let lsp_location = to_lsp_location(args.files, location.file, location.span); + Hover { + range: lsp_location.map(|location| location.range), + contents: HoverContents::Markup(MarkupContent { + kind: MarkupKind::Markdown, + value: format_reference(reference, args.interner), + }), + } + }) + }); future::ready(result) } diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 6dd7b7383ff..d9242eef825 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -271,13 +271,20 @@ pub(crate) fn on_shutdown( async { Ok(()) } } +pub(crate) struct ProcessRequestCallbackArgs<'a> { + location: noirc_errors::Location, + interner: &'a NodeInterner, + files: &'a FileMap, + interners: &'a HashMap, +} + pub(crate) fn process_request( state: &mut LspState, text_document_position_params: TextDocumentPositionParams, callback: F, ) -> Result where - F: FnOnce(noirc_errors::Location, &NodeInterner, &FileMap, &HashMap) -> T, + F: FnOnce(ProcessRequestCallbackArgs) -> T, { let file_path = text_document_position_params.text_document.uri.to_file_path().map_err(|_| { @@ -316,7 +323,12 @@ where &text_document_position_params.position, )?; - Ok(callback(location, interner, files, &state.cached_definitions)) + Ok(callback(ProcessRequestCallbackArgs { + location, + interner, + files, + interners: &state.cached_definitions, + })) } pub(crate) fn find_all_references_in_workspace( location: noirc_errors::Location, diff --git a/tooling/lsp/src/requests/references.rs b/tooling/lsp/src/requests/references.rs index 6c872656c28..badea8921b2 100644 --- a/tooling/lsp/src/requests/references.rs +++ b/tooling/lsp/src/requests/references.rs @@ -12,20 +12,16 @@ pub(crate) fn on_references_request( params: ReferenceParams, ) -> impl Future>, ResponseError>> { let include_declaration = params.context.include_declaration; - let result = process_request( - state, - params.text_document_position, - |location, interner, files, cached_interners| { - find_all_references_in_workspace( - location, - interner, - cached_interners, - files, - include_declaration, - true, - ) - }, - ); + let result = process_request(state, params.text_document_position, |args| { + find_all_references_in_workspace( + args.location, + args.interner, + args.interners, + args.files, + include_declaration, + true, + ) + }); future::ready(result) } diff --git a/tooling/lsp/src/requests/rename.rs b/tooling/lsp/src/requests/rename.rs index 245c2ed0882..fd68d4312c5 100644 --- a/tooling/lsp/src/requests/rename.rs +++ b/tooling/lsp/src/requests/rename.rs @@ -17,8 +17,8 @@ pub(crate) fn on_prepare_rename_request( state: &mut LspState, params: TextDocumentPositionParams, ) -> impl Future, ResponseError>> { - let result = process_request(state, params, |location, interner, _, _| { - let reference_id = interner.reference_at_location(location); + let result = process_request(state, params, |args| { + let reference_id = args.interner.reference_at_location(args.location); let rename_possible = match reference_id { // Rename shouldn't be possible when triggered on top of "Self" Some(ReferenceId::Reference(_, true /* is self type name */)) => false, @@ -34,40 +34,36 @@ pub(crate) fn on_rename_request( state: &mut LspState, params: RenameParams, ) -> impl Future, ResponseError>> { - let result = process_request( - state, - params.text_document_position, - |location, interner, files, cached_interners| { - let rename_changes = find_all_references_in_workspace( - location, - interner, - cached_interners, - files, - true, - false, - ) - .map(|locations| { - let rs = locations.iter().fold( - HashMap::new(), - |mut acc: HashMap>, location| { - let edit = - TextEdit { range: location.range, new_text: params.new_name.clone() }; - acc.entry(location.uri.clone()).or_default().push(edit); - acc - }, - ); - rs - }); - - let response = WorkspaceEdit { - changes: rename_changes, - document_changes: None, - change_annotations: None, - }; + let result = process_request(state, params.text_document_position, |args| { + let rename_changes = find_all_references_in_workspace( + args.location, + args.interner, + args.interners, + args.files, + true, + false, + ) + .map(|locations| { + let rs = locations.iter().fold( + HashMap::new(), + |mut acc: HashMap>, location| { + let edit = + TextEdit { range: location.range, new_text: params.new_name.clone() }; + acc.entry(location.uri.clone()).or_default().push(edit); + acc + }, + ); + rs + }); + + let response = WorkspaceEdit { + changes: rename_changes, + document_changes: None, + change_annotations: None, + }; - Some(response) - }, - ); + Some(response) + }); future::ready(result) } From 8044c5cf883e4b261f96b2e0fac8e0b79e0bd914 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 11:10:26 -0300 Subject: [PATCH 09/23] Include crate name --- tooling/lsp/src/requests/hover.rs | 125 ++++++++++++++++++------------ tooling/lsp/src/requests/mod.rs | 10 ++- 2 files changed, 82 insertions(+), 53 deletions(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 583dc427433..4b8b8becbbc 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -4,6 +4,7 @@ use async_lsp::ResponseError; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; use noirc_frontend::{ ast::Visibility, + graph::CrateId, hir::def_map::ModuleId, hir_def::stmt::HirPattern, macros_api::{NodeInterner, StructId}, @@ -15,7 +16,7 @@ use noirc_frontend::{ use crate::LspState; -use super::{process_request, to_lsp_location}; +use super::{process_request, to_lsp_location, ProcessRequestCallbackArgs}; pub(crate) fn on_hover_request( state: &mut LspState, @@ -29,7 +30,7 @@ pub(crate) fn on_hover_request( range: lsp_location.map(|location| location.range), contents: HoverContents::Markup(MarkupContent { kind: MarkupKind::Markdown, - value: format_reference(reference, args.interner), + value: format_reference(reference, &args), }), } }) @@ -38,30 +39,28 @@ pub(crate) fn on_hover_request( future::ready(result) } -fn format_reference(reference: ReferenceId, interner: &NodeInterner) -> String { +fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) -> String { match reference { - ReferenceId::Module(id) => format_module(id, interner), - ReferenceId::Struct(id) => format_struct(id, interner), - ReferenceId::StructMember(id, field_index) => { - format_struct_member(id, field_index, interner) - } - ReferenceId::Trait(id) => format_trait(id, interner), - ReferenceId::Global(id) => format_global(id, interner), - ReferenceId::Function(id) => format_function(id, interner), - ReferenceId::Alias(id) => format_alias(id, interner), - ReferenceId::Local(id) => format_local(id, &interner), + ReferenceId::Module(id) => format_module(id, args), + ReferenceId::Struct(id) => format_struct(id, args), + ReferenceId::StructMember(id, field_index) => format_struct_member(id, field_index, args), + ReferenceId::Trait(id) => format_trait(id, args), + ReferenceId::Global(id) => format_global(id, args), + ReferenceId::Function(id) => format_function(id, args), + ReferenceId::Alias(id) => format_alias(id, args), + ReferenceId::Local(id) => format_local(id, &args), ReferenceId::Reference(location, _) => { - format_reference(interner.find_referenced(location).unwrap(), interner) + format_reference(args.interner.find_referenced(location).unwrap(), args) } } } -fn format_module(id: ModuleId, interner: &NodeInterner) -> String { - let module_attributes = interner.module_attributes(&id); +fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> String { + let module_attributes = args.interner.module_attributes(&id); let mut string = String::new(); if format_parent_module_from_module_id( &ModuleId { krate: id.krate, local_id: module_attributes.parent }, - interner, + args, &mut string, ) { string.push_str("\n"); @@ -72,12 +71,12 @@ fn format_module(id: ModuleId, interner: &NodeInterner) -> String { string } -fn format_struct(id: StructId, interner: &NodeInterner) -> String { - let struct_type = interner.get_struct(id); +fn format_struct(id: StructId, args: &ProcessRequestCallbackArgs) -> String { + let struct_type = args.interner.get_struct(id); let struct_type = struct_type.borrow(); let mut string = String::new(); - if format_parent_module(ReferenceId::Struct(id), interner, &mut string) { + if format_parent_module(ReferenceId::Struct(id), args, &mut string) { string.push_str("\n"); } string.push_str(" "); @@ -96,13 +95,17 @@ fn format_struct(id: StructId, interner: &NodeInterner) -> String { string } -fn format_struct_member(id: StructId, field_index: usize, interner: &NodeInterner) -> String { - let struct_type = interner.get_struct(id); +fn format_struct_member( + id: StructId, + field_index: usize, + args: &ProcessRequestCallbackArgs, +) -> String { + let struct_type = args.interner.get_struct(id); let struct_type = struct_type.borrow(); let (field_name, field_type) = struct_type.field_at(field_index); let mut string = String::new(); - if format_parent_module(ReferenceId::Struct(id), interner, &mut string) { + if format_parent_module(ReferenceId::Struct(id), args, &mut string) { string.push_str("::"); } string.push_str(&struct_type.name.0.contents); @@ -114,11 +117,11 @@ fn format_struct_member(id: StructId, field_index: usize, interner: &NodeInterne string } -fn format_trait(id: TraitId, interner: &NodeInterner) -> String { - let a_trait = interner.get_trait(id); +fn format_trait(id: TraitId, args: &ProcessRequestCallbackArgs) -> String { + let a_trait = args.interner.get_trait(id); let mut string = String::new(); - if format_parent_module(ReferenceId::Trait(id), interner, &mut string) { + if format_parent_module(ReferenceId::Trait(id), args, &mut string) { string.push_str("\n"); } string.push_str(" "); @@ -128,13 +131,13 @@ fn format_trait(id: TraitId, interner: &NodeInterner) -> String { string } -fn format_global(id: GlobalId, interner: &NodeInterner) -> String { - let global_info = interner.get_global(id); +fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { + let global_info = args.interner.get_global(id); let definition_id = global_info.definition_id; - let typ = interner.definition_type(definition_id); + let typ = args.interner.definition_type(definition_id); let mut string = String::new(); - if format_parent_module(ReferenceId::Global(id), interner, &mut string) { + if format_parent_module(ReferenceId::Global(id), args, &mut string) { string.push_str("\n"); } string.push_str(" "); @@ -145,15 +148,15 @@ fn format_global(id: GlobalId, interner: &NodeInterner) -> String { string } -fn format_function(id: FuncId, interner: &NodeInterner) -> String { - let func_meta = interner.function_meta(&id); - let func_name_definition_id = interner.definition(func_meta.name.id); +fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { + let func_meta = args.interner.function_meta(&id); + let func_name_definition_id = args.interner.definition(func_meta.name.id); let mut string = String::new(); let formatted_parent_module = - format_parent_module(ReferenceId::Function(id), interner, &mut string); + format_parent_module(ReferenceId::Function(id), args, &mut string); let formatted_parent_struct = if let Some(struct_id) = func_meta.struct_id { - let struct_type = interner.get_struct(struct_id); + let struct_type = args.interner.get_struct(struct_id); let struct_type = struct_type.borrow(); if formatted_parent_module { string.push_str("::"); @@ -173,7 +176,7 @@ fn format_function(id: FuncId, interner: &NodeInterner) -> String { string.push('('); let parameters = &func_meta.parameters; for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { - format_pattern(pattern, interner, &mut string); + format_pattern(pattern, args.interner, &mut string); string.push_str(": "); if matches!(visibility, Visibility::Public) { string.push_str("pub "); @@ -198,12 +201,12 @@ fn format_function(id: FuncId, interner: &NodeInterner) -> String { string } -fn format_alias(id: TypeAliasId, interner: &NodeInterner) -> String { - let type_alias = interner.get_type_alias(id); +fn format_alias(id: TypeAliasId, args: &ProcessRequestCallbackArgs) -> String { + let type_alias = args.interner.get_type_alias(id); let type_alias = type_alias.borrow(); let mut string = String::new(); - format_parent_module(ReferenceId::Alias(id), interner, &mut string); + format_parent_module(ReferenceId::Alias(id), args, &mut string); string.push_str("\n"); string.push_str(" "); string.push_str("type "); @@ -213,12 +216,12 @@ fn format_alias(id: TypeAliasId, interner: &NodeInterner) -> String { string } -fn format_local(id: DefinitionId, interner: &NodeInterner) -> String { - let definition_info = interner.definition(id); +fn format_local(id: DefinitionId, args: &ProcessRequestCallbackArgs) -> String { + let definition_info = args.interner.definition(id); let DefinitionKind::Local(expr_id) = definition_info.kind else { panic!("Expected a local reference to reference a local definition") }; - let typ = interner.definition_type(id); + let typ = args.interner.definition_type(id); let mut string = String::new(); string.push_str(" "); @@ -271,30 +274,52 @@ fn format_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut St fn format_parent_module( referenced: ReferenceId, - interner: &NodeInterner, + args: &ProcessRequestCallbackArgs, string: &mut String, ) -> bool { - let Some(module) = interner.reference_module(referenced) else { + let Some(module) = args.interner.reference_module(referenced) else { return false; }; - return format_parent_module_from_module_id(&module, interner, string); + return format_parent_module_from_module_id(&module, args, string); } fn format_parent_module_from_module_id( module: &ModuleId, - interner: &NodeInterner, + args: &ProcessRequestCallbackArgs, string: &mut String, ) -> bool { - let Some(module_attributes) = interner.try_module_attributes(module) else { - return false; + let crate_id = module.krate; + let crate_name = match crate_id { + CrateId::Root(_) => Some(args.root_crate_name.clone()), + CrateId::Crate(_) => args + .root_crate_dependnencies + .iter() + .find(|dep| dep.crate_id == crate_id) + .map(|dep| format!("{}", dep.name)), + CrateId::Stdlib(_) => Some("std".to_string()), + CrateId::Dummy => None, }; - string.push_str(" "); + let wrote_crate = if let Some(crate_name) = crate_name { + string.push_str(" "); + string.push_str(&crate_name); + true + } else { + false + }; + + let Some(module_attributes) = args.interner.try_module_attributes(module) else { + return wrote_crate; + }; + + if wrote_crate { + string.push_str("::"); + } let mut segments = Vec::new(); let mut current_attributes = module_attributes; - while let Some(parent_attributes) = interner.try_module_attributes(&ModuleId { + while let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId { krate: module.krate, local_id: current_attributes.parent, }) { diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index d9242eef825..489e9237fa4 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -14,7 +14,7 @@ use lsp_types::{ use nargo::insert_all_files_for_workspace_into_file_manager; use nargo_fmt::Config; use noirc_driver::file_manager_with_stdlib; -use noirc_frontend::macros_api::NodeInterner; +use noirc_frontend::{graph::Dependency, macros_api::NodeInterner}; use serde::{Deserialize, Serialize}; use crate::{ @@ -273,9 +273,11 @@ pub(crate) fn on_shutdown( pub(crate) struct ProcessRequestCallbackArgs<'a> { location: noirc_errors::Location, - interner: &'a NodeInterner, files: &'a FileMap, + interner: &'a NodeInterner, interners: &'a HashMap, + root_crate_name: String, + root_crate_dependnencies: &'a Vec, } pub(crate) fn process_request( @@ -325,9 +327,11 @@ where Ok(callback(ProcessRequestCallbackArgs { location, - interner, files, + interner, interners: &state.cached_definitions, + root_crate_name: format!("{}", package.name), + root_crate_dependnencies: &context.crate_graph[context.root_crate_id()].dependencies, })) } pub(crate) fn find_all_references_in_workspace( From 5e9a1023cfa2a6416753cf072744f927bb85c127 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 11:48:10 -0300 Subject: [PATCH 10/23] Add a test for renaming a method --- tooling/lsp/src/requests/rename.rs | 5 +++++ .../lsp/test_programs/rename_function/src/main.nr | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/tooling/lsp/src/requests/rename.rs b/tooling/lsp/src/requests/rename.rs index fd68d4312c5..84956681167 100644 --- a/tooling/lsp/src/requests/rename.rs +++ b/tooling/lsp/src/requests/rename.rs @@ -170,6 +170,11 @@ mod rename_tests { check_rename_succeeds("rename_function_use", "some_function").await; } + #[test] + async fn test_rename_method() { + check_rename_succeeds("rename_function", "some_method").await; + } + #[test] async fn test_rename_struct() { check_rename_succeeds("rename_struct", "Foo").await; diff --git a/tooling/lsp/test_programs/rename_function/src/main.nr b/tooling/lsp/test_programs/rename_function/src/main.nr index 7a70084276e..e77b50c0b26 100644 --- a/tooling/lsp/test_programs/rename_function/src/main.nr +++ b/tooling/lsp/test_programs/rename_function/src/main.nr @@ -25,3 +25,16 @@ use foo::some_other_function as bar; fn x() { bar(); } + +struct SomeStruct { + +} + +impl SomeStruct { + fn some_method(self) {} +} + +fn y() { + let some_struct = SomeStruct {}; + some_struct.some_method(); +} From e7b282e81ee283f511eef231d22bbc8e4296be01 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 12:30:21 -0300 Subject: [PATCH 11/23] Add hover tests --- tooling/lsp/src/requests/hover.rs | 211 +++++++++++++++++- .../test_programs/workspace/one/src/lib.nr | 20 ++ .../test_programs/workspace/two/src/lib.nr | 30 +++ 3 files changed, 257 insertions(+), 4 deletions(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 4b8b8becbbc..e9d577ee3a3 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -177,11 +177,13 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { let parameters = &func_meta.parameters; for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { format_pattern(pattern, args.interner, &mut string); - string.push_str(": "); - if matches!(visibility, Visibility::Public) { - string.push_str("pub "); + if !pattern_is_self(pattern, args.interner) { + string.push_str(": "); + if matches!(visibility, Visibility::Public) { + string.push_str("pub "); + } + string.push_str(&format!("{}", typ)); } - string.push_str(&format!("{}", typ)); if index != parameters.len() - 1 { string.push_str(", "); } @@ -232,6 +234,9 @@ fn format_local(id: DefinitionId, args: &ProcessRequestCallbackArgs) -> String { string.push_str("let "); } if definition_info.mutable { + if expr_id.is_none() { + string.push_str("let "); + } string.push_str("mut "); } string.push_str(&definition_info.name); @@ -272,6 +277,17 @@ fn format_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut St } } +fn pattern_is_self(pattern: &HirPattern, interner: &NodeInterner) -> bool { + match pattern { + HirPattern::Identifier(ident) => { + let definition = interner.definition(ident.id); + definition.name == "self" + } + HirPattern::Mutable(pattern, _) => pattern_is_self(pattern, interner), + HirPattern::Tuple(..) | HirPattern::Struct(..) => false, + } +} + fn format_parent_module( referenced: ReferenceId, args: &ProcessRequestCallbackArgs, @@ -315,6 +331,8 @@ fn format_parent_module_from_module_id( if wrote_crate { string.push_str("::"); + } else { + string.push_str(" "); } let mut segments = Vec::new(); @@ -336,3 +354,188 @@ fn format_parent_module_from_module_id( true } + +#[cfg(test)] +mod hover_tests { + use crate::test_utils; + + use super::*; + use lsp_types::{ + Position, TextDocumentIdentifier, TextDocumentPositionParams, Url, WorkDoneProgressParams, + }; + use tokio::test; + + async fn assert_hover(directory: &str, file: &str, position: Position, expected_text: &str) { + let (mut state, noir_text_document) = test_utils::init_lsp_server(directory).await; + + // noir_text_document is always `src/main.nr` in the workspace directory, so let's go to the workspace dir + let noir_text_document = noir_text_document.to_file_path().unwrap(); + let workspace_dir = noir_text_document.parent().unwrap().parent().unwrap(); + + let file_uri = Url::from_file_path(workspace_dir.join(file)).unwrap(); + + let hover = on_hover_request( + &mut state, + HoverParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: file_uri }, + position: position, + }, + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + }, + ) + .await + .expect("Could not execute hover") + .unwrap(); + + let HoverContents::Markup(markup) = hover.contents else { + panic!("Expected hover contents to be Markup"); + }; + + assert_eq!(markup.value, expected_text); + } + + #[test] + async fn hover_on_module() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 6, character: 9 }, + r#" one + mod subone"#, + ) + .await; + } + + #[test] + async fn hover_on_struct() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 9, character: 20 }, + r#" one::subone + struct SubOneStruct { + some_field: i32, + some_other_field: Field, + }"#, + ) + .await; + } + + #[test] + async fn hover_on_struct_member() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 9, character: 35 }, + r#" one::subone::SubOneStruct + some_field: i32"#, + ) + .await; + } + + #[test] + async fn hover_on_trait() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 12, character: 17 }, + r#" one::subone + trait SomeTrait"#, + ) + .await; + } + + #[test] + async fn hover_on_global() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 15, character: 25 }, + r#" one::subone + global some_global: Field"#, + ) + .await; + } + + #[test] + async fn hover_on_function() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 3, character: 4 }, + r#" one + fn function_one()"#, + ) + .await; + } + + #[test] + async fn hover_on_local_function() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 2, character: 7 }, + r#" two + fn function_two()"#, + ) + .await; + } + + #[test] + async fn hover_on_struct_method() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 20, character: 6 }, + r#" one::subone::SubOneStruct + fn foo(self, x: i32, y: i32) -> Field"#, + ) + .await; + } + + #[test] + async fn hover_on_local_var() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 25, character: 12 }, + " let regular_var: Field", + ) + .await; + } + + #[test] + async fn hover_on_local_mut_var() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 27, character: 4 }, + " let mut mutable_var: Field", + ) + .await; + } + + #[test] + async fn hover_on_parameter() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 31, character: 12 }, + " some_param: i32", + ) + .await; + } + + #[test] + async fn hover_on_alias() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 34, character: 17 }, + r#" one::subone + type SomeAlias = i32"#, + ) + .await; + } +} diff --git a/tooling/lsp/test_programs/workspace/one/src/lib.nr b/tooling/lsp/test_programs/workspace/one/src/lib.nr index d4f660e35fb..6d580f61459 100644 --- a/tooling/lsp/test_programs/workspace/one/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/one/src/lib.nr @@ -1 +1,21 @@ pub fn function_one() {} + +mod subone { + struct SubOneStruct { + some_field: i32, + some_other_field: Field, + } + + impl SubOneStruct { + fn foo(self, x: i32, y: i32) -> Field { + 0 + } + } + + trait SomeTrait { + } + + global some_global = 2; + + type SomeAlias = i32; +} diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index adf7079b4c9..d73932f7846 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -3,3 +3,33 @@ use one::function_one; pub fn function_two() { function_one() } + +use one::subone; + +fn use_struct() { + let _ = subone::SubOneStruct { some_field: 0, some_other_field: 2 }; +} + +use one::subone::SomeTrait; + +fn use_global() { + let _ = one::subone::some_global; +} + +fn use_struct_method() { + let s = subone::SubOneStruct { some_field: 0, some_other_field: 2 }; + s.foo(0, 1); +} + +fn use_local_var() { + let regular_var = 0; + let _ = regular_var; + let mut mutable_var = 0; + mutable_var = 1; +} + +fn use_parameter(some_param: i32) { + let _ = some_param; +} + +use one::subone::SomeAlias; From dbb77338840e8a9e72bafce2ee9abfb84da056ca Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 14:54:22 -0300 Subject: [PATCH 12/23] Let hover work on traits that are in a method call --- .../noirc_frontend/src/elaborator/scope.rs | 10 +++++++ .../src/hir/def_collector/dc_crate.rs | 22 +------------- compiler/noirc_frontend/src/locations.rs | 30 ++++++++++++++++++- tooling/lsp/src/requests/hover.rs | 12 ++++++++ .../test_programs/workspace/two/src/lib.nr | 6 ++++ 5 files changed, 58 insertions(+), 22 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index af6fc0e5d5e..6d756d3efa6 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -48,6 +48,10 @@ impl<'context> Elaborator<'context> { let path_resolution; if self.interner.track_references { + let last_segment = path.last_segment(); + let location = Location::new(last_segment.span(), self.file); + let is_self_type_name = last_segment.is_self_type_name(); + let mut references: Vec = Vec::new(); path_resolution = resolver.resolve(self.def_maps, path.clone(), &mut Some(&mut references))?; @@ -59,6 +63,12 @@ impl<'context> Elaborator<'context> { ident.is_self_type_name(), ); } + + self.interner.add_module_def_id_reference( + path_resolution.module_def_id, + location, + is_self_type_name, + ); } else { path_resolution = resolver.resolve(self.def_maps, path, &mut None)?; } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index d8a701f266b..84aff265216 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -548,27 +548,7 @@ fn add_import_reference( } let location = Location::new(name.span(), file_id); - - match def_id { - crate::macros_api::ModuleDefId::ModuleId(module_id) => { - interner.add_module_reference(module_id, location); - } - crate::macros_api::ModuleDefId::FunctionId(func_id) => { - interner.add_function_reference(func_id, location); - } - crate::macros_api::ModuleDefId::TypeId(struct_id) => { - interner.add_struct_reference(struct_id, location, false); - } - crate::macros_api::ModuleDefId::TraitId(trait_id) => { - interner.add_trait_reference(trait_id, location, false); - } - crate::macros_api::ModuleDefId::TypeAliasId(type_alias_id) => { - interner.add_alias_reference(type_alias_id, location); - } - crate::macros_api::ModuleDefId::GlobalId(global_id) => { - interner.add_global_reference(global_id, location); - } - }; + interner.add_module_def_id_reference(def_id, location, false); } fn inject_prelude( diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index a09c1b89a40..ac05306fb77 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -4,7 +4,7 @@ use rangemap::RangeMap; use rustc_hash::FxHashMap; use crate::{ - hir::def_map::ModuleId, + hir::def_map::{ModuleDefId, ModuleId}, macros_api::{NodeInterner, StructId}, node_interner::{DefinitionId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId}, }; @@ -66,6 +66,34 @@ impl NodeInterner { self.reference_modules.get(&reference) } + pub(crate) fn add_module_def_id_reference( + &mut self, + def_id: ModuleDefId, + location: Location, + is_self_type: bool, + ) { + match def_id { + ModuleDefId::ModuleId(module_id) => { + self.add_module_reference(module_id, location); + } + ModuleDefId::FunctionId(func_id) => { + self.add_function_reference(func_id, location); + } + ModuleDefId::TypeId(struct_id) => { + self.add_struct_reference(struct_id, location, is_self_type); + } + ModuleDefId::TraitId(trait_id) => { + self.add_trait_reference(trait_id, location, is_self_type); + } + ModuleDefId::TypeAliasId(type_alias_id) => { + self.add_alias_reference(type_alias_id, location); + } + ModuleDefId::GlobalId(global_id) => { + self.add_global_reference(global_id, location); + } + }; + } + pub(crate) fn add_module_reference(&mut self, id: ModuleId, location: Location) { self.add_reference(ReferenceId::Module(id), location, false); } diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index e9d577ee3a3..79bfb0e493a 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -538,4 +538,16 @@ mod hover_tests { ) .await; } + + #[test] + async fn hover_on_trait_on_call() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 39, character: 17 }, + r#" std::default + trait Default"#, + ) + .await; + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index d73932f7846..724a058a1e8 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -33,3 +33,9 @@ fn use_parameter(some_param: i32) { } use one::subone::SomeAlias; + +use std::default::Default; + +fn use_impl_method() { + let _: i32 = Default::default(); +} From b731e136578d6ccd24e5970027148eb2440d75b4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 15:56:25 -0300 Subject: [PATCH 13/23] Some fixes. A couple of failing tests. --- .../src/elaborator/expressions.rs | 5 +- tooling/lsp/src/requests/hover.rs | 60 +++++++++++++++++++ .../test_programs/workspace/two/src/lib.nr | 4 ++ .../test_programs/workspace/two/src/other.nr | 2 + 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 tooling/lsp/test_programs/workspace/two/src/other.nr diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index d0d6a1cf7ce..72e2beea570 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -403,7 +403,8 @@ impl<'context> Elaborator<'context> { constructor: ConstructorExpression, ) -> (HirExpression, Type) { let span = constructor.type_name.span(); - let is_self_type = constructor.type_name.last_segment().is_self_type_name(); + let last_segment = constructor.type_name.last_segment(); + let is_self_type = last_segment.is_self_type_name(); let (r#type, struct_generics) = if let Some(struct_id) = constructor.struct_type { let typ = self.interner.get_struct(struct_id); @@ -434,7 +435,7 @@ impl<'context> Elaborator<'context> { }); let struct_id = struct_type.borrow().id; - let reference_location = Location::new(span, self.file); + let reference_location = Location::new(last_segment.span(), self.file); self.interner.add_struct_reference(struct_id, reference_location, is_self_type); (expr, Type::Struct(struct_type, generics)) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 79bfb0e493a..6715758882b 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -550,4 +550,64 @@ mod hover_tests { ) .await; } + + #[test] + async fn hover_on_std_module_in_use() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 36, character: 9 }, + r#" std + mod default"#, + ) + .await; + } + + #[test] + async fn hover_on_crate_module_in_call() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 15, character: 17 }, + r#" one + mod subone"#, + ) + .await; + } + + #[test] + async fn hover_on_module_without_crate_or_std_prefix() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 43, character: 4 }, + r#" two + mod other"#, + ) + .await; + } + + #[test] + async fn hover_on_module_with_crate_prefix() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 44, character: 11 }, + r#" two + mod other"#, + ) + .await; + } + + #[test] + async fn hover_on_module_on_struct_constructor() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 19, character: 12 }, + r#" one + mod subone"#, + ) + .await; + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index 724a058a1e8..647b72da2e6 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -39,3 +39,7 @@ use std::default::Default; fn use_impl_method() { let _: i32 = Default::default(); } + +mod other; +use other::another_function; +use crate::other::other_function; diff --git a/tooling/lsp/test_programs/workspace/two/src/other.nr b/tooling/lsp/test_programs/workspace/two/src/other.nr new file mode 100644 index 00000000000..4d2fffcee80 --- /dev/null +++ b/tooling/lsp/test_programs/workspace/two/src/other.nr @@ -0,0 +1,2 @@ +fn other_function() {} +fn another_function() {} From 45b7fb2327a058f3b4b0bbf66d5fa922ace6637c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:01:24 -0300 Subject: [PATCH 14/23] References when resolving path segmnets can be None --- .../noirc_frontend/src/elaborator/scope.rs | 6 ++++-- .../src/hir/def_collector/dc_crate.rs | 5 ++++- .../src/hir/resolution/import.rs | 21 ++++++++++++------- .../src/hir/resolution/path_resolver.rs | 9 ++++---- .../noirc_frontend/src/hir/type_check/mod.rs | 2 +- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index 6d756d3efa6..e01c7e997d4 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -6,7 +6,6 @@ use crate::hir::resolution::path_resolver::{PathResolver, StandardPathResolver}; use crate::hir::resolution::resolver::SELF_TYPE_NAME; use crate::hir::scope::{Scope as GenericScope, ScopeTree as GenericScopeTree}; use crate::macros_api::Ident; -use crate::node_interner::ReferenceId; use crate::{ hir::{ def_map::{ModuleDefId, TryFromModuleDefId}, @@ -52,11 +51,14 @@ impl<'context> Elaborator<'context> { let location = Location::new(last_segment.span(), self.file); let is_self_type_name = last_segment.is_self_type_name(); - let mut references: Vec = Vec::new(); + let mut references: Vec<_> = Vec::new(); path_resolution = resolver.resolve(self.def_maps, path.clone(), &mut Some(&mut references))?; for (referenced, ident) in references.iter().zip(path.segments) { + let Some(referenced) = referenced else { + continue; + }; self.interner.add_reference( *referenced, Location::new(ident.span(), self.file), diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 84aff265216..625a0a071f0 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -347,7 +347,7 @@ impl DefCollector { for collected_import in std::mem::take(&mut def_collector.imports) { let module_id = collected_import.module_id; let resolved_import = if context.def_interner.track_references { - let mut references: Vec = Vec::new(); + let mut references: Vec> = Vec::new(); let resolved_import = resolve_import( crate_id, &collected_import, @@ -359,6 +359,9 @@ impl DefCollector { let file_id = current_def_map.file_id(module_id); for (referenced, ident) in references.iter().zip(&collected_import.path.segments) { + let Some(referenced) = referenced else { + continue; + }; context.def_interner.add_reference( *referenced, Location::new(ident.span(), file_id), diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 710c12a91bf..e0c87e2d9e4 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -81,7 +81,7 @@ pub fn resolve_import( crate_id: CrateId, import_directive: &ImportDirective, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> Result { let module_scope = import_directive.module_id; let NamespaceResolution { @@ -126,7 +126,7 @@ fn resolve_path_to_ns( crate_id: CrateId, importing_crate: CrateId, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> NamespaceResolutionResult { let import_path = &import_directive.path.segments; let def_map = &def_maps[&crate_id]; @@ -196,7 +196,7 @@ fn resolve_path_from_crate_root( import_path: &[Ident], def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> NamespaceResolutionResult { resolve_name_in_module( crate_id, @@ -214,7 +214,7 @@ fn resolve_name_in_module( import_path: &[Ident], starting_mod: LocalModuleId, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> NamespaceResolutionResult { let def_map = &def_maps[&krate]; let mut current_mod_id = ModuleId { krate, local_id: starting_mod }; @@ -247,7 +247,7 @@ fn resolve_name_in_module( current_mod_id = match typ { ModuleDefId::ModuleId(id) => { if let Some(path_references) = path_references { - path_references.push(ReferenceId::Module(id)); + path_references.push(Some(ReferenceId::Module(id))); } id } @@ -255,14 +255,14 @@ fn resolve_name_in_module( // TODO: If impls are ever implemented, types can be used in a path ModuleDefId::TypeId(id) => { if let Some(path_references) = path_references { - path_references.push(ReferenceId::Struct(id)); + path_references.push(Some(ReferenceId::Struct(id))); } id.module_id() } ModuleDefId::TypeAliasId(_) => panic!("type aliases cannot be used in type namespace"), ModuleDefId::TraitId(id) => { if let Some(path_references) = path_references { - path_references.push(ReferenceId::Trait(id)); + path_references.push(Some(ReferenceId::Trait(id))); } id.0 } @@ -309,7 +309,7 @@ fn resolve_external_dep( current_def_map: &CrateDefMap, directive: &ImportDirective, def_maps: &BTreeMap, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, importing_crate: CrateId, ) -> NamespaceResolutionResult { // Use extern_prelude to get the dep @@ -327,6 +327,11 @@ fn resolve_external_dep( // See `singleton_import.nr` test case for a check that such cases are handled elsewhere. let path_without_crate_name = &path[1..]; + // Given that we skipped the first segment, record that it doesn't refer to any module or type. + if let Some(path_references) = path_references { + path_references.push(None); + } + let path = Path { segments: path_without_crate_name.to_vec(), kind: PathKind::Plain, diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index c3dc76b635f..7cd44a84018 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -9,12 +9,13 @@ use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; pub trait PathResolver { /// Resolve the given path returning the resolved ModuleDefId. /// If `path_references` is `Some`, a `ReferenceId` for each segment in `path` - /// will be resolved and pushed. + /// will be resolved and pushed (some entries will be None if they don't refer to + /// a module or type). fn resolve( &self, def_maps: &BTreeMap, path: Path, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult; fn local_module_id(&self) -> LocalModuleId; @@ -38,7 +39,7 @@ impl PathResolver for StandardPathResolver { &self, def_maps: &BTreeMap, path: Path, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult { resolve_path(def_maps, self.module_id, path, path_references) } @@ -58,7 +59,7 @@ pub fn resolve_path( def_maps: &BTreeMap, module_id: ModuleId, path: Path, - path_references: &mut Option<&mut Vec>, + path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult { // lets package up the path into an ImportDirective and resolve it using that let import = diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 60618e9fd0f..57125e1e2dd 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -697,7 +697,7 @@ pub mod test { &self, _def_maps: &BTreeMap, path: Path, - _path_references: &mut Option<&mut Vec>, + _path_references: &mut Option<&mut Vec>>, ) -> PathResolutionResult { // Not here that foo::bar and hello::foo::bar would fetch the same thing let name = path.segments.last().unwrap(); From 4c2aca2e9ce85893a152379883aaa89d97ed789e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:04:49 -0300 Subject: [PATCH 15/23] Fix for generic functions --- tooling/lsp/src/requests/hover.rs | 4 ++-- tooling/lsp/test_programs/workspace/one/src/lib.nr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 6715758882b..5617cf90174 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -171,8 +171,8 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { } string.push_str(" "); string.push_str("fn "); - format_generics(&func_meta.direct_generics, &mut string); string.push_str(&func_name_definition_id.name); + format_generics(&func_meta.direct_generics, &mut string); string.push('('); let parameters = &func_meta.parameters; for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { @@ -465,7 +465,7 @@ mod hover_tests { "two/src/lib.nr", Position { line: 3, character: 4 }, r#" one - fn function_one()"#, + fn function_one()"#, ) .await; } diff --git a/tooling/lsp/test_programs/workspace/one/src/lib.nr b/tooling/lsp/test_programs/workspace/one/src/lib.nr index 6d580f61459..d8392974b92 100644 --- a/tooling/lsp/test_programs/workspace/one/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/one/src/lib.nr @@ -1,4 +1,4 @@ -pub fn function_one() {} +pub fn function_one() {} mod subone { struct SubOneStruct { From 7240eb0b9c92014a5366dfa6e64f5a0e64b4af4e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:06:06 -0300 Subject: [PATCH 16/23] Test for generic struct --- tooling/lsp/src/requests/hover.rs | 13 +++++++++++++ tooling/lsp/test_programs/workspace/one/src/lib.nr | 4 ++++ tooling/lsp/test_programs/workspace/two/src/lib.nr | 2 ++ 3 files changed, 19 insertions(+) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 5617cf90174..c17c1471a8f 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -422,6 +422,19 @@ mod hover_tests { .await; } + #[test] + async fn hover_on_generic_struct() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 46, character: 17 }, + r#" one::subone + struct GenericStruct { + }"#, + ) + .await; + } + #[test] async fn hover_on_struct_member() { assert_hover( diff --git a/tooling/lsp/test_programs/workspace/one/src/lib.nr b/tooling/lsp/test_programs/workspace/one/src/lib.nr index d8392974b92..61f282fa2a7 100644 --- a/tooling/lsp/test_programs/workspace/one/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/one/src/lib.nr @@ -18,4 +18,8 @@ mod subone { global some_global = 2; type SomeAlias = i32; + + struct GenericStruct { + + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index 647b72da2e6..1639e2e38cd 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -43,3 +43,5 @@ fn use_impl_method() { mod other; use other::another_function; use crate::other::other_function; + +use one::subone::GenericStruct; From 9665376503d7cc3e3a99bb564fa55d102faf65a0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:13:29 -0300 Subject: [PATCH 17/23] clippy --- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 376db5ad805..d8beca9ebf0 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -731,7 +731,7 @@ impl<'context> Elaborator<'context> { typ, direct_generics, all_generics: self.generics.clone(), - struct_id: struct_id, + struct_id, trait_impl: self.current_trait_impl, parameters: parameters.into(), parameter_idents, diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 4b6739d8c3a..1f30b4388b6 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -320,7 +320,7 @@ impl<'a> ModCollector<'a> { context.def_interner.add_definition_location( ReferenceId::Struct(id), - Some(ModuleId { krate: krate, local_id: self.module_id }), + Some(ModuleId { krate, local_id: self.module_id }), ); } definition_errors @@ -371,7 +371,7 @@ impl<'a> ModCollector<'a> { context.def_interner.add_definition_location( ReferenceId::Alias(type_alias_id), - Some(ModuleId { krate: krate, local_id: self.module_id }), + Some(ModuleId { krate, local_id: self.module_id }), ); } errors @@ -541,7 +541,7 @@ impl<'a> ModCollector<'a> { context.def_interner.add_definition_location( ReferenceId::Trait(trait_id), - Some(ModuleId { krate: krate, local_id: self.module_id }), + Some(ModuleId { krate, local_id: self.module_id }), ); self.def_collector.items.traits.insert(trait_id, unresolved); From 2e081c8a7df43f38dcfcb87a85e71e6600284656 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:23:24 -0300 Subject: [PATCH 18/23] clippy --- tooling/lsp/src/requests/hover.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index c17c1471a8f..920767ecd68 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -48,7 +48,7 @@ fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) - ReferenceId::Global(id) => format_global(id, args), ReferenceId::Function(id) => format_function(id, args), ReferenceId::Alias(id) => format_alias(id, args), - ReferenceId::Local(id) => format_local(id, &args), + ReferenceId::Local(id) => format_local(id, args), ReferenceId::Reference(location, _) => { format_reference(args.interner.find_referenced(location).unwrap(), args) } @@ -63,7 +63,7 @@ fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> String { args, &mut string, ) { - string.push_str("\n"); + string.push('\n'); } string.push_str(" "); string.push_str("mod "); @@ -77,7 +77,7 @@ fn format_struct(id: StructId, args: &ProcessRequestCallbackArgs) -> String { let mut string = String::new(); if format_parent_module(ReferenceId::Struct(id), args, &mut string) { - string.push_str("\n"); + string.push('\n'); } string.push_str(" "); string.push_str("struct "); @@ -109,7 +109,7 @@ fn format_struct_member( string.push_str("::"); } string.push_str(&struct_type.name.0.contents); - string.push_str("\n"); + string.push('\n'); string.push_str(" "); string.push_str(&field_name.0.contents); string.push_str(": "); @@ -122,7 +122,7 @@ fn format_trait(id: TraitId, args: &ProcessRequestCallbackArgs) -> String { let mut string = String::new(); if format_parent_module(ReferenceId::Trait(id), args, &mut string) { - string.push_str("\n"); + string.push('\n'); } string.push_str(" "); string.push_str("trait "); @@ -138,7 +138,7 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { let mut string = String::new(); if format_parent_module(ReferenceId::Global(id), args, &mut string) { - string.push_str("\n"); + string.push('\n'); } string.push_str(" "); string.push_str("global "); @@ -167,7 +167,7 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { false }; if formatted_parent_module || formatted_parent_struct { - string.push_str("\n"); + string.push('\n'); } string.push_str(" "); string.push_str("fn "); @@ -209,7 +209,7 @@ fn format_alias(id: TypeAliasId, args: &ProcessRequestCallbackArgs) -> String { let mut string = String::new(); format_parent_module(ReferenceId::Alias(id), args, &mut string); - string.push_str("\n"); + string.push('\n'); string.push_str(" "); string.push_str("type "); string.push_str(&type_alias.name.0.contents); @@ -297,7 +297,7 @@ fn format_parent_module( return false; }; - return format_parent_module_from_module_id(&module, args, string); + format_parent_module_from_module_id(module, args, string) } fn format_parent_module_from_module_id( @@ -379,7 +379,7 @@ mod hover_tests { HoverParams { text_document_position_params: TextDocumentPositionParams { text_document: TextDocumentIdentifier { uri: file_uri }, - position: position, + position, }, work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, }, From 8dcb1c2ddeb8ec1a1166d091687b59db7f11e2c7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:29:00 -0300 Subject: [PATCH 19/23] Fix typo --- tooling/lsp/src/requests/hover.rs | 2 +- tooling/lsp/src/requests/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 920767ecd68..4211112a515 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -309,7 +309,7 @@ fn format_parent_module_from_module_id( let crate_name = match crate_id { CrateId::Root(_) => Some(args.root_crate_name.clone()), CrateId::Crate(_) => args - .root_crate_dependnencies + .root_crate_dependencies .iter() .find(|dep| dep.crate_id == crate_id) .map(|dep| format!("{}", dep.name)), diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 489e9237fa4..70f06335dce 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -277,7 +277,7 @@ pub(crate) struct ProcessRequestCallbackArgs<'a> { interner: &'a NodeInterner, interners: &'a HashMap, root_crate_name: String, - root_crate_dependnencies: &'a Vec, + root_crate_dependencies: &'a Vec, } pub(crate) fn process_request( @@ -331,7 +331,7 @@ where interner, interners: &state.cached_definitions, root_crate_name: format!("{}", package.name), - root_crate_dependnencies: &context.crate_graph[context.root_crate_id()].dependencies, + root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies, })) } pub(crate) fn find_all_references_in_workspace( From 48f3e0c59978ca3804adcc911d775d96c6d08340 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:29:40 -0300 Subject: [PATCH 20/23] Fix another typo --- compiler/noirc_frontend/src/locations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index ac05306fb77..656dfa05b96 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -207,7 +207,7 @@ impl NodeInterner { // Starting at the given location, find the node referenced by it. Then, gather // all locations that reference that node, and return all of them - // (the references and optionally the referenced node if `include_referencedd` is true). + // (the references and optionally the referenced node if `include_referenced` is true). // If `include_self_type_name` is true, references where "Self" is written are returned, // otherwise they are not. // Returns `None` if the location is not known to this interner. From a6f473e54763fa4bda28c765a361138d416abe70 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:31:36 -0300 Subject: [PATCH 21/23] And another typo --- compiler/noirc_frontend/src/locations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 656dfa05b96..0ba74e22781 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -17,7 +17,7 @@ pub(crate) struct LocationIndices { impl LocationIndices { pub(crate) fn add_location(&mut self, location: Location, node_index: PetGraphIndex) { - // Some location spans are empty: maybe they are from ficticious nodes? + // Some location spans are empty: maybe they are from fictitious nodes? if location.span.start() == location.span.end() { return; } From f9435e4c58b6c041c23ec54999a8c83cc4f71d68 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 11 Jul 2024 16:53:59 -0300 Subject: [PATCH 22/23] Fix: hover over generic argument didn't work --- .../noirc_frontend/src/elaborator/types.rs | 18 +++++++++++------- tooling/lsp/src/requests/hover.rs | 15 +++++++++++++++ .../lsp/test_programs/workspace/two/src/lib.nr | 6 ++++++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 9134fc851b9..35ff421ed32 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -57,12 +57,16 @@ impl<'context> Elaborator<'context> { use crate::ast::UnresolvedTypeData::*; let span = typ.span; - let (is_self_type_name, is_synthetic) = if let Named(ref named_path, _, synthetic) = typ.typ - { - (named_path.last_segment().is_self_type_name(), synthetic) - } else { - (false, false) - }; + let (named_path_span, is_self_type_name, is_synthetic) = + if let Named(ref named_path, _, synthetic) = typ.typ { + ( + Some(named_path.last_segment().span()), + named_path.last_segment().is_self_type_name(), + synthetic, + ) + } else { + (None, false, false) + }; let resolved_type = match typ.typ { FieldElement => Type::FieldElement, @@ -153,7 +157,7 @@ impl<'context> Elaborator<'context> { }; if let Some(unresolved_span) = typ.span { - let location = Location::new(unresolved_span, self.file); + let location = Location::new(named_path_span.unwrap_or(unresolved_span), self.file); match resolved_type { Type::Struct(ref struct_type, _) => { diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 4211112a515..161fd20f555 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -623,4 +623,19 @@ mod hover_tests { ) .await; } + + #[test] + async fn hover_on_type_inside_generic_arguments() { + assert_hover( + "workspace", + "two/src/lib.nr", + Position { line: 51, character: 30 }, + r#" one::subone + struct SubOneStruct { + some_field: i32, + some_other_field: Field, + }"#, + ) + .await; + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index 1639e2e38cd..3f0f0f117b7 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -45,3 +45,9 @@ use other::another_function; use crate::other::other_function; use one::subone::GenericStruct; + +use std::collections::bounded_vec::BoundedVec; + +fn instantiate_generic() { + let x: BoundedVec = BoundedVec::new(); +} From ec316c2cd424d501a6fd11f8216baf1315c1a2b7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 12 Jul 2024 10:13:14 -0300 Subject: [PATCH 23/23] Call `to_string()` instead of `format!` Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- tooling/lsp/src/requests/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 70f06335dce..e029750e589 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -330,7 +330,7 @@ where files, interner, interners: &state.cached_definitions, - root_crate_name: format!("{}", package.name), + root_crate_name: package.name.to_string(), root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies, })) }