From 8b97e1d2930a15fd267bdf50d3396b4759475f9c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 10:54:25 -0300 Subject: [PATCH 01/26] Autocomplete first segment in use path --- compiler/noirc_frontend/src/lib.rs | 15 ++ .../noirc_frontend/src/parser/parser/path.rs | 18 +- tooling/lsp/src/lib.rs | 23 +- tooling/lsp/src/notifications/mod.rs | 4 +- tooling/lsp/src/requests/completion.rs | 218 ++++++++++++++++++ tooling/lsp/src/requests/inlay_hint.rs | 62 +---- tooling/lsp/src/requests/mod.rs | 32 ++- tooling/lsp/src/types.rs | 9 +- tooling/lsp/src/utils.rs | 59 +++++ 9 files changed, 361 insertions(+), 79 deletions(-) create mode 100644 tooling/lsp/src/requests/completion.rs create mode 100644 tooling/lsp/src/utils.rs diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index b14f65a3e35..3f898f732d6 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -84,3 +84,18 @@ pub mod macros_api { ) -> Result<(), (MacroError, FileId)>; } } + +pub fn log(contents: &str) { + use std::fs::OpenOptions; + use std::io::prelude::*; + + let mut file = OpenOptions::new() + .write(true) + .append(true) + .open("/Users/asterite/Sandbox/output/lsp.txt") + .unwrap(); + + if let Err(e) = writeln!(file, "{}", contents) { + eprintln!("Couldn't write to file: {}", e); + } +} diff --git a/compiler/noirc_frontend/src/parser/parser/path.rs b/compiler/noirc_frontend/src/parser/parser/path.rs index 4a5085db172..624ed51a7b5 100644 --- a/compiler/noirc_frontend/src/parser/parser/path.rs +++ b/compiler/noirc_frontend/src/parser/parser/path.rs @@ -19,7 +19,14 @@ pub fn path_no_turbofish() -> impl NoirParser { } fn path_inner<'a>(segment: impl NoirParser + 'a) -> impl NoirParser + 'a { - let segments = segment.separated_by(just(Token::DoubleColon)).at_least(1); + let segments = segment + .separated_by(just(Token::DoubleColon)) + .at_least(1) + .then(just(Token::DoubleColon).then_ignore(none_of(Token::LeftBrace).rewind()).or_not()) + .map(|(path_segments, _trailing_colons)| { + // TODO: give an error if there are trailing colons + path_segments + }); let make_path = |kind| move |segments, span| Path { segments, kind, span }; let prefix = |key| keyword(key).ignore_then(just(Token::DoubleColon)); @@ -92,4 +99,13 @@ mod test { vec!["crate", "crate::std::crate", "foo::bar::crate", "foo::dep"], ); } + + // TODO + // #[test] + // fn parse_path_with_trailing_colons() { + // let src = "foo::bar::"; + + // let x = parse_with(path_no_turbofish(), src); + // dbg!(x); + // } } diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 88aab65c6fa..ca34d7686fd 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::{ - DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest, References, - Rename, + Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest, + References, Rename, }, CodeLens, }; @@ -36,7 +36,10 @@ use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelecti use noirc_driver::{file_manager_with_stdlib, prepare_crate, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::{ graph::{CrateId, CrateName}, - hir::{def_map::parse_file, Context, FunctionNameMatch, ParsedFiles}, + hir::{ + def_map::{parse_file, CrateDefMap}, + Context, FunctionNameMatch, ParsedFiles, + }, node_interner::NodeInterner, parser::ParserError, ParsedModule, @@ -48,11 +51,11 @@ use notifications::{ on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized, }; use requests::{ - on_code_lens_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_test_run_request, on_tests_request, - LspInitializationOptions, + 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_test_run_request, on_tests_request, LspInitializationOptions, }; use serde_json::Value as JsonValue; use thiserror::Error; @@ -62,6 +65,7 @@ mod notifications; mod requests; mod solver; mod types; +mod utils; #[cfg(test)] mod test_utils; @@ -86,6 +90,7 @@ pub struct LspState { cached_lenses: HashMap>, cached_definitions: HashMap, cached_parsed_files: HashMap))>, + cached_def_maps: HashMap>, options: LspInitializationOptions, } @@ -103,6 +108,7 @@ impl LspState { cached_definitions: HashMap::new(), open_documents_count: 0, cached_parsed_files: HashMap::new(), + cached_def_maps: HashMap::new(), options: Default::default(), } } @@ -136,6 +142,7 @@ impl NargoLspService { .request::(on_rename_request) .request::(on_hover_request) .request::(on_inlay_hint_request) + .request::(on_completion_request) .notification::(on_initialized) .notification::(on_did_change_configuration) .notification::(on_did_open_text_document) diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 56aef90cfde..1a27b9a89d6 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -153,8 +153,8 @@ pub(crate) fn process_workspace_for_noir_document( Some(&file_path), ); state.cached_lenses.insert(document_uri.to_string(), collected_lenses); - - state.cached_definitions.insert(package_root_dir, context.def_interner); + state.cached_definitions.insert(package_root_dir.clone(), context.def_interner); + state.cached_def_maps.insert(package_root_dir.clone(), context.def_maps); let fm = &context.file_manager; let files = fm.as_file_map(); diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs new file mode 100644 index 00000000000..c1f9a5cf64b --- /dev/null +++ b/tooling/lsp/src/requests/completion.rs @@ -0,0 +1,218 @@ +use std::{ + collections::BTreeMap, + future::{self, Future}, +}; + +use async_lsp::ResponseError; +use fm::PathString; +use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse}; +use noirc_errors::Span; +use noirc_frontend::{ + ast::{UseTree, UseTreeKind}, + graph::{CrateId, Dependency}, + hir::def_map::{CrateDefMap, ModuleId}, + parser::{Item, ItemKind}, + ParsedModule, +}; + +use crate::{utils, LspState}; + +use super::process_request; + +pub(crate) fn on_completion_request( + state: &mut LspState, + params: CompletionParams, +) -> impl Future, ResponseError>> { + noirc_frontend::log("Completion Request"); + + let uri = params.text_document_position.clone().text_document.uri; + + let result = process_request(state, params.text_document_position.clone(), |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, + ¶ms.text_document_position.position, + ) + .and_then(|byte_index| { + let file = args.files.get_file(file_id).unwrap(); + let source = file.source(); + let byte = source.bytes().nth(byte_index - 1); + let (parsed_module, _errors) = noirc_frontend::parse_program(source); + + let finder = NodeFinder::new( + byte_index, + byte, + args.root_crate_id, + args.def_maps, + args.root_crate_dependencies, + ); + finder.find(&parsed_module) + }) + }) + }); + future::ready(result) +} + +struct NodeFinder<'a> { + byte_index: usize, + byte: Option, + krate: CrateId, + current_module_id: ModuleId, + def_maps: &'a BTreeMap, + dependencies: &'a Vec, +} + +impl<'a> NodeFinder<'a> { + fn new( + byte_index: usize, + byte: Option, + krate: CrateId, + def_maps: &'a BTreeMap, + dependencies: &'a Vec, + ) -> Self { + let def_map = &def_maps[&krate]; + let current_module_id = ModuleId { krate: krate, local_id: def_map.root() }; + Self { byte_index, byte, krate, current_module_id, def_maps, dependencies } + } + + fn find(&self, parsed_module: &ParsedModule) -> Option { + for item in &parsed_module.items { + if let Some(response) = self.find_in_item(item) { + return Some(response); + } + } + + None + } + + fn find_in_item(&self, item: &Item) -> Option { + if !self.includes_span(item.span) { + return None; + } + + match &item.kind { + ItemKind::Import(use_tree) => { + if let Some(completion) = self.find_in_use_tree(use_tree, item.span) { + return Some(completion); + } + } + _ => (), + } + + None + } + + fn find_in_use_tree(&self, use_tree: &UseTree, span: Span) -> Option { + match &use_tree.kind { + UseTreeKind::Path(ident, alias) => { + if let Some(_alias) = alias { + // TODO: handle + None + } else { + // If we are at the end of the use statement (likely the most common scenario)... + if self.byte_index == span.end() as usize { + if let Some(b':') = self.byte { + let mut segments: Vec<_> = use_tree + .prefix + .segments + .iter() + .map(|segment| &segment.ident) + .collect(); + segments.push(ident); + + // If we are after a colon, find inside the last segment + // TODO: handle + None + } else { + // Otherwise we must complete the last segment + if use_tree.prefix.segments.is_empty() { + // We are at the start of the use segment and completing the first segment, + // let's start here. + return self.complete_in_module( + self.current_module_id, + ident.to_string(), + true, + ); + } + None + } + } else { + // TODO: handle + None + } + } + } + UseTreeKind::List(..) => { + // TODO: handle + None + } + } + } + + fn complete_in_module( + &self, + module_id: ModuleId, + prefix: String, + first_segment: bool, + ) -> Option { + let def_map = &self.def_maps[&module_id.krate]; + let module_data = def_map.modules().get(module_id.local_id.0)?; + let mut completion_items = Vec::new(); + + // Find in the target module + for child in module_data.children.keys() { + if name_matches(&child.0.contents, &prefix) { + completion_items.push(module_completion_item(child.0.contents.clone())); + } + } + + // If this is the first segment, also find in all crates + if first_segment { + for dependency in self.dependencies { + let dependency_name = dependency.as_name(); + if name_matches(&dependency_name, &prefix) { + completion_items.push(crate_completion_item(dependency_name)); + } + } + } + + Some(CompletionResponse::Array(completion_items)) + } + + fn includes_span(&self, span: Span) -> bool { + span.start() as usize <= self.byte_index && self.byte_index <= span.end() as usize + } +} + +fn name_matches(name: &str, prefix: &str) -> bool { + name.starts_with(prefix) +} + +fn module_completion_item(name: String) -> CompletionItem { + CompletionItem { + label: name, + label_details: None, + kind: Some(CompletionItemKind::MODULE), + detail: None, + documentation: None, + deprecated: None, + preselect: None, + sort_text: None, + filter_text: None, + insert_text: None, + insert_text_format: None, + insert_text_mode: None, + text_edit: None, + additional_text_edits: None, + command: None, + commit_characters: None, + data: None, + tags: None, + } +} + +fn crate_completion_item(name: String) -> CompletionItem { + module_completion_item(name) +} diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index c222d5bca01..3c608ef85c0 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -1,4 +1,3 @@ -use fm::codespan_files::Files; use std::future::{self, Future}; use async_lsp::ResponseError; @@ -21,7 +20,7 @@ use noirc_frontend::{ ParsedModule, Type, TypeBinding, TypeVariable, TypeVariableKind, }; -use crate::LspState; +use crate::{utils, LspState}; use super::{process_request, to_lsp_location, InlayHintsOptions}; @@ -43,7 +42,7 @@ pub(crate) fn on_inlay_hint_request( let source = file.source(); let (parsed_moduled, _errors) = noirc_frontend::parse_program(source); - let span = range_to_byte_span(args.files, file_id, ¶ms.range) + let span = utils::range_to_byte_span(args.files, file_id, ¶ms.range) .map(|range| Span::from(range.start as u32..range.end as u32)); let mut collector = @@ -687,63 +686,6 @@ fn get_expression_name(expression: &Expression) -> Option { } } -// These functions are copied from the codespan_lsp crate, except that they never panic -// (the library will sometimes panic, so functions returning Result are not always accurate) - -fn range_to_byte_span( - files: &FileMap, - file_id: FileId, - range: &lsp_types::Range, -) -> Option> { - Some( - position_to_byte_index(files, file_id, &range.start)? - ..position_to_byte_index(files, file_id, &range.end)?, - ) -} - -fn position_to_byte_index( - files: &FileMap, - file_id: FileId, - position: &lsp_types::Position, -) -> Option { - let Ok(source) = files.source(file_id) else { - return None; - }; - - let Ok(line_span) = files.line_range(file_id, position.line as usize) else { - return None; - }; - let line_str = source.get(line_span.clone())?; - - let byte_offset = character_to_line_offset(line_str, position.character)?; - - Some(line_span.start + byte_offset) -} - -fn character_to_line_offset(line: &str, character: u32) -> Option { - let line_len = line.len(); - let mut character_offset = 0; - - let mut chars = line.chars(); - while let Some(ch) = chars.next() { - if character_offset == character { - let chars_off = chars.as_str().len(); - let ch_off = ch.len_utf8(); - - return Some(line_len - chars_off - ch_off); - } - - character_offset += ch.len_utf16() as u32; - } - - // Handle positions after the last character on the line - if character_offset == character { - Some(line_len) - } else { - None - } -} - #[cfg(test)] mod inlay_hints_tests { use crate::{ diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index aef5d782c46..0f4a436174b 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; use std::path::PathBuf; use std::{collections::HashMap, future::Future}; @@ -15,6 +16,8 @@ use lsp_types::{ }; use nargo_fmt::Config; use noirc_driver::file_manager_with_stdlib; +use noirc_frontend::graph::CrateId; +use noirc_frontend::hir::def_map::CrateDefMap; use noirc_frontend::{graph::Dependency, macros_api::NodeInterner}; use serde::{Deserialize, Serialize}; @@ -34,6 +37,7 @@ use crate::{ // and params passed in. mod code_lens_request; +mod completion; mod document_symbol; mod goto_declaration; mod goto_definition; @@ -47,12 +51,12 @@ mod tests; pub(crate) use { code_lens_request::collect_lenses_for_package, code_lens_request::on_code_lens_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, test_run::on_test_run_request, - tests::on_tests_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, test_run::on_test_run_request, tests::on_tests_request, }; /// LSP client will send initialization request after the server has started. @@ -228,6 +232,15 @@ pub(crate) fn on_initialize( label: Some("Noir".to_string()), }, )), + completion_provider: Some(lsp_types::OneOf::Right(lsp_types::CompletionOptions { + resolve_provider: None, + trigger_characters: Some(vec![":".to_string()]), + all_commit_characters: None, + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: None, + }, + completion_item: None, + })), }, server_info: None, }) @@ -375,8 +388,10 @@ pub(crate) struct ProcessRequestCallbackArgs<'a> { files: &'a FileMap, interner: &'a NodeInterner, interners: &'a HashMap, + root_crate_id: CrateId, root_crate_name: String, root_crate_dependencies: &'a Vec, + def_maps: &'a BTreeMap, } pub(crate) fn process_request( @@ -411,12 +426,15 @@ where crate::prepare_package(&workspace_file_manager, &parsed_files, package); let interner; + let def_maps; if let Some(def_interner) = state.cached_definitions.get(&package_root_path) { interner = def_interner; + def_maps = state.cached_def_maps.get(&package_root_path).unwrap(); } else { // We ignore the warnings and errors produced by compilation while resolving the definition let _ = noirc_driver::check_crate(&mut context, crate_id, &Default::default()); interner = &context.def_interner; + def_maps = &context.def_maps; } let files = context.file_manager.as_file_map(); @@ -432,8 +450,10 @@ where files, interner, interners: &state.cached_definitions, + root_crate_id: crate_id, root_crate_name: package.name.to_string(), root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies, + def_maps, })) } pub(crate) fn find_all_references_in_workspace( diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index fa3234cf3bb..5afda0d292a 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -1,7 +1,8 @@ use fm::FileId; use lsp_types::{ - DeclarationCapability, DefinitionOptions, DocumentSymbolOptions, HoverOptions, - InlayHintOptions, OneOf, ReferencesOptions, RenameOptions, TypeDefinitionProviderCapability, + CompletionOptions, DeclarationCapability, DefinitionOptions, DocumentSymbolOptions, + HoverOptions, InlayHintOptions, OneOf, ReferencesOptions, RenameOptions, + TypeDefinitionProviderCapability, }; use noirc_driver::DebugFile; use noirc_errors::{debug_info::OpCodesCount, Location}; @@ -156,6 +157,10 @@ pub(crate) struct ServerCapabilities { /// The server provides document symbol support. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) document_symbol_provider: Option>, + + /// The server provides completion support. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) completion_provider: Option>, } #[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] diff --git a/tooling/lsp/src/utils.rs b/tooling/lsp/src/utils.rs new file mode 100644 index 00000000000..96db1f7bfa2 --- /dev/null +++ b/tooling/lsp/src/utils.rs @@ -0,0 +1,59 @@ +// These functions are copied from the codespan_lsp crate, except that they never panic +// (the library will sometimes panic, so functions returning Result are not always accurate) + +use fm::codespan_files::Files; +use fm::{FileId, FileMap}; + +pub(crate) fn range_to_byte_span( + files: &FileMap, + file_id: FileId, + range: &lsp_types::Range, +) -> Option> { + Some( + position_to_byte_index(files, file_id, &range.start)? + ..position_to_byte_index(files, file_id, &range.end)?, + ) +} + +pub(crate) fn position_to_byte_index( + files: &FileMap, + file_id: FileId, + position: &lsp_types::Position, +) -> Option { + let Ok(source) = files.source(file_id) else { + return None; + }; + + let Ok(line_span) = files.line_range(file_id, position.line as usize) else { + return None; + }; + let line_str = source.get(line_span.clone())?; + + let byte_offset = character_to_line_offset(line_str, position.character)?; + + Some(line_span.start + byte_offset) +} + +pub(crate) fn character_to_line_offset(line: &str, character: u32) -> Option { + let line_len = line.len(); + let mut character_offset = 0; + + let mut chars = line.chars(); + while let Some(ch) = chars.next() { + if character_offset == character { + let chars_off = chars.as_str().len(); + let ch_off = ch.len_utf8(); + + return Some(line_len - chars_off - ch_off); + } + + character_offset += ch.len_utf16() as u32; + } + + // Handle positions after the last character on the line + if character_offset == character { + Some(line_len) + } else { + None + } +} From 823850a2c90dfefc04e9dfe6e45e3df63699c310 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 12:09:49 -0300 Subject: [PATCH 02/26] Autocomplete other segments of a use statement --- tooling/lsp/src/requests/completion.rs | 111 ++++++++++++++++--------- 1 file changed, 70 insertions(+), 41 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index c1f9a5cf64b..1eccd0e91bf 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -8,9 +8,13 @@ use fm::PathString; use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse}; use noirc_errors::Span; use noirc_frontend::{ - ast::{UseTree, UseTreeKind}, + ast::{Ident, Path, PathKind, PathSegment, UseTree, UseTreeKind}, graph::{CrateId, Dependency}, - hir::def_map::{CrateDefMap, ModuleId}, + hir::{ + def_map::{CrateDefMap, ModuleId}, + resolution::path_resolver::{PathResolver, StandardPathResolver}, + }, + macros_api::ModuleDefId, parser::{Item, ItemKind}, ParsedModule, }; @@ -58,8 +62,7 @@ pub(crate) fn on_completion_request( struct NodeFinder<'a> { byte_index: usize, byte: Option, - krate: CrateId, - current_module_id: ModuleId, + module_id: ModuleId, def_maps: &'a BTreeMap, dependencies: &'a Vec, } @@ -74,7 +77,7 @@ impl<'a> NodeFinder<'a> { ) -> Self { let def_map = &def_maps[&krate]; let current_module_id = ModuleId { krate: krate, local_id: def_map.root() }; - Self { byte_index, byte, krate, current_module_id, def_maps, dependencies } + Self { byte_index, byte, module_id: current_module_id, def_maps, dependencies } } fn find(&self, parsed_module: &ParsedModule) -> Option { @@ -107,42 +110,7 @@ impl<'a> NodeFinder<'a> { fn find_in_use_tree(&self, use_tree: &UseTree, span: Span) -> Option { match &use_tree.kind { UseTreeKind::Path(ident, alias) => { - if let Some(_alias) = alias { - // TODO: handle - None - } else { - // If we are at the end of the use statement (likely the most common scenario)... - if self.byte_index == span.end() as usize { - if let Some(b':') = self.byte { - let mut segments: Vec<_> = use_tree - .prefix - .segments - .iter() - .map(|segment| &segment.ident) - .collect(); - segments.push(ident); - - // If we are after a colon, find inside the last segment - // TODO: handle - None - } else { - // Otherwise we must complete the last segment - if use_tree.prefix.segments.is_empty() { - // We are at the start of the use segment and completing the first segment, - // let's start here. - return self.complete_in_module( - self.current_module_id, - ident.to_string(), - true, - ); - } - None - } - } else { - // TODO: handle - None - } - } + self.find_in_use_tree_path(&use_tree.prefix, ident, alias, span) } UseTreeKind::List(..) => { // TODO: handle @@ -151,6 +119,48 @@ impl<'a> NodeFinder<'a> { } } + fn find_in_use_tree_path( + &self, + use_tree_prefix: &Path, + ident: &Ident, + alias: &Option, + span: Span, + ) -> Option { + if let Some(_alias) = alias { + // Won't handle completion if there's an alias (for now) + return None; + } + + if self.byte_index != span.end() as usize { + // Won't handle autocomplete if we are not at the end of the use statement + return None; + } + + let mut segments: Vec = + use_tree_prefix.segments.iter().map(|segment| segment.ident.clone()).collect(); + + if let Some(b':') = self.byte { + // We are after the colon + segments.push(ident.clone()); + + self.resolve_module(segments).and_then(|module_id| { + let prefix = String::new(); + self.complete_in_module(module_id, prefix, false) + }) + } else { + let prefix = ident.to_string(); + + // Otherwise we must complete the last segment + if segments.is_empty() { + // We are at the start of the use segment and completing the first segment + self.complete_in_module(self.module_id, prefix, true) + } else { + self.resolve_module(segments) + .and_then(|module_id| self.complete_in_module(module_id, prefix, false)) + } + } + } + fn complete_in_module( &self, module_id: ModuleId, @@ -181,6 +191,25 @@ impl<'a> NodeFinder<'a> { Some(CompletionResponse::Array(completion_items)) } + fn resolve_module(&self, segments: Vec) -> Option { + if let Some(ModuleDefId::ModuleId(module_id)) = self.resolve_path(segments) { + Some(module_id) + } else { + None + } + } + + fn resolve_path(&self, segments: Vec) -> Option { + let path_segments = segments.into_iter().map(PathSegment::from).collect(); + let path = Path { segments: path_segments, kind: PathKind::Plain, span: Span::default() }; + + let path_resolver = StandardPathResolver::new(self.module_id); + match path_resolver.resolve(&self.def_maps, path, &mut None) { + Ok(path_resolution) => Some(path_resolution.module_def_id), + Err(_) => None, + } + } + fn includes_span(&self, span: Span) -> bool { span.start() as usize <= self.byte_index && self.byte_index <= span.end() as usize } From 47c2cb3056ebab914fdf058cfa4caeb78eaea030 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 13:52:56 -0300 Subject: [PATCH 03/26] Complete types and values --- .../src/hir/def_map/module_data.rs | 4 + tooling/lsp/src/requests/completion.rs | 120 +++++++++++++++--- 2 files changed, 108 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index 488ccc476d7..b9e3528c29f 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -133,4 +133,8 @@ impl ModuleData { pub fn value_definitions(&self) -> impl Iterator + '_ { self.definitions.values().values().flat_map(|a| a.values().map(|(id, _, _)| *id)) } + + pub fn value_idents(&self) -> impl Iterator + '_ { + self.definitions.values().keys() + } } diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 1eccd0e91bf..7745b37cff4 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -5,7 +5,10 @@ use std::{ use async_lsp::ResponseError; use fm::PathString; -use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse}; +use lsp_types::{ + CompletionItem, CompletionItemKind, CompletionItemLabelDetails, CompletionParams, + CompletionResponse, +}; use noirc_errors::Span; use noirc_frontend::{ ast::{Ident, Path, PathKind, PathSegment, UseTree, UseTreeKind}, @@ -14,7 +17,8 @@ use noirc_frontend::{ def_map::{CrateDefMap, ModuleId}, resolution::path_resolver::{PathResolver, StandardPathResolver}, }, - macros_api::ModuleDefId, + macros_api::{ModuleDefId, NodeInterner, StructId}, + node_interner::{FuncId, GlobalId, TraitId, TypeAliasId}, parser::{Item, ItemKind}, ParsedModule, }; @@ -27,8 +31,6 @@ pub(crate) fn on_completion_request( state: &mut LspState, params: CompletionParams, ) -> impl Future, ResponseError>> { - noirc_frontend::log("Completion Request"); - let uri = params.text_document_position.clone().text_document.uri; let result = process_request(state, params.text_document_position.clone(), |args| { @@ -51,6 +53,7 @@ pub(crate) fn on_completion_request( args.root_crate_id, args.def_maps, args.root_crate_dependencies, + args.interner, ); finder.find(&parsed_module) }) @@ -65,6 +68,7 @@ struct NodeFinder<'a> { module_id: ModuleId, def_maps: &'a BTreeMap, dependencies: &'a Vec, + interner: &'a NodeInterner, } impl<'a> NodeFinder<'a> { @@ -74,10 +78,11 @@ impl<'a> NodeFinder<'a> { krate: CrateId, def_maps: &'a BTreeMap, dependencies: &'a Vec, + interner: &'a NodeInterner, ) -> Self { let def_map = &def_maps[&krate]; let current_module_id = ModuleId { krate: krate, local_id: def_map.root() }; - Self { byte_index, byte, module_id: current_module_id, def_maps, dependencies } + Self { byte_index, byte, module_id: current_module_id, def_maps, dependencies, interner } } fn find(&self, parsed_module: &ParsedModule) -> Option { @@ -171,10 +176,27 @@ impl<'a> NodeFinder<'a> { let module_data = def_map.modules().get(module_id.local_id.0)?; let mut completion_items = Vec::new(); - // Find in the target module - for child in module_data.children.keys() { - if name_matches(&child.0.contents, &prefix) { - completion_items.push(module_completion_item(child.0.contents.clone())); + // module_data.value_definitions() + + // Find in the target module types and values + let type_idents = module_data.children.keys(); + let value_idents = module_data.value_idents(); + let all_idents = type_idents.chain(value_idents); + + for ident in all_idents { + let name = &ident.0.contents; + + if name_matches(name, &prefix) { + let per_ns = module_data.find_name(ident); + if let Some((module_def_id, _, _)) = per_ns.types { + completion_items + .push(self.module_def_id_completion_item(module_def_id, name.clone())); + } + + if let Some((module_def_id, _, _)) = per_ns.values { + completion_items + .push(self.module_def_id_completion_item(module_def_id, name.clone())); + } } } @@ -191,6 +213,64 @@ impl<'a> NodeFinder<'a> { Some(CompletionResponse::Array(completion_items)) } + fn module_def_id_completion_item( + &self, + module_def_id: ModuleDefId, + name: String, + ) -> CompletionItem { + match module_def_id { + ModuleDefId::ModuleId(_) => module_completion_item(name), + ModuleDefId::FunctionId(func_id) => self.function_completion_item(func_id), + ModuleDefId::TypeId(struct_id) => self.struct_completion_item(struct_id), + ModuleDefId::TypeAliasId(type_alias_id) => { + self.type_alias_completion_item(type_alias_id) + } + ModuleDefId::TraitId(trait_id) => self.trait_completion_item(trait_id), + ModuleDefId::GlobalId(global_id) => self.global_completion_item(global_id), + } + } + + fn function_completion_item(&self, func_id: FuncId) -> CompletionItem { + let name = self.interner.function_name(&func_id).to_string(); + let description = self.interner.function_meta(&func_id).typ.to_string(); + + simple_completion_item(name, CompletionItemKind::FUNCTION, Some(description)) + } + + fn struct_completion_item(&self, struct_id: StructId) -> CompletionItem { + let struct_type = self.interner.get_struct(struct_id); + let struct_type = struct_type.borrow(); + let name = struct_type.name.to_string(); + + simple_completion_item(name.clone(), CompletionItemKind::STRUCT, Some(name)) + } + + fn type_alias_completion_item(&self, type_alias_id: TypeAliasId) -> CompletionItem { + let type_alias = self.interner.get_type_alias(type_alias_id); + let type_alias = type_alias.borrow(); + let name = type_alias.name.to_string(); + + simple_completion_item(name.clone(), CompletionItemKind::STRUCT, Some(name)) + } + + fn trait_completion_item(&self, trait_id: TraitId) -> CompletionItem { + let trait_ = self.interner.get_trait(trait_id); + let name = trait_.name.to_string(); + + simple_completion_item(name.clone(), CompletionItemKind::INTERFACE, Some(name)) + } + + fn global_completion_item(&self, global_id: GlobalId) -> CompletionItem { + let global_definition = self.interner.get_global_definition(global_id); + let name = global_definition.name.clone(); + + let global = self.interner.get_global(global_id); + let typ = self.interner.definition_type(global.definition_id); + let description = typ.to_string(); + + simple_completion_item(name, CompletionItemKind::CONSTANT, Some(description)) + } + fn resolve_module(&self, segments: Vec) -> Option { if let Some(ModuleDefId::ModuleId(module_id)) = self.resolve_path(segments) { Some(module_id) @@ -220,10 +300,22 @@ fn name_matches(name: &str, prefix: &str) -> bool { } fn module_completion_item(name: String) -> CompletionItem { + simple_completion_item(name, CompletionItemKind::MODULE, None) +} + +fn crate_completion_item(name: String) -> CompletionItem { + simple_completion_item(name, CompletionItemKind::MODULE, None) +} + +fn simple_completion_item( + label: String, + kind: CompletionItemKind, + description: Option, +) -> CompletionItem { CompletionItem { - label: name, - label_details: None, - kind: Some(CompletionItemKind::MODULE), + label, + label_details: Some(CompletionItemLabelDetails { detail: None, description: description }), + kind: Some(kind), detail: None, documentation: None, deprecated: None, @@ -241,7 +333,3 @@ fn module_completion_item(name: String) -> CompletionItem { tags: None, } } - -fn crate_completion_item(name: String) -> CompletionItem { - module_completion_item(name) -} From 189f4fc89bdc58c6196babb8a79d8e36b77c313b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 14:05:49 -0300 Subject: [PATCH 04/26] Show everything in a module's scope --- compiler/noirc_frontend/src/hir/def_map/module_data.rs | 2 +- tooling/lsp/src/requests/completion.rs | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index b9e3528c29f..e263c8eb303 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -38,7 +38,7 @@ impl ModuleData { } } - pub(crate) fn scope(&self) -> &ItemScope { + pub fn scope(&self) -> &ItemScope { &self.scope } diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 7745b37cff4..061044b0d96 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -176,14 +176,7 @@ impl<'a> NodeFinder<'a> { let module_data = def_map.modules().get(module_id.local_id.0)?; let mut completion_items = Vec::new(); - // module_data.value_definitions() - - // Find in the target module types and values - let type_idents = module_data.children.keys(); - let value_idents = module_data.value_idents(); - let all_idents = type_idents.chain(value_idents); - - for ident in all_idents { + for ident in module_data.scope().names() { let name = &ident.0.contents; if name_matches(name, &prefix) { From d9f1b8cb3de874eecb1afa562bd11906c9d934bc Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 14:26:58 -0300 Subject: [PATCH 05/26] Actually, only suggest direct children for now --- compiler/noirc_frontend/src/hir/def_map/module_data.rs | 6 +++++- tooling/lsp/src/requests/completion.rs | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index e263c8eb303..688e193fea4 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -38,10 +38,14 @@ impl ModuleData { } } - pub fn scope(&self) -> &ItemScope { + pub(crate) fn scope(&self) -> &ItemScope { &self.scope } + pub fn definitions(&self) -> &ItemScope { + &self.definitions + } + fn declare( &mut self, name: Ident, diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 061044b0d96..363490c6fa6 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -176,7 +176,7 @@ impl<'a> NodeFinder<'a> { let module_data = def_map.modules().get(module_id.local_id.0)?; let mut completion_items = Vec::new(); - for ident in module_data.scope().names() { + for ident in module_data.definitions().names() { let name = &ident.0.contents; if name_matches(name, &prefix) { From a2bf2bc8826cc291ad633e40b921cb93b12beb7c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 14:27:08 -0300 Subject: [PATCH 06/26] Remove `forall ...` from function type --- tooling/lsp/src/requests/completion.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 363490c6fa6..a85d2d8fa38 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -20,7 +20,7 @@ use noirc_frontend::{ macros_api::{ModuleDefId, NodeInterner, StructId}, node_interner::{FuncId, GlobalId, TraitId, TypeAliasId}, parser::{Item, ItemKind}, - ParsedModule, + ParsedModule, Type, }; use crate::{utils, LspState}; @@ -225,7 +225,11 @@ impl<'a> NodeFinder<'a> { fn function_completion_item(&self, func_id: FuncId) -> CompletionItem { let name = self.interner.function_name(&func_id).to_string(); - let description = self.interner.function_meta(&func_id).typ.to_string(); + let mut typ = &self.interner.function_meta(&func_id).typ; + if let Type::Forall(_, typ_) = typ { + typ = typ_; + } + let description = typ.to_string(); simple_completion_item(name, CompletionItemKind::FUNCTION, Some(description)) } From dc45168f1a14ae727eb28352bc720ecc7706a678 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 15:20:46 -0300 Subject: [PATCH 07/26] Add a test --- tooling/lsp/src/notifications/mod.rs | 2 +- tooling/lsp/src/requests/completion.rs | 85 +++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 1a27b9a89d6..3a60de15c4a 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -30,7 +30,7 @@ pub(super) fn on_did_change_configuration( ControlFlow::Continue(()) } -pub(super) fn on_did_open_text_document( +pub(crate) fn on_did_open_text_document( state: &mut LspState, params: DidOpenTextDocumentParams, ) -> ControlFlow> { diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index a85d2d8fa38..40b7d305a39 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -296,8 +296,8 @@ fn name_matches(name: &str, prefix: &str) -> bool { name.starts_with(prefix) } -fn module_completion_item(name: String) -> CompletionItem { - simple_completion_item(name, CompletionItemKind::MODULE, None) +fn module_completion_item(name: impl Into) -> CompletionItem { + simple_completion_item(name.into(), CompletionItemKind::MODULE, None) } fn crate_completion_item(name: String) -> CompletionItem { @@ -330,3 +330,84 @@ fn simple_completion_item( tags: None, } } + +#[cfg(test)] +mod completion_tests { + use crate::{notifications::on_did_open_text_document, test_utils}; + + use super::*; + use lsp_types::{ + DidOpenTextDocumentParams, PartialResultParams, Position, TextDocumentIdentifier, + TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams, + }; + use tokio::test; + + async fn assert_completion(src: &str, expected: Vec) { + let (mut state, noir_text_document) = test_utils::init_lsp_server("document_symbol").await; + + let (line, column) = src + .lines() + .enumerate() + .filter_map(|(line_index, line)| { + line.find(">|<").map(|char_index| (line_index, char_index)) + }) + .nth(0) + .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(), + }, + }, + ); + + // Get inlay hints. These should now be relative to the changed text, + // not the saved file's text. + let response = on_completion_request( + &mut state, + CompletionParams { + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: noir_text_document }, + position: Position { line: line as u32, character: column as u32 }, + }, + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + partial_result_params: PartialResultParams { partial_result_token: None }, + context: None, + }, + ) + .await + .expect("Could not execute on_completion_request") + .unwrap(); + + let CompletionResponse::Array(items) = response else { + panic!("Expected response to be CompletionResponse::Array"); + }; + + let items = items.clone().sort_by_key(|item| item.label.clone()); + let expected = expected.clone().sort_by_key(|item| item.label.clone()); + + assert_eq!(items, expected); + } + + #[test] + async fn test_use_first_segment() { + let src = r#" + mod foo {} + mod foobar {} + use f>|< + "#; + + assert_completion( + src, + vec![module_completion_item("foo"), module_completion_item("foobar")], + ) + .await; + } +} From b8441c7d34d4a0b690883fb0abec526b6deb28a4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 15:26:15 -0300 Subject: [PATCH 08/26] More tests --- tooling/lsp/src/requests/completion.rs | 71 +++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 40b7d305a39..a09dab72c95 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -305,12 +305,12 @@ fn crate_completion_item(name: String) -> CompletionItem { } fn simple_completion_item( - label: String, + label: impl Into, kind: CompletionItemKind, description: Option, ) -> CompletionItem { CompletionItem { - label, + label: label.into(), label_details: Some(CompletionItemLabelDetails { detail: None, description: description }), kind: Some(kind), detail: None, @@ -410,4 +410,71 @@ mod completion_tests { ) .await; } + + #[test] + async fn test_use_second_segment() { + let src = r#" + mod foo { + mod bar {} + mod baz {} + } + use foo::>|< + "#; + + assert_completion(src, vec![module_completion_item("bar"), module_completion_item("baz")]) + .await; + } + + #[test] + async fn test_use_second_segment_after_typing() { + let src = r#" + mod foo { + mod bar {} + mod brave {} + } + use foo::ba>|< + "#; + + assert_completion(src, vec![module_completion_item("bar")]).await; + } + + #[test] + async fn test_use_struct() { + let src = r#" + mod foo { + struct Foo {} + } + use foo::>|< + "#; + + assert_completion( + src, + vec![simple_completion_item( + "Foo", + CompletionItemKind::STRUCT, + Some("Foo".to_string()), + )], + ) + .await; + } + + #[test] + async fn test_use_function() { + let src = r#" + mod foo { + fn bar(x: i32) -> u64 { 0 } + } + use foo::>|< + "#; + + assert_completion( + src, + vec![simple_completion_item( + "bar", + CompletionItemKind::FUNCTION, + Some("fn(i32) -> u64".to_string()), + )], + ) + .await; + } } From ad557626db47ff7cf792877fcf15c938f6d0c3aa Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 15:52:02 -0300 Subject: [PATCH 09/26] Don't suggest crate completions on `crate::` --- tooling/lsp/src/requests/completion.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index a09dab72c95..8d51f62659e 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -118,7 +118,7 @@ impl<'a> NodeFinder<'a> { self.find_in_use_tree_path(&use_tree.prefix, ident, alias, span) } UseTreeKind::List(..) => { - // TODO: handle + // For now we don't offer autocompletion in use trees None } } @@ -158,7 +158,11 @@ impl<'a> NodeFinder<'a> { // Otherwise we must complete the last segment if segments.is_empty() { // We are at the start of the use segment and completing the first segment - self.complete_in_module(self.module_id, prefix, true) + self.complete_in_module( + self.module_id, + prefix, + use_tree_prefix.kind != PathKind::Crate, + ) } else { self.resolve_module(segments) .and_then(|module_id| self.complete_in_module(module_id, prefix, false)) @@ -170,7 +174,7 @@ impl<'a> NodeFinder<'a> { &self, module_id: ModuleId, prefix: String, - first_segment: bool, + suggest_crates: bool, ) -> Option { let def_map = &self.def_maps[&module_id.krate]; let module_data = def_map.modules().get(module_id.local_id.0)?; @@ -194,7 +198,7 @@ impl<'a> NodeFinder<'a> { } // If this is the first segment, also find in all crates - if first_segment { + if suggest_crates { for dependency in self.dependencies { let dependency_name = dependency.as_name(); if name_matches(&dependency_name, &prefix) { @@ -477,4 +481,14 @@ mod completion_tests { ) .await; } + + #[test] + async fn test_use_after_crate_and_letter() { + let src = r#" + mod foo {} + use crate::f>|< + "#; + + assert_completion(src, vec![module_completion_item("foo")]).await; + } } From 16dd3b5ff5924b55e4ccbeb8a934a511df710309 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 16:12:23 -0300 Subject: [PATCH 10/26] Fix tests --- tooling/lsp/src/requests/completion.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 8d51f62659e..a8ed8d6b298 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -394,8 +394,11 @@ mod completion_tests { panic!("Expected response to be CompletionResponse::Array"); }; - let items = items.clone().sort_by_key(|item| item.label.clone()); - let expected = expected.clone().sort_by_key(|item| item.label.clone()); + let mut items = items.clone(); + items.sort_by_key(|item| item.label.clone()); + + let mut expected = expected.clone(); + expected.sort_by_key(|item| item.label.clone()); assert_eq!(items, expected); } From 296467e0c891e788697fc85e8143cb4d6aba1c58 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 16:24:48 -0300 Subject: [PATCH 11/26] Suggest "crate::" --- tooling/lsp/src/requests/completion.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index a8ed8d6b298..381110ce8d4 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -205,6 +205,14 @@ impl<'a> NodeFinder<'a> { completion_items.push(crate_completion_item(dependency_name)); } } + + if name_matches("crate::", &prefix) { + completion_items.push(simple_completion_item( + "crate::", + CompletionItemKind::KEYWORD, + None, + )); + } } Some(CompletionResponse::Array(completion_items)) @@ -494,4 +502,17 @@ mod completion_tests { assert_completion(src, vec![module_completion_item("foo")]).await; } + + #[test] + async fn test_use_suggests_hardcoded_crate() { + let src = r#" + use c>|< + "#; + + assert_completion( + src, + vec![simple_completion_item("crate::", CompletionItemKind::KEYWORD, None)], + ) + .await; + } } From de6cd1c446d1c88e37a24aa808fdb2cdab0010e0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 16:37:25 -0300 Subject: [PATCH 12/26] Autocomplete in use tree --- tooling/lsp/src/requests/completion.rs | 53 +++++++++++++++++++++----- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 381110ce8d4..a1deb4fe36d 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -102,7 +102,9 @@ impl<'a> NodeFinder<'a> { match &item.kind { ItemKind::Import(use_tree) => { - if let Some(completion) = self.find_in_use_tree(use_tree, item.span) { + let mut prefixes = Vec::new(); + if let Some(completion) = self.find_in_use_tree(use_tree, item.span, &mut prefixes) + { return Some(completion); } } @@ -112,13 +114,27 @@ impl<'a> NodeFinder<'a> { None } - fn find_in_use_tree(&self, use_tree: &UseTree, span: Span) -> Option { + fn find_in_use_tree( + &self, + use_tree: &UseTree, + span: Span, + prefixes: &mut Vec, + ) -> Option { match &use_tree.kind { UseTreeKind::Path(ident, alias) => { - self.find_in_use_tree_path(&use_tree.prefix, ident, alias, span) + prefixes.push(use_tree.prefix.clone()); + let response = self.find_in_use_tree_path(&prefixes, ident, alias, span); + prefixes.pop(); + response } - UseTreeKind::List(..) => { - // For now we don't offer autocompletion in use trees + UseTreeKind::List(use_trees) => { + prefixes.push(use_tree.prefix.clone()); + for use_tree in use_trees { + if let Some(completion) = self.find_in_use_tree(use_tree, span, prefixes) { + return Some(completion); + } + } + prefixes.pop(); None } } @@ -126,7 +142,7 @@ impl<'a> NodeFinder<'a> { fn find_in_use_tree_path( &self, - use_tree_prefix: &Path, + prefixes: &Vec, ident: &Ident, alias: &Option, span: Span, @@ -136,13 +152,18 @@ impl<'a> NodeFinder<'a> { return None; } - if self.byte_index != span.end() as usize { + if self.byte_index != ident.span().end() as usize && self.byte_index != span.end() as usize + { // Won't handle autocomplete if we are not at the end of the use statement return None; } - let mut segments: Vec = - use_tree_prefix.segments.iter().map(|segment| segment.ident.clone()).collect(); + let mut segments: Vec = Vec::new(); + for prefix in prefixes { + for segment in &prefix.segments { + segments.push(segment.ident.clone()); + } + } if let Some(b':') = self.byte { // We are after the colon @@ -161,7 +182,7 @@ impl<'a> NodeFinder<'a> { self.complete_in_module( self.module_id, prefix, - use_tree_prefix.kind != PathKind::Crate, + prefixes.first().unwrap().kind != PathKind::Crate, ) } else { self.resolve_module(segments) @@ -515,4 +536,16 @@ mod completion_tests { ) .await; } + + #[test] + async fn test_use_in_tree() { + let src = r#" + mod foo { + mod bar {} + } + use foo::{b>|<} + "#; + + assert_completion(src, vec![module_completion_item("bar")]).await; + } } From d70fbff9910cbddcba5352aa1173a1895c77da04 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 16:42:33 -0300 Subject: [PATCH 13/26] Complete after colons in use tree --- tooling/lsp/src/requests/completion.rs | 28 +++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index a1deb4fe36d..be312fe9fa0 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -152,9 +152,13 @@ impl<'a> NodeFinder<'a> { return None; } - if self.byte_index != ident.span().end() as usize && self.byte_index != span.end() as usize - { - // Won't handle autocomplete if we are not at the end of the use statement + let after_colons = self.byte == Some(b':'); + let at_use_end = self.byte_index != span.end() as usize; + let at_ident_end = self.byte_index == ident.span().end() as usize; + let at_ident_colons_end = + after_colons && self.byte_index - 2 == ident.span().end() as usize; + + if !(at_use_end || at_ident_end || at_ident_colons_end) { return None; } @@ -165,7 +169,7 @@ impl<'a> NodeFinder<'a> { } } - if let Some(b':') = self.byte { + if after_colons { // We are after the colon segments.push(ident.clone()); @@ -538,7 +542,7 @@ mod completion_tests { } #[test] - async fn test_use_in_tree() { + async fn test_use_in_tree_after_letter() { let src = r#" mod foo { mod bar {} @@ -548,4 +552,18 @@ mod completion_tests { assert_completion(src, vec![module_completion_item("bar")]).await; } + + #[test] + async fn test_use_in_tree_after_colons() { + let src = r#" + mod foo { + mod bar { + mod baz {} + } + } + use foo::{bar::>|<} + "#; + + assert_completion(src, vec![module_completion_item("baz")]).await; + } } From 522f7486326a530de5bd4550d42f7615e362cdb2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Aug 2024 16:51:04 -0300 Subject: [PATCH 14/26] Some fixes --- tooling/lsp/src/requests/completion.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index be312fe9fa0..2c1dfbf304c 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -103,8 +103,7 @@ impl<'a> NodeFinder<'a> { match &item.kind { ItemKind::Import(use_tree) => { let mut prefixes = Vec::new(); - if let Some(completion) = self.find_in_use_tree(use_tree, item.span, &mut prefixes) - { + if let Some(completion) = self.find_in_use_tree(use_tree, &mut prefixes) { return Some(completion); } } @@ -117,20 +116,19 @@ impl<'a> NodeFinder<'a> { fn find_in_use_tree( &self, use_tree: &UseTree, - span: Span, prefixes: &mut Vec, ) -> Option { match &use_tree.kind { UseTreeKind::Path(ident, alias) => { prefixes.push(use_tree.prefix.clone()); - let response = self.find_in_use_tree_path(&prefixes, ident, alias, span); + let response = self.find_in_use_tree_path(&prefixes, ident, alias); prefixes.pop(); response } UseTreeKind::List(use_trees) => { prefixes.push(use_tree.prefix.clone()); for use_tree in use_trees { - if let Some(completion) = self.find_in_use_tree(use_tree, span, prefixes) { + if let Some(completion) = self.find_in_use_tree(use_tree, prefixes) { return Some(completion); } } @@ -145,7 +143,6 @@ impl<'a> NodeFinder<'a> { prefixes: &Vec, ident: &Ident, alias: &Option, - span: Span, ) -> Option { if let Some(_alias) = alias { // Won't handle completion if there's an alias (for now) @@ -153,12 +150,11 @@ impl<'a> NodeFinder<'a> { } let after_colons = self.byte == Some(b':'); - let at_use_end = self.byte_index != span.end() as usize; let at_ident_end = self.byte_index == ident.span().end() as usize; let at_ident_colons_end = after_colons && self.byte_index - 2 == ident.span().end() as usize; - if !(at_use_end || at_ident_end || at_ident_colons_end) { + if !(at_ident_end || at_ident_colons_end) { return None; } @@ -566,4 +562,17 @@ mod completion_tests { assert_completion(src, vec![module_completion_item("baz")]).await; } + + #[test] + async fn test_use_in_tree_after_colons_after_another_segment() { + let src = r#" + mod foo { + mod bar {} + mod qux {} + } + use foo::{bar, q>|<} + "#; + + assert_completion(src, vec![module_completion_item("qux")]).await; + } } From 3c82b82e592f7c6dbbd63407642c097d5c1ae78d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 07:26:23 -0300 Subject: [PATCH 15/26] Produce an error when a path has trailing colons --- compiler/noirc_frontend/src/parser/errors.rs | 2 ++ .../noirc_frontend/src/parser/parser/path.rs | 32 ++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index 80adb01dc9a..dc587dbdc11 100644 --- a/compiler/noirc_frontend/src/parser/errors.rs +++ b/compiler/noirc_frontend/src/parser/errors.rs @@ -16,6 +16,8 @@ pub enum ParserErrorReason { ExpectedFieldName(Token), #[error("expected a pattern but found a type - {0}")] ExpectedPatternButFoundType(Token), + #[error("expected an identifier after ::")] + ExpectedIdentifierAfterColons, #[error("Expected a ; separating these two statements")] MissingSeparatingSemi, #[error("constrain keyword is deprecated")] diff --git a/compiler/noirc_frontend/src/parser/parser/path.rs b/compiler/noirc_frontend/src/parser/parser/path.rs index 624ed51a7b5..2eb0807d684 100644 --- a/compiler/noirc_frontend/src/parser/parser/path.rs +++ b/compiler/noirc_frontend/src/parser/parser/path.rs @@ -1,5 +1,5 @@ use crate::ast::{Path, PathKind, PathSegment, UnresolvedType}; -use crate::parser::NoirParser; +use crate::parser::{NoirParser, ParserError, ParserErrorReason}; use crate::token::{Keyword, Token}; @@ -23,8 +23,13 @@ fn path_inner<'a>(segment: impl NoirParser + 'a) -> impl NoirParser .separated_by(just(Token::DoubleColon)) .at_least(1) .then(just(Token::DoubleColon).then_ignore(none_of(Token::LeftBrace).rewind()).or_not()) - .map(|(path_segments, _trailing_colons)| { - // TODO: give an error if there are trailing colons + .validate(|(path_segments, trailing_colons), span, emit_error| { + if trailing_colons.is_some() { + emit_error(ParserError::with_reason( + ParserErrorReason::ExpectedIdentifierAfterColons, + span, + )) + } path_segments }); let make_path = |kind| move |segments, span| Path { segments, kind, span }; @@ -57,7 +62,7 @@ mod test { use super::*; use crate::parser::{ parse_type, - parser::test_helpers::{parse_all_failing, parse_with}, + parser::test_helpers::{parse_all_failing, parse_recover, parse_with}, }; #[test] @@ -100,12 +105,17 @@ mod test { ); } - // TODO - // #[test] - // fn parse_path_with_trailing_colons() { - // let src = "foo::bar::"; + #[test] + fn parse_path_with_trailing_colons() { + let src = "foo::bar::"; + + let (path, errors) = parse_recover(path_no_turbofish(), src); + let path = path.unwrap(); + assert_eq!(path.segments.len(), 2); + assert_eq!(path.segments[0].ident.0.contents, "foo"); + assert_eq!(path.segments[1].ident.0.contents, "bar"); - // let x = parse_with(path_no_turbofish(), src); - // dbg!(x); - // } + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].message, "expected an identifier after ::"); + } } From 879c4ecaa6209d17a353ac2de96f7135c8936e2b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 09:33:02 -0300 Subject: [PATCH 16/26] Consider submodules --- tooling/lsp/src/requests/completion.rs | 77 ++++++++++++++++++++++++-- tooling/lsp/src/requests/hover.rs | 4 +- tooling/lsp/src/requests/mod.rs | 12 ++-- 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 2c1dfbf304c..335648eba94 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -47,12 +47,12 @@ pub(crate) fn on_completion_request( let byte = source.bytes().nth(byte_index - 1); let (parsed_module, _errors) = noirc_frontend::parse_program(source); - let finder = NodeFinder::new( + let mut finder = NodeFinder::new( byte_index, byte, - args.root_crate_id, + args.crate_id, args.def_maps, - args.root_crate_dependencies, + args.dependencies, args.interner, ); finder.find(&parsed_module) @@ -85,7 +85,14 @@ impl<'a> NodeFinder<'a> { Self { byte_index, byte, module_id: current_module_id, def_maps, dependencies, interner } } - fn find(&self, parsed_module: &ParsedModule) -> Option { + fn find(&mut self, parsed_module: &ParsedModule) -> Option { + self.find_in_parsed_module(parsed_module) + } + + fn find_in_parsed_module( + &mut self, + parsed_module: &ParsedModule, + ) -> Option { for item in &parsed_module.items { if let Some(response) = self.find_in_item(item) { return Some(response); @@ -95,7 +102,7 @@ impl<'a> NodeFinder<'a> { None } - fn find_in_item(&self, item: &Item) -> Option { + fn find_in_item(&mut self, item: &Item) -> Option { if !self.includes_span(item.span) { return None; } @@ -107,6 +114,26 @@ impl<'a> NodeFinder<'a> { return Some(completion); } } + ItemKind::Submodules(parsed_sub_module) => { + // 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 module_data = def_map.modules().get(self.module_id.local_id.0)?; + 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 completion = self.find_in_parsed_module(&parsed_sub_module.contents); + + // Restore the old module before continuing + self.module_id = previous_module_id; + + if let Some(completion) = completion { + return Some(completion); + } + } _ => (), } @@ -234,6 +261,14 @@ impl<'a> NodeFinder<'a> { None, )); } + + if module_data.parent.is_some() && name_matches("super::", &prefix) { + completion_items.push(simple_completion_item( + "super::", + CompletionItemKind::KEYWORD, + None, + )); + } } Some(CompletionResponse::Array(completion_items)) @@ -429,6 +464,17 @@ mod completion_tests { let mut expected = expected.clone(); expected.sort_by_key(|item| item.label.clone()); + if items != expected { + println!( + "Items: {:?}", + items.iter().map(|item| item.label.clone()).collect::>() + ); + println!( + "Expected: {:?}", + expected.iter().map(|item| item.label.clone()).collect::>() + ); + } + assert_eq!(items, expected); } @@ -575,4 +621,25 @@ mod completion_tests { assert_completion(src, vec![module_completion_item("qux")]).await; } + + #[test] + async fn test_use_in_nested_module() { + let src = r#" + mod foo { + mod something {} + + use s>|< + } + "#; + + assert_completion( + src, + vec![ + module_completion_item("something"), + module_completion_item("std"), + simple_completion_item("super::", CompletionItemKind::KEYWORD, None), + ], + ) + .await; + } } diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 73ea504b496..b6fdc6f7842 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -321,9 +321,9 @@ fn format_parent_module_from_module_id( ) -> bool { let crate_id = module.krate; let crate_name = match crate_id { - CrateId::Root(_) => Some(args.root_crate_name.clone()), + CrateId::Root(_) => Some(args.crate_name.clone()), CrateId::Crate(_) => args - .root_crate_dependencies + .dependencies .iter() .find(|dep| dep.crate_id == crate_id) .map(|dep| format!("{}", dep.name)), diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 0f4a436174b..e138f839600 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -388,9 +388,9 @@ pub(crate) struct ProcessRequestCallbackArgs<'a> { files: &'a FileMap, interner: &'a NodeInterner, interners: &'a HashMap, - root_crate_id: CrateId, - root_crate_name: String, - root_crate_dependencies: &'a Vec, + crate_id: CrateId, + crate_name: String, + dependencies: &'a Vec, def_maps: &'a BTreeMap, } @@ -450,9 +450,9 @@ where files, interner, interners: &state.cached_definitions, - root_crate_id: crate_id, - root_crate_name: package.name.to_string(), - root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies, + crate_id, + crate_name: package.name.to_string(), + dependencies: &context.crate_graph[context.root_crate_id()].dependencies, def_maps, })) } From 6899df57b3cca3fd021d92e126bd163fa4f284c0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 09:50:32 -0300 Subject: [PATCH 17/26] Use current module_id --- tooling/lsp/src/requests/completion.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 335648eba94..ab61cfbf631 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -4,7 +4,7 @@ use std::{ }; use async_lsp::ResponseError; -use fm::PathString; +use fm::{FileId, PathString}; use lsp_types::{ CompletionItem, CompletionItemKind, CompletionItemLabelDetails, CompletionParams, CompletionResponse, @@ -14,7 +14,7 @@ use noirc_frontend::{ ast::{Ident, Path, PathKind, PathSegment, UseTree, UseTreeKind}, graph::{CrateId, Dependency}, hir::{ - def_map::{CrateDefMap, ModuleId}, + def_map::{CrateDefMap, LocalModuleId, ModuleId}, resolution::path_resolver::{PathResolver, StandardPathResolver}, }, macros_api::{ModuleDefId, NodeInterner, StructId}, @@ -48,6 +48,7 @@ pub(crate) fn on_completion_request( let (parsed_module, _errors) = noirc_frontend::parse_program(source); let mut finder = NodeFinder::new( + file_id, byte_index, byte, args.crate_id, @@ -73,6 +74,7 @@ struct NodeFinder<'a> { impl<'a> NodeFinder<'a> { fn new( + file: FileId, byte_index: usize, byte: Option, krate: CrateId, @@ -80,9 +82,17 @@ impl<'a> NodeFinder<'a> { dependencies: &'a Vec, interner: &'a NodeInterner, ) -> Self { + // Find the module the current file belongs to let def_map = &def_maps[&krate]; - let current_module_id = ModuleId { krate: krate, local_id: def_map.root() }; - Self { byte_index, byte, module_id: current_module_id, def_maps, dependencies, interner } + 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 { byte_index, byte, module_id, def_maps, dependencies, interner } } fn find(&mut self, parsed_module: &ParsedModule) -> Option { From 7bfa5cdfc0208c63b4cfefaafbee5c48c574a856 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 09:52:14 -0300 Subject: [PATCH 18/26] Take a sad test and make it better --- tooling/lsp/src/requests/completion.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index ab61cfbf631..f404828939a 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -572,12 +572,18 @@ mod completion_tests { #[test] async fn test_use_after_crate_and_letter() { + // Prove that "std" shows up let src = r#" - mod foo {} - use crate::f>|< + use s>|< "#; + assert_completion(src, vec![module_completion_item("std")]).await; - assert_completion(src, vec![module_completion_item("foo")]).await; + // "std" doesn't show up anymore because of the "crate::" prefix + let src = r#" + mod something {} + use crate::s>|< + "#; + assert_completion(src, vec![module_completion_item("something")]).await; } #[test] From 0d4c7e44694155dee6753415564897319354efa7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 10:03:06 -0300 Subject: [PATCH 19/26] Correctly handle super autocompletions --- tooling/lsp/src/requests/completion.rs | 44 ++++++++++++++++++++------ 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index f404828939a..85c8e6de1da 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -195,6 +195,8 @@ impl<'a> NodeFinder<'a> { return None; } + let is_super = prefixes.get(0).map(|prefix| prefix.kind) == Some(PathKind::Super); + let mut segments: Vec = Vec::new(); for prefix in prefixes { for segment in &prefix.segments { @@ -208,7 +210,8 @@ impl<'a> NodeFinder<'a> { self.resolve_module(segments).and_then(|module_id| { let prefix = String::new(); - self.complete_in_module(module_id, prefix, false) + let suggest_crates = false; + self.complete_in_module(module_id, prefix, is_super, suggest_crates) }) } else { let prefix = ident.to_string(); @@ -216,14 +219,13 @@ impl<'a> NodeFinder<'a> { // Otherwise we must complete the last segment if segments.is_empty() { // We are at the start of the use segment and completing the first segment - self.complete_in_module( - self.module_id, - prefix, - prefixes.first().unwrap().kind != PathKind::Crate, - ) + let suggest_crates = prefixes.first().unwrap().kind != PathKind::Crate; + self.complete_in_module(self.module_id, prefix, is_super, suggest_crates) } else { - self.resolve_module(segments) - .and_then(|module_id| self.complete_in_module(module_id, prefix, false)) + self.resolve_module(segments).and_then(|module_id| { + let suggest_crates = false; + self.complete_in_module(module_id, prefix, is_super, suggest_crates) + }) } } } @@ -232,10 +234,17 @@ impl<'a> NodeFinder<'a> { &self, module_id: ModuleId, prefix: String, - suggest_crates: bool, + is_super: bool, + mut suggest_crates: bool, ) -> Option { let def_map = &self.def_maps[&module_id.krate]; - let module_data = def_map.modules().get(module_id.local_id.0)?; + let mut module_data = def_map.modules().get(module_id.local_id.0)?; + + if is_super { + module_data = def_map.modules().get(module_data.parent?.0)?; + suggest_crates = false; + } + let mut completion_items = Vec::new(); for ident in module_data.definitions().names() { @@ -658,4 +667,19 @@ mod completion_tests { ) .await; } + + #[test] + async fn test_use_after_super() { + let src = r#" + mod foo {} + + mod bar { + mod something {} + + use super::f>|< + } + "#; + + assert_completion(src, vec![module_completion_item("foo")]).await; + } } From ab477334ff5b24a17412d6c5d2ea5bc6120ccc98 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 10:04:11 -0300 Subject: [PATCH 20/26] Use `crate_completion_item` in tests --- tooling/lsp/src/requests/completion.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 85c8e6de1da..1dc5964a6a5 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -384,10 +384,10 @@ fn name_matches(name: &str, prefix: &str) -> bool { } fn module_completion_item(name: impl Into) -> CompletionItem { - simple_completion_item(name.into(), CompletionItemKind::MODULE, None) + simple_completion_item(name, CompletionItemKind::MODULE, None) } -fn crate_completion_item(name: String) -> CompletionItem { +fn crate_completion_item(name: impl Into) -> CompletionItem { simple_completion_item(name, CompletionItemKind::MODULE, None) } @@ -585,7 +585,7 @@ mod completion_tests { let src = r#" use s>|< "#; - assert_completion(src, vec![module_completion_item("std")]).await; + assert_completion(src, vec![crate_completion_item("std")]).await; // "std" doesn't show up anymore because of the "crate::" prefix let src = r#" @@ -661,7 +661,7 @@ mod completion_tests { src, vec![ module_completion_item("something"), - module_completion_item("std"), + crate_completion_item("std"), simple_completion_item("super::", CompletionItemKind::KEYWORD, None), ], ) From b02928d0c8aeeec2cd7ce3d656f84f35cecd8164 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 10:04:31 -0300 Subject: [PATCH 21/26] Remove log function --- compiler/noirc_frontend/src/lib.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index 3f898f732d6..b14f65a3e35 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -84,18 +84,3 @@ pub mod macros_api { ) -> Result<(), (MacroError, FileId)>; } } - -pub fn log(contents: &str) { - use std::fs::OpenOptions; - use std::io::prelude::*; - - let mut file = OpenOptions::new() - .write(true) - .append(true) - .open("/Users/asterite/Sandbox/output/lsp.txt") - .unwrap(); - - if let Err(e) = writeln!(file, "{}", contents) { - eprintln!("Couldn't write to file: {}", e); - } -} From 621fa846f6a9a7deac1348a5b6498bf71f3f83e1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 10:06:21 -0300 Subject: [PATCH 22/26] Remove unused `value_idents` --- compiler/noirc_frontend/src/hir/def_map/module_data.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index 688e193fea4..22875ffe18a 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -137,8 +137,4 @@ impl ModuleData { pub fn value_definitions(&self) -> impl Iterator + '_ { self.definitions.values().values().flat_map(|a| a.values().map(|(id, _, _)| *id)) } - - pub fn value_idents(&self) -> impl Iterator + '_ { - self.definitions.values().keys() - } } From 5371634a74fd323a0f582686c38d1cebce4c3540 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 10:07:24 -0300 Subject: [PATCH 23/26] clippy --- compiler/noirc_frontend/src/parser/parser/path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/parser/parser/path.rs b/compiler/noirc_frontend/src/parser/parser/path.rs index a61c1fa1176..ae3a1bc0b93 100644 --- a/compiler/noirc_frontend/src/parser/parser/path.rs +++ b/compiler/noirc_frontend/src/parser/parser/path.rs @@ -28,7 +28,7 @@ fn path_inner<'a>(segment: impl NoirParser + 'a) -> impl NoirParser emit_error(ParserError::with_reason( ParserErrorReason::ExpectedIdentifierAfterColons, span, - )) + )); } path_segments }); From 2afa955eeda753dfc4fe34139354f59e1ff0d000 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 10:11:17 -0300 Subject: [PATCH 24/26] clippy --- tooling/lsp/src/requests/completion.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 1dc5964a6a5..f9357557af5 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -44,7 +44,7 @@ pub(crate) fn on_completion_request( .and_then(|byte_index| { let file = args.files.get_file(file_id).unwrap(); let source = file.source(); - let byte = source.bytes().nth(byte_index - 1); + let byte = source.as_bytes().get(byte_index - 1).copied(); let (parsed_module, _errors) = noirc_frontend::parse_program(source); let mut finder = NodeFinder::new( @@ -158,7 +158,7 @@ impl<'a> NodeFinder<'a> { match &use_tree.kind { UseTreeKind::Path(ident, alias) => { prefixes.push(use_tree.prefix.clone()); - let response = self.find_in_use_tree_path(&prefixes, ident, alias); + let response = self.find_in_use_tree_path(prefixes, ident, alias); prefixes.pop(); response } @@ -368,7 +368,7 @@ impl<'a> NodeFinder<'a> { let path = Path { segments: path_segments, kind: PathKind::Plain, span: Span::default() }; let path_resolver = StandardPathResolver::new(self.module_id); - match path_resolver.resolve(&self.def_maps, path, &mut None) { + match path_resolver.resolve(self.def_maps, path, &mut None) { Ok(path_resolution) => Some(path_resolution.module_def_id), Err(_) => None, } @@ -398,7 +398,7 @@ fn simple_completion_item( ) -> CompletionItem { CompletionItem { label: label.into(), - label_details: Some(CompletionItemLabelDetails { detail: None, description: description }), + label_details: Some(CompletionItemLabelDetails { detail: None, description }), kind: Some(kind), detail: None, documentation: None, @@ -438,7 +438,7 @@ mod completion_tests { .filter_map(|(line_index, line)| { line.find(">|<").map(|char_index| (line_index, char_index)) }) - .nth(0) + .next() .expect("Expected to find one >|< in the source code"); let src = src.replace(">|<", ""); From 3b3b610596ffdadb03254d3fdab3f897c3074daa Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 10:32:39 -0300 Subject: [PATCH 25/26] Clean up some comments --- tooling/lsp/src/requests/completion.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index f9357557af5..8c070c56b6a 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -205,7 +205,7 @@ impl<'a> NodeFinder<'a> { } if after_colons { - // We are after the colon + // We are right after "::" segments.push(ident.clone()); self.resolve_module(segments).and_then(|module_id| { @@ -214,9 +214,8 @@ impl<'a> NodeFinder<'a> { self.complete_in_module(module_id, prefix, is_super, suggest_crates) }) } else { + // We are right after the last segment let prefix = ident.to_string(); - - // Otherwise we must complete the last segment if segments.is_empty() { // We are at the start of the use segment and completing the first segment let suggest_crates = prefixes.first().unwrap().kind != PathKind::Crate; @@ -264,7 +263,6 @@ impl<'a> NodeFinder<'a> { } } - // If this is the first segment, also find in all crates if suggest_crates { for dependency in self.dependencies { let dependency_name = dependency.as_name(); From 785e47643a3ac43b573f7805cc2b24af0895391a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 11:03:54 -0300 Subject: [PATCH 26/26] Some fixes regarding `crate::` and looking up modules from the root module --- tooling/lsp/src/requests/completion.rs | 65 +++++++++++++++++++------- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 8c070c56b6a..0c1f7e724dc 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -66,6 +66,7 @@ pub(crate) fn on_completion_request( struct NodeFinder<'a> { byte_index: usize, byte: Option, + root_module_id: ModuleId, module_id: ModuleId, def_maps: &'a BTreeMap, dependencies: &'a Vec, @@ -84,6 +85,7 @@ impl<'a> NodeFinder<'a> { ) -> Self { // Find the module the current file belongs to let def_map = &def_maps[&krate]; + let root_module_id = ModuleId { krate, local_id: def_map.root() }; let local_id = if let Some((module_index, _)) = def_map.modules().iter().find(|(_, module_data)| module_data.location.file == file) { @@ -92,7 +94,7 @@ impl<'a> NodeFinder<'a> { def_map.root() }; let module_id = ModuleId { krate, local_id }; - Self { byte_index, byte, module_id, def_maps, dependencies, interner } + Self { byte_index, byte, root_module_id, module_id, def_maps, dependencies, interner } } fn find(&mut self, parsed_module: &ParsedModule) -> Option { @@ -195,7 +197,7 @@ impl<'a> NodeFinder<'a> { return None; } - let is_super = prefixes.get(0).map(|prefix| prefix.kind) == Some(PathKind::Super); + let path_kind = prefixes[0].kind; let mut segments: Vec = Vec::new(); for prefix in prefixes { @@ -210,20 +212,19 @@ impl<'a> NodeFinder<'a> { self.resolve_module(segments).and_then(|module_id| { let prefix = String::new(); - let suggest_crates = false; - self.complete_in_module(module_id, prefix, is_super, suggest_crates) + let at_root = false; + self.complete_in_module(module_id, prefix, path_kind, at_root) }) } else { // We are right after the last segment let prefix = ident.to_string(); if segments.is_empty() { - // We are at the start of the use segment and completing the first segment - let suggest_crates = prefixes.first().unwrap().kind != PathKind::Crate; - self.complete_in_module(self.module_id, prefix, is_super, suggest_crates) + let at_root = true; + self.complete_in_module(self.module_id, prefix, path_kind, at_root) } else { + let at_root = false; self.resolve_module(segments).and_then(|module_id| { - let suggest_crates = false; - self.complete_in_module(module_id, prefix, is_super, suggest_crates) + self.complete_in_module(module_id, prefix, path_kind, at_root) }) } } @@ -233,15 +234,23 @@ impl<'a> NodeFinder<'a> { &self, module_id: ModuleId, prefix: String, - is_super: bool, - mut suggest_crates: bool, + path_kind: PathKind, + at_root: bool, ) -> Option { let def_map = &self.def_maps[&module_id.krate]; let mut module_data = def_map.modules().get(module_id.local_id.0)?; - if is_super { - module_data = def_map.modules().get(module_data.parent?.0)?; - suggest_crates = false; + if at_root { + match path_kind { + PathKind::Crate => { + module_data = def_map.modules().get(def_map.root().0)?; + } + PathKind::Super => { + module_data = def_map.modules().get(module_data.parent?.0)?; + } + PathKind::Dep => (), + PathKind::Plain => (), + } } let mut completion_items = Vec::new(); @@ -263,7 +272,7 @@ impl<'a> NodeFinder<'a> { } } - if suggest_crates { + if at_root && path_kind == PathKind::Plain { for dependency in self.dependencies { let dependency_name = dependency.as_name(); if name_matches(&dependency_name, &prefix) { @@ -365,7 +374,7 @@ impl<'a> NodeFinder<'a> { let path_segments = segments.into_iter().map(PathSegment::from).collect(); let path = Path { segments: path_segments, kind: PathKind::Plain, span: Span::default() }; - let path_resolver = StandardPathResolver::new(self.module_id); + let path_resolver = StandardPathResolver::new(self.root_module_id); match path_resolver.resolve(self.def_maps, path, &mut None) { Ok(path_resolution) => Some(path_resolution.module_def_id), Err(_) => None, @@ -680,4 +689,28 @@ mod completion_tests { assert_completion(src, vec![module_completion_item("foo")]).await; } + + #[test] + async fn test_use_after_crate_and_letter_nested_in_module() { + let src = r#" + mod something { + mod something_else {} + use crate::s>|< + } + + "#; + assert_completion(src, vec![module_completion_item("something")]).await; + } + #[test] + + async fn test_use_after_crate_segment_and_letter_nested_in_module() { + let src = r#" + mod something { + mod something_else {} + use crate::something::s>|< + } + + "#; + assert_completion(src, vec![module_completion_item("something_else")]).await; + } }