From f89b3db1b850cf71cc03dc800e3ff13c931bc816 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 09:53:54 -0300 Subject: [PATCH 1/9] Apply review suggestions --- compiler/noirc_frontend/src/elaborator/mod.rs | 4 ++-- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 4 ++-- compiler/noirc_frontend/src/hir/def_map/module_data.rs | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 47222de80c6..24b0bff457a 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1832,8 +1832,6 @@ impl<'context> Elaborator<'context> { }; for (i, variant) in typ.enum_def.variants.iter().enumerate() { - self.interner.add_definition_location(ReferenceId::EnumVariant(*type_id, i), None); - let types = vecmap(&variant.item.parameters, |typ| self.resolve_type(typ.clone())); let name = variant.item.name.clone(); datatype.borrow_mut().push_variant(EnumVariant::new(name, types.clone())); @@ -1849,6 +1847,8 @@ impl<'context> Elaborator<'context> { &self_type, unresolved.clone(), ); + + self.interner.add_definition_location(ReferenceId::EnumVariant(*type_id, i), None); } } } 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 fcb21e18050..440426c5e75 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -336,12 +336,12 @@ impl<'a> ModCollector<'a> { krate: CrateId, ) -> Vec<(CompilationError, FileId)> { let mut definition_errors = vec![]; - for struct_definition in types { + for enum_definition in types { if let Some((id, the_enum)) = collect_enum( &mut context.def_interner, &mut self.def_collector.def_map, &mut context.usage_tracker, - struct_definition, + enum_definition, self.file_id, self.module_id, krate, diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index 559a6acb536..5df0e08cbb2 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -31,7 +31,7 @@ pub struct ModuleData { /// True if this module is a `contract Foo { ... }` module containing contract functions pub is_contract: bool, - /// True if this module is actually a struct + /// True if this module is actually a type pub is_type: bool, pub attributes: Vec, @@ -44,7 +44,7 @@ impl ModuleData { outer_attributes: Vec, inner_attributes: Vec, is_contract: bool, - is_struct: bool, + is_type: bool, ) -> ModuleData { let mut attributes = outer_attributes; attributes.extend(inner_attributes); @@ -57,7 +57,7 @@ impl ModuleData { definitions: ItemScope::default(), location, is_contract, - is_type: is_struct, + is_type, attributes, } } From 733cc01a4cc5e912e6d5bc73785fc7044dea7259 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 10:08:57 -0300 Subject: [PATCH 2/9] Unify ReferenceId::Type with ReferenceId::Enum and fix hover --- .../src/elaborator/expressions.rs | 2 +- .../noirc_frontend/src/elaborator/patterns.rs | 2 +- .../noirc_frontend/src/elaborator/types.rs | 2 +- .../src/hir/def_collector/dc_mod.rs | 8 ++-- compiler/noirc_frontend/src/locations.rs | 25 ++++-------- compiler/noirc_frontend/src/node_interner.rs | 3 +- tooling/lsp/src/modules.rs | 2 +- .../code_action/fill_struct_fields.rs | 13 +++++-- tooling/lsp/src/requests/completion.rs | 5 +-- .../requests/completion/completion_items.rs | 2 +- tooling/lsp/src/requests/hover.rs | 39 ++++++++++--------- tooling/lsp/src/requests/inlay_hint.rs | 3 +- 12 files changed, 50 insertions(+), 56 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 650bd579f2f..8b55ddfa425 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -660,7 +660,7 @@ impl<'context> Elaborator<'context> { let struct_id = struct_type.borrow().id; let reference_location = Location::new(last_segment.ident.span(), self.file); - self.interner.add_struct_reference(struct_id, reference_location, is_self_type); + self.interner.add_type_reference(struct_id, reference_location, is_self_type); (expr, Type::DataType(struct_type, generics)) } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 6ab12d1e537..5c45c587823 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -234,7 +234,7 @@ impl<'context> Elaborator<'context> { let struct_id = struct_type.borrow().id; let reference_location = Location::new(name_span, self.file); - self.interner.add_struct_reference(struct_id, reference_location, is_self_type); + self.interner.add_type_reference(struct_id, reference_location, is_self_type); for (field_index, field) in fields.iter().enumerate() { let reference_location = Location::new(field.0.span(), self.file); diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 4d8a90d25d4..e24f69fa632 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -158,7 +158,7 @@ impl<'context> Elaborator<'context> { // Record the location of the type reference self.interner.push_type_ref_location(resolved_type.clone(), location); if !is_synthetic { - self.interner.add_struct_reference( + self.interner.add_type_reference( data_type.borrow().id, location, is_self_type_name, 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 440426c5e75..c77b3f07a7f 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1072,7 +1072,7 @@ pub fn collect_struct( } }; - interner.set_doc_comments(ReferenceId::Struct(id), doc_comments); + interner.set_doc_comments(ReferenceId::Type(id), doc_comments); for (index, field) in unresolved.struct_def.fields.iter().enumerate() { if !field.doc_comments.is_empty() { @@ -1106,7 +1106,7 @@ pub fn collect_struct( } if interner.is_in_lsp_mode() { - interner.register_struct(id, name.to_string(), visibility, parent_module_id); + interner.register_type(id, name.to_string(), visibility, parent_module_id); } Some((id, unresolved)) @@ -1167,7 +1167,7 @@ pub fn collect_enum( } }; - interner.set_doc_comments(ReferenceId::Enum(id), doc_comments); + interner.set_doc_comments(ReferenceId::Type(id), doc_comments); for (index, variant) in unresolved.enum_def.variants.iter().enumerate() { if !variant.doc_comments.is_empty() { @@ -1201,7 +1201,7 @@ pub fn collect_enum( } if interner.is_in_lsp_mode() { - interner.register_enum(id, name.to_string(), visibility, parent_module_id); + interner.register_type(id, name.to_string(), visibility, parent_module_id); } Some((id, unresolved)) diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 33c37172b50..fcc666f13e8 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -60,7 +60,7 @@ impl NodeInterner { match reference { ReferenceId::Module(id) => self.module_attributes(&id).location, ReferenceId::Function(id) => self.function_modifiers(&id).name_location, - ReferenceId::Struct(id) | ReferenceId::Enum(id) => { + ReferenceId::Type(id) => { let typ = self.get_type(id); let typ = typ.borrow(); Location::new(typ.name.span(), typ.location.file) @@ -109,8 +109,8 @@ impl NodeInterner { 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::TypeId(type_id) => { + self.add_type_reference(type_id, location, is_self_type); } ModuleDefId::TraitId(trait_id) => { self.add_trait_reference(trait_id, location, is_self_type); @@ -128,13 +128,13 @@ impl NodeInterner { self.add_reference(ReferenceId::Module(id), location, false); } - pub(crate) fn add_struct_reference( + pub(crate) fn add_type_reference( &mut self, id: TypeId, location: Location, is_self_type: bool, ) { - self.add_reference(ReferenceId::Struct(id), location, is_self_type); + self.add_reference(ReferenceId::Type(id), location, is_self_type); } pub(crate) fn add_struct_member_reference( @@ -326,25 +326,14 @@ impl NodeInterner { self.register_name_for_auto_import(name, ModuleDefId::GlobalId(id), visibility, None); } - pub(crate) fn register_struct( + pub(crate) fn register_type( &mut self, id: TypeId, name: String, visibility: ItemVisibility, parent_module_id: ModuleId, ) { - self.add_definition_location(ReferenceId::Struct(id), Some(parent_module_id)); - self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None); - } - - pub(crate) fn register_enum( - &mut self, - id: TypeId, - name: String, - visibility: ItemVisibility, - parent_module_id: ModuleId, - ) { - self.add_definition_location(ReferenceId::Enum(id), Some(parent_module_id)); + self.add_definition_location(ReferenceId::Type(id), Some(parent_module_id)); self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None); } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 3d74cc1bf37..024a98d8475 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -299,9 +299,8 @@ pub enum DependencyId { #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum ReferenceId { Module(ModuleId), - Struct(TypeId), + Type(TypeId), StructMember(TypeId, usize), - Enum(TypeId), EnumVariant(TypeId, usize), Trait(TraitId), Global(GlobalId), diff --git a/tooling/lsp/src/modules.rs b/tooling/lsp/src/modules.rs index b023f3886c3..758322fa4bc 100644 --- a/tooling/lsp/src/modules.rs +++ b/tooling/lsp/src/modules.rs @@ -16,7 +16,7 @@ pub(crate) fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> Refer match module_def_id { ModuleDefId::ModuleId(id) => ReferenceId::Module(id), ModuleDefId::FunctionId(id) => ReferenceId::Function(id), - ModuleDefId::TypeId(id) => ReferenceId::Struct(id), + ModuleDefId::TypeId(id) => ReferenceId::Type(id), ModuleDefId::TypeAliasId(id) => ReferenceId::Alias(id), ModuleDefId::TraitId(id) => ReferenceId::Trait(id), ModuleDefId::GlobalId(id) => ReferenceId::Global(id), diff --git a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs index 7a4d562e402..8aa47160b38 100644 --- a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -20,15 +20,20 @@ impl<'a> CodeActionFinder<'a> { }; let location = Location::new(path.span, self.file); - let Some(ReferenceId::Struct(struct_id)) = self.interner.find_referenced(location) else { + let Some(ReferenceId::Type(type_id)) = self.interner.find_referenced(location) else { return; }; - let struct_type = self.interner.get_type(struct_id); - let struct_type = struct_type.borrow(); + let typ = self.interner.get_type(type_id); + let typ = typ.borrow(); + + // Might be an enum + if !typ.is_struct() { + return; + } // First get all of the struct's fields - let mut fields = struct_type.get_fields_as_written(); + let mut fields = typ.get_fields_as_written(); // Remove the ones that already exists in the constructor for (constructor_field, _) in &constructor.fields { diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 9948a29691e..03205836858 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -199,7 +199,7 @@ impl<'a> NodeFinder<'a> { }; let location = Location::new(span, self.file); - let Some(ReferenceId::Struct(struct_id)) = self.interner.find_referenced(location) else { + let Some(ReferenceId::Type(struct_id)) = self.interner.find_referenced(location) else { return; }; @@ -1953,8 +1953,7 @@ fn name_matches(name: &str, prefix: &str) -> bool { fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option { match reference_id { ReferenceId::Module(module_id) => Some(ModuleDefId::ModuleId(module_id)), - ReferenceId::Struct(struct_id) => Some(ModuleDefId::TypeId(struct_id)), - ReferenceId::Enum(enum_id) => Some(ModuleDefId::TypeId(enum_id)), + ReferenceId::Type(struct_id) => Some(ModuleDefId::TypeId(struct_id)), ReferenceId::Trait(trait_id) => Some(ModuleDefId::TraitId(trait_id)), ReferenceId::Function(func_id) => Some(ModuleDefId::FunctionId(func_id)), ReferenceId::Alias(type_alias_id) => Some(ModuleDefId::TypeAliasId(type_alias_id)), diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index c8ae16bf1f4..c9d06292657 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -113,7 +113,7 @@ impl<'a> NodeFinder<'a> { fn struct_completion_item(&self, name: String, struct_id: TypeId) -> CompletionItem { let completion_item = simple_completion_item(name.clone(), CompletionItemKind::STRUCT, Some(name)); - self.completion_item_with_doc_comments(ReferenceId::Struct(struct_id), completion_item) + self.completion_item_with_doc_comments(ReferenceId::Type(struct_id), completion_item) } pub(super) fn struct_field_completion_item( diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 8b9c0bb4b28..965224acbb8 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -73,11 +73,10 @@ pub(crate) fn on_hover_request( fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) -> Option { match reference { ReferenceId::Module(id) => format_module(id, args), - ReferenceId::Struct(id) => Some(format_struct(id, args)), + ReferenceId::Type(id) => Some(format_type(id, args)), ReferenceId::StructMember(id, field_index) => { Some(format_struct_member(id, field_index, args)) } - ReferenceId::Enum(id) => Some(format_enum(id, args)), ReferenceId::EnumVariant(id, variant_index) => { Some(format_enum_variant(id, variant_index, args)) } @@ -126,20 +125,27 @@ fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> Option String { - let struct_type = args.interner.get_type(id); - let struct_type = struct_type.borrow(); +fn format_type(id: TypeId, args: &ProcessRequestCallbackArgs) -> String { + let typ = args.interner.get_type(id); + let typ = typ.borrow(); + if typ.is_struct() { + format_struct(&typ, args) + } else { + format_enum(&typ, args) + } +} +fn format_struct(typ: &DataType, args: &ProcessRequestCallbackArgs) -> String { let mut string = String::new(); - if format_parent_module(ReferenceId::Struct(id), args, &mut string) { + if format_parent_module(ReferenceId::Type(typ.id), args, &mut string) { string.push('\n'); } 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(&typ.name.0.contents); + format_generics(&typ.generics, &mut string); string.push_str(" {\n"); - for field in struct_type.get_fields_as_written() { + for field in typ.get_fields_as_written() { string.push_str(" "); string.push_str(&field.name.0.contents); string.push_str(": "); @@ -148,17 +154,14 @@ fn format_struct(id: TypeId, args: &ProcessRequestCallbackArgs) -> String { } string.push_str(" }"); - append_doc_comments(args.interner, ReferenceId::Struct(id), &mut string); + append_doc_comments(args.interner, ReferenceId::Type(typ.id), &mut string); string } -fn format_enum(id: TypeId, args: &ProcessRequestCallbackArgs) -> String { - let typ = args.interner.get_type(id); - let typ = typ.borrow(); - +fn format_enum(typ: &DataType, args: &ProcessRequestCallbackArgs) -> String { let mut string = String::new(); - if format_parent_module(ReferenceId::Enum(id), args, &mut string) { + if format_parent_module(ReferenceId::Type(typ.id), args, &mut string) { string.push('\n'); } string.push_str(" "); @@ -181,7 +184,7 @@ fn format_enum(id: TypeId, args: &ProcessRequestCallbackArgs) -> String { } string.push_str(" }"); - append_doc_comments(args.interner, ReferenceId::Enum(id), &mut string); + append_doc_comments(args.interner, ReferenceId::Type(typ.id), &mut string); string } @@ -196,7 +199,7 @@ fn format_struct_member( let field = struct_type.field_at(field_index); let mut string = String::new(); - if format_parent_module(ReferenceId::Struct(id), args, &mut string) { + if format_parent_module(ReferenceId::Type(id), args, &mut string) { string.push_str("::"); } string.push_str(&struct_type.name.0.contents); @@ -222,7 +225,7 @@ fn format_enum_variant( let variant = enum_type.variant_at(field_index); let mut string = String::new(); - if format_parent_module(ReferenceId::Enum(id), args, &mut string) { + if format_parent_module(ReferenceId::Type(id), args, &mut string) { string.push_str("::"); } string.push_str(&enum_type.name.0.contents); diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 1798f845a31..792dd4352bc 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -109,8 +109,7 @@ impl<'a> InlayHintCollector<'a> { self.push_type_hint(lsp_location, &variant_type, false, include_colon); } ReferenceId::Module(_) - | ReferenceId::Struct(_) - | ReferenceId::Enum(_) + | ReferenceId::Type(_) | ReferenceId::Trait(_) | ReferenceId::Function(_) | ReferenceId::Alias(_) From 217676c63ca08dd207a06fc506840b003fa16e03 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 10:48:06 -0300 Subject: [PATCH 3/9] Better function completion for enum variants --- .../noirc_frontend/src/elaborator/enums.rs | 6 ++- compiler/noirc_frontend/src/elaborator/mod.rs | 3 +- .../noirc_frontend/src/hir_def/function.rs | 3 ++ .../requests/completion/completion_items.rs | 43 +++++++++++++------ tooling/lsp/src/requests/completion/tests.rs | 33 +++++++++++++- 5 files changed, 71 insertions(+), 17 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index f574aa81619..0c65ad7aafa 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -2,7 +2,7 @@ use iter_extended::vecmap; use noirc_errors::Location; use crate::{ - ast::{EnumVariant, FunctionKind, NoirEnumeration, UnresolvedType, Visibility}, + ast::{Documented, EnumVariant, FunctionKind, NoirEnumeration, UnresolvedType, Visibility}, hir_def::{ expr::{HirEnumConstructorExpression, HirExpression, HirIdent}, function::{FuncMeta, FunctionBody, HirFunction, Parameters}, @@ -21,13 +21,14 @@ impl Elaborator<'_> { &mut self, enum_: &NoirEnumeration, type_id: TypeId, - variant: &EnumVariant, + documented_variant: &Documented, variant_arg_types: Vec, variant_index: usize, datatype: &Shared, self_type: &Type, self_type_unresolved: UnresolvedType, ) { + let variant = &documented_variant.item; let name_string = variant.name.to_string(); let datatype_ref = datatype.borrow(); let location = Location::new(variant.name.span(), self.file); @@ -70,6 +71,7 @@ impl Elaborator<'_> { type_id: Some(type_id), trait_id: None, trait_impl: None, + enum_variant_index: Some(variant_index), is_entry_point: false, has_inline_attribute: false, function_body: FunctionBody::Resolved, diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 24b0bff457a..662dd3d587d 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1006,6 +1006,7 @@ impl<'context> Elaborator<'context> { type_id: struct_id, trait_id, trait_impl: self.current_trait_impl, + enum_variant_index: None, parameters: parameters.into(), parameter_idents, return_type: func.def.return_type.clone(), @@ -1840,7 +1841,7 @@ impl<'context> Elaborator<'context> { self.define_enum_variant_function( &typ.enum_def, *type_id, - &variant.item, + &variant, types, i, &datatype, diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index 0e51539de91..75bb4f50541 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -141,6 +141,9 @@ pub struct FuncMeta { /// The trait impl this function belongs to, if any pub trait_impl: Option, + /// If this function is the one related to an enum variant, this holds its index (relative to `type_id`) + pub enum_variant_index: Option, + /// True if this function is an entry point to the program. /// For non-contracts, this means the function is `main`. pub is_entry_point: bool, diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index c9d06292657..5131f9554a1 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -288,10 +288,10 @@ impl<'a> NodeFinder<'a> { } else { false }; + let description = func_meta_type_to_string(func_meta, name, func_self_type.is_some()); let name = if self_prefix { format!("self.{}", name) } else { name.clone() }; let name = if is_macro_call { format!("{}!", name) } else { name }; let name = &name; - let description = func_meta_type_to_string(func_meta, func_self_type.is_some()); let mut has_arguments = false; let completion_item = match function_completion_kind { @@ -351,7 +351,16 @@ impl<'a> NodeFinder<'a> { self.auto_import_trait_if_trait_method(func_id, trait_info, &mut completion_item); - self.completion_item_with_doc_comments(ReferenceId::Function(func_id), completion_item) + if let (Some(type_id), Some(variant_index)) = + (func_meta.type_id, func_meta.enum_variant_index) + { + self.completion_item_with_doc_comments( + ReferenceId::EnumVariant(type_id, variant_index), + completion_item, + ) + } else { + self.completion_item_with_doc_comments(ReferenceId::Function(func_id), completion_item) + } } fn auto_import_trait_if_trait_method( @@ -512,18 +521,25 @@ pub(super) fn trait_impl_method_completion_item( snippet_completion_item(label, CompletionItemKind::METHOD, insert_text, None) } -fn func_meta_type_to_string(func_meta: &FuncMeta, has_self_type: bool) -> String { +fn func_meta_type_to_string(func_meta: &FuncMeta, name: &str, has_self_type: bool) -> String { let mut typ = &func_meta.typ; if let Type::Forall(_, typ_) = typ { typ = typ_; } + let is_enum_variant = func_meta.enum_variant_index.is_some(); + if let Type::Function(args, ret, _env, unconstrained) = typ { let mut string = String::new(); - if *unconstrained { - string.push_str("unconstrained "); + if is_enum_variant { + string.push_str(name); + string.push('('); + } else { + if *unconstrained { + string.push_str("unconstrained "); + } + string.push_str("fn("); } - string.push_str("fn("); for (index, arg) in args.iter().enumerate() { if index > 0 { string.push_str(", "); @@ -536,13 +552,16 @@ fn func_meta_type_to_string(func_meta: &FuncMeta, has_self_type: bool) -> String } string.push(')'); - let ret: &Type = ret; - if let Type::Unit = ret { - // Nothing - } else { - string.push_str(" -> "); - string.push_str(&ret.to_string()); + if !is_enum_variant { + let ret: &Type = ret; + if let Type::Unit = ret { + // Nothing + } else { + string.push_str(" -> "); + string.push_str(&ret.to_string()); + } } + string } else { typ.to_string() diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 8ff568e3c26..dcb622fd7e1 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -20,8 +20,8 @@ mod completion_tests { use lsp_types::{ CompletionItem, CompletionItemKind, CompletionItemLabelDetails, CompletionParams, - CompletionResponse, DidOpenTextDocumentParams, PartialResultParams, Position, - TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, + CompletionResponse, DidOpenTextDocumentParams, Documentation, PartialResultParams, + Position, TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams, }; use tokio::test; @@ -3077,4 +3077,33 @@ fn main() { "#; assert_eq!(new_code, expected); } + + #[test] + async fn test_suggests_enum_variant_differently_than_a_function_call() { + let src = r#" + enum Enum { + /// Some docs + Variant(Field, i32) + } + + fn foo() { + Enum::Var>|< + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "Variant(…)".to_string()); + + let details = item.label_details.as_ref().unwrap(); + assert_eq!(details.description, Some("Variant(Field, i32)".to_string())); + + assert_eq!(item.detail, Some("Variant(Field, i32)".to_string())); + + let Documentation::MarkupContent(markdown) = item.documentation.as_ref().unwrap() else { + panic!("Expected markdown docs"); + }; + assert!(markdown.value.contains("Some docs")); + } } From b408401cd8790b6ef51afb53b1df6eb141f0c638 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 10:59:50 -0300 Subject: [PATCH 4/9] Fix hover on empty enum --- compiler/noirc_frontend/src/elaborator/mod.rs | 2 ++ compiler/noirc_frontend/src/hir_def/types.rs | 12 +++++-- tooling/lsp/src/requests/hover.rs | 33 +++++++++++++++++++ .../test_programs/workspace/two/src/lib.nr | 8 +++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 662dd3d587d..b75fa57f34f 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1832,6 +1832,8 @@ impl<'context> Elaborator<'context> { span: typ.enum_def.span, }; + datatype.borrow_mut().init_variants(); + for (i, variant) in typ.enum_def.variants.iter().enumerate() { let types = vecmap(&variant.item.parameters, |typ| self.resolve_type(typ.clone())); let name = variant.item.name.clone(); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index bab8f566f7c..54a3f73efeb 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -448,13 +448,19 @@ impl DataType { self.body = TypeBody::Struct(fields); } - pub(crate) fn push_variant(&mut self, variant: EnumVariant) { + pub(crate) fn init_variants(&mut self) { match &mut self.body { TypeBody::None => { - self.body = TypeBody::Enum(vec![variant]); + self.body = TypeBody::Enum(vec![]); } + _ => panic!("Called init_variants but body was None"), + } + } + + pub(crate) fn push_variant(&mut self, variant: EnumVariant) { + match &mut self.body { TypeBody::Enum(variants) => variants.push(variant), - TypeBody::Struct(_) => panic!("Called push_variant on a non-variant type {self}"), + _ => panic!("Called push_variant on {self} but body wasn't an enum"), } } diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 965224acbb8..21b4ded9ca3 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -1259,4 +1259,37 @@ mod hover_tests { .await; assert!(hover_text.contains("fn mut_self(&mut self)")); } + + #[test] + async fn hover_on_empty_enum_type() { + let hover_text = + get_hover_text("workspace", "two/src/lib.nr", Position { line: 100, character: 8 }) + .await; + assert!(hover_text.contains( + " two + enum EmptyColor { + } + +--- + + Red, blue, etc." + )); + } + + #[test] + async fn hover_on_non_empty_enum_type() { + let hover_text = + get_hover_text("workspace", "two/src/lib.nr", Position { line: 103, character: 8 }) + .await; + assert!(hover_text.contains( + " two + enum Color { + Red, + } + +--- + + Red, blue, etc." + )); + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index aacc4508756..f0c29502937 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -96,3 +96,11 @@ impl TraitWithDocs for Field { impl Foo { fn mut_self(&mut self) {} } + +/// Red, blue, etc. +enum EmptyColor {} + +/// Red, blue, etc. +enum Color { + Red, +} From e0da390ea1b1749734ea6c820952a900ff6a565b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 11:03:03 -0300 Subject: [PATCH 5/9] Test for hover over enum variant --- tooling/lsp/src/requests/hover.rs | 15 +++++++++++++++ .../lsp/test_programs/workspace/two/src/lib.nr | 1 + 2 files changed, 16 insertions(+) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 21b4ded9ca3..318965cec25 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -1292,4 +1292,19 @@ mod hover_tests { Red, blue, etc." )); } + + #[test] + async fn hover_on_enum_variant() { + let hover_text = + get_hover_text("workspace", "two/src/lib.nr", Position { line: 105, character: 6 }) + .await; + assert!(hover_text.contains( + " two::Color + Red + +--- + + Like a tomato" + )); + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index f0c29502937..0843b49c572 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -102,5 +102,6 @@ enum EmptyColor {} /// Red, blue, etc. enum Color { + /// Like a tomato Red, } From af2a9dffc9e7d0a2659c4077384f240092d75e68 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 11:36:21 -0300 Subject: [PATCH 6/9] Fix signature help for enum variants --- tooling/lsp/src/requests/signature_help.rs | 32 ++++++++++++++----- .../lsp/src/requests/signature_help/tests.rs | 27 ++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/tooling/lsp/src/requests/signature_help.rs b/tooling/lsp/src/requests/signature_help.rs index c0d40656c19..9847586fad1 100644 --- a/tooling/lsp/src/requests/signature_help.rs +++ b/tooling/lsp/src/requests/signature_help.rs @@ -122,10 +122,22 @@ impl<'a> SignatureFinder<'a> { active_parameter: Option, has_self: bool, ) -> SignatureInformation { + let enum_type_id = match (func_meta.type_id, func_meta.enum_variant_index) { + (Some(type_id), Some(_)) => Some(type_id), + _ => None, + }; + let mut label = String::new(); let mut parameters = Vec::new(); - label.push_str("fn "); + if let Some(enum_type_id) = enum_type_id { + label.push_str("enum "); + label.push_str(&self.interner.get_type(enum_type_id).borrow().name.0.contents); + label.push_str("::"); + } else { + label.push_str("fn "); + } + label.push_str(name); label.push('('); for (index, (pattern, typ, _)) in func_meta.parameters.0.iter().enumerate() { @@ -142,8 +154,10 @@ impl<'a> SignatureFinder<'a> { } else { let parameter_start = label.chars().count(); - self.hir_pattern_to_argument(pattern, &mut label); - label.push_str(": "); + if enum_type_id.is_none() { + self.hir_pattern_to_argument(pattern, &mut label); + label.push_str(": "); + } label.push_str(&typ.to_string()); let parameter_end = label.chars().count(); @@ -159,11 +173,13 @@ impl<'a> SignatureFinder<'a> { } label.push(')'); - match &func_meta.return_type { - FunctionReturnType::Default(_) => (), - FunctionReturnType::Ty(typ) => { - label.push_str(" -> "); - label.push_str(&typ.to_string()); + if enum_type_id.is_none() { + match &func_meta.return_type { + FunctionReturnType::Default(_) => (), + FunctionReturnType::Ty(typ) => { + label.push_str(" -> "); + label.push_str(&typ.to_string()); + } } } diff --git a/tooling/lsp/src/requests/signature_help/tests.rs b/tooling/lsp/src/requests/signature_help/tests.rs index 4b3f3c38156..0aedc256652 100644 --- a/tooling/lsp/src/requests/signature_help/tests.rs +++ b/tooling/lsp/src/requests/signature_help/tests.rs @@ -240,4 +240,31 @@ mod signature_help_tests { assert_eq!(signature.active_parameter, Some(0)); } + + #[test] + async fn test_signature_help_for_enum_variant() { + let src = r#" + enum Enum { + Variant(Field, i32) + } + + fn bar() { + Enum::Variant(>|<(), ()); + } + "#; + + let signature_help = get_signature_help(src).await; + assert_eq!(signature_help.signatures.len(), 1); + + let signature = &signature_help.signatures[0]; + assert_eq!(signature.label, "enum Enum::Variant(Field, i32)"); + + let params = signature.parameters.as_ref().unwrap(); + assert_eq!(params.len(), 2); + + check_label(&signature.label, ¶ms[0].label, "Field"); + check_label(&signature.label, ¶ms[1].label, "i32"); + + assert_eq!(signature.active_parameter, Some(0)); + } } From b8118c8ab2838195bb291aea3dd3483f57c5a0bb Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 11:36:34 -0300 Subject: [PATCH 7/9] Improve hover for enum variant --- .../noirc_frontend/src/elaborator/enums.rs | 5 +- compiler/noirc_frontend/src/elaborator/mod.rs | 23 ++-- .../requests/completion/completion_items.rs | 8 +- tooling/lsp/src/requests/completion/tests.rs | 2 + tooling/lsp/src/requests/hover.rs | 127 ++++++++++++------ .../test_programs/workspace/two/src/lib.nr | 6 +- 6 files changed, 111 insertions(+), 60 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 0c65ad7aafa..2ccd2b25561 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -2,7 +2,7 @@ use iter_extended::vecmap; use noirc_errors::Location; use crate::{ - ast::{Documented, EnumVariant, FunctionKind, NoirEnumeration, UnresolvedType, Visibility}, + ast::{EnumVariant, FunctionKind, NoirEnumeration, UnresolvedType, Visibility}, hir_def::{ expr::{HirEnumConstructorExpression, HirExpression, HirIdent}, function::{FuncMeta, FunctionBody, HirFunction, Parameters}, @@ -21,14 +21,13 @@ impl Elaborator<'_> { &mut self, enum_: &NoirEnumeration, type_id: TypeId, - documented_variant: &Documented, + variant: &EnumVariant, variant_arg_types: Vec, variant_index: usize, datatype: &Shared, self_type: &Type, self_type_unresolved: UnresolvedType, ) { - let variant = &documented_variant.item; let name_string = variant.name.to_string(); let datatype_ref = datatype.borrow(); let location = Location::new(variant.name.span(), self.file); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index b75fa57f34f..bf7fcceb32d 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -11,26 +11,24 @@ use crate::{ }, graph::CrateId, hir::{ - def_collector::dc_crate::{ - filter_literal_globals, CompilationError, ImplMap, UnresolvedFunctions, - UnresolvedGlobal, UnresolvedStruct, UnresolvedTraitImpl, UnresolvedTypeAlias, - }, def_collector::{ - dc_crate::{CollectedItems, UnresolvedEnum}, + dc_crate::{ + filter_literal_globals, CollectedItems, CompilationError, ImplMap, UnresolvedEnum, + UnresolvedFunctions, UnresolvedGlobal, UnresolvedStruct, UnresolvedTraitImpl, + UnresolvedTypeAlias, + }, errors::DefCollectorErrorKind, }, - def_map::{DefMaps, ModuleData}, - def_map::{LocalModuleId, ModuleId, MAIN_FUNCTION}, + def_map::{DefMaps, LocalModuleId, ModuleData, ModuleId, MAIN_FUNCTION}, resolution::errors::ResolverError, scope::ScopeForest as GenericScopeForest, type_check::{generics::TraitGenerics, TypeCheckError}, Context, }, - hir_def::traits::TraitImpl, hir_def::{ expr::{HirCapturedVar, HirIdent}, function::{FuncMeta, FunctionBody, HirFunction}, - traits::TraitConstraint, + traits::{TraitConstraint, TraitImpl}, types::{Generics, Kind, ResolvedGeneric}, }, node_interner::{ @@ -1834,6 +1832,8 @@ impl<'context> Elaborator<'context> { datatype.borrow_mut().init_variants(); + let module_id = ModuleId { krate: self.crate_id, local_id: typ.module_id }; + for (i, variant) in typ.enum_def.variants.iter().enumerate() { let types = vecmap(&variant.item.parameters, |typ| self.resolve_type(typ.clone())); let name = variant.item.name.clone(); @@ -1843,7 +1843,7 @@ impl<'context> Elaborator<'context> { self.define_enum_variant_function( &typ.enum_def, *type_id, - &variant, + &variant.item, types, i, &datatype, @@ -1851,7 +1851,8 @@ impl<'context> Elaborator<'context> { unresolved.clone(), ); - self.interner.add_definition_location(ReferenceId::EnumVariant(*type_id, i), None); + let reference_id = ReferenceId::EnumVariant(*type_id, i); + self.interner.add_definition_location(reference_id, Some(module_id)); } } } diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 5131f9554a1..039b745172b 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -428,6 +428,8 @@ impl<'a> NodeFinder<'a> { function_kind: FunctionKind, skip_first_argument: bool, ) -> String { + let is_enum_variant = func_meta.enum_variant_index.is_some(); + let mut text = String::new(); text.push_str(name); text.push('('); @@ -457,7 +459,11 @@ impl<'a> NodeFinder<'a> { text.push_str("${"); text.push_str(&index.to_string()); text.push(':'); - self.hir_pattern_to_argument(pattern, &mut text); + if is_enum_variant { + text.push_str("()"); + } else { + self.hir_pattern_to_argument(pattern, &mut text); + } text.push('}'); index += 1; diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index dcb622fd7e1..a3cd6b0d024 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -3101,6 +3101,8 @@ fn main() { assert_eq!(item.detail, Some("Variant(Field, i32)".to_string())); + assert_eq!(item.insert_text, Some("Variant(${1:()}, ${2:()})".to_string())); + let Documentation::MarkupContent(markdown) = item.documentation.as_ref().unwrap() else { panic!("Expected markdown docs"); }; diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 318965cec25..67467f01c15 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -380,9 +380,19 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { let func_name_definition_id = args.interner.definition(func_meta.name.id); + let enum_variant = match (func_meta.type_id, func_meta.enum_variant_index) { + (Some(type_id), Some(index)) => Some((type_id, index)), + _ => None, + }; + + let reference_id = if let Some((type_id, variant_index)) = enum_variant { + ReferenceId::EnumVariant(type_id, variant_index) + } else { + ReferenceId::Function(id) + }; + let mut string = String::new(); - let formatted_parent_module = - format_parent_module(ReferenceId::Function(id), args, &mut string); + let formatted_parent_module = format_parent_module(reference_id, args, &mut string); let formatted_parent_type = if let Some(trait_impl_id) = func_meta.trait_impl { let trait_impl = args.interner.get_trait_implementation(trait_impl_id); @@ -447,21 +457,23 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { string.push_str("::"); } string.push_str(&data_type.name.0.contents); - string.push('\n'); - string.push_str(" "); - string.push_str("impl"); + if enum_variant.is_none() { + string.push('\n'); + string.push_str(" "); + string.push_str("impl"); - let impl_generics: Vec<_> = func_meta - .all_generics - .iter() - .take(func_meta.all_generics.len() - func_meta.direct_generics.len()) - .cloned() - .collect(); - format_generics(&impl_generics, &mut string); + let impl_generics: Vec<_> = func_meta + .all_generics + .iter() + .take(func_meta.all_generics.len() - func_meta.direct_generics.len()) + .cloned() + .collect(); + format_generics(&impl_generics, &mut string); - string.push(' '); - string.push_str(&data_type.name.0.contents); - format_generic_names(&impl_generics, &mut string); + string.push(' '); + string.push_str(&data_type.name.0.contents); + format_generic_names(&impl_generics, &mut string); + } true } else { @@ -488,7 +500,9 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { let func_name = &func_name_definition_id.name; - string.push_str("fn "); + if enum_variant.is_none() { + string.push_str("fn "); + } string.push_str(func_name); format_generics(&func_meta.direct_generics, &mut string); string.push('('); @@ -501,15 +515,19 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { string.push_str("&mut "); } - format_pattern(pattern, args.interner, &mut string); + if enum_variant.is_some() { + string.push_str(&format!("{}", typ)); + } else { + format_pattern(pattern, args.interner, &mut string); - // Don't add type for `self` param - if !is_self { - string.push_str(": "); - if matches!(visibility, Visibility::Public) { - string.push_str("pub "); + // Don't add type for `self` param + if !is_self { + 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 { @@ -519,28 +537,34 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { string.push(')'); - let return_type = func_meta.return_type(); - match return_type { - Type::Unit => (), - _ => { - string.push_str(" -> "); - string.push_str(&format!("{}", return_type)); + if enum_variant.is_none() { + let return_type = func_meta.return_type(); + match return_type { + Type::Unit => (), + _ => { + string.push_str(" -> "); + string.push_str(&format!("{}", return_type)); + } } - } - string.push_str(&go_to_type_links(return_type, args.interner, args.files)); + string.push_str(&go_to_type_links(return_type, args.interner, args.files)); + } - let had_doc_comments = - append_doc_comments(args.interner, ReferenceId::Function(id), &mut string); - if !had_doc_comments { - // If this function doesn't have doc comments, but it's a trait impl method, - // use the trait method doc comments. - if let Some(trait_impl_id) = func_meta.trait_impl { - let trait_impl = args.interner.get_trait_implementation(trait_impl_id); - let trait_impl = trait_impl.borrow(); - let trait_ = args.interner.get_trait(trait_impl.trait_id); - if let Some(func_id) = trait_.method_ids.get(func_name) { - append_doc_comments(args.interner, ReferenceId::Function(*func_id), &mut string); + if enum_variant.is_some() { + append_doc_comments(args.interner, reference_id, &mut string); + } else { + let had_doc_comments = append_doc_comments(args.interner, reference_id, &mut string); + if !had_doc_comments { + // If this function doesn't have doc comments, but it's a trait impl method, + // use the trait method doc comments. + if let Some(trait_impl_id) = func_meta.trait_impl { + let trait_impl = args.interner.get_trait_implementation(trait_impl_id); + let trait_impl = trait_impl.borrow(); + let trait_ = args.interner.get_trait(trait_impl.trait_id); + if let Some(func_id) = trait_.method_ids.get(func_name) { + let reference_id = ReferenceId::Function(*func_id); + append_doc_comments(args.interner, reference_id, &mut string); + } } } } @@ -1284,7 +1308,7 @@ mod hover_tests { assert!(hover_text.contains( " two enum Color { - Red, + Red(Field), } --- @@ -1300,7 +1324,22 @@ mod hover_tests { .await; assert!(hover_text.contains( " two::Color - Red + Red(Field) + +--- + + Like a tomato" + )); + } + + #[test] + async fn hover_on_enum_variant_in_call() { + let hover_text = + get_hover_text("workspace", "two/src/lib.nr", Position { line: 109, character: 12 }) + .await; + assert!(hover_text.contains( + " two::Color + Red(Field) --- diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index 0843b49c572..0baeb83d5c1 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -103,5 +103,9 @@ enum EmptyColor {} /// Red, blue, etc. enum Color { /// Like a tomato - Red, + Red(Field), +} + +fn test_enum() -> Color { + Color::Red(1) } From ee18dd8c6d511e44ed9ddc2b63ec984a781f24f5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 11:38:23 -0300 Subject: [PATCH 8/9] Don't show parameter name inlay hints for enum variants --- tooling/lsp/src/requests/inlay_hint.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 792dd4352bc..cbf4ed26ef9 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -173,6 +173,11 @@ impl<'a> InlayHintCollector<'a> { if let Some(ReferenceId::Function(func_id)) = referenced { let func_meta = self.interner.function_meta(&func_id); + // No hints for enum variants + if func_meta.enum_variant_index.is_some() { + return; + } + let mut parameters = func_meta.parameters.iter().peekable(); let mut parameters_count = func_meta.parameters.len(); From bd337610a6e050d60d14d56f510a7fbc392e4989 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 24 Jan 2025 12:16:14 -0300 Subject: [PATCH 9/9] Fixes after merge --- .../code_action/fill_struct_fields.rs | 13 +++------ tooling/lsp/src/requests/completion.rs | 15 +++++++---- tooling/lsp/src/requests/hover.rs | 27 +++++++++++++------ 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs index 8aa47160b38..fc8be7c5163 100644 --- a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -27,23 +27,16 @@ impl<'a> CodeActionFinder<'a> { let typ = self.interner.get_type(type_id); let typ = typ.borrow(); - // Might be an enum - if !typ.is_struct() { - return; - } - // First get all of the struct's fields - let mut fields = typ.get_fields_as_written(); + let Some(mut fields) = typ.get_fields_as_written() else { + return; + }; // Remove the ones that already exists in the constructor for (constructor_field, _) in &constructor.fields { fields.retain(|field| field.name.0.contents != constructor_field.0.contents); } - if fields.is_empty() { - return; - } - // Some fields are missing. Let's suggest a quick fix that adds them. let bytes = self.source.as_bytes(); let right_brace_index = span.end() as usize - 1; diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 03205836858..0c51772935a 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -207,8 +207,11 @@ impl<'a> NodeFinder<'a> { let struct_type = struct_type.borrow(); // First get all of the struct's fields - let mut fields: Vec<_> = - struct_type.get_fields_as_written().into_iter().enumerate().collect(); + let Some(fields) = struct_type.get_fields_as_written() else { + return; + }; + + let mut fields = fields.into_iter().enumerate().collect::>(); // Remove the ones that already exists in the constructor for (used_name, _) in &constructor_expression.fields { @@ -805,9 +808,11 @@ impl<'a> NodeFinder<'a> { prefix: &str, self_prefix: bool, ) { - for (field_index, (name, visibility, typ)) in - struct_type.get_fields_with_visibility(generics).iter().enumerate() - { + let Some(fields) = struct_type.get_fields_with_visibility(generics) else { + return; + }; + + for (field_index, (name, visibility, typ)) in fields.iter().enumerate() { if !struct_member_is_visible(struct_type.id, *visibility, self.module_id, self.def_maps) { continue; diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 67467f01c15..60c2a686a62 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -16,7 +16,8 @@ use noirc_frontend::{ DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, NodeInterner, ReferenceId, TraitId, TraitImplKind, TypeAliasId, TypeId, }, - DataType, Generics, Shared, Type, TypeAlias, TypeBinding, TypeVariable, + DataType, EnumVariant, Generics, Shared, StructField, Type, TypeAlias, TypeBinding, + TypeVariable, }; use crate::{ @@ -128,14 +129,20 @@ fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> Option String { let typ = args.interner.get_type(id); let typ = typ.borrow(); - if typ.is_struct() { - format_struct(&typ, args) + if let Some(fields) = typ.get_fields_as_written() { + format_struct(&typ, fields, args) + } else if let Some(variants) = typ.get_variants_as_written() { + format_enum(&typ, variants, args) } else { - format_enum(&typ, args) + unreachable!("Type should either be a struct or an enum") } } -fn format_struct(typ: &DataType, args: &ProcessRequestCallbackArgs) -> String { +fn format_struct( + typ: &DataType, + fields: Vec, + args: &ProcessRequestCallbackArgs, +) -> String { let mut string = String::new(); if format_parent_module(ReferenceId::Type(typ.id), args, &mut string) { string.push('\n'); @@ -145,7 +152,7 @@ fn format_struct(typ: &DataType, args: &ProcessRequestCallbackArgs) -> String { string.push_str(&typ.name.0.contents); format_generics(&typ.generics, &mut string); string.push_str(" {\n"); - for field in typ.get_fields_as_written() { + for field in fields { string.push_str(" "); string.push_str(&field.name.0.contents); string.push_str(": "); @@ -159,7 +166,11 @@ fn format_struct(typ: &DataType, args: &ProcessRequestCallbackArgs) -> String { string } -fn format_enum(typ: &DataType, args: &ProcessRequestCallbackArgs) -> String { +fn format_enum( + typ: &DataType, + variants: Vec, + args: &ProcessRequestCallbackArgs, +) -> String { let mut string = String::new(); if format_parent_module(ReferenceId::Type(typ.id), args, &mut string) { string.push('\n'); @@ -169,7 +180,7 @@ fn format_enum(typ: &DataType, args: &ProcessRequestCallbackArgs) -> String { string.push_str(&typ.name.0.contents); format_generics(&typ.generics, &mut string); string.push_str(" {\n"); - for field in typ.get_variants_as_written() { + for field in variants { string.push_str(" "); string.push_str(&field.name.0.contents);