diff --git a/crates/ide_completion/src/completions/fn_param.rs b/crates/ide_completion/src/completions/fn_param.rs index 5acda464982..bb4ce0a24dc 100644 --- a/crates/ide_completion/src/completions/fn_param.rs +++ b/crates/ide_completion/src/completions/fn_param.rs @@ -3,7 +3,7 @@ use rustc_hash::FxHashMap; use syntax::{ ast::{self, HasModuleItem}, - match_ast, AstNode, + match_ast, AstNode, SyntaxKind, }; use crate::{ @@ -16,24 +16,22 @@ use crate::{ /// `spam: &mut Spam` insert text/label and `spam` lookup string will be /// suggested. pub(crate) fn complete_fn_param(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - if !matches!(ctx.pattern_ctx, Some(PatternContext { is_param: Some(ParamKind::Function), .. })) - { + let param_of_fn = + matches!(ctx.pattern_ctx, Some(PatternContext { is_param: Some(ParamKind::Function), .. })); + + if !param_of_fn { return None; } - let mut params = FxHashMap::default(); + let mut file_params = FxHashMap::default(); - let me = ctx.token.ancestors().find_map(ast::Fn::cast); - let mut process_fn = |func: ast::Fn| { - if Some(&func) == me.as_ref() { - return; - } - func.param_list().into_iter().flat_map(|it| it.params()).for_each(|param| { + let mut extract_params = |f: ast::Fn| { + f.param_list().into_iter().flat_map(|it| it.params()).for_each(|param| { if let Some(pat) = param.pat() { // FIXME: We should be able to turn these into SmolStr without having to allocate a String - let text = param.syntax().text().to_string(); - let lookup = pat.syntax().text().to_string(); - params.entry(text).or_insert(lookup); + let whole_param = param.syntax().text().to_string(); + let binding = pat.syntax().text().to_string(); + file_params.entry(whole_param).or_insert(binding); } }); }; @@ -44,32 +42,93 @@ pub(crate) fn complete_fn_param(acc: &mut Completions, ctx: &CompletionContext) ast::SourceFile(it) => it.items().filter_map(|item| match item { ast::Item::Fn(it) => Some(it), _ => None, - }).for_each(&mut process_fn), + }).for_each(&mut extract_params), ast::ItemList(it) => it.items().filter_map(|item| match item { ast::Item::Fn(it) => Some(it), _ => None, - }).for_each(&mut process_fn), + }).for_each(&mut extract_params), ast::AssocItemList(it) => it.assoc_items().filter_map(|item| match item { ast::AssocItem::Fn(it) => Some(it), _ => None, - }).for_each(&mut process_fn), + }).for_each(&mut extract_params), _ => continue, } }; } + let function = ctx.token.ancestors().find_map(ast::Fn::cast)?; + let param_list = function.param_list()?; + + remove_duplicated(&mut file_params, param_list.params()); + let self_completion_items = ["self", "&self", "mut self", "&mut self"]; - if ctx.impl_def.is_some() && me?.param_list()?.params().next().is_none() { + if should_add_self_completions(ctx, param_list) { self_completion_items.into_iter().for_each(|self_item| { add_new_item_to_acc(ctx, acc, self_item.to_string(), self_item.to_string()) }); } - params.into_iter().for_each(|(label, lookup)| add_new_item_to_acc(ctx, acc, label, lookup)); + file_params.into_iter().try_for_each(|(whole_param, binding)| { + Some(add_new_item_to_acc(ctx, acc, surround_with_commas(ctx, whole_param), binding)) + })?; Some(()) } +fn remove_duplicated( + file_params: &mut FxHashMap, + fn_params: ast::AstChildren, +) { + fn_params.for_each(|param| { + let whole_param = param.syntax().text().to_string(); + file_params.remove(&whole_param); + + if let Some(pattern) = param.pat() { + let binding = pattern.syntax().text().to_string(); + file_params.retain(|_, v| v != &binding); + } + }) +} + +fn should_add_self_completions(ctx: &CompletionContext, param_list: ast::ParamList) -> bool { + let inside_impl = ctx.impl_def.is_some(); + let no_params = param_list.params().next().is_none() && param_list.self_param().is_none(); + + inside_impl && no_params +} + +fn surround_with_commas(ctx: &CompletionContext, param: String) -> String { + match fallible_surround_with_commas(ctx, ¶m) { + Some(surrounded) => surrounded, + // fallback to the original parameter + None => param, + } +} + +fn fallible_surround_with_commas(ctx: &CompletionContext, param: &str) -> Option { + let next_token = { + let t = ctx.token.next_token()?; + match t.kind() { + SyntaxKind::WHITESPACE => t.next_token()?, + _ => t, + } + }; + + let trailing_comma_missing = matches!(next_token.kind(), SyntaxKind::IDENT); + let trailing = if trailing_comma_missing { "," } else { "" }; + + let previous_token = if matches!(ctx.token.kind(), SyntaxKind::IDENT | SyntaxKind::WHITESPACE) { + ctx.previous_token.as_ref()? + } else { + &ctx.token + }; + + let needs_leading = !matches!(previous_token.kind(), SyntaxKind::L_PAREN | SyntaxKind::COMMA); + let leading = if needs_leading { ", " } else { "" }; + + Some(format!("{}{}{}", leading, param, trailing)) +} + fn add_new_item_to_acc( ctx: &CompletionContext, acc: &mut Completions, diff --git a/crates/ide_completion/src/tests/fn_param.rs b/crates/ide_completion/src/tests/fn_param.rs index 8a07aefafe3..940cecf395d 100644 --- a/crates/ide_completion/src/tests/fn_param.rs +++ b/crates/ide_completion/src/tests/fn_param.rs @@ -46,7 +46,20 @@ fn bar(file_id: usize) {} fn baz(file$0 id: u32) {} "#, expect![[r#" - bn file_id: usize + bn file_id: usize, + kw mut + "#]], + ); +} + +#[test] +fn repeated_param_name() { + check( + r#" +fn foo(file_id: usize) {} +fn bar(file_id: u32, $0) {} +"#, + expect![[r#" kw mut "#]], ); @@ -126,7 +139,6 @@ impl A { #[test] fn in_impl_after_self() { - // FIXME: self completions should not be here check( r#" struct A {} @@ -137,10 +149,6 @@ impl A { } "#, expect![[r#" - bn self - bn &self - bn mut self - bn &mut self bn file_id: usize kw mut sp Self