From 33ce3d5b1576764133df55e85fafe7c48012ab15 Mon Sep 17 00:00:00 2001 From: Marek Kaput Date: Wed, 5 Feb 2025 11:22:11 +0100 Subject: [PATCH] Create a dedicated `SymbolDef::Variant` to fix finding references of enum variants Previously, `SymbolDef::Item` was used for enum variants. `ItemDef::name` returned the name of a **lookup item** related to the item, which for enum variants was **the enum**, not the variant itself. This caused a bug because usage finding uses the symbol's name as a text search needle, which made the algorithm attempt looking for references to the enum, yielding effectively no usage results. fix #129 - this was the last FIXME comment related to that issue that was left, even though the bug eventually appeared to be completely unrelated. commit-id:374fbe2d --- src/ide/hover/render/definition.rs | 18 +++++++- src/lang/defs.rs | 69 ++++++++++++++++++++++++------ tests/e2e/find_references/enums.rs | 61 ++++++++++++++++++++++---- 3 files changed, 126 insertions(+), 22 deletions(-) diff --git a/src/ide/hover/render/definition.rs b/src/ide/hover/render/definition.rs index 660ff76f..15778b1b 100644 --- a/src/ide/hover/render/definition.rs +++ b/src/ide/hover/render/definition.rs @@ -57,8 +57,8 @@ pub fn definition( // Signature is the signature of the struct, so it makes sense that the definition // path is too. - md += &fenced_code_block(&member.structure().definition_path(db)); - md += &fenced_code_block(&member.structure().signature(db)); + md += &fenced_code_block(&member.struct_item().definition_path(db)); + md += &fenced_code_block(&member.struct_item().signature(db)); if let Some(doc) = db.get_item_documentation(member.member_id().into()) { md += RULE; @@ -66,6 +66,20 @@ pub fn definition( } md } + SymbolDef::Variant(variant) => { + let mut md = String::new(); + + // Signature is the signature of the enum, so it makes sense that the definition + // path is too. + md += &fenced_code_block(&variant.enum_item().definition_path(db)); + md += &fenced_code_block(&variant.enum_item().signature(db)); + + if let Some(doc) = db.get_item_documentation(variant.variant_id().into()) { + md += RULE; + md += &doc; + } + md + } }; Some(Hover { diff --git a/src/lang/defs.rs b/src/lang/defs.rs index 4cb196da..56d8e3ed 100644 --- a/src/lang/defs.rs +++ b/src/lang/defs.rs @@ -4,7 +4,7 @@ use cairo_lang_defs::db::DefsGroup; use cairo_lang_defs::ids::{ EnumLongId, GenericTypeId, LanguageElementId, LocalVarLongId, LookupItemId, MemberId, ModuleId, ModuleItemId, NamedLanguageElementId, StructLongId, SubmoduleLongId, TopLevelLanguageElementId, - TraitItemId, VarId, + TraitItemId, VarId, VariantId, }; use cairo_lang_diagnostics::ToOption; use cairo_lang_doc::db::DocGroup; @@ -44,6 +44,7 @@ pub enum SymbolDef { Variable(VariableDef), ExprInlineMacro(SmolStr), Member(MemberDef), + Variant(VariantDef), Module(ModuleDef), } @@ -77,24 +78,19 @@ impl SymbolDef { | ResolvedItem::Generic(ResolvedGenericItem::GenericType(_)) | ResolvedItem::Generic(ResolvedGenericItem::GenericTypeAlias(_)) | ResolvedItem::Generic(ResolvedGenericItem::GenericImplAlias(_)) - | ResolvedItem::Generic(ResolvedGenericItem::Variant(_)) | ResolvedItem::Generic(ResolvedGenericItem::Trait(_)) | ResolvedItem::Generic(ResolvedGenericItem::Impl(_)) | ResolvedItem::Concrete(ResolvedConcreteItem::Constant(_)) | ResolvedItem::Concrete(ResolvedConcreteItem::Function(_)) | ResolvedItem::Concrete(ResolvedConcreteItem::Type(_)) - | ResolvedItem::Concrete(ResolvedConcreteItem::Variant(_)) | ResolvedItem::Concrete(ResolvedConcreteItem::Trait(_)) | ResolvedItem::Concrete(ResolvedConcreteItem::Impl(_)) | ResolvedItem::Concrete(ResolvedConcreteItem::SelfTrait(_)) => { ItemDef::new(db, &definition_node).map(Self::Item) } - ResolvedItem::Generic(ResolvedGenericItem::Module(id)) => { - Some(Self::Module(ModuleDef::new(db, id, definition_node))) - } - - ResolvedItem::Concrete(ResolvedConcreteItem::Module(id)) => { + ResolvedItem::Generic(ResolvedGenericItem::Module(id)) + | ResolvedItem::Concrete(ResolvedConcreteItem::Module(id)) => { Some(Self::Module(ModuleDef::new(db, id, definition_node))) } @@ -105,6 +101,14 @@ impl SymbolDef { ResolvedItem::Member(member_id) => { MemberDef::new(db, member_id, definition_node).map(Self::Member) } + + ResolvedItem::Generic(ResolvedGenericItem::Variant(variant)) => { + VariantDef::new(db, variant.id, definition_node).map(Self::Variant) + } + + ResolvedItem::Concrete(ResolvedConcreteItem::Variant(concrete_variant)) => { + VariantDef::new(db, concrete_variant.id, definition_node).map(Self::Variant) + } } } @@ -117,6 +121,7 @@ impl SymbolDef { Self::Variable(d) => Some(d.definition_stable_ptr()), Self::ExprInlineMacro(_) => None, Self::Member(d) => Some(d.definition_stable_ptr), + Self::Variant(d) => Some(d.definition_stable_ptr), Self::Module(d) => Some(d.definition_stable_ptr), } } @@ -141,6 +146,7 @@ impl SymbolDef { Self::Variable(it) => it.name(db), Self::ExprInlineMacro(name) => name.clone(), Self::Member(it) => it.name(db), + Self::Variant(it) => it.name(db), Self::Module(it) => it.name(db), } } @@ -321,7 +327,7 @@ impl VariableDef { #[derive(Eq, PartialEq)] pub struct MemberDef { member_id: MemberId, - structure: ItemDef, + struct_item: ItemDef, definition_stable_ptr: SyntaxStablePtrId, } @@ -333,7 +339,11 @@ impl MemberDef { definition_node: SyntaxNode, ) -> Option { let structure = ItemDef::new(db, &definition_node)?; - Some(Self { member_id, structure, definition_stable_ptr: definition_node.stable_ptr() }) + Some(Self { + member_id, + struct_item: structure, + definition_stable_ptr: definition_node.stable_ptr(), + }) } /// Gets [`MemberId`] associated with this symbol. @@ -342,8 +352,8 @@ impl MemberDef { } /// Gets a definition of the structure which this symbol is a member of. - pub fn structure(&self) -> &ItemDef { - &self.structure + pub fn struct_item(&self) -> &ItemDef { + &self.struct_item } /// Gets member's name. @@ -352,6 +362,41 @@ impl MemberDef { } } +/// Information about an enum variant. +#[derive(Eq, PartialEq)] +pub struct VariantDef { + variant_id: VariantId, + enum_item: ItemDef, + definition_stable_ptr: SyntaxStablePtrId, +} + +impl VariantDef { + /// Constructs a new [`VariantDef`] instance. + pub fn new( + db: &AnalysisDatabase, + variant_id: VariantId, + definition_node: SyntaxNode, + ) -> Option { + let enum_item = ItemDef::new(db, &definition_node)?; + Some(Self { variant_id, enum_item, definition_stable_ptr: definition_node.stable_ptr() }) + } + + /// Gets [`VariantId`] associated with this symbol. + pub fn variant_id(&self) -> VariantId { + self.variant_id + } + + /// Gets a definition of the enum which this symbol is a variant of. + pub fn enum_item(&self) -> &ItemDef { + &self.enum_item + } + + /// Gets variant's name. + pub fn name(&self, db: &AnalysisDatabase) -> SmolStr { + self.variant_id.name(db) + } +} + /// Information about the definition of a module. #[derive(Eq, PartialEq)] pub struct ModuleDef { diff --git a/tests/e2e/find_references/enums.rs b/tests/e2e/find_references/enums.rs index b079ddde..1ca13ad5 100644 --- a/tests/e2e/find_references/enums.rs +++ b/tests/e2e/find_references/enums.rs @@ -8,18 +8,18 @@ fn enum_name() { Bar, Baz, } - + fn main() { let foo = Foo::Bar; let foobar: Foo = foo; } - + fn calc(foo: Foo) {} - + mod rectangle { use super::Foo; } - + mod trick { struct Foo {} } @@ -46,24 +46,69 @@ fn enum_name() { ") } -// FIXME(#129): Pattern should also be selected. #[test] -fn enum_variants() { +fn enum_variants_via_declaration() { + test_transform!(find_references, r#" + enum Foo { Bar, Baz } + fn main() { + let foo = Foo::Bar; + match foo { + Foo::Bar => {}, + _ => {} + } + } + "#, @r" + enum Foo { Bar, Baz } + fn main() { + let foo = Foo::Bar; + match foo { + Foo::Bar => {}, + _ => {} + } + } + ") +} + +#[test] +fn enum_variants_via_expr() { test_transform!(find_references, r#" enum Foo { Bar, Baz } fn main() { let foo = Foo::Bar; match foo { - Foo::Bar => {} + Foo::Bar => {}, _ => {} } } "#, @r" enum Foo { Bar, Baz } + fn main() { + let foo = Foo::Bar; + match foo { + Foo::Bar => {}, + _ => {} + } + } + ") +} + +#[test] +fn enum_variants_via_pattern() { + test_transform!(find_references, r#" + enum Foo { Bar, Baz } fn main() { let foo = Foo::Bar; match foo { - Foo::Bar => {} + Foo::Bar => {}, + _ => {} + } + } + "#, @r" + enum Foo { Bar, Baz } + fn main() { + let foo = Foo::Bar; + match foo { + Foo::Bar => {}, _ => {} } }