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

Provide intention to fill struct members #5945

Open mkaput opened 2 months ago

mkaput commented 2 months ago

Implement the following intention in CairoLS:

Image

the-first-elder commented 1 month ago

Hi @mkaput can you provide a link to this on the codebase

mkaput commented 1 month ago

Hey! Here's the starting point: https://github.com/starkware-libs/cairo/blob/cdaceb909ccf0644b7e385669d2c48417ceb90d4/crates/cairo-lang-language-server/src/ide/code_actions/mod.rs#L34-L78

You can follow the implementation of unused variable intention as the hooking point. First, you need to assign an error code to this particular diagnostic; you can grep for it by error message.

You will need to get information about struct members. This is provided in the SemanticGroup (this trait-Salsa group is implemented by AnalysisDatabase) by this query: https://github.com/starkware-libs/cairo/blob/50d465cd4facb6742f72d3d43f87378ebd92e13f/crates/cairo-lang-semantic/src/db.rs#L233-L238

the-first-elder commented 1 month ago

Hi @mkaput I'm having issues getting the struct_Id, also there is no fix struct field error, i created an error code for missing members

piotmag769 commented 1 month ago

@the-first-elder

there is no fix struct field error, i created an error code for missing members

That's good, that's what was supposed to be done!

I'm having issues getting the struct_Id

You need to get the StructId from the SyntaxNode when you already recognised that the MissingMember diagnostic was returned for the node. Since you can deduct (from the diagnostic) that the SyntaxNode contains struct expression, you need to find an expr node of appropriate kind (probably SyntaxKind::ExprStructCtorCal, but not sure about that). Checking the code from dot_completions may be helpful. Then when you get the type, do this:

https://github.com/starkware-libs/cairo/blob/cdaceb909ccf0644b7e385669d2c48417ceb90d4/crates/cairo-lang-language-server/src/ide/completion/completions.rs#L241-L244

and you have the member names 🙂

You can check this PR, maybe it will help you with implementation

mkaput commented 1 month ago

@the-first-elder any progress? how can we help?

the-first-elder commented 1 month ago

This is what i have currently done, I do need some guidance in getting it right.

pub fn fill_struct_fields(
    db: &AnalysisDatabase,
    node: &SyntaxNode,
    diagnostic: Diagnostic,
    uri: Url,
) -> CodeAction {
    // Resolve the StructId from the SyntaxNode
    let struct_id = resolve_struct_id(db, node).expect("Failed to resolve StructId");

    let members = get_struct_members(db.upcast(), struct_id)
    .expect("Failed to extract struct members");

    // Create the text edit
    let text_edit = TextEdit {
        range: diagnostic.range,
        new_text: format!("{{ {} }}", generate_default_values(&members)),
    };

    CodeAction {
        title: "Fill missing struct fields".to_string(),
        edit: Some(WorkspaceEdit {
            changes: Some(HashMap::from([(uri, vec![text_edit])])),
            document_changes: None,
            change_annotations: None,
        }),
        diagnostics: Some(vec![diagnostic]),
        ..Default::default()
    }
}

fn resolve_struct_id(db: &AnalysisDatabase, node: &SyntaxNode) -> Option<StructId> {
    let syntax_db = db.upcast();

    if node.kind(syntax_db) == SyntaxKind::ExprStructCtorCall {

        let lookup_item_id = db.find_lookup_item(&node)?;
        let function_id = lookup_item_id.function_with_body()?;
        let expr_id = db.lookup_expr_by_ptr(function_id, node.stable_ptr()).ok()?;
        let semantic_expr = db.expr_semantic(function_id, expr_id);

        let ty = semantic_expr.ty();
        if ty.is_missing(db) {
            return None;
        }

        let (_, long_ty) = peel_snapshots(db, ty);
        if let TypeLongId::Concrete(ConcreteTypeId::Struct(concrete_struct_id)) = long_ty {
            return Some(concrete_struct_id.struct_id(db));
        }
    }

    None
}

fn get_struct_members(db: &dyn SemanticGroup, struct_id: StructId) -> Maybe<OrderedHashMap<SmolStr, Member>> {
    db.struct_members(struct_id)
}

fn generate_default_values(members: &OrderedHashMap<SmolStr, Member>) -> String {
    members.iter().map(|(name, _)| format!("{}: default_value", name)).collect::<Vec<_>>().join(", ")
}
piotmag769 commented 1 month ago

@the-first-elder looks like a solid work! Have you tried if it works? You can do it via building the LS from source and setting

{    
    "cairo1.languageServerPath": "/Users/piotrmagiera/SWM/Dev/cairo/target/release/cairo-language-server",
    "cairo1.preferScarbLanguageServer": false,
 }

in .vscode/settings.json You can also access the file via command/crtl + shift + P + > ... open user settings (JSON) shortcut.

Regarding the implementation I am worried that checking if the syntax node is of kind ExprStructCtorCall directly may not work, it will be probably one of the parents of the node.

Regarding the default_value that you want to fill the struct members with I think it would be reasonable to check if the type has Default implementation: if so, we can just drop Default::default() there. Not sure what we should do if there is no Default implemented for the member, any idea @mkaput?

mkaput commented 1 month ago

Regarding the default_value that you want to fill the struct members with I think it would be reasonable to check if the type has Default implementation: if so, we can just drop Default::default() there. Not sure what we should do if there is no Default implemented for the member, any idea @mkaput?

Just put () (unit value/empty tuple) in such case and expect people to fix compilation errors.

the-first-elder commented 1 month ago

i get this when i try to hover Request textDocument/hover failed.

piotmag769 commented 1 month ago

Does it happen on your branch? Or the main one?

the-first-elder commented 1 month ago

mine but same on main as well

piotmag769 commented 1 month ago

Could you provide reproduction for that in another issue?

mkaput commented 2 weeks ago

Unassigning due to lack of activity.