fix: Fix snippets triggering where they shouldn't

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<SelfParam> {
-        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<Param> {
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<Option<PathKind>> 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, .. }) => {