Skip to content

Commit

Permalink
Merge branch 'master' into ab/lsp-autocomplete-super
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite committed Aug 19, 2024
2 parents 10de6f9 + cb0d490 commit ec45e27
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

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 @@ 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 @@ -1557,6 +1560,20 @@ 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;
}

#[test]
async fn test_auto_import_with_super() {
let src = r#"
Expand Down

0 comments on commit ec45e27

Please sign in to comment.