From adbcedde1812b728726419f24000bf123b22fef9 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 3 Apr 2020 19:59:28 +0200 Subject: [PATCH 1/3] Remove the second code-path for completing names in patterns --- .../ra_ide/src/completion/complete_pattern.rs | 57 ++++--------------- .../ra_ide/src/completion/complete_scope.rs | 10 +--- .../src/completion/completion_context.rs | 31 +++++----- crates/ra_syntax/src/ast/extensions.rs | 3 + 4 files changed, 31 insertions(+), 70 deletions(-) diff --git a/crates/ra_ide/src/completion/complete_pattern.rs b/crates/ra_ide/src/completion/complete_pattern.rs index bc8fade6f3..1c8b50eeca 100644 --- a/crates/ra_ide/src/completion/complete_pattern.rs +++ b/crates/ra_ide/src/completion/complete_pattern.rs @@ -4,23 +4,25 @@ use crate::completion::{CompletionContext, Completions}; /// Completes constats and paths in patterns. pub(super) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) { - if !ctx.is_pat_binding { + if !ctx.is_pat_binding_or_const { return; } // FIXME: ideally, we should look at the type we are matching against and // suggest variants + auto-imports ctx.scope().process_all_names(&mut |name, res| { - let def = match &res { - hir::ScopeDef::ModuleDef(def) => def, + match &res { + hir::ScopeDef::ModuleDef(def) => match def { + hir::ModuleDef::Adt(hir::Adt::Enum(..)) + | hir::ModuleDef::Adt(hir::Adt::Struct(..)) + | hir::ModuleDef::EnumVariant(..) + | hir::ModuleDef::Const(..) + | hir::ModuleDef::Module(..) => (), + _ => return, + }, + hir::ScopeDef::MacroDef(_) => (), _ => return, }; - match def { - hir::ModuleDef::Adt(hir::Adt::Enum(..)) - | hir::ModuleDef::EnumVariant(..) - | hir::ModuleDef::Const(..) - | hir::ModuleDef::Module(..) => (), - _ => return, - } + acc.add_resolution(ctx, name.to_string(), &res) }); } @@ -69,20 +71,6 @@ mod tests { insert: "E", kind: Enum, }, - CompletionItem { - label: "E", - source_range: [246; 246), - delete: [246; 246), - insert: "E", - kind: Enum, - }, - CompletionItem { - label: "X", - source_range: [246; 246), - delete: [246; 246), - insert: "X", - kind: EnumVariant, - }, CompletionItem { label: "X", source_range: [246; 246), @@ -97,20 +85,6 @@ mod tests { insert: "Z", kind: Const, }, - CompletionItem { - label: "Z", - source_range: [246; 246), - delete: [246; 246), - insert: "Z", - kind: Const, - }, - CompletionItem { - label: "m", - source_range: [246; 246), - delete: [246; 246), - insert: "m", - kind: Module, - }, CompletionItem { label: "m", source_range: [246; 246), @@ -138,13 +112,6 @@ mod tests { ); assert_debug_snapshot!(completions, @r###" [ - CompletionItem { - label: "E", - source_range: [151; 151), - delete: [151; 151), - insert: "E", - kind: Enum, - }, CompletionItem { label: "E", source_range: [151; 151), diff --git a/crates/ra_ide/src/completion/complete_scope.rs b/crates/ra_ide/src/completion/complete_scope.rs index 2ca5527331..665597e4cb 100644 --- a/crates/ra_ide/src/completion/complete_scope.rs +++ b/crates/ra_ide/src/completion/complete_scope.rs @@ -1,19 +1,13 @@ //! Completion of names from the current scope, e.g. locals and imported items. use crate::completion::{CompletionContext, Completions}; -use hir::{ModuleDef, ScopeDef}; pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) { - if !ctx.is_trivial_path && !ctx.is_pat_binding_and_path { + if !(ctx.is_trivial_path && !ctx.is_pat_binding_or_const) { return; } - ctx.scope().process_all_names(&mut |name, res| match (ctx.is_pat_binding_and_path, &res) { - (true, ScopeDef::ModuleDef(ModuleDef::Function(..))) => (), - (true, ScopeDef::ModuleDef(ModuleDef::Static(..))) => (), - (true, ScopeDef::Local(..)) => (), - _ => acc.add_resolution(ctx, name.to_string(), &res), - }); + ctx.scope().process_all_names(&mut |name, res| acc.add_resolution(ctx, name.to_string(), &res)); } #[cfg(test)] diff --git a/crates/ra_ide/src/completion/completion_context.rs b/crates/ra_ide/src/completion/completion_context.rs index fdc0da2c5e..b8213d62f5 100644 --- a/crates/ra_ide/src/completion/completion_context.rs +++ b/crates/ra_ide/src/completion/completion_context.rs @@ -35,10 +35,7 @@ pub(crate) struct CompletionContext<'a> { pub(super) is_param: bool, /// If a name-binding or reference to a const in a pattern. /// Irrefutable patterns (like let) are excluded. - pub(super) is_pat_binding: bool, - // A bind battern which may also be part of a path. - // if let Some(En<|>) = Some(Enum::A) - pub(super) is_pat_binding_and_path: bool, + pub(super) is_pat_binding_or_const: bool, /// A single-indent path, like `foo`. `::foo` should not be considered a trivial path. pub(super) is_trivial_path: bool, /// If not a trivial path, the prefix (qualifier). @@ -97,8 +94,7 @@ impl<'a> CompletionContext<'a> { record_lit_pat: None, impl_def: None, is_param: false, - is_pat_binding: false, - is_pat_binding_and_path: false, + is_pat_binding_or_const: false, is_trivial_path: false, path_prefix: None, after_if: false, @@ -190,18 +186,19 @@ impl<'a> CompletionContext<'a> { // suggest declaration names, see `CompletionKind::Magic`. if let Some(name) = find_node_at_offset::(&file_with_fake_ident, offset) { if let Some(bind_pat) = name.syntax().ancestors().find_map(ast::BindPat::cast) { - let parent = bind_pat.syntax().parent(); - if parent.clone().and_then(ast::MatchArm::cast).is_some() - || parent.clone().and_then(ast::Condition::cast).is_some() - { - self.is_pat_binding = true; + self.is_pat_binding_or_const = true; + if bind_pat.has_at() || bind_pat.is_ref() || bind_pat.is_mutable() { + self.is_pat_binding_or_const = false; } - - if parent.and_then(ast::RecordFieldPatList::cast).is_none() - && bind_pat.pat().is_none() - && !bind_pat.is_ref() - { - self.is_pat_binding_and_path = true; + if bind_pat.syntax().parent().and_then(ast::RecordFieldPatList::cast).is_some() { + self.is_pat_binding_or_const = false; + } + if let Some(let_stmt) = bind_pat.syntax().ancestors().find_map(ast::LetStmt::cast) { + if let Some(pat) = let_stmt.pat() { + if bind_pat.syntax().text_range().is_subrange(&pat.syntax().text_range()) { + self.is_pat_binding_or_const = false; + } + } } } if is_node::(name.syntax()) { diff --git a/crates/ra_syntax/src/ast/extensions.rs b/crates/ra_syntax/src/ast/extensions.rs index 392731dac2..bf7d137be7 100644 --- a/crates/ra_syntax/src/ast/extensions.rs +++ b/crates/ra_syntax/src/ast/extensions.rs @@ -325,6 +325,9 @@ impl ast::BindPat { pub fn is_ref(&self) -> bool { self.syntax().children_with_tokens().any(|n| n.kind() == T![ref]) } + pub fn has_at(&self) -> bool { + self.syntax().children_with_tokens().any(|it| it.kind() == T![@]) + } } pub struct SlicePatComponents { From b1cf95f691cf919b3933d659e3f394f0e7f292cd Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 3 Apr 2020 18:56:23 +0200 Subject: [PATCH 2/3] Generalize call parenthesis insertion --- crates/ra_ide/src/completion/presentation.rs | 73 ++++++++++++-------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/crates/ra_ide/src/completion/presentation.rs b/crates/ra_ide/src/completion/presentation.rs index 1c7c0924db..3930316b0c 100644 --- a/crates/ra_ide/src/completion/presentation.rs +++ b/crates/ra_ide/src/completion/presentation.rs @@ -7,7 +7,8 @@ use test_utils::tested_by; use crate::{ completion::{ - CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, Completions, + completion_item::Builder, CompletionContext, CompletionItem, CompletionItemKind, + CompletionKind, Completions, }, display::{const_label, macro_label, type_label, FunctionSignature}, RootDatabase, @@ -193,7 +194,6 @@ impl Completions { func: hir::Function, ) { let has_self_param = func.has_self_param(ctx.db); - let params = func.params(ctx.db); let name = name.unwrap_or_else(|| func.name(ctx.db).to_string()); let ast_node = func.source(ctx.db).value; @@ -210,32 +210,14 @@ impl Completions { .set_deprecated(is_deprecated(func, ctx.db)) .detail(function_signature.to_string()); - // If not an import, add parenthesis automatically. - if ctx.use_item_syntax.is_none() && !ctx.is_call && ctx.config.add_call_parenthesis { - tested_by!(inserts_parens_for_function_calls); + let params = function_signature + .parameter_names + .iter() + .skip(if function_signature.has_self_param { 1 } else { 0 }) + .cloned() + .collect(); - let (snippet, label) = if params.is_empty() || has_self_param && params.len() == 1 { - (format!("{}()$0", name), format!("{}()", name)) - } else { - builder = builder.trigger_call_info(); - let snippet = if ctx.config.add_call_argument_snippets { - let to_skip = if has_self_param { 1 } else { 0 }; - let function_params_snippet = function_signature - .parameter_names - .iter() - .skip(to_skip) - .enumerate() - .map(|(index, param_name)| format!("${{{}:{}}}", index + 1, param_name)) - .sep_by(", "); - format!("{}({})$0", name, function_params_snippet) - } else { - format!("{}($0)", name) - }; - - (snippet, format!("{}(…)", name)) - }; - builder = builder.lookup_by(name).label(label).insert_snippet(snippet); - } + builder = builder.add_call_parens(ctx, name, params); self.add(builder) } @@ -300,6 +282,43 @@ impl Completions { } } +impl Builder { + fn add_call_parens( + mut self, + ctx: &CompletionContext, + name: String, + params: Vec, + ) -> Builder { + if !ctx.config.add_call_parenthesis { + return self; + } + if ctx.use_item_syntax.is_some() || ctx.is_call { + return self; + } + // If not an import, add parenthesis automatically. + tested_by!(inserts_parens_for_function_calls); + + let (snippet, label) = if params.is_empty() { + (format!("{}()$0", name), format!("{}()", name)) + } else { + self = self.trigger_call_info(); + let snippet = if ctx.config.add_call_argument_snippets { + let function_params_snippet = params + .iter() + .enumerate() + .map(|(index, param_name)| format!("${{{}:{}}}", index + 1, param_name)) + .sep_by(", "); + format!("{}({})$0", name, function_params_snippet) + } else { + format!("{}($0)", name) + }; + + (snippet, format!("{}(…)", name)) + }; + self.lookup_by(name).label(label).insert_snippet(snippet) + } +} + fn is_deprecated(node: impl HasAttrs, db: &RootDatabase) -> bool { node.attrs(db).by_key("deprecated").exists() } From a5e8dfd0247648d8108386f4f98b3af0e48181f7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 3 Apr 2020 19:33:12 +0200 Subject: [PATCH 3/3] Add parens for enums --- crates/ra_ide/src/completion/complete_dot.rs | 2 +- crates/ra_ide/src/completion/complete_path.rs | 24 ++- .../ra_ide/src/completion/complete_pattern.rs | 1 + crates/ra_ide/src/completion/presentation.rs | 194 ++++++++++++++---- 4 files changed, 175 insertions(+), 46 deletions(-) diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs index 82ec169135..f433faef36 100644 --- a/crates/ra_ide/src/completion/complete_dot.rs +++ b/crates/ra_ide/src/completion/complete_dot.rs @@ -61,7 +61,7 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: &T && ctx.scope().module().map_or(true, |m| func.is_visible_from(ctx.db, m)) && seen_methods.insert(func.name(ctx.db)) { - acc.add_function(ctx, func); + acc.add_function(ctx, func, None); } None::<()> }); diff --git a/crates/ra_ide/src/completion/complete_path.rs b/crates/ra_ide/src/completion/complete_path.rs index 3db17f15ff..3ed2ae2b63 100644 --- a/crates/ra_ide/src/completion/complete_path.rs +++ b/crates/ra_ide/src/completion/complete_path.rs @@ -38,7 +38,7 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { hir::ModuleDef::Adt(_) | hir::ModuleDef::TypeAlias(_) => { if let hir::ModuleDef::Adt(Adt::Enum(e)) = def { for variant in e.variants(ctx.db) { - acc.add_enum_variant(ctx, variant); + acc.add_enum_variant(ctx, variant, None); } } let ty = match def { @@ -58,7 +58,7 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { match item { hir::AssocItem::Function(func) => { if !func.has_self_param(ctx.db) { - acc.add_function(ctx, func); + acc.add_function(ctx, func, None); } } hir::AssocItem::Const(ct) => acc.add_const(ctx, ct), @@ -87,7 +87,7 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { match item { hir::AssocItem::Function(func) => { if !func.has_self_param(ctx.db) { - acc.add_function(ctx, func); + acc.add_function(ctx, func, None); } } hir::AssocItem::Const(ct) => acc.add_const(ctx, ct), @@ -355,15 +355,17 @@ mod tests { @r###" [ CompletionItem { - label: "Bar", + label: "Bar(…)", source_range: [116; 116), delete: [116; 116), - insert: "Bar", + insert: "Bar($0)", kind: EnumVariant, + lookup: "Bar", detail: "(i32)", documentation: Documentation( "Bar Variant with i32", ), + trigger_call_info: true, }, CompletionItem { label: "Foo", @@ -403,15 +405,17 @@ mod tests { @r###" [ CompletionItem { - label: "Bar", + label: "Bar(…)", source_range: [180; 180), delete: [180; 180), - insert: "Bar", + insert: "Bar($0)", kind: EnumVariant, + lookup: "Bar", detail: "(i32, u32)", documentation: Documentation( "Bar Variant with i32 and u32", ), + trigger_call_info: true, }, CompletionItem { label: "Foo", @@ -425,15 +429,17 @@ mod tests { ), }, CompletionItem { - label: "S", + label: "S(…)", source_range: [180; 180), delete: [180; 180), - insert: "S", + insert: "S($0)", kind: EnumVariant, + lookup: "S", detail: "(S)", documentation: Documentation( "", ), + trigger_call_info: true, }, ] "### diff --git a/crates/ra_ide/src/completion/complete_pattern.rs b/crates/ra_ide/src/completion/complete_pattern.rs index 1c8b50eeca..1b7d3122f8 100644 --- a/crates/ra_ide/src/completion/complete_pattern.rs +++ b/crates/ra_ide/src/completion/complete_pattern.rs @@ -77,6 +77,7 @@ mod tests { delete: [246; 246), insert: "X", kind: EnumVariant, + detail: "()", }, CompletionItem { label: "Z", diff --git a/crates/ra_ide/src/completion/presentation.rs b/crates/ra_ide/src/completion/presentation.rs index 3930316b0c..cdfd7bc324 100644 --- a/crates/ra_ide/src/completion/presentation.rs +++ b/crates/ra_ide/src/completion/presentation.rs @@ -57,14 +57,16 @@ impl Completions { let kind = match resolution { ScopeDef::ModuleDef(Module(..)) => CompletionItemKind::Module, ScopeDef::ModuleDef(Function(func)) => { - return self.add_function_with_name(ctx, Some(local_name), *func); + return self.add_function(ctx, *func, Some(local_name)); } ScopeDef::ModuleDef(Adt(hir::Adt::Struct(_))) => CompletionItemKind::Struct, // FIXME: add CompletionItemKind::Union ScopeDef::ModuleDef(Adt(hir::Adt::Union(_))) => CompletionItemKind::Struct, ScopeDef::ModuleDef(Adt(hir::Adt::Enum(_))) => CompletionItemKind::Enum, - ScopeDef::ModuleDef(EnumVariant(..)) => CompletionItemKind::EnumVariant, + ScopeDef::ModuleDef(EnumVariant(var)) => { + return self.add_enum_variant(ctx, *var, Some(local_name)); + } ScopeDef::ModuleDef(Const(..)) => CompletionItemKind::Const, ScopeDef::ModuleDef(Static(..)) => CompletionItemKind::Static, ScopeDef::ModuleDef(Trait(..)) => CompletionItemKind::Trait, @@ -125,10 +127,6 @@ impl Completions { completion_item.kind(kind).set_documentation(docs).add_to(self) } - pub(crate) fn add_function(&mut self, ctx: &CompletionContext, func: hir::Function) { - self.add_function_with_name(ctx, None, func) - } - fn guess_macro_braces(&self, macro_name: &str, docs: &str) -> &'static str { let mut votes = [0, 0, 0]; for (idx, s) in docs.match_indices(¯o_name) { @@ -187,15 +185,15 @@ impl Completions { self.add(builder); } - fn add_function_with_name( + pub(crate) fn add_function( &mut self, ctx: &CompletionContext, - name: Option, func: hir::Function, + local_name: Option, ) { let has_self_param = func.has_self_param(ctx.db); - let name = name.unwrap_or_else(|| func.name(ctx.db).to_string()); + let name = local_name.unwrap_or_else(|| func.name(ctx.db).to_string()); let ast_node = func.source(ctx.db).value; let function_signature = FunctionSignature::from(&ast_node); @@ -217,7 +215,7 @@ impl Completions { .cloned() .collect(); - builder = builder.add_call_parens(ctx, name, params); + builder = builder.add_call_parens(ctx, name, Params::Named(params)); self.add(builder) } @@ -254,14 +252,20 @@ impl Completions { .add_to(self); } - pub(crate) fn add_enum_variant(&mut self, ctx: &CompletionContext, variant: hir::EnumVariant) { + pub(crate) fn add_enum_variant( + &mut self, + ctx: &CompletionContext, + variant: hir::EnumVariant, + local_name: Option, + ) { let is_deprecated = is_deprecated(variant, ctx.db); - let name = variant.name(ctx.db); + let name = local_name.unwrap_or_else(|| variant.name(ctx.db).to_string()); let detail_types = variant .fields(ctx.db) .into_iter() .map(|field| (field.name(ctx.db), field.signature_ty(ctx.db))); - let detail = match variant.kind(ctx.db) { + let variant_kind = variant.kind(ctx.db); + let detail = match variant_kind { StructKind::Tuple | StructKind::Unit => detail_types .map(|(_, t)| t.display(ctx.db).to_string()) .sep_by(", ") @@ -273,22 +277,42 @@ impl Completions { .surround_with("{ ", " }") .to_string(), }; - CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.to_string()) - .kind(CompletionItemKind::EnumVariant) - .set_documentation(variant.docs(ctx.db)) - .set_deprecated(is_deprecated) - .detail(detail) - .add_to(self); + let mut res = + CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.clone()) + .kind(CompletionItemKind::EnumVariant) + .set_documentation(variant.docs(ctx.db)) + .set_deprecated(is_deprecated) + .detail(detail); + + if variant_kind == StructKind::Tuple { + let params = Params::Anonymous(variant.fields(ctx.db).len()); + res = res.add_call_parens(ctx, name, params) + } + + res.add_to(self); + } +} + +enum Params { + Named(Vec), + Anonymous(usize), +} + +impl Params { + fn len(&self) -> usize { + match self { + Params::Named(xs) => xs.len(), + Params::Anonymous(len) => *len, + } + } + + fn is_empty(&self) -> bool { + self.len() == 0 } } impl Builder { - fn add_call_parens( - mut self, - ctx: &CompletionContext, - name: String, - params: Vec, - ) -> Builder { + fn add_call_parens(mut self, ctx: &CompletionContext, name: String, params: Params) -> Builder { if !ctx.config.add_call_parenthesis { return self; } @@ -302,15 +326,16 @@ impl Builder { (format!("{}()$0", name), format!("{}()", name)) } else { self = self.trigger_call_info(); - let snippet = if ctx.config.add_call_argument_snippets { - let function_params_snippet = params - .iter() - .enumerate() - .map(|(index, param_name)| format!("${{{}:{}}}", index + 1, param_name)) - .sep_by(", "); - format!("{}({})$0", name, function_params_snippet) - } else { - format!("{}($0)", name) + let snippet = match (ctx.config.add_call_argument_snippets, params) { + (true, Params::Named(params)) => { + let function_params_snippet = params + .iter() + .enumerate() + .map(|(index, param_name)| format!("${{{}:{}}}", index + 1, param_name)) + .sep_by(", "); + format!("{}({})$0", name, function_params_snippet) + } + _ => format!("{}($0)", name), }; (snippet, format!("{}(…)", name)) @@ -385,12 +410,14 @@ mod tests { @r###" [ CompletionItem { - label: "Foo", + label: "Foo(…)", source_range: [115; 117), delete: [115; 117), - insert: "Foo", + insert: "Foo($0)", kind: EnumVariant, + lookup: "Foo", detail: "(i32, i32)", + trigger_call_info: true, }, ]"### ); @@ -564,6 +591,101 @@ mod tests { ); } + #[test] + fn inserts_parens_for_tuple_enums() { + assert_debug_snapshot!( + do_reference_completion( + r" + enum Option { Some(T), None } + use Option::*; + fn main() -> Option { + Som<|> + } + " + ), + @r###" + [ + CompletionItem { + label: "None", + source_range: [144; 147), + delete: [144; 147), + insert: "None", + kind: EnumVariant, + detail: "()", + }, + CompletionItem { + label: "Option", + source_range: [144; 147), + delete: [144; 147), + insert: "Option", + kind: Enum, + }, + CompletionItem { + label: "Some(…)", + source_range: [144; 147), + delete: [144; 147), + insert: "Some($0)", + kind: EnumVariant, + lookup: "Some", + detail: "(T)", + trigger_call_info: true, + }, + CompletionItem { + label: "main()", + source_range: [144; 147), + delete: [144; 147), + insert: "main()$0", + kind: Function, + lookup: "main", + detail: "fn main() -> Option", + }, + ] + "### + ); + assert_debug_snapshot!( + do_reference_completion( + r" + enum Option { Some(T), None } + use Option::*; + fn main(value: Option) { + match value { + Som<|> + } + } + " + ), + @r###" + [ + CompletionItem { + label: "None", + source_range: [185; 188), + delete: [185; 188), + insert: "None", + kind: EnumVariant, + detail: "()", + }, + CompletionItem { + label: "Option", + source_range: [185; 188), + delete: [185; 188), + insert: "Option", + kind: Enum, + }, + CompletionItem { + label: "Some(…)", + source_range: [185; 188), + delete: [185; 188), + insert: "Some($0)", + kind: EnumVariant, + lookup: "Some", + detail: "(T)", + trigger_call_info: true, + }, + ] + "### + ); + } + #[test] fn arg_snippets_for_method_call() { assert_debug_snapshot!(