From 5964758765c35a4810e3f88c8241a7a8c12047b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sm=C3=B3=C5=82ka?= Date: Thu, 31 Oct 2024 10:55:07 +0100 Subject: [PATCH] LS: Add "fill struct fields" code action commit-id:0051578a --- .../ide/code_actions/fill_struct_fields.rs | 127 ++++++++++++++++++ .../src/ide/code_actions/mod.rs | 104 +++++++++----- .../tests/e2e/code_actions.rs | 1 + .../code_actions/fill_struct_fields.txt | 88 ++++++++++++ crates/cairo-lang-semantic/src/diagnostic.rs | 1 + .../src/expr/test_data/pattern | 4 +- 6 files changed, 286 insertions(+), 39 deletions(-) create mode 100644 crates/cairo-lang-language-server/src/ide/code_actions/fill_struct_fields.rs create mode 100644 crates/cairo-lang-language-server/tests/test_data/code_actions/fill_struct_fields.txt diff --git a/crates/cairo-lang-language-server/src/ide/code_actions/fill_struct_fields.rs b/crates/cairo-lang-language-server/src/ide/code_actions/fill_struct_fields.rs new file mode 100644 index 00000000000..524cddca625 --- /dev/null +++ b/crates/cairo-lang-language-server/src/ide/code_actions/fill_struct_fields.rs @@ -0,0 +1,127 @@ +use std::collections::HashMap; + +use cairo_lang_defs::ids::LanguageElementId; +use cairo_lang_semantic::Expr; +use cairo_lang_semantic::db::SemanticGroup; +use cairo_lang_semantic::items::function_with_body::SemanticExprLookup; +use cairo_lang_semantic::items::visibility::peek_visible_in; +use cairo_lang_semantic::lookup_item::LookupItemEx; +use cairo_lang_syntax::node::ast::{ExprStructCtorCall, StructArg}; +use cairo_lang_syntax::node::kind::SyntaxKind; +use cairo_lang_syntax::node::{SyntaxNode, TypedSyntaxNode}; +use lsp_types::{CodeAction, CodeActionKind, CodeActionParams, Range, TextEdit, WorkspaceEdit}; +use tracing::error; + +use crate::lang::db::{AnalysisDatabase, LsSemanticGroup, LsSyntaxGroup}; +use crate::lang::lsp::ToLsp; + +/// Generates a completion adding all visible struct members that have not yet been specified +/// to the constructor call, filling their values with a placeholder unit type. +pub fn fill_struct_fields( + db: &AnalysisDatabase, + node: SyntaxNode, + params: &CodeActionParams, +) -> Option { + let module_file_id = db.find_module_file_containing_node(&node)?; + let module_id = module_file_id.0; + let file_id = module_file_id.file_id(db).ok()?; + let function_id = db.find_lookup_item(&node)?.function_with_body()?; + + let constructor = db.first_ancestor_of_kind(node, SyntaxKind::ExprStructCtorCall)?; + let constructor_expr = ExprStructCtorCall::from_syntax_node(db, constructor.clone()); + + let mut last_important_element = None; + let mut has_trailing_comma = false; + + for node in constructor.descendants(db) { + match node.kind(db) { + SyntaxKind::TokenComma => { + has_trailing_comma = true; + last_important_element = Some(node) + } + SyntaxKind::StructArgSingle => { + has_trailing_comma = false; + last_important_element = Some(node) + } + // Don't complete any fields if initialization contains tail. + SyntaxKind::StructArgTail => return None, + _ => {} + } + } + + let code_prefix = String::from(if !has_trailing_comma && last_important_element.is_some() { + ", " + } else { + " " + }); + + let struct_arguments = constructor_expr.arguments(db); + let left_brace = struct_arguments.lbrace(db); + let struct_arguments = struct_arguments.arguments(db).elements(db); + + let already_present_arguments = struct_arguments + .iter() + .map(|member| match member { + StructArg::StructArgSingle(argument) => { + argument.identifier(db).token(db).as_syntax_node().get_text_without_trivia(db) + } + StructArg::StructArgTail(_) => unreachable!(), + }) + .collect::>(); + + let constructor_expr_id = + db.lookup_expr_by_ptr(function_id, constructor_expr.stable_ptr().into()).ok()?; + + let constructor_semantic = match db.expr_semantic(function_id, constructor_expr_id) { + Expr::StructCtor(semantic) => semantic, + _ => { + error!( + "Semantic expression obtained from StructCtorCall doesn't refer to constructor." + ); + return None; + } + }; + + let concrete_struct_id = constructor_semantic.concrete_struct_id; + let struct_parent_module_id = concrete_struct_id.struct_id(db).parent_module(db); + + let arguments_to_complete = db + .concrete_struct_members(concrete_struct_id) + .ok()? + .iter() + .filter_map(|(name, member)| { + let name = name.to_string(); + + if already_present_arguments.contains(&name) { + None + } else if peek_visible_in(db, member.visibility, struct_parent_module_id, module_id) { + Some(format!("{name}: ()")) + } else { + None + } + }) + .collect::>(); + + let code_to_insert = code_prefix + &arguments_to_complete.join(", "); + + let edit_start = last_important_element + .unwrap_or(left_brace.as_syntax_node()) + .span_end_without_trivia(db) + .position_in_file(db, file_id)? + .to_lsp(); + + let mut changes = HashMap::new(); + let url = params.text_document.uri.clone(); + let change = TextEdit { range: Range::new(edit_start, edit_start), new_text: code_to_insert }; + + changes.insert(url, vec![change]); + + let edit = WorkspaceEdit::new(changes); + + Some(CodeAction { + title: String::from("Fill struct fields"), + kind: Some(CodeActionKind::QUICKFIX), + edit: Some(edit), + ..Default::default() + }) +} diff --git a/crates/cairo-lang-language-server/src/ide/code_actions/mod.rs b/crates/cairo-lang-language-server/src/ide/code_actions/mod.rs index 1fbb092d9bf..3d0f0da8330 100644 --- a/crates/cairo-lang-language-server/src/ide/code_actions/mod.rs +++ b/crates/cairo-lang-language-server/src/ide/code_actions/mod.rs @@ -1,15 +1,18 @@ use cairo_lang_syntax::node::SyntaxNode; +use cairo_lang_utils::ordered_hash_map::{Entry, OrderedHashMap}; +use itertools::Itertools; use lsp_types::{ CodeAction, CodeActionOrCommand, CodeActionParams, CodeActionResponse, Diagnostic, NumberOrString, }; -use tracing::debug; +use tracing::{debug, warn}; use crate::lang::db::{AnalysisDatabase, LsSyntaxGroup}; use crate::lang::lsp::{LsProtoGroup, ToCairo}; mod add_missing_trait; mod expand_macro; +mod fill_struct_fields; mod rename_unused_variable; /// Compute commands for a given text document and range. These commands are typically code fixes to @@ -18,61 +21,88 @@ pub fn code_actions(params: CodeActionParams, db: &AnalysisDatabase) -> Option Vec { - let code = match &diagnostic.code { - Some(NumberOrString::String(code)) => code, - Some(NumberOrString::Number(code)) => { - debug!("diagnostic code is not a string: `{code}`"); - return vec![]; - } - None => { - debug!("diagnostic code is missing"); - return vec![]; - } - }; + let mut diagnostic_groups_by_codes: OrderedHashMap> = + OrderedHashMap::default(); - match code.as_str() { - "E0001" => { - vec![rename_unused_variable::rename_unused_variable( - db, - node, - diagnostic.clone(), - params.text_document.uri.clone(), - )] + for diagnostic in params.context.diagnostics.iter() { + if let Some(code) = extract_code(diagnostic) { + match diagnostic_groups_by_codes.entry(code.to_owned()) { + Entry::Occupied(mut entry) => { + entry.get_mut().push(diagnostic); + } + Entry::Vacant(entry) => { + entry.insert(vec![diagnostic]); + } + } } - "E0002" => add_missing_trait::add_missing_trait(db, node, params.text_document.uri.clone()), - code => { - debug!("no code actions for diagnostic code: {code}"); - vec![] + } + + diagnostic_groups_by_codes + .into_iter() + .flat_map(|(code, diagnostics)| match code.as_str() { + "E0001" => diagnostics + .into_iter() + .map(|diagnostic| { + rename_unused_variable::rename_unused_variable( + db, + node, + diagnostic.clone(), + params.text_document.uri.clone(), + ) + }) + .collect_vec(), + "E0002" => { + add_missing_trait::add_missing_trait(db, node, params.text_document.uri.clone()) + } + "E0003" => fill_struct_fields::fill_struct_fields(db, node.clone(), params) + .map(|result| vec![result]) + .unwrap_or_default(), + _ => { + debug!("no code actions for diagnostic code: {code}"); + vec![] + } + }) + .collect_vec() +} + +/// Extracts [`Diagnostic`] code if it's given as a string, returns None otherwise. +fn extract_code(diagnostic: &Diagnostic) -> Option<&str> { + match &diagnostic.code { + Some(NumberOrString::String(code)) => Some(code), + Some(NumberOrString::Number(code)) => { + warn!("diagnostic code is not a string: `{code}`"); + None } + None => None, } } diff --git a/crates/cairo-lang-language-server/tests/e2e/code_actions.rs b/crates/cairo-lang-language-server/tests/e2e/code_actions.rs index 047ae40f7ae..823d75b78b4 100644 --- a/crates/cairo-lang-language-server/tests/e2e/code_actions.rs +++ b/crates/cairo-lang-language-server/tests/e2e/code_actions.rs @@ -14,6 +14,7 @@ cairo_lang_test_utils::test_file_test!( { missing_trait: "missing_trait.txt", macro_expand: "macro_expand.txt", + fill_struct_fields: "fill_struct_fields.txt", }, test_quick_fix ); diff --git a/crates/cairo-lang-language-server/tests/test_data/code_actions/fill_struct_fields.txt b/crates/cairo-lang-language-server/tests/test_data/code_actions/fill_struct_fields.txt new file mode 100644 index 00000000000..eded152395f --- /dev/null +++ b/crates/cairo-lang-language-server/tests/test_data/code_actions/fill_struct_fields.txt @@ -0,0 +1,88 @@ +//! > Test filling missing members in struct constructor. + +//! > test_runner_name +test_quick_fix + +//! > cairo_project.toml +[crate_roots] +hello = "src" + +[config.global] +edition = "2024_07" + +//! > cairo_code +mod some_module { + pub struct Struct { + x: u32, + pub y: felt252, + pub z: i16 + } + + fn build_struct() { + let s = Struct { + x: 0x0, + y: 0x0, + z: 0x0 + }; + + let _a = Struct { }; + + let _b = Struct { x: 0x0, }; + + let _c = Struct { ..s }; + } +} + +mod happy_cases { + use super::some_module::Struct; + + fn foo() { + let _a = Struct { }; + let _b = Struct { y: 0x0, }; + let _c = Struct { y: 0x0, x: 0x0, } + } +} + +mod unhappy_cases { + fn foo() { + let _a = NonexsitentStruct { }; + } +} + +//! > Code action #0 + let _a = Struct { }; +Title: Fill struct fields +Add new text: " x: (), y: (), z: ()" +At: Range { start: Position { line: 14, character: 25 }, end: Position { line: 14, character: 25 } } + +//! > Code action #1 + let _b = Struct { x: 0x0, }; +Title: Fill struct fields +Add new text: " y: (), z: ()" +At: Range { start: Position { line: 16, character: 33 }, end: Position { line: 16, character: 33 } } + +//! > Code action #2 + let _c = Struct { ..s }; +No code actions. + +//! > Code action #3 + let _a = Struct { }; +Title: Fill struct fields +Add new text: " y: (), z: ()" +At: Range { start: Position { line: 26, character: 25 }, end: Position { line: 26, character: 25 } } + +//! > Code action #4 + let _b = Struct { y: 0x0, }; +Title: Fill struct fields +Add new text: " z: ()" +At: Range { start: Position { line: 27, character: 33 }, end: Position { line: 27, character: 33 } } + +//! > Code action #5 + let _c = Struct { y: 0x0, x: 0x0, } +Title: Fill struct fields +Add new text: " z: ()" +At: Range { start: Position { line: 28, character: 41 }, end: Position { line: 28, character: 41 } } + +//! > Code action #6 + let _a = NonexsitentStruct { }; +No code actions. diff --git a/crates/cairo-lang-semantic/src/diagnostic.rs b/crates/cairo-lang-semantic/src/diagnostic.rs index 290e343897b..7435e8443b0 100644 --- a/crates/cairo-lang-semantic/src/diagnostic.rs +++ b/crates/cairo-lang-semantic/src/diagnostic.rs @@ -1256,6 +1256,7 @@ impl SemanticDiagnosticKind { Self::CannotCallMethod { .. } => { error_code!(E0002) } + Self::MissingMember(_) => error_code!(E0003), _ => return None, }) } diff --git a/crates/cairo-lang-semantic/src/expr/test_data/pattern b/crates/cairo-lang-semantic/src/expr/test_data/pattern index 4f9e9e0141c..85a5bf3cbc4 100644 --- a/crates/cairo-lang-semantic/src/expr/test_data/pattern +++ b/crates/cairo-lang-semantic/src/expr/test_data/pattern @@ -65,7 +65,7 @@ struct MyStruct { } //! > expected_diagnostics -error: Missing member "b". +error[E0003]: Missing member "b". --> lib.cairo:6:9 let MyStruct { a: _ } = s; ^***************^ @@ -163,7 +163,7 @@ error: Redefinition of member "a" on struct "test::MyStruct". let MyStruct { a: _, c: _, a: _ } = s; ^ -error: Missing member "b". +error[E0003]: Missing member "b". --> lib.cairo:6:9 let MyStruct { a: _, c: _, a: _ } = s; ^***************************^