Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: (LSP) suggest names that match any part of the current prefix #5752

Merged
merged 3 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@ -38,7 +39,7 @@
use super::process_request;

mod auto_import;
mod builtins;

Check warning on line 42 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (builtins)
mod completion_items;
mod kinds;
mod sort_text;
Expand Down Expand Up @@ -563,7 +564,7 @@
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 @@
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 @@
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 @@
};
} 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 @@
}
}

/// 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

Check warning on line 1169 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (merk)
/// name_matches("compute_merkle_root", "merk_ro") == true

Check warning on line 1170 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (merk)
///
/// // "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 @@
| 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 @@
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 @@

impl SomeStruct {
fn foo() {
S>|<
So>|<
}
}
"#;
Expand All @@ -517,7 +517,7 @@

impl Trait for SomeStruct {
fn foo() {
S>|<
So>|<
}
}
"#;
Expand All @@ -537,7 +537,7 @@
let src = r#"
fn main() {
for index in 0..10 {
i>|<
ind>|<
}
}
"#;
Expand All @@ -558,13 +558,13 @@
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 @@
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 @@
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 @@
#[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 @@ -1534,7 +1537,7 @@
async fn test_auto_import_suggests_modules_too() {
let src = r#"
mod foo {
mod barbaz {

Check warning on line 1540 in tooling/lsp/src/requests/completion/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (barbaz)
fn hello_world() {}
}
}
Expand All @@ -1547,13 +1550,27 @@
assert_eq!(items.len(), 1);

let item = &items[0];
assert_eq!(item.label, "barbaz");

Check warning on line 1553 in tooling/lsp/src/requests/completion/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (barbaz)
assert_eq!(
item.label_details,
Some(CompletionItemLabelDetails {
detail: Some("(use foo::barbaz)".to_string()),

Check warning on line 1557 in tooling/lsp/src/requests/completion/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (barbaz)
description: None
})
);
}

#[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;
}
}
Loading