Skip to content

Commit

Permalink
Create a dedicated SymbolDef::Variant to fix finding references of …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
mkaput committed Feb 5, 2025
1 parent 6dd5b25 commit 33ce3d5
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 22 deletions.
18 changes: 16 additions & 2 deletions src/ide/hover/render/definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,29 @@ 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;
md += &doc;
}
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 {
Expand Down
69 changes: 57 additions & 12 deletions src/lang/defs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -44,6 +44,7 @@ pub enum SymbolDef {
Variable(VariableDef),
ExprInlineMacro(SmolStr),
Member(MemberDef),
Variant(VariantDef),
Module(ModuleDef),
}

Expand Down Expand Up @@ -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)))
}

Expand All @@ -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)
}
}
}

Expand All @@ -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),
}
}
Expand All @@ -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),
}
}
Expand Down Expand Up @@ -321,7 +327,7 @@ impl VariableDef {
#[derive(Eq, PartialEq)]
pub struct MemberDef {
member_id: MemberId,
structure: ItemDef,
struct_item: ItemDef,
definition_stable_ptr: SyntaxStablePtrId,
}

Expand All @@ -333,7 +339,11 @@ impl MemberDef {
definition_node: SyntaxNode,
) -> Option<Self> {
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.
Expand All @@ -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.
Expand All @@ -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<Self> {
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 {
Expand Down
61 changes: 53 additions & 8 deletions tests/e2e/find_references/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
}
Expand All @@ -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 { Ba<caret>r, Baz }
fn main() {
let foo = Foo::Bar;
match foo {
Foo::Bar => {},
_ => {}
}
}
"#, @r"
enum Foo { <sel=declaration>Bar</sel>, Baz }
fn main() {
let foo = Foo::<sel>Bar</sel>;
match foo {
Foo::<sel>Bar</sel> => {},
_ => {}
}
}
")
}

#[test]
fn enum_variants_via_expr() {
test_transform!(find_references, r#"
enum Foo { Bar, Baz }
fn main() {
let foo = Foo::Ba<caret>r;
match foo {
Foo::Bar => {}
Foo::Bar => {},
_ => {}
}
}
"#, @r"
enum Foo { <sel=declaration>Bar</sel>, Baz }
fn main() {
let foo = Foo::<sel>Bar</sel>;
match foo {
Foo::<sel>Bar</sel> => {},
_ => {}
}
}
")
}

#[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::B<caret>ar => {},
_ => {}
}
}
"#, @r"
enum Foo { <sel=declaration>Bar</sel>, Baz }
fn main() {
let foo = Foo::<sel>Bar</sel>;
match foo {
Foo::<sel>Bar</sel> => {},
_ => {}
}
}
Expand Down

0 comments on commit 33ce3d5

Please sign in to comment.