diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index 8e153bb0b46..95cdc0b88b4 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -7,26 +7,26 @@ use async_lsp::ResponseError; use fm::{FileId, FileMap, PathString}; use lsp_types::{ CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionParams, CodeActionResponse, - Position, Range, TextDocumentPositionParams, TextEdit, Url, WorkspaceEdit, + TextDocumentPositionParams, TextEdit, Url, WorkspaceEdit, }; -use noirc_errors::{Location, Span}; +use noirc_errors::Span; use noirc_frontend::{ - ast::{Ident, Path, Visitor}, + ast::{ConstructorExpression, Path, Visitor}, graph::CrateId, hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}, - macros_api::{ModuleDefId, NodeInterner}, + macros_api::NodeInterner, +}; +use noirc_frontend::{ parser::{Item, ItemKind, ParsedSubModule}, ParsedModule, }; -use crate::{ - byte_span_to_range, - modules::{get_parent_module_id, module_full_path, module_id_path}, - utils, LspState, -}; +use crate::{utils, LspState}; use super::{process_request, to_lsp_location}; +mod fill_struct_fields; +mod import_or_qualify; #[cfg(test)] mod tests; @@ -68,6 +68,7 @@ struct CodeActionFinder<'a> { uri: Url, files: &'a FileMap, file: FileId, + source: &'a str, lines: Vec<&'a str>, byte_index: usize, /// The module ID in scope. This might change as we traverse the AST @@ -108,6 +109,7 @@ impl<'a> CodeActionFinder<'a> { uri, files, file, + source, lines: source.lines().collect(), byte_index, module_id, @@ -137,46 +139,7 @@ impl<'a> CodeActionFinder<'a> { Some(code_actions) } - fn push_import_code_action(&mut self, full_path: &str) { - let line = self.auto_import_line as u32; - let character = (self.nesting * 4) as u32; - let indent = " ".repeat(self.nesting * 4); - let mut newlines = "\n"; - - // If the line we are inserting into is not an empty line, insert an extra line to make some room - if let Some(line_text) = self.lines.get(line as usize) { - if !line_text.trim().is_empty() { - newlines = "\n\n"; - } - } - - let title = format!("Import {}", full_path); - let text_edit = TextEdit { - range: Range { start: Position { line, character }, end: Position { line, character } }, - new_text: format!("use {};{}{}", full_path, newlines, indent), - }; - - let code_action = self.new_quick_fix(title, text_edit); - self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); - } - - fn push_qualify_code_action(&mut self, ident: &Ident, prefix: &str, full_path: &str) { - let Some(range) = byte_span_to_range( - self.files, - self.file, - ident.span().start() as usize..ident.span().start() as usize, - ) else { - return; - }; - - let title = format!("Qualify as {}", full_path); - let text_edit = TextEdit { range, new_text: format!("{}::", prefix) }; - - let code_action = self.new_quick_fix(title, text_edit); - self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); - } - - fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeAction { + fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeActionOrCommand { let mut changes = HashMap::new(); changes.insert(self.uri.clone(), vec![text_edit]); @@ -186,7 +149,7 @@ impl<'a> CodeActionFinder<'a> { change_annotations: None, }; - CodeAction { + CodeActionOrCommand::CodeAction(CodeAction { title, kind: Some(CodeActionKind::QUICKFIX), diagnostics: None, @@ -195,7 +158,7 @@ impl<'a> CodeActionFinder<'a> { is_preferred: None, disabled: None, data: None, - } + }) } fn includes_span(&self, span: Span) -> bool { @@ -244,69 +207,16 @@ impl<'a> Visitor for CodeActionFinder<'a> { } fn visit_path(&mut self, path: &Path) { - if path.segments.len() != 1 { - return; - } - - let ident = &path.segments[0].ident; - if !self.includes_span(ident.span()) { - return; - } - - let location = Location::new(ident.span(), self.file); - if self.interner.find_referenced(location).is_some() { - return; - } - - let current_module_parent_id = get_parent_module_id(self.def_maps, self.module_id); - - // The Path doesn't resolve to anything so it means it's an error and maybe we - // can suggest an import or to fully-qualify the path. - for (name, entries) in self.interner.get_auto_import_names() { - if name != &ident.0.contents { - continue; - } - - for (module_def_id, visibility, defining_module) in entries { - let module_full_path = if let Some(defining_module) = defining_module { - module_id_path( - *defining_module, - &self.module_id, - current_module_parent_id, - self.interner, - ) - } else { - let Some(module_full_path) = module_full_path( - *module_def_id, - *visibility, - self.module_id, - current_module_parent_id, - self.interner, - ) else { - continue; - }; - module_full_path - }; - - let full_path = if defining_module.is_some() - || !matches!(module_def_id, ModuleDefId::ModuleId(..)) - { - format!("{}::{}", module_full_path, name) - } else { - module_full_path.clone() - }; + self.import_or_qualify(path); + } - let qualify_prefix = if let ModuleDefId::ModuleId(..) = module_def_id { - let mut segments: Vec<_> = module_full_path.split("::").collect(); - segments.pop(); - segments.join("::") - } else { - module_full_path - }; + fn visit_constructor_expression( + &mut self, + constructor: &ConstructorExpression, + span: Span, + ) -> bool { + self.fill_struct_fields(constructor, span); - self.push_import_code_action(&full_path); - self.push_qualify_code_action(ident, &qualify_prefix, &full_path); - } - } + true } } diff --git a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs new file mode 100644 index 00000000000..f57fbc652ad --- /dev/null +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -0,0 +1,307 @@ +use lsp_types::TextEdit; +use noirc_errors::{Location, Span}; +use noirc_frontend::{ast::ConstructorExpression, node_interner::ReferenceId}; + +use crate::byte_span_to_range; + +use super::CodeActionFinder; + +impl<'a> CodeActionFinder<'a> { + pub(super) fn fill_struct_fields(&mut self, constructor: &ConstructorExpression, span: Span) { + if !self.includes_span(span) { + return; + } + + // Find out which struct this is + let location = Location::new(constructor.type_name.last_ident().span(), self.file); + let Some(ReferenceId::Struct(struct_id)) = self.interner.find_referenced(location) else { + return; + }; + + let struct_type = self.interner.get_struct(struct_id); + let struct_type = struct_type.borrow(); + + // First get all of the struct's fields + let mut fields = struct_type.get_fields_as_written(); + + // Remove the ones that already exists in the constructor + for (field, _) in &constructor.fields { + fields.retain(|(name, _)| name != &field.0.contents); + } + + if fields.is_empty() { + return; + } + + // Some fields are missing. Let's suggest a quick fix that adds them. + let bytes = self.source.as_bytes(); + let right_brace_index = span.end() as usize - 1; + let mut index = right_brace_index - 1; + while bytes[index].is_ascii_whitespace() { + index -= 1; + } + + let char_before_right_brace = bytes[index] as char; + + index += 1; + + let Some(range) = byte_span_to_range(self.files, self.file, index..index) else { + return; + }; + + // If the constructor spans multiple lines, we'll add the new fields in new lines too. + // Otherwise we'll add all the fields in a single line. + let constructor_range = + byte_span_to_range(self.files, self.file, span.start() as usize..span.end() as usize); + + // If it's multiline, find out the indent of the beginning line: we'll add new fields + // with that indent "plus one" (4 more spaces). + let line_indent = if let Some(constructor_range) = constructor_range { + if constructor_range.start.line == constructor_range.end.line { + None + } else { + let line = self.lines[constructor_range.start.line as usize]; + let whitespace_bytes = + line.bytes().take_while(|byte| byte.is_ascii_whitespace()).count(); + Some(whitespace_bytes) + } + } else { + None + }; + let line_indent = line_indent.map(|indent| " ".repeat(indent + 4)); + + let on_whitespace = bytes[index].is_ascii_whitespace(); + + let mut new_text = String::new(); + + // Add a comma if there's not a trailing one (if there are existing fields) + if !constructor.fields.is_empty() && char_before_right_brace != ',' { + new_text.push(','); + } + + // Add space or newline depending on whether it's multiline or not + if let Some(line_indent) = &line_indent { + new_text.push('\n'); + new_text.push_str(line_indent); + } else if !on_whitespace || constructor.fields.is_empty() { + new_text.push(' '); + } + + for (index, (name, _)) in fields.iter().enumerate() { + if index > 0 { + new_text.push(','); + if let Some(line_indent) = &line_indent { + new_text.push('\n'); + new_text.push_str(line_indent); + } else { + new_text.push(' '); + } + } + new_text.push_str(name); + new_text.push_str(": ()"); + } + + if !bytes[right_brace_index - 1].is_ascii_whitespace() { + new_text.push(' '); + } + + let title = "Fill struct fields".to_string(); + let text_edit = TextEdit { range, new_text }; + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } +} + +#[cfg(test)] +mod tests { + use tokio::test; + + use crate::requests::code_action::tests::assert_code_action; + + #[test] + async fn test_fill_struct_fields_code_action_no_space() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo {>|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { one: (), two: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_space() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { >|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { one: (), two: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_some_fields() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1>|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1, one: (), three: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_some_fields_trailing_comma() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1,>|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1, one: (), three: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_multiline_empty() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo {>|< + } + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { + one: (), + two: () + } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_multiline_some_fields() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo {>|< + one: 1, + } + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { + one: 1, + two: () + } + } + "#; + + assert_code_action(title, src, expected).await; + } +} diff --git a/tooling/lsp/src/requests/code_action/import_or_qualify.rs b/tooling/lsp/src/requests/code_action/import_or_qualify.rs new file mode 100644 index 00000000000..d07d117a317 --- /dev/null +++ b/tooling/lsp/src/requests/code_action/import_or_qualify.rs @@ -0,0 +1,240 @@ +use lsp_types::{Position, Range, TextEdit}; +use noirc_errors::Location; +use noirc_frontend::{ + ast::{Ident, Path}, + macros_api::ModuleDefId, +}; + +use crate::{ + byte_span_to_range, + modules::{get_parent_module_id, module_full_path, module_id_path}, +}; + +use super::CodeActionFinder; + +impl<'a> CodeActionFinder<'a> { + pub(super) fn import_or_qualify(&mut self, path: &Path) { + if path.segments.len() != 1 { + return; + } + + let ident = &path.segments[0].ident; + if !self.includes_span(ident.span()) { + return; + } + + let location = Location::new(ident.span(), self.file); + if self.interner.find_referenced(location).is_some() { + return; + } + + let current_module_parent_id = get_parent_module_id(self.def_maps, self.module_id); + + // The Path doesn't resolve to anything so it means it's an error and maybe we + // can suggest an import or to fully-qualify the path. + for (name, entries) in self.interner.get_auto_import_names() { + if name != &ident.0.contents { + continue; + } + + for (module_def_id, visibility, defining_module) in entries { + let module_full_path = if let Some(defining_module) = defining_module { + module_id_path( + *defining_module, + &self.module_id, + current_module_parent_id, + self.interner, + ) + } else { + let Some(module_full_path) = module_full_path( + *module_def_id, + *visibility, + self.module_id, + current_module_parent_id, + self.interner, + ) else { + continue; + }; + module_full_path + }; + + let full_path = if defining_module.is_some() + || !matches!(module_def_id, ModuleDefId::ModuleId(..)) + { + format!("{}::{}", module_full_path, name) + } else { + module_full_path.clone() + }; + + let qualify_prefix = if let ModuleDefId::ModuleId(..) = module_def_id { + let mut segments: Vec<_> = module_full_path.split("::").collect(); + segments.pop(); + segments.join("::") + } else { + module_full_path + }; + + self.push_import_code_action(&full_path); + self.push_qualify_code_action(ident, &qualify_prefix, &full_path); + } + } + } + + fn push_import_code_action(&mut self, full_path: &str) { + let line = self.auto_import_line as u32; + let character = (self.nesting * 4) as u32; + let indent = " ".repeat(self.nesting * 4); + let mut newlines = "\n"; + + // If the line we are inserting into is not an empty line, insert an extra line to make some room + if let Some(line_text) = self.lines.get(line as usize) { + if !line_text.trim().is_empty() { + newlines = "\n\n"; + } + } + + let title = format!("Import {}", full_path); + let text_edit = TextEdit { + range: Range { start: Position { line, character }, end: Position { line, character } }, + new_text: format!("use {};{}{}", full_path, newlines, indent), + }; + + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } + + fn push_qualify_code_action(&mut self, ident: &Ident, prefix: &str, full_path: &str) { + let Some(range) = byte_span_to_range( + self.files, + self.file, + ident.span().start() as usize..ident.span().start() as usize, + ) else { + return; + }; + + let title = format!("Qualify as {}", full_path); + let text_edit = TextEdit { range, new_text: format!("{}::", prefix) }; + + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } +} + +#[cfg(test)] +mod tests { + use tokio::test; + + use crate::requests::code_action::tests::assert_code_action; + + #[test] + async fn test_qualify_code_action_for_struct() { + let title = "Qualify as foo::bar::SomeTypeInBar"; + + let src = r#" + mod foo { + mod bar { + struct SomeTypeInBar {} + } + } + + fn foo(x: SomeType>|||| CodeActionResponse { .unwrap() } -async fn assert_code_action(title: &str, src: &str, expected: &str) { +pub(crate) async fn assert_code_action(title: &str, src: &str, expected: &str) { let actions = get_code_action(src).await; let action = actions .iter() @@ -87,150 +86,3 @@ fn apply_text_edit(src: &str, text_edit: &TextEdit) -> String { lines[text_edit.range.start.line as usize] = &line; lines.join("\n") } - -#[test] -async fn test_qualify_code_action_for_struct() { - let title = "Qualify as foo::bar::SomeTypeInBar"; - - let src = r#" - mod foo { - mod bar { - struct SomeTypeInBar {} - } - } - - fn foo(x: SomeType>|||||