Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
misicnenad opened this issue May 28, 2024 · 12 comments
Open
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed ide This issue refers to CairoLS or editor extensions

Comments

@misicnenad
Copy link

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:

  • create a new Scarb package
  • open lib.cairo in VS Code (ensure Cairo 1.0 extension is installed)
  • paste the code from "Related code" section below into the opened file
  • put the cursor inside any of the struct instance creation expressions (between curly brackets)
  • prompt the completion dialog to open (usually CTRL + Space)

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: #5660

@misicnenad misicnenad added the bug Something isn't working label May 28, 2024
@mkaput mkaput added good first issue Good for newcomers ide This issue refers to CairoLS or editor extensions labels May 28, 2024
@danielcdz
Copy link

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

@misicnenad
Copy link
Author

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

@danielcdz
Copy link

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

@mkaput
Copy link
Contributor

mkaput commented Jun 10, 2024

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

@danielcdz
Copy link

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

@mkaput
Copy link
Contributor

mkaput commented Jun 11, 2024

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

I think so

@mkaput
Copy link
Contributor

mkaput commented Jun 17, 2024

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

@danielcdz
Copy link

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

@danielcdz
Copy link

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
Copy link
Contributor

mkaput commented Jun 18, 2024

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

match node.kind(db) {
SyntaxKind::TerminalDot => {
let parent = node.parent().unwrap();
if parent.kind(db) == SyntaxKind::ExprBinary {
return CompletionKind::Dot(ast::ExprBinary::from_syntax_node(db, parent));
}
}
SyntaxKind::TerminalColonColon => {
let parent = node.parent().unwrap();
debug!("parent.kind: {:#?}", parent.kind(db));
if parent.kind(db) == SyntaxKind::ExprPath {
return completion_kind_from_path_node(db, parent);
}
let grandparent = parent.parent().unwrap();
debug!("grandparent.kind: {:#?}", grandparent.kind(db));
if grandparent.kind(db) == SyntaxKind::ExprPath {
return completion_kind_from_path_node(db, grandparent);
}
let (use_ast, should_pop) = if parent.kind(db) == SyntaxKind::UsePathLeaf {
(ast::UsePath::Leaf(ast::UsePathLeaf::from_syntax_node(db, parent)), true)
} else if grandparent.kind(db) == SyntaxKind::UsePathLeaf {
(ast::UsePath::Leaf(ast::UsePathLeaf::from_syntax_node(db, grandparent)), true)
} else if parent.kind(db) == SyntaxKind::UsePathSingle {
(ast::UsePath::Single(ast::UsePathSingle::from_syntax_node(db, parent)), false)
} else if grandparent.kind(db) == SyntaxKind::UsePathSingle {
(ast::UsePath::Single(ast::UsePathSingle::from_syntax_node(db, grandparent)), false)
} else {
debug!("Generic");
return CompletionKind::ColonColon(vec![]);
};
let mut segments = vec![];
let Ok(()) = get_use_segments(db.upcast(), &use_ast, &mut segments) else {
debug!("Generic");
return CompletionKind::ColonColon(vec![]);
};
if should_pop {
segments.pop();
}
debug!("ColonColon");
return CompletionKind::ColonColon(segments);
}
SyntaxKind::TerminalIdentifier => {
let parent = node.parent().unwrap();
debug!("parent.kind: {:#?}", parent.kind(db));
let grandparent = parent.parent().unwrap();
debug!("grandparent.kind: {:#?}", grandparent.kind(db));
if grandparent.kind(db) == SyntaxKind::ExprPath {
if db.get_children(grandparent.clone())[0].stable_ptr() != parent.stable_ptr() {
// Not the first segment.
debug!("Not first segment");
return completion_kind_from_path_node(db, grandparent);
}
// First segment.
let grandgrandparent = grandparent.parent().unwrap();
debug!("grandgrandparent.kind: {:#?}", grandgrandparent.kind(db));
if grandgrandparent.kind(db) == SyntaxKind::ExprBinary {
let expr = ast::ExprBinary::from_syntax_node(db, grandgrandparent.clone());
if matches!(
ast::ExprBinary::from_syntax_node(db, grandgrandparent).op(db),
ast::BinaryOperator::Dot(_)
) {
debug!("Dot");
return CompletionKind::Dot(expr);
}
}
}
if grandparent.kind(db) == SyntaxKind::UsePathLeaf {
let use_ast = ast::UsePathLeaf::from_syntax_node(db, grandparent);
let mut segments = vec![];
let Ok(()) =
get_use_segments(db.upcast(), &ast::UsePath::Leaf(use_ast), &mut segments)
else {
debug!("Generic");
return CompletionKind::ColonColon(vec![]);
};
segments.pop();
debug!("ColonColon");
return CompletionKind::ColonColon(segments);
}
}
_ => (),
}

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:

