LS: Fix hover on definition

[Draggu]

fix #5826

---

This change is 

[piotmag769]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @mkaput, and @orizi)

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 216 at r1 (raw file):

fn main()

Well xD Seems like it doesn't work :c

[orizi]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, and @mkaput)

[Draggu]

Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @Arcticae, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 216 at r1 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Well xD Seems like it doesn't work :c

Done.

[piotmag769]

Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, and @orizi)

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 323 at r2 (raw file):

impl RectangleImpl of RectangleTrait

This one seems wrong

[orizi]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, and @mkaput)

[mkaput]

Reviewed 1 of 4 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

---

-- commits line 4 at r2:

Suggestion:

- 590e171: LS: Fix hover on definition

  fix #5826

  commit-id:7654b189

---

crates/cairo-lang-language-server/src/lib.rs line 999 at r2 (raw file):

    }

    for lookup_item_id in lookup_items.iter().rev().copied() {

please add a comment stating that you're looking for variable definitions here

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 323 at r2 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

This one seems wrong

I'm not sure. Technically, it is because we're inside an impl so we are in a concrete context. But UX-wise, it would also be nice to get trait info alongside. Topic for a separate issue nonetheless.

[mkaput]

Reviewed 1 of 2 files at r3, all commit messages.
Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/lib.rs line 1000 at r3 (raw file):

    }

    // Skip variable definition.

skip?

---

crates/cairo-lang-language-server/src/lib.rs line 1021 at r3 (raw file):

    } else {
        None
    }

let's stick with single style of returning

Suggestion:

    // Skip variable definition.
    if db.first_ancestor_of_kind(identifier.as_syntax_node(), SyntaxKind::StatementLet).is_none() {
        let item = match lookup_items.first().copied()? {
            LookupItemId::ModuleItem(item) => {
                ResolvedGenericItem::from_module_item(db, item).to_option()?
            }
            LookupItemId::TraitItem(trait_item) => {
                if let TraitItemId::Function(trait_fn) = trait_item {
                    ResolvedGenericItem::TraitFunction(trait_fn)
                } else {
                    ResolvedGenericItem::Trait(trait_item.trait_id(db))
                }
            }
            LookupItemId::ImplItem(impl_item) => {
                ResolvedGenericItem::Impl(impl_item.impl_def_id(db))
            }
        };

        return Some((ResolvedItem::Generic(item.clone()), resolved_generic_item_def(db, item)?));
    }
    
    None

[Draggu]

Reviewable status: 2 of 4 files reviewed, 3 unresolved discussions (waiting on @Arcticae, @mkaput, @orizi, and @piotmag769)

---

-- commits line 4 at r2:
Done.

---

crates/cairo-lang-language-server/src/lib.rs line 999 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

please add a comment stating that you're looking for variable definitions here

Done.

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 323 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I'm not sure. Technically, it is because we're inside an impl so we are in a concrete context. But UX-wise, it would also be nice to get trait info alongside. Topic for a separate issue nonetheless.

Not a simple fix. Track it here #6287

[mkaput]

Reviewed 1 of 2 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Arcticae and @piotmag769)

[piotmag769]

Reviewed 1 of 4 files at r1, 1 of 3 files at r2, 1 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Arcticae, @Draggu, and @mkaput)

---

crates/cairo-lang-language-server/src/lib.rs line 1007 at r3 (raw file):

            }
            LookupItemId::TraitItem(trait_item) => {
                if let TraitItemId::Function(trait_fn) = trait_item {

Why is it here, but in the impl we don't do it?

---

crates/cairo-lang-language-server/src/lib.rs line 1021 at r3 (raw file):

Previously, mkaput (Marek Kaput) wrote…

let's stick with single style of returning

Imo the one you proposed is worse

[mkaput]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Arcticae and @Draggu)

[Arcticae]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Draggu)

[Draggu]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mkaput and @piotmag769)

---

crates/cairo-lang-language-server/src/lib.rs line 1000 at r3 (raw file):

Previously, mkaput (Marek Kaput) wrote…

skip?

If this check is missing we would get parent ModuleItem for variable.

---

crates/cairo-lang-language-server/src/lib.rs line 1007 at r3 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Why is it here, but in the impl we don't do it?

Because ResolvedGenericItem has only 1 variant for Impl and 2 for Trait.

[mkaput]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Draggu and @piotmag769)

---

crates/cairo-lang-language-server/src/lib.rs line 1000 at r3 (raw file):

Previously, Draggu (Piotr Figiela) wrote…

If this check is missing we would get parent ModuleItem for variable.

please leave this information in code as a comment

[piotmag769]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Draggu)

[piotmag769]

Reviewed 1 of 2 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Draggu)

[Draggu]

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @mkaput and @piotmag769)

---

crates/cairo-lang-language-server/src/lib.rs line 1000 at r3 (raw file):

Previously, mkaput (Marek Kaput) wrote…

please leave this information in code as a comment

Done.

[mkaput]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Draggu)