From cb0d49017a3b592afc2002e592a61d33bf3ac3a4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 19 Aug 2024 07:17:21 -0300 Subject: [PATCH] feat: (LSP) suggest names that match any part of the current prefix (#5752) # Description ## Problem Part of https://github.com/noir-lang/noir/issues/1577 ## Summary As I mentioned in #5741, I noticed Rust Analyzer will suggests methods and names where the prefix you are writing matches any part of the name. Well, kind of, the logic is not exactly that, but that's what's implemented in this PR. It should make finding things much easier (if you are looking for stuff related to "merkle" or "hasher" you just need to type that). ![lsp-complete-any-part](https://github.com/user-attachments/assets/a65adc20-fc96-4682-b1c3-8961c434a133) It works in any context, for example struct fields: ![lsp-complete-any-part-struct-field](https://github.com/user-attachments/assets/f52193ef-adf7-493b-afa5-dbae9009857e) ## Additional Context None. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- Cargo.lock | 1 + tooling/lsp/Cargo.toml | 1 + tooling/lsp/src/requests/completion.rs | 62 +++++++++++++++++-- .../requests/completion/completion_items.rs | 15 ++++- .../lsp/src/requests/completion/sort_text.rs | 18 ++++-- tooling/lsp/src/requests/completion/tests.rs | 45 +++++++++----- 6 files changed, 114 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bacbf939786..e1159c71201 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2722,6 +2722,7 @@ dependencies = [ "acvm", "async-lsp", "codespan-lsp", + "convert_case 0.6.0", "fm", "fxhash", "lsp-types 0.94.1", diff --git a/tooling/lsp/Cargo.toml b/tooling/lsp/Cargo.toml index c5203478f5a..353a6ade904 100644 --- a/tooling/lsp/Cargo.toml +++ b/tooling/lsp/Cargo.toml @@ -33,6 +33,7 @@ thiserror.workspace = true fm.workspace = true rayon = "1.8.0" fxhash.workspace = true +convert_case = "0.6.0" [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies] wasm-bindgen.workspace = true diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index ad0cf0c5b9b..95ed49ac2f1 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -8,6 +8,7 @@ use completion_items::{ crate_completion_item, field_completion_item, simple_completion_item, struct_field_completion_item, }; +use convert_case::{Case, Casing}; use fm::{FileId, FileMap, PathString}; use kinds::{FunctionCompletionKind, FunctionKind, ModuleCompletionKind, RequestedItems}; use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse}; @@ -563,7 +564,7 @@ impl<'a> NodeFinder<'a> { let location = Location::new(member_access_expression.lhs.span, self.file); if let Some(typ) = self.interner.type_at_location(location) { let typ = typ.follow_bindings(); - let prefix = ident.to_string(); + let prefix = ident.to_string().to_case(Case::Snake); self.complete_type_fields_and_methods(&typ, &prefix); return; } @@ -679,6 +680,8 @@ impl<'a> NodeFinder<'a> { at_root = idents.is_empty(); } + let prefix = prefix.to_case(Case::Snake); + let is_single_segment = !after_colons && idents.is_empty() && path.kind == PathKind::Plain; let module_id; @@ -841,11 +844,11 @@ impl<'a> NodeFinder<'a> { segments.push(ident.clone()); if let Some(module_id) = self.resolve_module(segments) { - let prefix = String::new(); + let prefix = ""; let at_root = false; self.complete_in_module( module_id, - &prefix, + prefix, path_kind, at_root, module_completion_kind, @@ -855,7 +858,7 @@ impl<'a> NodeFinder<'a> { }; } else { // We are right after the last segment - let prefix = ident.to_string(); + let prefix = ident.to_string().to_case(Case::Snake); if segments.is_empty() { let at_root = true; self.complete_in_module( @@ -1155,8 +1158,41 @@ impl<'a> NodeFinder<'a> { } } +/// Returns true if name matches a prefix written in code. +/// `prefix` must already be in snake case. +/// This method splits both name and prefix by underscore, +/// then checks that every part of name starts with a part of +/// prefix, in order. +/// +/// For example: +/// +/// // "merk" and "ro" match "merkle" and "root" and are in order +/// name_matches("compute_merkle_root", "merk_ro") == true +/// +/// // "ro" matches "root", but "merkle" comes before it, so no match +/// name_matches("compute_merkle_root", "ro_mer") == false +/// +/// // neither "compute" nor "merkle" nor "root" start with "oot" +/// name_matches("compute_merkle_root", "oot") == false fn name_matches(name: &str, prefix: &str) -> bool { - name.starts_with(prefix) + let name = name.to_case(Case::Snake); + let name_parts: Vec<&str> = name.split('_').collect(); + + let mut last_index: i32 = -1; + for prefix_part in prefix.split('_') { + if let Some(name_part_index) = + name_parts.iter().position(|name_part| name_part.starts_with(prefix_part)) + { + if last_index >= name_part_index as i32 { + return false; + } + last_index = name_part_index as i32; + } else { + return false; + } + } + + true } fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option { @@ -1172,3 +1208,19 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option None, } } + +#[cfg(test)] +mod completion_name_matches_tests { + use crate::requests::completion::name_matches; + + #[test] + fn test_name_matches() { + assert!(name_matches("foo", "foo")); + assert!(name_matches("foo_bar", "bar")); + assert!(name_matches("FooBar", "foo")); + assert!(name_matches("FooBar", "bar")); + assert!(name_matches("FooBar", "foo_bar")); + + assert!(!name_matches("foo_bar", "o_b")); + } +} diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 70afc43fe55..d4190f5241c 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -9,7 +9,10 @@ use noirc_frontend::{ }; use super::{ - sort_text::{default_sort_text, new_sort_text, operator_sort_text, self_mismatch_sort_text}, + sort_text::{ + crate_or_module_sort_text, default_sort_text, new_sort_text, operator_sort_text, + self_mismatch_sort_text, + }, FunctionCompletionKind, FunctionKind, NodeFinder, RequestedItems, }; @@ -246,11 +249,17 @@ impl<'a> NodeFinder<'a> { } pub(super) fn module_completion_item(name: impl Into) -> CompletionItem { - simple_completion_item(name, CompletionItemKind::MODULE, None) + completion_item_with_sort_text( + simple_completion_item(name, CompletionItemKind::MODULE, None), + crate_or_module_sort_text(), + ) } pub(super) fn crate_completion_item(name: impl Into) -> CompletionItem { - simple_completion_item(name, CompletionItemKind::MODULE, None) + completion_item_with_sort_text( + simple_completion_item(name, CompletionItemKind::MODULE, None), + crate_or_module_sort_text(), + ) } fn func_meta_type_to_string(func_meta: &FuncMeta, has_self_type: bool) -> String { diff --git a/tooling/lsp/src/requests/completion/sort_text.rs b/tooling/lsp/src/requests/completion/sort_text.rs index a1dd0ba5e94..9bdc603660f 100644 --- a/tooling/lsp/src/requests/completion/sort_text.rs +++ b/tooling/lsp/src/requests/completion/sort_text.rs @@ -1,33 +1,39 @@ /// Sort text for "new" methods: we want these to show up before anything else, /// if we are completing at something like `Foo::` pub(super) fn new_sort_text() -> String { - "3".to_string() + "a".to_string() } /// This is the default sort text. pub(super) fn default_sort_text() -> String { - "5".to_string() + "b".to_string() +} + +/// We want crates and modules to show up after other things (for example +/// local variables, functions or types) +pub(super) fn crate_or_module_sort_text() -> String { + "c".to_string() } /// Sort text for auto-import items. We want these to show up after local definitions. pub(super) fn auto_import_sort_text() -> String { - "6".to_string() + "d".to_string() } /// When completing something like `Foo::`, we want to show methods that take /// self after the other ones. pub(super) fn self_mismatch_sort_text() -> String { - "7".to_string() + "e".to_string() } /// We want to show operator methods last. pub(super) fn operator_sort_text() -> String { - "8".to_string() + "f".to_string() } /// If a name begins with underscore it's likely something that's meant to /// be private (but visibility doesn't exist everywhere yet, so for now /// we assume that) pub(super) fn underscore_sort_text() -> String { - "9".to_string() + "g".to_string() } diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 825a7dd0081..40f89ac533a 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -356,14 +356,14 @@ mod completion_tests { async fn test_complete_path_after_colons_and_letter_shows_submodule() { let src = r#" mod foo { - mod bar {} + mod qux {} } fn main() { - foo::b>|< + foo::q>|< } "#; - assert_completion(src, vec![module_completion_item("bar")]).await; + assert_completion(src, vec![module_completion_item("qux")]).await; } #[test] @@ -494,7 +494,7 @@ mod completion_tests { impl SomeStruct { fn foo() { - S>|< + So>|< } } "#; @@ -517,7 +517,7 @@ mod completion_tests { impl Trait for SomeStruct { fn foo() { - S>|< + So>|< } } "#; @@ -537,7 +537,7 @@ mod completion_tests { let src = r#" fn main() { for index in 0..10 { - i>|< + ind>|< } } "#; @@ -558,13 +558,13 @@ mod completion_tests { fn lambda(f: fn(i32)) { } fn main() { - lambda(|var| v>|<) + lambda(|lambda_var| lambda_v>|<) } "#; assert_completion_excluding_auto_import( src, vec![simple_completion_item( - "var", + "lambda_var", CompletionItemKind::VARIABLE, Some("_".to_string()), )], @@ -733,16 +733,19 @@ mod completion_tests { let src = r#" fn foo(x: i>|<) {} "#; - assert_completion( - src, + + let items = get_completions(src).await; + let items = items.into_iter().filter(|item| item.label.starts_with('i')).collect(); + + assert_items_match( + items, vec![ simple_completion_item("i8", CompletionItemKind::STRUCT, Some("i8".to_string())), simple_completion_item("i16", CompletionItemKind::STRUCT, Some("i16".to_string())), simple_completion_item("i32", CompletionItemKind::STRUCT, Some("i32".to_string())), simple_completion_item("i64", CompletionItemKind::STRUCT, Some("i64".to_string())), ], - ) - .await; + ); } #[test] @@ -895,7 +898,7 @@ mod completion_tests { async fn test_suggest_struct_type_parameter() { let src = r#" struct Foo { - context: C>|< + context: Cont>|< } "#; assert_completion_excluding_auto_import( @@ -962,7 +965,7 @@ mod completion_tests { #[test] async fn test_suggest_function_type_parameters() { let src = r#" - fn foo(x: C>|<) {} + fn foo(x: Cont>|<) {} "#; assert_completion_excluding_auto_import( src, @@ -1556,4 +1559,18 @@ mod completion_tests { }) ); } + + #[test] + async fn test_completes_matching_any_part_of_an_identifier_by_underscore() { + let src = r#" + struct Foo { + some_property: i32, + } + + fn foo(f: Foo) { + f.prop>|< + } + "#; + assert_completion(src, vec![field_completion_item("some_property", "i32")]).await; + } }