From 4c03ce702ea23099f244abeb45cde5a78a7a5d09 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 18 Sep 2024 15:58:30 -0300 Subject: [PATCH 1/5] LSP completion: Suggest macro calls --- tooling/lsp/src/requests/completion.rs | 28 +++-- .../src/requests/completion/auto_import.rs | 105 +++++++++--------- .../requests/completion/completion_items.rs | 81 ++++++++++---- tooling/lsp/src/requests/completion/tests.rs | 35 ++++++ 4 files changed, 167 insertions(+), 82 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 17652d37509..de222cbebbb 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -627,15 +627,16 @@ impl<'a> NodeFinder<'a> { for (name, methods) in methods_by_name { for func_id in methods.iter() { if name_matches(name, prefix) { - if let Some(completion_item) = self.function_completion_item( + let completion_items = self.function_completion_items( name, func_id, function_completion_kind, function_kind, None, // attribute first type self_prefix, - ) { - self.completion_items.push(completion_item); + ); + if !completion_items.is_empty() { + self.completion_items.extend(completion_items); self.suggested_module_def_ids.insert(ModuleDefId::FunctionId(func_id)); } } @@ -654,15 +655,16 @@ impl<'a> NodeFinder<'a> { for (name, func_id) in &trait_.method_ids { if name_matches(name, prefix) { - if let Some(completion_item) = self.function_completion_item( + let completion_items = self.function_completion_items( name, *func_id, function_completion_kind, function_kind, None, // attribute first type self_prefix, - ) { - self.completion_items.push(completion_item); + ); + if !completion_items.is_empty() { + self.completion_items.extend(completion_items); self.suggested_module_def_ids.insert(ModuleDefId::FunctionId(*func_id)); } } @@ -742,14 +744,15 @@ impl<'a> NodeFinder<'a> { let per_ns = module_data.find_name(ident); if let Some((module_def_id, visibility, _)) = per_ns.types { if is_visible(module_id, self.module_id, visibility, self.def_maps) { - if let Some(completion_item) = self.module_def_id_completion_item( + let completion_items = self.module_def_id_completion_items( module_def_id, name.clone(), function_completion_kind, function_kind, requested_items, - ) { - self.completion_items.push(completion_item); + ); + if !completion_items.is_empty() { + self.completion_items.extend(completion_items); self.suggested_module_def_ids.insert(module_def_id); } } @@ -757,14 +760,15 @@ impl<'a> NodeFinder<'a> { if let Some((module_def_id, visibility, _)) = per_ns.values { if is_visible(module_id, self.module_id, visibility, self.def_maps) { - if let Some(completion_item) = self.module_def_id_completion_item( + let completion_items = self.module_def_id_completion_items( module_def_id, name.clone(), function_completion_kind, function_kind, requested_items, - ) { - self.completion_items.push(completion_item); + ); + if !completion_items.is_empty() { + self.completion_items.extend(completion_items); self.suggested_module_def_ids.insert(module_def_id); } } diff --git a/tooling/lsp/src/requests/completion/auto_import.rs b/tooling/lsp/src/requests/completion/auto_import.rs index f9c5dab0672..2713ae252bf 100644 --- a/tooling/lsp/src/requests/completion/auto_import.rs +++ b/tooling/lsp/src/requests/completion/auto_import.rs @@ -29,73 +29,78 @@ impl<'a> NodeFinder<'a> { continue; } - let Some(mut completion_item) = self.module_def_id_completion_item( + let completion_items = self.module_def_id_completion_items( *module_def_id, name.clone(), function_completion_kind, FunctionKind::Any, requested_items, - ) else { + ); + + if completion_items.is_empty() { continue; }; - let module_full_path = if let Some(defining_module) = defining_module { - relative_module_id_path( - *defining_module, - &self.module_id, - current_module_parent_id, - self.interner, - ) - } else { - let Some(module_full_path) = relative_module_full_path( - *module_def_id, - *visibility, - self.module_id, - current_module_parent_id, - self.interner, - self.def_maps, - ) else { - continue; + self.suggested_module_def_ids.insert(*module_def_id); + + for mut completion_item in completion_items { + let module_full_path = if let Some(defining_module) = defining_module { + relative_module_id_path( + *defining_module, + &self.module_id, + current_module_parent_id, + self.interner, + ) + } else { + let Some(module_full_path) = relative_module_full_path( + *module_def_id, + *visibility, + self.module_id, + current_module_parent_id, + self.interner, + self.def_maps, + ) else { + continue; + }; + module_full_path }; - module_full_path - }; - let full_path = if defining_module.is_some() - || !matches!(module_def_id, ModuleDefId::ModuleId(..)) - { - format!("{}::{}", module_full_path, name) - } else { - module_full_path - }; + let full_path = if defining_module.is_some() + || !matches!(module_def_id, ModuleDefId::ModuleId(..)) + { + format!("{}::{}", module_full_path, name) + } else { + module_full_path + }; - let mut label_details = completion_item.label_details.unwrap(); - label_details.detail = Some(format!("(use {})", full_path)); - completion_item.label_details = Some(label_details); + let mut label_details = completion_item.label_details.unwrap(); + label_details.detail = Some(format!("(use {})", full_path)); + completion_item.label_details = Some(label_details); - let line = self.auto_import_line as u32; - let character = (self.nesting * 4) as u32; - let indent = " ".repeat(self.nesting * 4); - let mut newlines = "\n"; + let line = self.auto_import_line as u32; + let character = (self.nesting * 4) as u32; + let indent = " ".repeat(self.nesting * 4); + let mut newlines = "\n"; - // If the line we are inserting into is not an empty line, insert an extra line to make some room - if let Some(line_text) = self.lines.get(line as usize) { - if !line_text.trim().is_empty() { - newlines = "\n\n"; + // If the line we are inserting into is not an empty line, insert an extra line to make some room + if let Some(line_text) = self.lines.get(line as usize) { + if !line_text.trim().is_empty() { + newlines = "\n\n"; + } } - } - completion_item.additional_text_edits = Some(vec![TextEdit { - range: Range { - start: Position { line, character }, - end: Position { line, character }, - }, - new_text: format!("use {};{}{}", full_path, newlines, indent), - }]); + completion_item.additional_text_edits = Some(vec![TextEdit { + range: Range { + start: Position { line, character }, + end: Position { line, character }, + }, + new_text: format!("use {};{}{}", full_path, newlines, indent), + }]); - completion_item.sort_text = Some(auto_import_sort_text()); + completion_item.sort_text = Some(auto_import_sort_text()); - self.completion_items.push(completion_item); - self.suggested_module_def_ids.insert(*module_def_id); + self.completion_items.push(completion_item); + } } } } diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 56b1776b228..7e0ecb1e0df 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -20,17 +20,17 @@ use super::{ }; impl<'a> NodeFinder<'a> { - pub(super) fn module_def_id_completion_item( + pub(super) fn module_def_id_completion_items( &self, module_def_id: ModuleDefId, name: String, function_completion_kind: FunctionCompletionKind, function_kind: FunctionKind, requested_items: RequestedItems, - ) -> Option { + ) -> Vec { match requested_items { RequestedItems::OnlyTypes => match module_def_id { - ModuleDefId::FunctionId(_) | ModuleDefId::GlobalId(_) => return None, + ModuleDefId::FunctionId(_) | ModuleDefId::GlobalId(_) => return Vec::new(), ModuleDefId::ModuleId(_) | ModuleDefId::TypeId(_) | ModuleDefId::TypeAliasId(_) @@ -38,7 +38,7 @@ impl<'a> NodeFinder<'a> { }, RequestedItems::OnlyAttributeFunctions(..) => { if !matches!(module_def_id, ModuleDefId::FunctionId(..)) { - return None; + return Vec::new(); } } RequestedItems::AnyItems => (), @@ -57,8 +57,8 @@ impl<'a> NodeFinder<'a> { }; match module_def_id { - ModuleDefId::ModuleId(id) => Some(self.module_completion_item(name, id)), - ModuleDefId::FunctionId(func_id) => self.function_completion_item( + ModuleDefId::ModuleId(id) => vec![self.module_completion_item(name, id)], + ModuleDefId::FunctionId(func_id) => self.function_completion_items( &name, func_id, function_completion_kind, @@ -66,10 +66,10 @@ impl<'a> NodeFinder<'a> { attribute_first_type.as_ref(), false, // self_prefix ), - ModuleDefId::TypeId(struct_id) => Some(self.struct_completion_item(name, struct_id)), - ModuleDefId::TypeAliasId(id) => Some(self.type_alias_completion_item(name, id)), - ModuleDefId::TraitId(trait_id) => Some(self.trait_completion_item(name, trait_id)), - ModuleDefId::GlobalId(global_id) => Some(self.global_completion_item(name, global_id)), + ModuleDefId::TypeId(struct_id) => vec![self.struct_completion_item(name, struct_id)], + ModuleDefId::TypeAliasId(id) => vec![self.type_alias_completion_item(name, id)], + ModuleDefId::TraitId(trait_id) => vec![self.trait_completion_item(name, trait_id)], + ModuleDefId::GlobalId(global_id) => vec![self.global_completion_item(name, global_id)], } } @@ -133,7 +133,7 @@ impl<'a> NodeFinder<'a> { self.completion_item_with_doc_comments(ReferenceId::Global(global_id), completion_item) } - pub(super) fn function_completion_item( + pub(super) fn function_completion_items( &self, name: &String, func_id: FuncId, @@ -141,7 +141,7 @@ impl<'a> NodeFinder<'a> { function_kind: FunctionKind, attribute_first_type: Option<&Type>, self_prefix: bool, - ) -> Option { + ) -> Vec { let func_meta = self.interner.function_meta(&func_id); let func_self_type = if let Some((pattern, typ, _)) = func_meta.parameters.0.first() { @@ -161,12 +161,12 @@ impl<'a> NodeFinder<'a> { if let Some(attribute_first_type) = attribute_first_type { if func_meta.parameters.is_empty() { - return None; + return Vec::new(); } let (_, typ, _) = &func_meta.parameters.0[0]; if typ != attribute_first_type { - return None; + return Vec::new(); } } @@ -186,23 +186,65 @@ impl<'a> NodeFinder<'a> { } if self_type != func_self_type { - return None; + return Vec::new(); } } else if let Type::Tuple(self_tuple_types) = self_type { // Tuple types of different lengths seem to also have methods defined on all of them, // so here we reject methods for tuples where the length doesn't match. if let Type::Tuple(func_self_tuple_types) = func_self_type { if self_tuple_types.len() != func_self_tuple_types.len() { - return None; + return Vec::new(); } } } } else { - return None; + return Vec::new(); } } } + let make_completion_item = |is_macro_call| { + self.function_completion_item( + name, + func_id, + func_meta, + func_self_type, + function_completion_kind, + function_kind, + attribute_first_type, + self_prefix, + is_macro_call, + ) + }; + + // It's unlikely users will define a function named `unquote` that does something different than std's unquote. + if name == "unquote" { + vec![make_completion_item(true)] + } else { + let modifiers = self.interner.function_modifiers(&func_id); + + if modifiers.is_comptime + && matches!(func_meta.return_type(), Type::Quoted(QuotedType::Quoted)) + { + vec![make_completion_item(false), make_completion_item(true)] + } else { + vec![make_completion_item(false)] + } + } + } + + pub(super) fn function_completion_item( + &self, + name: &String, + func_id: FuncId, + func_meta: &FuncMeta, + func_self_type: Option<&Type>, + function_completion_kind: FunctionCompletionKind, + function_kind: FunctionKind, + attribute_first_type: Option<&Type>, + self_prefix: bool, + is_macro_call: bool, + ) -> CompletionItem { let is_operator = if let Some(trait_impl_id) = &func_meta.trait_impl { let trait_impl = self.interner.get_trait_implementation(*trait_impl_id); let trait_impl = trait_impl.borrow(); @@ -211,6 +253,7 @@ impl<'a> NodeFinder<'a> { false }; let name = if self_prefix { format!("self.{}", name) } else { name.clone() }; + let name = if is_macro_call { format!("{}!", name) } else { name }; let name = &name; let description = func_meta_type_to_string(func_meta, func_self_type.is_some()); let mut has_arguments = false; @@ -269,10 +312,8 @@ impl<'a> NodeFinder<'a> { } } }; - let completion_item = - self.completion_item_with_doc_comments(ReferenceId::Function(func_id), completion_item); - Some(completion_item) + self.completion_item_with_doc_comments(ReferenceId::Function(func_id), completion_item) } fn compute_function_insert_text( diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 50f4412d7a8..9abaabcc83e 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -2119,4 +2119,39 @@ mod completion_tests { assert_completion(src, vec![field_completion_item("bar", "Field")]).await; } + + #[test] + async fn test_suggests_macro_call_if_comptime_function_returns_quoted() { + let src = r#" + comptime fn foobar() -> Quoted {} + + fn main() { + fooba>|< + } + "#; + + assert_completion_excluding_auto_import( + src, + vec![ + function_completion_item("foobar!()", "foobar!()", "fn() -> Quoted"), + function_completion_item("foobar()", "foobar()", "fn() -> Quoted"), + ], + ) + .await; + } + + #[test] + async fn test_only_suggests_macro_call_for_unquote() { + let src = r#" + use std::meta::unquote; + + fn main() { + unquot>|< + } + "#; + + let completions = get_completions(src).await; + assert_eq!(completions.len(), 1); + assert_eq!(completions[0].label, "unquote!(…)"); + } } From addcc3a5ea8dd700ee247f372370e3234418e327 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 18 Sep 2024 16:00:39 -0300 Subject: [PATCH 2/5] Don't suggest macro calls when suggsting attribute functions --- .../requests/completion/completion_items.rs | 27 +++++++++++-------- tooling/lsp/src/requests/completion/tests.rs | 4 +-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 7e0ecb1e0df..4a6a0af6654 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -217,19 +217,24 @@ impl<'a> NodeFinder<'a> { ) }; - // It's unlikely users will define a function named `unquote` that does something different than std's unquote. + // When suggestion functions in attributes, never suggest a macro call + if attribute_first_type.is_some() { + return vec![make_completion_item(false)]; + } + + // Special case: the `unquote` macro + // (it's unlikely users will define a function named `unquote` that does something different than std's unquote) if name == "unquote" { - vec![make_completion_item(true)] - } else { - let modifiers = self.interner.function_modifiers(&func_id); + return vec![make_completion_item(true)]; + } - if modifiers.is_comptime - && matches!(func_meta.return_type(), Type::Quoted(QuotedType::Quoted)) - { - vec![make_completion_item(false), make_completion_item(true)] - } else { - vec![make_completion_item(false)] - } + let modifiers = self.interner.function_modifiers(&func_id); + if modifiers.is_comptime + && matches!(func_meta.return_type(), Type::Quoted(QuotedType::Quoted)) + { + vec![make_completion_item(false), make_completion_item(true)] + } else { + vec![make_completion_item(false)] } } diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 9abaabcc83e..23d137c1647 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -1919,7 +1919,7 @@ mod completion_tests { #[some>|<] fn foo() {} - fn some_attr(f: FunctionDefinition, x: Field) {} + comptime fn some_attr(f: FunctionDefinition, x: Field) -> Quoted {} fn some_other_function(x: Field) {} "#; @@ -1928,7 +1928,7 @@ mod completion_tests { vec![function_completion_item( "some_attr(…)", "some_attr(${1:x})", - "fn(FunctionDefinition, Field)", + "fn(FunctionDefinition, Field) -> Quoted", )], ) .await; From 85f12981becc3bf44fa5b6db11cade9fcbce03c9 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 18 Sep 2024 16:04:55 -0300 Subject: [PATCH 3/5] Add missing return --- tooling/lsp/src/requests/completion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index de222cbebbb..0d1d7af7ae6 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -578,7 +578,7 @@ impl<'a> NodeFinder<'a> { } Type::TypeVariable(var, _) | Type::NamedGeneric(var, _, _) => { if let TypeBinding::Bound(typ) = &*var.borrow() { - self.complete_type_fields_and_methods( + return self.complete_type_fields_and_methods( typ, prefix, function_completion_kind, From a6965e44ec2bab737ef66f144991b8c1665eb003 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 18 Sep 2024 16:05:40 -0300 Subject: [PATCH 4/5] Clippy --- tooling/lsp/src/requests/completion/completion_items.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 4a6a0af6654..4906ab58844 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -238,6 +238,7 @@ impl<'a> NodeFinder<'a> { } } + #[allow(clippy::too_many_arguments)] pub(super) fn function_completion_item( &self, name: &String, From 056f9f45f8b8e4c73ef0fdf80b23989aa0630e86 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 18 Sep 2024 16:32:23 -0300 Subject: [PATCH 5/5] Update tooling/lsp/src/requests/completion/completion_items.rs Co-authored-by: jfecher --- tooling/lsp/src/requests/completion/completion_items.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 4906ab58844..5273e49d4b4 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -217,7 +217,7 @@ impl<'a> NodeFinder<'a> { ) }; - // When suggestion functions in attributes, never suggest a macro call + // When suggesting functions in attributes, never suggest a macro call if attribute_first_type.is_some() { return vec![make_completion_item(false)]; }