From 5cdade3415abd0c3eaeeab7ca511242fab308aec Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 29 Aug 2024 18:32:43 -0300 Subject: [PATCH 01/11] feat: LSP code actions to import or qualified unresolved paths --- tooling/lsp/src/lib.rs | 18 +- tooling/lsp/src/modules.rs | 131 ++++++++ tooling/lsp/src/requests/code_action.rs | 305 ++++++++++++++++++ .../src/requests/completion/auto_import.rs | 141 +------- tooling/lsp/src/requests/mod.rs | 27 +- tooling/lsp/src/types.rs | 10 +- tooling/lsp/src/visibility.rs | 13 + 7 files changed, 497 insertions(+), 148 deletions(-) create mode 100644 tooling/lsp/src/modules.rs create mode 100644 tooling/lsp/src/requests/code_action.rs create mode 100644 tooling/lsp/src/visibility.rs diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index c2885543844..4a764f4268b 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -22,8 +22,8 @@ use fm::{codespan_files as files, FileManager}; use fxhash::FxHashSet; use lsp_types::{ request::{ - Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest, - References, Rename, SignatureHelpRequest, + CodeActionRequest, Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, + PrepareRenameRequest, References, Rename, SignatureHelpRequest, }, CodeLens, }; @@ -51,21 +51,24 @@ use notifications::{ on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized, }; use requests::{ - on_code_lens_request, on_completion_request, on_document_symbol_request, on_formatting, - on_goto_declaration_request, on_goto_definition_request, on_goto_type_definition_request, - on_hover_request, on_initialize, on_inlay_hint_request, on_prepare_rename_request, - on_profile_run_request, on_references_request, on_rename_request, on_shutdown, - on_signature_help_request, on_test_run_request, on_tests_request, LspInitializationOptions, + on_code_action_request, on_code_lens_request, on_completion_request, + on_document_symbol_request, on_formatting, on_goto_declaration_request, + on_goto_definition_request, on_goto_type_definition_request, on_hover_request, on_initialize, + on_inlay_hint_request, on_prepare_rename_request, on_profile_run_request, + on_references_request, on_rename_request, on_shutdown, on_signature_help_request, + on_test_run_request, on_tests_request, LspInitializationOptions, }; use serde_json::Value as JsonValue; use thiserror::Error; use tower::Service; +mod modules; mod notifications; mod requests; mod solver; mod types; mod utils; +mod visibility; #[cfg(test)] mod test_utils; @@ -144,6 +147,7 @@ impl NargoLspService { .request::(on_inlay_hint_request) .request::(on_completion_request) .request::(on_signature_help_request) + .request::(on_code_action_request) .notification::(on_initialized) .notification::(on_did_change_configuration) .notification::(on_did_open_text_document) diff --git a/tooling/lsp/src/modules.rs b/tooling/lsp/src/modules.rs new file mode 100644 index 00000000000..53ee2c82f86 --- /dev/null +++ b/tooling/lsp/src/modules.rs @@ -0,0 +1,131 @@ +use std::collections::BTreeMap; + +use noirc_frontend::{ + ast::ItemVisibility, + graph::CrateId, + hir::def_map::{CrateDefMap, ModuleId}, + macros_api::{ModuleDefId, NodeInterner}, + node_interner::ReferenceId, +}; + +use crate::visibility::is_visible; + +pub(crate) fn get_parent_module( + interner: &NodeInterner, + module_def_id: ModuleDefId, +) -> Option { + let reference_id = module_def_id_to_reference_id(module_def_id); + interner.reference_module(reference_id).copied() +} + +pub(crate) fn get_parent_module_id( + def_maps: &BTreeMap, + module_id: ModuleId, +) -> Option { + let crate_def_map = &def_maps[&module_id.krate]; + let module_data = &crate_def_map.modules()[module_id.local_id.0]; + module_data.parent.map(|parent| ModuleId { krate: module_id.krate, local_id: parent }) +} + +pub(crate) fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { + match module_def_id { + ModuleDefId::ModuleId(id) => ReferenceId::Module(id), + ModuleDefId::FunctionId(id) => ReferenceId::Function(id), + ModuleDefId::TypeId(id) => ReferenceId::Struct(id), + ModuleDefId::TypeAliasId(id) => ReferenceId::Alias(id), + ModuleDefId::TraitId(id) => ReferenceId::Trait(id), + ModuleDefId::GlobalId(id) => ReferenceId::Global(id), + } +} + +/// Returns the fully-qualified path of the given `ModuleDefId` relative to `current_module_id`: +/// - If `ModuleDefId` is a module, that module's path is returned +/// - Otherwise, that item's parent module's path is returned +pub(crate) fn module_full_path( + module_def_id: ModuleDefId, + visibility: ItemVisibility, + current_module_id: ModuleId, + current_module_parent_id: Option, + interner: &NodeInterner, +) -> Option { + let full_path; + if let ModuleDefId::ModuleId(module_id) = module_def_id { + if !is_visible(visibility, current_module_id, module_id) { + return None; + } + + full_path = + module_id_path(module_id, ¤t_module_id, current_module_parent_id, interner); + } else { + let Some(parent_module) = get_parent_module(interner, module_def_id) else { + return None; + }; + + if !is_visible(visibility, current_module_id, parent_module) { + return None; + } + + full_path = + module_id_path(parent_module, ¤t_module_id, current_module_parent_id, interner); + } + Some(full_path) +} + +/// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`. +/// Returns a relative path if possible. +pub(crate) fn module_id_path( + target_module_id: ModuleId, + current_module_id: &ModuleId, + current_module_parent_id: Option, + interner: &NodeInterner, +) -> String { + if Some(target_module_id) == current_module_parent_id { + return "super".to_string(); + } + + let mut segments: Vec<&str> = Vec::new(); + let mut is_relative = false; + + if let Some(module_attributes) = interner.try_module_attributes(&target_module_id) { + segments.push(&module_attributes.name); + + let mut current_attributes = module_attributes; + loop { + let Some(parent_local_id) = current_attributes.parent else { + break; + }; + + let parent_module_id = + &ModuleId { krate: target_module_id.krate, local_id: parent_local_id }; + + if current_module_id == parent_module_id { + is_relative = true; + break; + } + + if current_module_parent_id == Some(*parent_module_id) { + segments.push("super"); + is_relative = true; + break; + } + + let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { + break; + }; + + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } + } + + if !is_relative { + // We don't record module attriubtes for the root module, + // so we handle that case separately + if let CrateId::Root(_) = target_module_id.krate { + segments.push("crate"); + } + } + + segments.reverse(); + segments.join("::") +} diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs new file mode 100644 index 00000000000..5a01ccc6c36 --- /dev/null +++ b/tooling/lsp/src/requests/code_action.rs @@ -0,0 +1,305 @@ +use std::{ + collections::{BTreeMap, HashMap}, + future::{self, Future}, +}; + +use async_lsp::ResponseError; +use fm::{FileId, FileMap, PathString}; +use lsp_types::{ + CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionParams, CodeActionResponse, + Position, Range, TextDocumentPositionParams, TextEdit, Url, WorkspaceEdit, +}; +use noirc_errors::{Location, Span}; +use noirc_frontend::{ + ast::{Ident, Path, Visitor}, + graph::CrateId, + hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}, + macros_api::{ModuleDefId, NodeInterner}, + parser::{Item, ItemKind, ParsedSubModule}, + ParsedModule, +}; + +use crate::{ + byte_span_to_range, + modules::{get_parent_module_id, module_full_path}, + utils, LspState, +}; + +use super::{process_request, to_lsp_location}; + +pub(crate) fn on_code_action_request( + state: &mut LspState, + params: CodeActionParams, +) -> impl Future, ResponseError>> { + let uri = params.text_document.clone().uri; + let position = params.range.start; + let text_document_position_params = + TextDocumentPositionParams { text_document: params.text_document, position }; + + let result = process_request(state, text_document_position_params, |args| { + let path = PathString::from_path(uri.to_file_path().unwrap()); + args.files.get_file_id(&path).and_then(|file_id| { + utils::position_to_byte_index(args.files, file_id, &position).and_then(|byte_index| { + let file = args.files.get_file(file_id).unwrap(); + let source = file.source(); + let (parsed_module, _errors) = noirc_frontend::parse_program(source); + + let mut finder = CodeActionFinder::new( + uri, + args.files, + file_id, + source, + byte_index, + args.crate_id, + args.def_maps, + args.interner, + ); + finder.find(&parsed_module) + }) + }) + }); + future::ready(result) +} + +struct CodeActionFinder<'a> { + uri: Url, + files: &'a FileMap, + file: FileId, + lines: Vec<&'a str>, + byte_index: usize, + /// The module ID in scope. This might change as we traverse the AST + /// if we are analyzing something inside an inline module declaration. + module_id: ModuleId, + def_maps: &'a BTreeMap, + interner: &'a NodeInterner, + /// How many nested `mod` we are in deep + nesting: usize, + /// The line where an auto_import must be inserted + auto_import_line: usize, + code_actions: Vec, +} + +impl<'a> CodeActionFinder<'a> { + #[allow(clippy::too_many_arguments)] + fn new( + uri: Url, + files: &'a FileMap, + file: FileId, + source: &'a str, + byte_index: usize, + krate: CrateId, + def_maps: &'a BTreeMap, + interner: &'a NodeInterner, + ) -> Self { + // Find the module the current file belongs to + let def_map = &def_maps[&krate]; + let local_id = if let Some((module_index, _)) = + def_map.modules().iter().find(|(_, module_data)| module_data.location.file == file) + { + LocalModuleId(module_index) + } else { + def_map.root() + }; + let module_id = ModuleId { krate, local_id }; + Self { + uri, + files, + file, + lines: source.lines().collect(), + byte_index, + module_id, + def_maps, + interner, + nesting: 0, + auto_import_line: 0, + code_actions: vec![], + } + } + + fn find(&mut self, parsed_module: &ParsedModule) -> Option { + parsed_module.accept(self); + + if self.code_actions.is_empty() { + return None; + } + + let mut code_actions = std::mem::take(&mut self.code_actions); + code_actions.sort_by_key(|code_action| { + let CodeActionOrCommand::CodeAction(code_action) = code_action else { + panic!("We only gather code actions, never commands"); + }; + code_action.title.clone() + }); + + 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 text_edit = TextEdit { + range: Range { start: Position { line, character }, end: Position { line, character } }, + new_text: format!("use {};{}{}", full_path, newlines, indent), + }; + + let mut changes = HashMap::new(); + changes.insert(self.uri.clone(), vec![text_edit]); + + let workspace_edit = WorkspaceEdit { + changes: Some(changes), + document_changes: None, + change_annotations: None, + }; + + let title = format!("Import {}", full_path); + let code_action = new_quick_fix(title, workspace_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 text_edit = TextEdit { range, new_text: format!("{}::", prefix) }; + let mut changes = HashMap::new(); + changes.insert(self.uri.clone(), vec![text_edit]); + + let workspace_edit = WorkspaceEdit { + changes: Some(changes), + document_changes: None, + change_annotations: None, + }; + + let title = format!("Qualify as {}", full_path); + let code_action = new_quick_fix(title, workspace_edit); + self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); + } + + fn includes_span(&self, span: Span) -> bool { + span.start() as usize <= self.byte_index && self.byte_index <= span.end() as usize + } +} + +impl<'a> Visitor for CodeActionFinder<'a> { + fn visit_item(&mut self, item: &Item) -> bool { + if let ItemKind::Import(..) = &item.kind { + if let Some(lsp_location) = to_lsp_location(self.files, self.file, item.span) { + self.auto_import_line = (lsp_location.range.end.line + 1) as usize; + } + } + + self.includes_span(item.span) + } + + fn visit_parsed_submodule(&mut self, parsed_sub_module: &ParsedSubModule, span: Span) -> bool { + // Switch `self.module_id` to the submodule + let previous_module_id = self.module_id; + + let def_map = &self.def_maps[&self.module_id.krate]; + let Some(module_data) = def_map.modules().get(self.module_id.local_id.0) else { + return false; + }; + if let Some(child_module) = module_data.children.get(&parsed_sub_module.name) { + self.module_id = ModuleId { krate: self.module_id.krate, local_id: *child_module }; + } + + let old_auto_import_line = self.auto_import_line; + self.nesting += 1; + + if let Some(lsp_location) = to_lsp_location(self.files, self.file, span) { + self.auto_import_line = (lsp_location.range.start.line + 1) as usize; + } + + parsed_sub_module.contents.accept(self); + + // Restore the old module before continuing + self.module_id = previous_module_id; + self.nesting -= 1; + self.auto_import_line = old_auto_import_line; + + false + } + + 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) in entries { + let Some(module_full_path) = module_full_path( + *module_def_id, + *visibility, + self.module_id, + current_module_parent_id, + self.interner, + ) else { + continue; + }; + + let full_path = if let ModuleDefId::ModuleId(..) = module_def_id { + module_full_path.clone() + } else { + format!("{}::{}", module_full_path, name) + }; + + 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 new_quick_fix(title: String, workspace_edit: WorkspaceEdit) -> CodeAction { + CodeAction { + title, + kind: Some(CodeActionKind::QUICKFIX), + diagnostics: None, + edit: Some(workspace_edit), + command: None, + is_preferred: None, + disabled: None, + data: None, + } +} diff --git a/tooling/lsp/src/requests/completion/auto_import.rs b/tooling/lsp/src/requests/completion/auto_import.rs index 32f777dec34..bf3ff7f0291 100644 --- a/tooling/lsp/src/requests/completion/auto_import.rs +++ b/tooling/lsp/src/requests/completion/auto_import.rs @@ -1,13 +1,7 @@ -use std::collections::BTreeMap; - use lsp_types::{Position, Range, TextEdit}; -use noirc_frontend::{ - ast::ItemVisibility, - graph::CrateId, - hir::def_map::{CrateDefMap, ModuleId}, - macros_api::{ModuleDefId, NodeInterner}, - node_interner::ReferenceId, -}; +use noirc_frontend::macros_api::ModuleDefId; + +use crate::modules::{get_parent_module_id, module_full_path}; use super::{ kinds::{FunctionCompletionKind, FunctionKind, RequestedItems}, @@ -45,41 +39,15 @@ impl<'a> NodeFinder<'a> { continue; }; - let module_full_path; - if let ModuleDefId::ModuleId(module_id) = module_def_id { - module_full_path = module_id_path( - *module_id, - &self.module_id, - current_module_parent_id, - self.interner, - ); - } else { - let Some(parent_module) = get_parent_module(self.interner, *module_def_id) - else { - continue; - }; - - match *visibility { - ItemVisibility::Public => (), - ItemVisibility::Private => { - // Technically this can't be reached because we don't record private items for auto-import, - // but this is here for completeness. - continue; - } - ItemVisibility::PublicCrate => { - if self.module_id.krate != parent_module.krate { - continue; - } - } - } - - module_full_path = module_id_path( - parent_module, - &self.module_id, - current_module_parent_id, - self.interner, - ); - } + let Some(module_full_path) = module_full_path( + *module_def_id, + *visibility, + self.module_id, + current_module_parent_id, + self.interner, + ) else { + continue; + }; let full_path = if let ModuleDefId::ModuleId(..) = module_def_id { module_full_path @@ -119,88 +87,3 @@ impl<'a> NodeFinder<'a> { } } } - -fn get_parent_module(interner: &NodeInterner, module_def_id: ModuleDefId) -> Option { - let reference_id = module_def_id_to_reference_id(module_def_id); - interner.reference_module(reference_id).copied() -} - -fn get_parent_module_id( - def_maps: &BTreeMap, - module_id: ModuleId, -) -> Option { - let crate_def_map = &def_maps[&module_id.krate]; - let module_data = &crate_def_map.modules()[module_id.local_id.0]; - module_data.parent.map(|parent| ModuleId { krate: module_id.krate, local_id: parent }) -} - -fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { - match module_def_id { - ModuleDefId::ModuleId(id) => ReferenceId::Module(id), - ModuleDefId::FunctionId(id) => ReferenceId::Function(id), - ModuleDefId::TypeId(id) => ReferenceId::Struct(id), - ModuleDefId::TypeAliasId(id) => ReferenceId::Alias(id), - ModuleDefId::TraitId(id) => ReferenceId::Trait(id), - ModuleDefId::GlobalId(id) => ReferenceId::Global(id), - } -} - -/// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`. -/// Returns a relative path if possible. -fn module_id_path( - target_module_id: ModuleId, - current_module_id: &ModuleId, - current_module_parent_id: Option, - interner: &NodeInterner, -) -> String { - if Some(target_module_id) == current_module_parent_id { - return "super".to_string(); - } - - let mut segments: Vec<&str> = Vec::new(); - let mut is_relative = false; - - if let Some(module_attributes) = interner.try_module_attributes(&target_module_id) { - segments.push(&module_attributes.name); - - let mut current_attributes = module_attributes; - - loop { - let Some(parent_local_id) = current_attributes.parent else { - break; - }; - - let parent_module_id = - &ModuleId { krate: target_module_id.krate, local_id: parent_local_id }; - - if current_module_id == parent_module_id { - is_relative = true; - break; - } - - if current_module_parent_id == Some(*parent_module_id) { - segments.push("super"); - is_relative = true; - break; - } - - let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { - break; - }; - - segments.push(&parent_attributes.name); - current_attributes = parent_attributes; - } - } - - if !is_relative { - // We don't record module attributes for the root module, - // so we handle that case separately - if let CrateId::Root(_) = target_module_id.krate { - segments.push("crate"); - } - } - - segments.reverse(); - segments.join("::") -} diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index e88c7f11465..5bd9959fd63 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -10,7 +10,7 @@ use crate::{ use async_lsp::{ErrorCode, ResponseError}; use fm::{codespan_files::Error, FileMap, PathString}; use lsp_types::{ - DeclarationCapability, Location, Position, TextDocumentPositionParams, + CodeActionKind, DeclarationCapability, Location, Position, TextDocumentPositionParams, TextDocumentSyncCapability, TextDocumentSyncKind, TypeDefinitionProviderCapability, Url, WorkDoneProgressOptions, }; @@ -36,6 +36,7 @@ use crate::{ // They are not attached to the `NargoLspService` struct so they can be unit tested with only `LspState` // and params passed in. +mod code_action; mod code_lens_request; mod completion; mod document_symbol; @@ -51,14 +52,15 @@ mod test_run; mod tests; pub(crate) use { - code_lens_request::collect_lenses_for_package, code_lens_request::on_code_lens_request, - completion::on_completion_request, document_symbol::on_document_symbol_request, - goto_declaration::on_goto_declaration_request, goto_definition::on_goto_definition_request, - goto_definition::on_goto_type_definition_request, hover::on_hover_request, - inlay_hint::on_inlay_hint_request, profile_run::on_profile_run_request, - references::on_references_request, rename::on_prepare_rename_request, - rename::on_rename_request, signature_help::on_signature_help_request, - test_run::on_test_run_request, tests::on_tests_request, + code_action::on_code_action_request, code_lens_request::collect_lenses_for_package, + code_lens_request::on_code_lens_request, completion::on_completion_request, + document_symbol::on_document_symbol_request, goto_declaration::on_goto_declaration_request, + goto_definition::on_goto_definition_request, goto_definition::on_goto_type_definition_request, + hover::on_hover_request, inlay_hint::on_inlay_hint_request, + profile_run::on_profile_run_request, references::on_references_request, + rename::on_prepare_rename_request, rename::on_rename_request, + signature_help::on_signature_help_request, test_run::on_test_run_request, + tests::on_tests_request, }; /// LSP client will send initialization request after the server has started. @@ -252,6 +254,13 @@ pub(crate) fn on_initialize( }, }, )), + code_action_provider: Some(lsp_types::OneOf::Right(lsp_types::CodeActionOptions { + code_action_kinds: Some(vec![CodeActionKind::QUICKFIX]), + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: None, + }, + resolve_provider: None, + })), }, server_info: None, }) diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index 3ac1f35e18e..043c50a87fd 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -1,8 +1,8 @@ use fm::FileId; use lsp_types::{ - CompletionOptions, DeclarationCapability, DefinitionOptions, DocumentSymbolOptions, - HoverOptions, InlayHintOptions, OneOf, ReferencesOptions, RenameOptions, SignatureHelpOptions, - TypeDefinitionProviderCapability, + CodeActionOptions, CompletionOptions, DeclarationCapability, DefinitionOptions, + DocumentSymbolOptions, HoverOptions, InlayHintOptions, OneOf, ReferencesOptions, RenameOptions, + SignatureHelpOptions, TypeDefinitionProviderCapability, }; use noirc_driver::DebugFile; use noirc_errors::{debug_info::OpCodesCount, Location}; @@ -165,6 +165,10 @@ pub(crate) struct ServerCapabilities { /// The server provides signature help support. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) signature_help_provider: Option>, + + /// The server provides code action support. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) code_action_provider: Option>, } #[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] diff --git a/tooling/lsp/src/visibility.rs b/tooling/lsp/src/visibility.rs new file mode 100644 index 00000000000..aad8b47fbbe --- /dev/null +++ b/tooling/lsp/src/visibility.rs @@ -0,0 +1,13 @@ +use noirc_frontend::{ast::ItemVisibility, hir::def_map::ModuleId}; + +pub(super) fn is_visible( + visibility: ItemVisibility, + current_module: ModuleId, + target_module: ModuleId, +) -> bool { + match visibility { + ItemVisibility::Public => true, + ItemVisibility::Private => false, + ItemVisibility::PublicCrate => current_module.krate == target_module.krate, + } +} From de9e2270bb97269a7040c196bf7e1ef3957cfcc3 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 30 Aug 2024 17:37:28 -0300 Subject: [PATCH 02/11] Don't crash on "go to definition" when id is dummy --- .../noirc_frontend/src/resolve_locations.rs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/resolve_locations.rs b/compiler/noirc_frontend/src/resolve_locations.rs index 61ff6baf919..b9e86bf0ef7 100644 --- a/compiler/noirc_frontend/src/resolve_locations.rs +++ b/compiler/noirc_frontend/src/resolve_locations.rs @@ -4,7 +4,7 @@ use noirc_errors::Location; use crate::hir_def::expr::HirExpression; use crate::hir_def::types::Type; -use crate::node_interner::{DefinitionKind, Node, NodeInterner}; +use crate::node_interner::{DefinitionId, DefinitionKind, Node, NodeInterner}; impl NodeInterner { /// Scans the interner for the item which is located at that [Location] @@ -108,14 +108,18 @@ impl NodeInterner { ) -> Option { match expression { HirExpression::Ident(ident, _) => { - let definition_info = self.definition(ident.id); - match definition_info.kind { - DefinitionKind::Function(func_id) => { - Some(self.function_meta(&func_id).location) + if ident.id != DefinitionId::dummy_id() { + let definition_info = self.definition(ident.id); + match definition_info.kind { + DefinitionKind::Function(func_id) => { + Some(self.function_meta(&func_id).location) + } + DefinitionKind::Local(_local_id) => Some(definition_info.location), + DefinitionKind::Global(_global_id) => Some(definition_info.location), + _ => None, } - DefinitionKind::Local(_local_id) => Some(definition_info.location), - DefinitionKind::Global(_global_id) => Some(definition_info.location), - _ => None, + } else { + None } } HirExpression::Constructor(expr) => { From c5eaacff4338f8853c6aff0b81a95a1cbd84cffa Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 30 Aug 2024 17:53:40 -0300 Subject: [PATCH 03/11] Add tests --- tooling/lsp/src/requests/code_action.rs | 3 + tooling/lsp/src/requests/code_action/tests.rs | 158 ++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 tooling/lsp/src/requests/code_action/tests.rs diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index 5a01ccc6c36..dd48aba6447 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -27,6 +27,9 @@ use crate::{ use super::{process_request, to_lsp_location}; +#[cfg(test)] +mod tests; + pub(crate) fn on_code_action_request( state: &mut LspState, params: CodeActionParams, diff --git a/tooling/lsp/src/requests/code_action/tests.rs b/tooling/lsp/src/requests/code_action/tests.rs new file mode 100644 index 00000000000..91d211fc922 --- /dev/null +++ b/tooling/lsp/src/requests/code_action/tests.rs @@ -0,0 +1,158 @@ +use crate::{notifications::on_did_open_text_document, test_utils}; + +use lsp_types::{ + CodeActionContext, CodeActionOrCommand, CodeActionParams, CodeActionResponse, + DidOpenTextDocumentParams, PartialResultParams, Position, Range, TextDocumentIdentifier, + TextDocumentItem, WorkDoneProgressParams, +}; +use tokio::test; + +use super::on_code_action_request; + +async fn get_code_action(src: &str) -> CodeActionResponse { + let (mut state, noir_text_document) = test_utils::init_lsp_server("document_symbol").await; + + let (line, column) = src + .lines() + .enumerate() + .find_map(|(line_index, line)| line.find(">|<").map(|char_index| (line_index, char_index))) + .expect("Expected to find one >|< in the source code"); + + let src = src.replace(">|<", ""); + + on_did_open_text_document( + &mut state, + DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: noir_text_document.clone(), + language_id: "noir".to_string(), + version: 0, + text: src.to_string(), + }, + }, + ); + + let position = Position { line: line as u32, character: column as u32 }; + + on_code_action_request( + &mut state, + CodeActionParams { + text_document: TextDocumentIdentifier { uri: noir_text_document }, + range: Range { start: position, end: position }, + context: CodeActionContext { diagnostics: Vec::new(), only: None, trigger_kind: None }, + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + partial_result_params: PartialResultParams { partial_result_token: None }, + }, + ) + .await + .expect("Could not execute on_code_action_request") + .unwrap() +} + +#[test] +async fn test_code_action_for_unresolved_path_for_struct() { + let src = r#" + mod foo { + mod bar { + struct SomeTypeInBar {} + } + } + + fn foo(x: SomeType>|| Date: Sun, 1 Sep 2024 10:29:52 -0300 Subject: [PATCH 04/11] Simplify --- tooling/lsp/src/requests/code_action.rs | 46 ++++++++++--------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index dd48aba6447..fd8c42a3b87 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -150,22 +150,13 @@ impl<'a> CodeActionFinder<'a> { } } + 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 mut changes = HashMap::new(); - changes.insert(self.uri.clone(), vec![text_edit]); - - let workspace_edit = WorkspaceEdit { - changes: Some(changes), - document_changes: None, - change_annotations: None, - }; - - let title = format!("Import {}", full_path); - let code_action = new_quick_fix(title, workspace_edit); + let code_action = self.new_quick_fix(title, text_edit); self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); } @@ -178,7 +169,14 @@ impl<'a> CodeActionFinder<'a> { 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 { let mut changes = HashMap::new(); changes.insert(self.uri.clone(), vec![text_edit]); @@ -188,9 +186,16 @@ impl<'a> CodeActionFinder<'a> { change_annotations: None, }; - let title = format!("Qualify as {}", full_path); - let code_action = new_quick_fix(title, workspace_edit); - self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); + CodeAction { + title, + kind: Some(CodeActionKind::QUICKFIX), + diagnostics: None, + edit: Some(workspace_edit), + command: None, + is_preferred: None, + disabled: None, + data: None, + } } fn includes_span(&self, span: Span) -> bool { @@ -293,16 +298,3 @@ impl<'a> Visitor for CodeActionFinder<'a> { } } } - -fn new_quick_fix(title: String, workspace_edit: WorkspaceEdit) -> CodeAction { - CodeAction { - title, - kind: Some(CodeActionKind::QUICKFIX), - diagnostics: None, - edit: Some(workspace_edit), - command: None, - is_preferred: None, - disabled: None, - data: None, - } -} From 23d0cca034c9ee9813c0408081b976a65e8acb88 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Sun, 1 Sep 2024 16:12:42 -0300 Subject: [PATCH 05/11] Better tests --- tooling/lsp/src/requests/code_action/tests.rs | 169 +++++++++++------- 1 file changed, 106 insertions(+), 63 deletions(-) diff --git a/tooling/lsp/src/requests/code_action/tests.rs b/tooling/lsp/src/requests/code_action/tests.rs index 91d211fc922..21b5fab96c6 100644 --- a/tooling/lsp/src/requests/code_action/tests.rs +++ b/tooling/lsp/src/requests/code_action/tests.rs @@ -3,7 +3,7 @@ use crate::{notifications::on_did_open_text_document, test_utils}; use lsp_types::{ CodeActionContext, CodeActionOrCommand, CodeActionParams, CodeActionResponse, DidOpenTextDocumentParams, PartialResultParams, Position, Range, TextDocumentIdentifier, - TextDocumentItem, WorkDoneProgressParams, + TextDocumentItem, TextEdit, WorkDoneProgressParams, }; use tokio::test; @@ -49,8 +49,49 @@ async fn get_code_action(src: &str) -> CodeActionResponse { .unwrap() } +async fn assert_code_action(title: &str, src: &str, expected: &str) { + let actions = get_code_action(src).await; + let action = actions + .iter() + .filter_map(|action| { + if let CodeActionOrCommand::CodeAction(action) = action { + if action.title == title { + Some(action) + } else { + None + } + } else { + None + } + }) + .next() + .expect("Couldn't find an action with the given title"); + + let workspace_edit = action.edit.as_ref().unwrap(); + let text_edits = workspace_edit.changes.as_ref().unwrap().iter().next().unwrap().1; + assert_eq!(text_edits.len(), 1); + + let result = apply_text_edit(&src.replace(">|<", ""), &text_edits[0]); + assert_eq!(result, expected); +} + +fn apply_text_edit(src: &str, text_edit: &TextEdit) -> String { + let mut lines: Vec<_> = src.lines().collect(); + assert_eq!(text_edit.range.start.line, text_edit.range.end.line); + + let mut line = lines[text_edit.range.start.line as usize].to_string(); + line.replace_range( + text_edit.range.start.character as usize..text_edit.range.end.character as usize, + &text_edit.new_text, + ); + lines[text_edit.range.start.line as usize] = &line; + lines.join("\n") +} + #[test] -async fn test_code_action_for_unresolved_path_for_struct() { +async fn test_qualify_code_action_for_struct() { + let title = "Qualify as foo::bar::SomeTypeInBar"; + let src = r#" mod foo { mod bar { @@ -61,49 +102,48 @@ async fn test_code_action_for_unresolved_path_for_struct() { fn foo(x: SomeType>||| Date: Sun, 1 Sep 2024 16:03:33 -0300 Subject: [PATCH 06/11] feat: LSP code action "Fill struct fields" --- tooling/lsp/src/requests/code_action.rs | 81 +++++++++++- tooling/lsp/src/requests/code_action/tests.rs | 120 ++++++++++++++++++ 2 files changed, 200 insertions(+), 1 deletion(-) diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index fd8c42a3b87..cd7ff890a41 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -11,10 +11,11 @@ use lsp_types::{ }; use noirc_errors::{Location, Span}; use noirc_frontend::{ - ast::{Ident, Path, Visitor}, + ast::{ConstructorExpression, Ident, Path, Visitor}, graph::CrateId, hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}, macros_api::{ModuleDefId, NodeInterner}, + node_interner::ReferenceId, parser::{Item, ItemKind, ParsedSubModule}, ParsedModule, }; @@ -68,6 +69,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 +110,7 @@ impl<'a> CodeActionFinder<'a> { uri, files, file, + source, lines: source.lines().collect(), byte_index, module_id, @@ -297,4 +300,80 @@ impl<'a> Visitor for CodeActionFinder<'a> { } } } + + fn visit_constructor_expression( + &mut self, + constructor: &ConstructorExpression, + span: Span, + ) -> bool { + if !self.includes_span(span) { + return false; + } + + // 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 true; + }; + + 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 true; + } + + // 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 true; + }; + + let on_whitespace = bytes[index].is_ascii_whitespace(); + + let mut new_text = String::new(); + if !constructor.fields.is_empty() && char_before_right_brace != ',' { + new_text.push(',') + } + if !on_whitespace || constructor.fields.is_empty() { + new_text.push(' '); + } + + for (index, (name, _)) in fields.iter().enumerate() { + if index > 0 { + new_text.push_str(", "); + } + 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(CodeActionOrCommand::CodeAction(code_action)); + + true + } } diff --git a/tooling/lsp/src/requests/code_action/tests.rs b/tooling/lsp/src/requests/code_action/tests.rs index 21b5fab96c6..2ee7d955645 100644 --- a/tooling/lsp/src/requests/code_action/tests.rs +++ b/tooling/lsp/src/requests/code_action/tests.rs @@ -199,3 +199,123 @@ fn main() { assert_code_action(title, src, expected).await; } + +#[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; +} From e3eb86a7d458b00716be06b30cc13b993a933be2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 14:15:53 -0300 Subject: [PATCH 07/11] Better "Fill struct fields" for multiline constructor --- tooling/lsp/src/requests/code_action.rs | 40 ++++++++++- tooling/lsp/src/requests/code_action/tests.rs | 67 +++++++++++++++++++ 2 files changed, 104 insertions(+), 3 deletions(-) diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index cd7ff890a41..830a30d7f44 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -347,19 +347,53 @@ impl<'a> Visitor for CodeActionFinder<'a> { return true; }; + // 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(',') + new_text.push(','); } - if !on_whitespace || constructor.fields.is_empty() { + + // 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_str(", "); + 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(": ()"); diff --git a/tooling/lsp/src/requests/code_action/tests.rs b/tooling/lsp/src/requests/code_action/tests.rs index 2ee7d955645..69cab321bea 100644 --- a/tooling/lsp/src/requests/code_action/tests.rs +++ b/tooling/lsp/src/requests/code_action/tests.rs @@ -319,3 +319,70 @@ async fn test_fill_struct_fields_code_action_some_fields_trailing_comma() { 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; +} From 63d86f5e69323f74556737bebab1d787f7a1b68e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 14:22:53 -0300 Subject: [PATCH 08/11] Split code actions into several files --- tooling/lsp/src/requests/code_action.rs | 207 +----------------- .../code_action/fill_struct_fields.rs | 109 +++++++++ .../requests/code_action/import_or_qualify.rs | 109 +++++++++ 3 files changed, 227 insertions(+), 198 deletions(-) create mode 100644 tooling/lsp/src/requests/code_action/fill_struct_fields.rs create mode 100644 tooling/lsp/src/requests/code_action/import_or_qualify.rs diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index 830a30d7f44..2b03a445642 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -7,27 +7,24 @@ 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::{ConstructorExpression, Ident, Path, Visitor}, + ast::{ConstructorExpression, Path, Visitor}, graph::CrateId, hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}, - macros_api::{ModuleDefId, NodeInterner}, - node_interner::ReferenceId, + macros_api::NodeInterner, parser::{Item, ItemKind, ParsedSubModule}, ParsedModule, }; -use crate::{ - byte_span_to_range, - modules::{get_parent_module_id, module_full_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; @@ -140,45 +137,6 @@ 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 { let mut changes = HashMap::new(); changes.insert(self.uri.clone(), vec![text_edit]); @@ -247,58 +205,7 @@ 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) in entries { - let Some(module_full_path) = module_full_path( - *module_def_id, - *visibility, - self.module_id, - current_module_parent_id, - self.interner, - ) else { - continue; - }; - - let full_path = if let ModuleDefId::ModuleId(..) = module_def_id { - module_full_path.clone() - } else { - format!("{}::{}", module_full_path, name) - }; - - 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); - } - } + self.import_or_qualify(path); } fn visit_constructor_expression( @@ -310,103 +217,7 @@ impl<'a> Visitor for CodeActionFinder<'a> { return false; } - // 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 true; - }; - - 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 true; - } - - // 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 true; - }; - - // 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(CodeActionOrCommand::CodeAction(code_action)); + self.fill_struct_fields(constructor, span); 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..07534bc4c31 --- /dev/null +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -0,0 +1,109 @@ +use lsp_types::{CodeActionOrCommand, 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) { + // 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(CodeActionOrCommand::CodeAction(code_action)); + } +} 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..d1339f25206 --- /dev/null +++ b/tooling/lsp/src/requests/code_action/import_or_qualify.rs @@ -0,0 +1,109 @@ +use lsp_types::{CodeActionOrCommand, 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}, +}; + +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) in entries { + let Some(module_full_path) = module_full_path( + *module_def_id, + *visibility, + self.module_id, + current_module_parent_id, + self.interner, + ) else { + continue; + }; + + let full_path = if let ModuleDefId::ModuleId(..) = module_def_id { + module_full_path.clone() + } else { + format!("{}::{}", module_full_path, name) + }; + + 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(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)); + } +} From 5514f3cc4e0222a3b5fed62bfcce375b5b594cff Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 14:24:01 -0300 Subject: [PATCH 09/11] Let `new_quick_fix` return a `CodeActionOrCommand` --- tooling/lsp/src/requests/code_action.rs | 6 +++--- tooling/lsp/src/requests/code_action/fill_struct_fields.rs | 4 ++-- tooling/lsp/src/requests/code_action/import_or_qualify.rs | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index 2b03a445642..150932933c8 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -137,7 +137,7 @@ impl<'a> CodeActionFinder<'a> { Some(code_actions) } - 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]); @@ -147,7 +147,7 @@ impl<'a> CodeActionFinder<'a> { change_annotations: None, }; - CodeAction { + CodeActionOrCommand::CodeAction(CodeAction { title, kind: Some(CodeActionKind::QUICKFIX), diagnostics: None, @@ -156,7 +156,7 @@ impl<'a> CodeActionFinder<'a> { is_preferred: None, disabled: None, data: None, - } + }) } fn includes_span(&self, span: Span) -> bool { diff --git a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs index 07534bc4c31..a0e3f552436 100644 --- a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -1,4 +1,4 @@ -use lsp_types::{CodeActionOrCommand, TextEdit}; +use lsp_types::TextEdit; use noirc_errors::{Location, Span}; use noirc_frontend::{ast::ConstructorExpression, node_interner::ReferenceId}; @@ -104,6 +104,6 @@ impl<'a> CodeActionFinder<'a> { 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(CodeActionOrCommand::CodeAction(code_action)); + self.code_actions.push(code_action); } } diff --git a/tooling/lsp/src/requests/code_action/import_or_qualify.rs b/tooling/lsp/src/requests/code_action/import_or_qualify.rs index d1339f25206..deb8b0a1593 100644 --- a/tooling/lsp/src/requests/code_action/import_or_qualify.rs +++ b/tooling/lsp/src/requests/code_action/import_or_qualify.rs @@ -1,4 +1,4 @@ -use lsp_types::{CodeActionOrCommand, Position, Range, TextEdit}; +use lsp_types::{Position, Range, TextEdit}; use noirc_errors::Location; use noirc_frontend::{ ast::{Ident, Path}, @@ -88,7 +88,7 @@ impl<'a> CodeActionFinder<'a> { }; let code_action = self.new_quick_fix(title, text_edit); - self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); + self.code_actions.push(code_action); } fn push_qualify_code_action(&mut self, ident: &Ident, prefix: &str, full_path: &str) { @@ -104,6 +104,6 @@ impl<'a> CodeActionFinder<'a> { 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)); + self.code_actions.push(code_action); } } From 4868b0a7c8066aeaa5adac446ed4d4206d90d5a7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 14:24:45 -0300 Subject: [PATCH 10/11] Move more code into specific code action file --- tooling/lsp/src/requests/code_action.rs | 4 ---- tooling/lsp/src/requests/code_action/fill_struct_fields.rs | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tooling/lsp/src/requests/code_action.rs b/tooling/lsp/src/requests/code_action.rs index 150932933c8..0640674e762 100644 --- a/tooling/lsp/src/requests/code_action.rs +++ b/tooling/lsp/src/requests/code_action.rs @@ -213,10 +213,6 @@ impl<'a> Visitor for CodeActionFinder<'a> { constructor: &ConstructorExpression, span: Span, ) -> bool { - if !self.includes_span(span) { - return false; - } - self.fill_struct_fields(constructor, span); 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 index a0e3f552436..d2fb9674110 100644 --- a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -8,6 +8,10 @@ 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 { From 57df1baec26b0172aa2f7ff88b60c09e2cbd6e2b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 14:52:13 -0300 Subject: [PATCH 11/11] Also move code action tests to their own files --- .../code_action/fill_struct_fields.rs | 194 +++++++++++ .../requests/code_action/import_or_qualify.rs | 119 +++++++ tooling/lsp/src/requests/code_action/tests.rs | 302 +----------------- 3 files changed, 314 insertions(+), 301 deletions(-) diff --git a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs index d2fb9674110..f57fbc652ad 100644 --- a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -111,3 +111,197 @@ impl<'a> CodeActionFinder<'a> { 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 index deb8b0a1593..b2dbf66be13 100644 --- a/tooling/lsp/src/requests/code_action/import_or_qualify.rs +++ b/tooling/lsp/src/requests/code_action/import_or_qualify.rs @@ -107,3 +107,122 @@ impl<'a> CodeActionFinder<'a> { 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,302 +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>|||||<} - } - "#; - - 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; -}