starkware-libs / cairo

Cairo is the first Turing-complete language for creating provable programs for general computation.
Apache License 2.0
1.52k stars 470 forks source link

bug: LSP completion dialog should only recommend members of a struct #5664

Open 0xNeshi opened 3 months ago

0xNeshi commented 3 months ago

Bug Report

Cairo version: 2.6.3

Current behavior:

When creating a struct instance, there is no completion dialog support - the one that does appear contains some values that can only be described as random.

Example 1: struct in the same module: image

Example 2: public struct in a different module with a public field: image

Example 3: public struct in a different module with a private field: image

Expected behavior:

Completion dialog for struct instances should only list its available fields and nothing else.

Expected for examples above: Example 1: struct in the same module - should only list field. Example 2: public struct in a different module with a public field - should only list field_2. Example 3: public struct in a different module with a private field - should list nothing.

Steps to reproduce:

Related code:

#[derive(Drop)]
struct Type {
    field: felt252,
}

mod some_mod {
    #[derive(Drop, Debug)]
    pub struct SomeType {
        pub field_2: felt252
    }
}

mod another_mod {
    #[derive(Drop, Debug)]
    pub struct AnotherType {
        field_3: felt252
    }
}

fn main() {
    let struct_instance = Type {

    };
    let struct_instance_2 = some_mod::SomeType {

    };
    let struct_instance_3 = another_mod::AnotherType {

    };
}

Other information:

Related issue: https://github.com/starkware-libs/cairo/issues/5660

danielcdz commented 3 months ago

Hello @misicnenad, I would like to help with this one if possible!

0xNeshi commented 3 months ago

@danielcdz awesome, please check with @mkaput if he can assign this to you

danielcdz commented 3 months ago

ty @misicnenad, hey @mkaput can I help with this?

mkaput commented 3 months ago

@danielcdz yes of course! Task is assigned to you :) Thanks 💖

danielcdz commented 2 months ago

Hey @mkaput this is similar to #5660, can I follow the same instructions you left in the comments?

mkaput commented 2 months ago

Hey @mkaput this is similar to #5660, can I follow the same instructions you left in the comments?

I think so

mkaput commented 2 months ago

@danielcdz hey, how's it going? Would you like some assistance from my side in this area?

danielcdz commented 2 months ago

@mkaput that would be great! I would write some questions below 👇

danielcdz commented 2 months ago

I'm trying to look where the completions for a struct are being made but I didn't find anything useful, I saw that on issue #5660 you left some comments with examples of where completions are computed, can you help me with something similar for this 🙏

mkaput commented 2 months ago

It looks like you have to add a new completion kind for struct fields, here:

https://github.com/starkware-libs/cairo/blob/88f0506651106300e3d05bb3adf9eef0ee77ff24/crates/cairo-lang-language-server/src/ide/completion/mod.rs#L68-L149

This is basically traversing AST up to learn where you are. So you have to check out if you are within braces ({}) of struct construction expression. I guess either the left brace, or comma would be the nodes that you get at the input, but this is an exercise for you to investigate :)

Then, you have to hook completion implementation, starting here add a clause for your new completion kind:

https://github.com/starkware-libs/cairo/blob/88f0506651106300e3d05bb3adf9eef0ee77ff24/crates/cairo-lang-language-server/src/ide/completion/mod.rs#L45-L57

The body of this function is pretty wild west now. You have to get the semantic model of the struct that you are completing. I think you can find a lot of logic to copy in dot completions (i.e. foobar.<complete>), here:

https://github.com/starkware-libs/cairo/blob/57ffe1893a351f587e648e530ba0deb97e853ed8/crates/cairo-lang-language-server/src/ide/completion/completions.rs#L182-L254

For example, this is the function to get struct members (concrete, i.e. with generic parameters resolved to concrete types according to type inference context): https://github.com/starkware-libs/cairo/blob/448c234a612d9012b118f260ca47bcd33425f7d0/crates/cairo-lang-semantic/src/items/structure.rs#L260-L284

danielcdz commented 2 months ago

Hey @mkaput ty for all the explanations you gave me, I think I got the main idea on how to solve it, but I prefer to let this issue to a person more experienced in Rust, also I'm participating in a Hackathon and I don't have enough time to solve this issue at the moment, I'm sorry for this 😞

mkaput commented 2 months ago

@danielcdz understood :)

I think I got the main idea on how to solve it

Do you think you can add your own two cents to this? This would be a great contribution on its own! 😍

mkaput commented 1 week ago

@KevinMB0220 feel free to submit a PR. We're not assigning tasks to external contributors anymore