Skip to content

Commit

Permalink
feat: (LSP) suggest names that match any part of the current prefix (n…
Browse files Browse the repository at this point in the history
…oir-lang#5752)

# Description

## Problem

Part of noir-lang#1577

## Summary

As I mentioned in noir-lang#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.
  • Loading branch information
asterite authored Aug 19, 2024
1 parent cdbb940 commit cb0d490
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 28 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tooling/lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 57 additions & 5 deletions tooling/lsp/src/requests/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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<ModuleDefId> {
Expand All @@ -1172,3 +1208,19 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option<ModuleDe
| ReferenceId::Reference(_, _) => 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"));
}
}
15 changes: 12 additions & 3 deletions tooling/lsp/src/requests/completion/completion_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -246,11 +249,17 @@ impl<'a> NodeFinder<'a> {
}

pub(super) fn module_completion_item(name: impl Into<String>) -> 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<String>) -> 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 {
Expand Down
18 changes: 12 additions & 6 deletions tooling/lsp/src/requests/completion/sort_text.rs
Original file line number Diff line number Diff line change
@@ -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()
}
45 changes: 31 additions & 14 deletions tooling/lsp/src/requests/completion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -494,7 +494,7 @@ mod completion_tests {
impl SomeStruct {
fn foo() {
S>|<
So>|<
}
}
"#;
Expand All @@ -517,7 +517,7 @@ mod completion_tests {
impl Trait for SomeStruct {
fn foo() {
S>|<
So>|<
}
}
"#;
Expand All @@ -537,7 +537,7 @@ mod completion_tests {
let src = r#"
fn main() {
for index in 0..10 {
i>|<
ind>|<
}
}
"#;
Expand All @@ -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()),
)],
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -895,7 +898,7 @@ mod completion_tests {
async fn test_suggest_struct_type_parameter() {
let src = r#"
struct Foo<Context> {
context: C>|<
context: Cont>|<
}
"#;
assert_completion_excluding_auto_import(
Expand Down Expand Up @@ -962,7 +965,7 @@ mod completion_tests {
#[test]
async fn test_suggest_function_type_parameters() {
let src = r#"
fn foo<Context>(x: C>|<) {}
fn foo<Context>(x: Cont>|<) {}
"#;
assert_completion_excluding_auto_import(
src,
Expand Down Expand Up @@ -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;
}
}

0 comments on commit cb0d490

Please sign in to comment.