match completion_kind(db, node) {
CompletionKind::Dot(expr) => {
dot_completions(db, file_id, lookup_items, expr).map(CompletionResponse::Array)
}
CompletionKind::ColonColon(segments) if !segments.is_empty() => {
colon_colon_completions(db, module_file_id, lookup_items, segments)
.map(CompletionResponse::Array)
}
_ if trigger_kind == CompletionTriggerKind::INVOKED => {
Some(CompletionResponse::Array(generic_completions(db, module_file_id, lookup_items)))
}
_ => None,
}

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:

#[tracing::instrument(level = "trace", skip_all)]
pub fn dot_completions(
db: &(dyn SemanticGroup + 'static),
file_id: FileId,
lookup_items: Vec<LookupItemId>,
expr: ast::ExprBinary,
) -> Option<Vec<CompletionItem>> {
let syntax_db = db.upcast();
// Get a resolver in the current context.
let lookup_item_id = lookup_items.into_iter().next()?;
let function_with_body = lookup_item_id.function_with_body()?;
let module_id = function_with_body.module_file_id(db.upcast()).0;
let resolver_data = lookup_item_id.resolver_data(db).ok()?;
let resolver = Resolver::with_data(
db,
resolver_data.as_ref().clone_with_inference_id(db, InferenceId::NoContext),
);
// Extract lhs node.
let node = expr.lhs(syntax_db);
let stable_ptr = node.stable_ptr().untyped();
// Get its semantic model.
let expr_id = db.lookup_expr_by_ptr(function_with_body, node.stable_ptr()).ok()?;
let semantic_expr = db.expr_semantic(function_with_body, expr_id);
// Get the type.
let ty = semantic_expr.ty();
if ty.is_missing(db) {
debug!("type is missing");
return None;
}
// Find relevant methods for type.
let offset = if let Some(ModuleId::Submodule(submodule_id)) =
db.find_module_containing_node(&expr.as_syntax_node())
{
let module_def_ast = submodule_id.stable_ptr(db.upcast()).lookup(syntax_db);
if let ast::MaybeModuleBody::Some(body) = module_def_ast.body(syntax_db) {
body.items(syntax_db).as_syntax_node().span_start_without_trivia(syntax_db)
} else {
TextOffset::default()
}
} else {
TextOffset::default()
};
let position = offset.position_in_file(db.upcast(), file_id).unwrap().to_lsp();
let relevant_methods = find_methods_for_type(db, resolver, ty, stable_ptr);
let mut completions = Vec::new();
for trait_function in relevant_methods {
let Some(completion) = completion_for_method(db, module_id, trait_function, position)
else {
continue;
};
completions.push(completion);
}
// Find members of the type.
let (_, long_ty) = peel_snapshots(db, ty);
if let TypeLongId::Concrete(ConcreteTypeId::Struct(concrete_struct_id)) = long_ty {
db.concrete_struct_members(concrete_struct_id).ok()?.into_iter().for_each(
|(name, member)| {
let completion = CompletionItem {
label: name.to_string(),
detail: Some(member.ty.format(db.upcast())),
kind: Some(CompletionItemKind::FIELD),
..CompletionItem::default()
};
completions.push(completion);
},
);
}
Some(completions)
}

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):

pub trait SemanticStructEx<'a>: Upcast<dyn SemanticGroup + 'a> {
fn concrete_struct_members(
&self,
concrete_struct_id: ConcreteStructId,
) -> Maybe<OrderedHashMap<SmolStr, semantic::Member>> {
// TODO(spapini): Uphold the invariant that constructed ConcreteEnumId instances
// always have the correct number of generic arguments.
let db = self.upcast();
let generic_params = db.struct_generic_params(concrete_struct_id.struct_id(db))?;
let generic_args = concrete_struct_id.lookup_intern(db).generic_args;
let substitution = GenericSubstitution::new(&generic_params, &generic_args);
let generic_members =
self.upcast().struct_members(concrete_struct_id.struct_id(self.upcast()))?;
generic_members
.into_iter()
.map(|(name, member)| {
let ty =
SubstitutionRewriter { db, substitution: &substitution }.rewrite(member.ty)?;
let member = semantic::Member { ty, ..member };
Ok((name, member))
})
.collect::<Maybe<_>>()
}
}

@mkaput mkaput added the help wanted Extra attention is needed label Jun 19, 2024
@danielcdz
Copy link

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 😞

@danielcdz danielcdz removed their assignment Jun 20, 2024
@mkaput
Copy link
Contributor

mkaput commented Jun 20, 2024

@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! 😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed ide This issue refers to CairoLS or editor extensions
Projects
None yet
Development

No branches or pull requests

3 participants