From 0ce620686cd3c8a9ada6da9b0fcd08b9d9e544b6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 6 May 2022 15:44:41 +0200 Subject: [PATCH] fix: Fix snippets triggering where they shouldn't --- crates/hir/src/lib.rs | 9 +- crates/ide-completion/src/completions/dot.rs | 2 +- crates/ide-completion/src/completions/expr.rs | 9 +- .../src/completions/flyimport.rs | 6 +- .../src/completions/item_list.rs | 9 +- .../ide-completion/src/completions/keyword.rs | 8 +- .../ide-completion/src/completions/snippet.rs | 53 +++++----- .../src/completions/trait_impl.rs | 37 ++++--- crates/ide-completion/src/context.rs | 99 +++++++++++-------- crates/ide-completion/src/render/function.rs | 2 +- 10 files changed, 133 insertions(+), 101 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 8b537a1d385..496b4168c02 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1370,11 +1370,12 @@ impl Function { None } + pub fn has_self_param(self, db: &dyn HirDatabase) -> bool { + db.function_data(self.id).has_self_param() + } + pub fn self_param(self, db: &dyn HirDatabase) -> Option { - if !db.function_data(self.id).has_self_param() { - return None; - } - Some(SelfParam { func: self.id }) + self.has_self_param(db).then(|| SelfParam { func: self.id }) } pub fn assoc_fn_params(self, db: &dyn HirDatabase) -> Vec { diff --git a/crates/ide-completion/src/completions/dot.rs b/crates/ide-completion/src/completions/dot.rs index 3e9bd6078de..33af76c7100 100644 --- a/crates/ide-completion/src/completions/dot.rs +++ b/crates/ide-completion/src/completions/dot.rs @@ -42,7 +42,7 @@ fn complete_undotted_self(acc: &mut Completions, ctx: &CompletionContext) { Some(PathCompletionCtx { is_absolute_path: false, qualifier: None, - kind: PathKind::Expr, + kind: PathKind::Expr { .. }, .. }) if !ctx.is_path_disallowed() => {} _ => return, diff --git a/crates/ide-completion/src/completions/expr.rs b/crates/ide-completion/src/completions/expr.rs index 1278564e195..b7e2b886574 100644 --- a/crates/ide-completion/src/completions/expr.rs +++ b/crates/ide-completion/src/completions/expr.rs @@ -15,9 +15,12 @@ pub(crate) fn complete_expr_path(acc: &mut Completions, ctx: &CompletionContext) } let (&is_absolute_path, qualifier) = match &ctx.path_context { - Some(PathCompletionCtx { kind: PathKind::Expr, is_absolute_path, qualifier, .. }) => { - (is_absolute_path, qualifier) - } + Some(PathCompletionCtx { + kind: PathKind::Expr { .. }, + is_absolute_path, + qualifier, + .. + }) => (is_absolute_path, qualifier), _ => return, }; diff --git a/crates/ide-completion/src/completions/flyimport.rs b/crates/ide-completion/src/completions/flyimport.rs index 7a02a3b3333..7a90126916f 100644 --- a/crates/ide-completion/src/completions/flyimport.rs +++ b/crates/ide-completion/src/completions/flyimport.rs @@ -161,12 +161,12 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) (_, ItemInNs::Types(hir::ModuleDef::Module(_))) => true, // and so are macros(except for attributes) ( - PathKind::Expr | PathKind::Type | PathKind::Item | PathKind::Pat, + PathKind::Expr { .. } | PathKind::Type | PathKind::Item { .. } | PathKind::Pat, ItemInNs::Macros(mac), ) => mac.is_fn_like(ctx.db), - (PathKind::Item, _) => true, + (PathKind::Item { .. }, _) => true, - (PathKind::Expr, ItemInNs::Types(_) | ItemInNs::Values(_)) => true, + (PathKind::Expr { .. }, ItemInNs::Types(_) | ItemInNs::Values(_)) => true, (PathKind::Pat, ItemInNs::Types(_)) => true, (PathKind::Pat, ItemInNs::Values(def)) => matches!(def, hir::ModuleDef::Const(_)), diff --git a/crates/ide-completion/src/completions/item_list.rs b/crates/ide-completion/src/completions/item_list.rs index 80825a633fd..df03120dfe1 100644 --- a/crates/ide-completion/src/completions/item_list.rs +++ b/crates/ide-completion/src/completions/item_list.rs @@ -13,9 +13,12 @@ pub(crate) fn complete_item_list(acc: &mut Completions, ctx: &CompletionContext) } let (&is_absolute_path, qualifier) = match &ctx.path_context { - Some(PathCompletionCtx { kind: PathKind::Item, is_absolute_path, qualifier, .. }) => { - (is_absolute_path, qualifier) - } + Some(PathCompletionCtx { + kind: PathKind::Item { .. }, + is_absolute_path, + qualifier, + .. + }) => (is_absolute_path, qualifier), _ => return, }; diff --git a/crates/ide-completion/src/completions/keyword.rs b/crates/ide-completion/src/completions/keyword.rs index 93bd3468469..557992d14a9 100644 --- a/crates/ide-completion/src/completions/keyword.rs +++ b/crates/ide-completion/src/completions/keyword.rs @@ -125,9 +125,11 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte } let (can_be_stmt, in_loop_body) = match ctx.path_context { - Some(PathCompletionCtx { is_absolute_path: false, can_be_stmt, in_loop_body, .. }) => { - (can_be_stmt, in_loop_body) - } + Some(PathCompletionCtx { + is_absolute_path: false, + kind: PathKind::Expr { in_block_expr, in_loop_body, .. }, + .. + }) => (in_block_expr, in_loop_body), _ => return, }; diff --git a/crates/ide-completion/src/completions/snippet.rs b/crates/ide-completion/src/completions/snippet.rs index a1675b896dd..38136db4a9f 100644 --- a/crates/ide-completion/src/completions/snippet.rs +++ b/crates/ide-completion/src/completions/snippet.rs @@ -5,8 +5,9 @@ use ide_db::{imports::insert_use::ImportScope, SnippetCap}; use syntax::T; use crate::{ - context::PathCompletionCtx, item::Builder, CompletionContext, CompletionItem, - CompletionItemKind, Completions, SnippetScope, + context::{ItemListKind, PathCompletionCtx, PathKind}, + item::Builder, + CompletionContext, CompletionItem, CompletionItemKind, Completions, SnippetScope, }; fn snippet(ctx: &CompletionContext, cap: SnippetCap, label: &str, snippet: &str) -> Builder { @@ -16,14 +17,13 @@ fn snippet(ctx: &CompletionContext, cap: SnippetCap, label: &str, snippet: &str) } pub(crate) fn complete_expr_snippet(acc: &mut Completions, ctx: &CompletionContext) { - if ctx.function_def.is_none() { - return; - } - let can_be_stmt = match ctx.path_context { Some(PathCompletionCtx { - is_absolute_path: false, qualifier: None, can_be_stmt, .. - }) => can_be_stmt, + is_absolute_path: false, + qualifier: None, + kind: PathKind::Expr { in_block_expr, .. }, + .. + }) => in_block_expr, _ => return, }; @@ -43,11 +43,16 @@ pub(crate) fn complete_expr_snippet(acc: &mut Completions, ctx: &CompletionConte } pub(crate) fn complete_item_snippet(acc: &mut Completions, ctx: &CompletionContext) { - if !(ctx.expects_item() || ctx.has_block_expr_parent()) - || ctx.previous_token_is(T![unsafe]) - || ctx.path_qual().is_some() - || ctx.has_unfinished_impl_or_trait_prev_sibling() - { + let path_kind = match ctx.path_context { + Some(PathCompletionCtx { + is_absolute_path: false, + qualifier: None, + kind: kind @ (PathKind::Item { .. } | PathKind::Expr { in_block_expr: true, .. }), + .. + }) => kind, + _ => return, + }; + if ctx.previous_token_is(T![unsafe]) || ctx.has_unfinished_impl_or_trait_prev_sibling() { return; } if ctx.has_visibility_prev_sibling() { @@ -64,7 +69,8 @@ pub(crate) fn complete_item_snippet(acc: &mut Completions, ctx: &CompletionConte } // Test-related snippets shouldn't be shown in blocks. - if !ctx.has_block_expr_parent() { + if let PathKind::Item { kind: ItemListKind::SourceFile | ItemListKind::Module, .. } = path_kind + { let mut item = snippet( ctx, cap, @@ -96,19 +102,22 @@ fn ${1:feature}() { item.lookup_by("tfn"); item.add_to(acc); } - - let item = snippet( - ctx, - cap, - "macro_rules", - "\ + if let PathKind::Item { kind: ItemListKind::SourceFile | ItemListKind::Module, .. } + | PathKind::Expr { .. } = path_kind + { + let item = snippet( + ctx, + cap, + "macro_rules", + "\ macro_rules! $1 { ($2) => { $0 }; }", - ); - item.add_to(acc); + ); + item.add_to(acc); + } } fn add_custom_completions( diff --git a/crates/ide-completion/src/completions/trait_impl.rs b/crates/ide-completion/src/completions/trait_impl.rs index d5272be882a..e3c15038d07 100644 --- a/crates/ide-completion/src/completions/trait_impl.rs +++ b/crates/ide-completion/src/completions/trait_impl.rs @@ -56,19 +56,17 @@ pub(crate) fn complete_trait_impl(acc: &mut Completions, ctx: &CompletionContext if let Some((kind, replacement_range, impl_def)) = completion_match(ctx) { if let Some(hir_impl) = ctx.sema.to_def(&impl_def) { get_missing_assoc_items(&ctx.sema, &impl_def).into_iter().for_each(|item| { + use self::ImplCompletionKind::*; match (item, kind) { - ( - hir::AssocItem::Function(fn_item), - ImplCompletionKind::All | ImplCompletionKind::Fn, - ) => add_function_impl(acc, ctx, replacement_range, fn_item, hir_impl), - ( - hir::AssocItem::TypeAlias(type_item), - ImplCompletionKind::All | ImplCompletionKind::TypeAlias, - ) => add_type_alias_impl(acc, ctx, replacement_range, type_item), - ( - hir::AssocItem::Const(const_item), - ImplCompletionKind::All | ImplCompletionKind::Const, - ) => add_const_impl(acc, ctx, replacement_range, const_item, hir_impl), + (hir::AssocItem::Function(func), All | Fn) => { + add_function_impl(acc, ctx, replacement_range, func, hir_impl) + } + (hir::AssocItem::TypeAlias(type_alias), All | TypeAlias) => { + add_type_alias_impl(acc, ctx, replacement_range, type_alias) + } + (hir::AssocItem::Const(const_), All | Const) => { + add_const_impl(acc, ctx, replacement_range, const_, hir_impl) + } _ => {} } }); @@ -76,6 +74,7 @@ pub(crate) fn complete_trait_impl(acc: &mut Completions, ctx: &CompletionContext } } +// FIXME: This should be lifted out so that we can do proper smart item keyword completions fn completion_match(ctx: &CompletionContext) -> Option<(ImplCompletionKind, TextRange, ast::Impl)> { let token = ctx.token.clone(); @@ -152,15 +151,15 @@ fn add_function_impl( func: hir::Function, impl_def: hir::Impl, ) { - let fn_name = func.name(ctx.db).to_smol_str(); + let fn_name = func.name(ctx.db); - let label = if func.assoc_fn_params(ctx.db).is_empty() { - format!("fn {}()", fn_name) - } else { - format!("fn {}(..)", fn_name) - }; + let label = format!( + "fn {}({})", + fn_name, + if func.assoc_fn_params(ctx.db).is_empty() { "" } else { ".." } + ); - let completion_kind = if func.self_param(ctx.db).is_some() { + let completion_kind = if func.has_self_param(ctx.db) { CompletionItemKind::Method } else { CompletionItemKind::SymbolKind(SymbolKind::Function) diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs index 86d7edccc95..802e48f23a6 100644 --- a/crates/ide-completion/src/context.rs +++ b/crates/ide-completion/src/context.rs @@ -45,7 +45,10 @@ pub(crate) enum Visible { #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub(super) enum PathKind { - Expr, + Expr { + in_block_expr: bool, + in_loop_body: bool, + }, Type, Attr { kind: AttrKind, @@ -53,7 +56,9 @@ pub(super) enum PathKind { }, Derive, /// Path in item position, that is inside an (Assoc)ItemList - Item, + Item { + kind: ItemListKind, + }, Pat, Vis { has_in_token: bool, @@ -61,6 +66,15 @@ pub(super) enum PathKind { Use, } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(super) enum ItemListKind { + SourceFile, + Module, + Impl, + Trait, + ExternBlock, +} + #[derive(Debug)] pub(crate) struct PathCompletionCtx { /// If this is a call with () already there (or {} in case of record patterns) @@ -78,9 +92,6 @@ pub(crate) struct PathCompletionCtx { pub(super) kind: PathKind, /// Whether the path segment has type args or not. pub(super) has_type_args: bool, - /// `true` if we are a statement or a last expr in the block. - pub(super) can_be_stmt: bool, - pub(super) in_loop_body: bool, } #[derive(Debug)] @@ -314,7 +325,7 @@ impl<'a> CompletionContext<'a> { } pub(crate) fn expects_expression(&self) -> bool { - matches!(self.path_context, Some(PathCompletionCtx { kind: PathKind::Expr, .. })) + matches!(self.path_context, Some(PathCompletionCtx { kind: PathKind::Expr { .. }, .. })) } pub(crate) fn expects_type(&self) -> bool { @@ -968,13 +979,18 @@ impl<'a> CompletionContext<'a> { is_absolute_path: false, qualifier: None, parent: path.parent_path(), - kind: PathKind::Item, + kind: PathKind::Item { kind: ItemListKind::SourceFile }, has_type_args: false, - can_be_stmt: false, - in_loop_body: false, }; let mut pat_ctx = None; - path_ctx.in_loop_body = is_in_loop_body(name_ref.syntax()); + + let is_in_block = |it: &SyntaxNode| { + it.parent() + .map(|node| { + ast::ExprStmt::can_cast(node.kind()) || ast::StmtList::can_cast(node.kind()) + }) + .unwrap_or(false) + }; path_ctx.kind = path.syntax().ancestors().find_map(|it| { // using Option> as extra controlflow @@ -983,7 +999,10 @@ impl<'a> CompletionContext<'a> { ast::PathType(_) => Some(PathKind::Type), ast::PathExpr(it) => { path_ctx.has_call_parens = it.syntax().parent().map_or(false, |it| ast::CallExpr::can_cast(it.kind())); - Some(PathKind::Expr) + let in_block_expr = is_in_block(it.syntax()); + let in_loop_body = is_in_loop_body(it.syntax()); + + Some(PathKind::Expr { in_block_expr, in_loop_body }) }, ast::TupleStructPat(it) => { path_ctx.has_call_parens = true; @@ -1001,17 +1020,25 @@ impl<'a> CompletionContext<'a> { }, ast::MacroCall(it) => { path_ctx.has_macro_bang = it.excl_token().is_some(); - match it.syntax().parent().map(|it| it.kind()) { + let parent = it.syntax().parent(); + match parent.as_ref().map(|it| it.kind()) { Some(SyntaxKind::MACRO_PAT) => Some(PathKind::Pat), Some(SyntaxKind::MACRO_TYPE) => Some(PathKind::Type), - Some(SyntaxKind::MACRO_EXPR) => Some(PathKind::Expr), - Some( - SyntaxKind::ITEM_LIST - | SyntaxKind::ASSOC_ITEM_LIST - | SyntaxKind::EXTERN_ITEM_LIST - | SyntaxKind::SOURCE_FILE - ) => Some(PathKind::Item), - _ => return Some(None), + Some(SyntaxKind::ITEM_LIST) => Some(PathKind::Item { kind: ItemListKind::Module }), + Some(SyntaxKind::ASSOC_ITEM_LIST) => Some(PathKind::Item { kind: match parent.and_then(|it| it.parent()).map(|it| it.kind()) { + Some(SyntaxKind::TRAIT) => ItemListKind::Trait, + Some(SyntaxKind::IMPL) => ItemListKind::Impl, + _ => return Some(None), + } }), + Some(SyntaxKind::EXTERN_ITEM_LIST) => Some(PathKind::Item { kind: ItemListKind::ExternBlock }), + Some(SyntaxKind::SOURCE_FILE) => Some(PathKind::Item { kind: ItemListKind::SourceFile }), + _ => { + return Some(parent.and_then(ast::MacroExpr::cast).map(|it| { + let in_loop_body = is_in_loop_body(it.syntax()); + let in_block_expr = is_in_block(it.syntax()); + PathKind::Expr { in_block_expr, in_loop_body } + })); + }, } }, ast::Meta(meta) => (|| { @@ -1032,10 +1059,16 @@ impl<'a> CompletionContext<'a> { })(), ast::Visibility(it) => Some(PathKind::Vis { has_in_token: it.in_token().is_some() }), ast::UseTree(_) => Some(PathKind::Use), - ast::ItemList(_) => Some(PathKind::Item), - ast::AssocItemList(_) => Some(PathKind::Item), - ast::ExternItemList(_) => Some(PathKind::Item), - ast::SourceFile(_) => Some(PathKind::Item), + ast::ItemList(_) => Some(PathKind::Item { kind: ItemListKind::Module }), + ast::AssocItemList(it) => Some(PathKind::Item { kind: { + match it.syntax().parent()?.kind() { + SyntaxKind::TRAIT => ItemListKind::Trait, + SyntaxKind::IMPL => ItemListKind::Impl, + _ => return None, + } + }}), + ast::ExternItemList(_) => Some(PathKind::Item { kind: ItemListKind::ExternBlock }), + ast::SourceFile(_) => Some(PathKind::Item { kind: ItemListKind::SourceFile }), _ => return None, } }; @@ -1086,24 +1119,6 @@ impl<'a> CompletionContext<'a> { } } - // Find either enclosing expr statement (thing with `;`) or a - // block. If block, check that we are the last expr. - path_ctx.can_be_stmt = name_ref - .syntax() - .ancestors() - .find_map(|node| { - if let Some(stmt) = ast::ExprStmt::cast(node.clone()) { - return Some(stmt.syntax().text_range() == name_ref.syntax().text_range()); - } - if let Some(stmt_list) = ast::StmtList::cast(node) { - return Some( - stmt_list.tail_expr().map(|e| e.syntax().text_range()) - == Some(name_ref.syntax().text_range()), - ); - } - None - }) - .unwrap_or(false); Some((path_ctx, pat_ctx)) } } diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index 6430b1e46a6..0117d869ea4 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/crates/ide-completion/src/render/function.rs @@ -197,7 +197,7 @@ fn should_add_parens(ctx: &CompletionContext) -> bool { } match ctx.path_context { - Some(PathCompletionCtx { kind: PathKind::Expr, has_call_parens: true, .. }) => { + Some(PathCompletionCtx { kind: PathKind::Expr { .. }, has_call_parens: true, .. }) => { return false } Some(PathCompletionCtx { kind: PathKind::Use | PathKind::Type, .. }) => {