7 files changed, 105 insertions, 52 deletions

diff --git a/crates/ide_completion/src/completions.rs b/crates/ide_completion/src/completions.rs
index e2994eed4..78154bf3e 100644
--- a/crates/ide_completion/src/completions.rs
+++ b/crates/ide_completion/src/completions.rs
@@ -203,41 +203,37 @@ impl Completions {
 fn complete_enum_variants(
     acc: &mut Completions,
     ctx: &CompletionContext,
-    ty: &hir::Type,
+    enum_data: hir::Enum,
     cb: impl Fn(&mut Completions, &CompletionContext, hir::Variant, hir::ModPath),
 ) {
-    if let Some(hir::Adt::Enum(enum_data)) =
-        iter::successors(Some(ty.clone()), |ty| ty.remove_ref()).last().and_then(|ty| ty.as_adt())
-    {
-        let variants = enum_data.variants(ctx.db);
-
-        let module = if let Some(module) = ctx.scope.module() {
-            // Compute path from the completion site if available.
-            module
-        } else {
-            // Otherwise fall back to the enum's definition site.
-            enum_data.module(ctx.db)
-        };
-
-        if let Some(impl_) = ctx.impl_def.as_ref().and_then(|impl_| ctx.sema.to_def(impl_)) {
-            if impl_.self_ty(ctx.db) == *ty {
-                for &variant in &variants {
-                    let self_path = hir::ModPath::from_segments(
-                        hir::PathKind::Plain,
-                        iter::once(known::SELF_TYPE).chain(iter::once(variant.name(ctx.db))),
-                    );
-                    cb(acc, ctx, variant, self_path);
-                }
+    let variants = enum_data.variants(ctx.db);
+
+    let module = if let Some(module) = ctx.scope.module() {
+        // Compute path from the completion site if available.
+        module
+    } else {
+        // Otherwise fall back to the enum's definition site.
+        enum_data.module(ctx.db)
+    };
+
+    if let Some(impl_) = ctx.impl_def.as_ref().and_then(|impl_| ctx.sema.to_def(impl_)) {
+        if impl_.self_ty(ctx.db).as_adt() == Some(hir::Adt::Enum(enum_data)) {
+            for &variant in &variants {
+                let self_path = hir::ModPath::from_segments(
+                    hir::PathKind::Plain,
+                    iter::once(known::SELF_TYPE).chain(iter::once(variant.name(ctx.db))),
+                );
+                cb(acc, ctx, variant, self_path);
             }
         }
+    }
 
-        for variant in variants {
-            if let Some(path) = module.find_use_path(ctx.db, hir::ModuleDef::from(variant)) {
-                // Variants with trivial paths are already added by the existing completion logic,
-                // so we should avoid adding these twice
-                if path.segments().len() > 1 {
-                    cb(acc, ctx, variant, path);
-                }
+    for variant in variants {
+        if let Some(path) = module.find_use_path(ctx.db, hir::ModuleDef::from(variant)) {
+            // Variants with trivial paths are already added by the existing completion logic,
+            // so we should avoid adding these twice
+            if path.segments().len() > 1 {
+                cb(acc, ctx, variant, path);
             }
         }
     }
diff --git a/crates/ide_completion/src/completions/pattern.rs b/crates/ide_completion/src/completions/pattern.rs
index 808d7ff7e..8dc9ab73c 100644
--- a/crates/ide_completion/src/completions/pattern.rs
+++ b/crates/ide_completion/src/completions/pattern.rs
@@ -12,8 +12,10 @@ pub(crate) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) {
     }
 
     if !ctx.is_irrefutable_pat_binding {
-        if let Some(ty) = ctx.expected_type.as_ref() {
-            super::complete_enum_variants(acc, ctx, ty, |acc, ctx, variant, path| {
+        if let Some(hir::Adt::Enum(e)) =
+            ctx.expected_type.as_ref().and_then(|ty| ty.strip_references().as_adt())
+        {
+            super::complete_enum_variants(acc, ctx, e, |acc, ctx, variant, path| {
                 acc.add_qualified_variant_pat(ctx, variant, path.clone());
                 acc.add_qualified_enum_variant(ctx, variant, path);
             });
diff --git a/crates/ide_completion/src/completions/postfix.rs b/crates/ide_completion/src/completions/postfix.rs
index ac69b720a..962aaf0df 100644
--- a/crates/ide_completion/src/completions/postfix.rs
+++ b/crates/ide_completion/src/completions/postfix.rs
@@ -35,14 +35,11 @@ pub(crate) fn complete_postfix(acc: &mut Completions, ctx: &CompletionContext) {
         None => return,
     };
 
-    let ref_removed_ty =
-        std::iter::successors(Some(receiver_ty.clone()), |ty| ty.remove_ref()).last().unwrap();
-
     let cap = match ctx.config.snippet_cap {
         Some(it) => it,
         None => return,
     };
-    let try_enum = TryEnum::from_ty(&ctx.sema, &ref_removed_ty);
+    let try_enum = TryEnum::from_ty(&ctx.sema, &receiver_ty.strip_references());
     if let Some(try_enum) = &try_enum {
         match try_enum {
             TryEnum::Result => {
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index d2ebba65f..eedb44873 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -59,6 +59,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
                 hir::ModuleDef::TypeAlias(a) => {
                     let ty = a.ty(ctx.db);
                     if let Some(Adt::Enum(e)) = ty.as_adt() {
+                        cov_mark::hit!(completes_variant_through_alias);
                         add_enum_variants(ctx, acc, e);
                     }
                     ty
@@ -68,6 +69,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
                         Some(it) => it,
                         None => return,
                     };
+                    cov_mark::hit!(completes_primitive_assoc_const);
                     builtin.ty(ctx.db, module)
                 }
                 _ => unreachable!(),
@@ -96,9 +98,8 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
                     if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) {
                         return None;
                     }
-                    match item {
-                        hir::AssocItem::Function(_) | hir::AssocItem::Const(_) => {}
-                        hir::AssocItem::TypeAlias(ty) => acc.add_type_alias(ctx, ty),
+                    if let hir::AssocItem::TypeAlias(ty) = item {
+                        acc.add_type_alias(ctx, ty)
                     }
                     None::<()>
                 });
@@ -745,7 +746,7 @@ fn f() {}
     }
 
     #[test]
-    fn completes_self_enum() {
+    fn completes_variant_through_self() {
         check(
             r#"
 enum Foo {
@@ -769,6 +770,7 @@ impl Foo {
 
     #[test]
     fn completes_primitive_assoc_const() {
+        cov_mark::check!(completes_primitive_assoc_const);
         check(
             r#"
 //- /lib.rs crate:lib deps:core
@@ -792,7 +794,8 @@ impl u8 {
     }
 
     #[test]
-    fn completes_through_alias() {
+    fn completes_variant_through_alias() {
+        cov_mark::check!(completes_variant_through_alias);
         check(
             r#"
 enum Foo {
diff --git a/crates/ide_completion/src/completions/trait_impl.rs b/crates/ide_completion/src/completions/trait_impl.rs
index a26fe7c6c..968c0254d 100644
--- a/crates/ide_completion/src/completions/trait_impl.rs
+++ b/crates/ide_completion/src/completions/trait_impl.rs
@@ -36,7 +36,7 @@ use ide_db::{traits::get_missing_assoc_items, SymbolKind};
 use syntax::{
     ast::{self, edit, Impl},
     display::function_declaration,
-    AstNode, SyntaxKind, SyntaxNode, TextRange, T,
+    AstNode, SyntaxElement, SyntaxKind, SyntaxNode, TextRange, T,
 };
 use text_edit::TextEdit;
 
@@ -154,8 +154,7 @@ fn add_function_impl(
     } else {
         CompletionItemKind::SymbolKind(SymbolKind::Function)
     };
-    let range = TextRange::new(fn_def_node.text_range().start(), ctx.source_range().end());
-
+    let range = replacement_range(ctx, fn_def_node);
     if let Some(src) = func.source(ctx.db) {
         let function_decl = function_declaration(&src.value);
         match ctx.config.snippet_cap {
@@ -183,8 +182,7 @@ fn add_type_alias_impl(
 
     let snippet = format!("type {} = ", alias_name);
 
-    let range = TextRange::new(type_def_node.text_range().start(), ctx.source_range().end());
-
+    let range = replacement_range(ctx, type_def_node);
     let mut item = CompletionItem::new(CompletionKind::Magic, ctx.source_range(), snippet.clone());
     item.text_edit(TextEdit::replace(range, snippet))
         .lookup_by(alias_name)
@@ -205,9 +203,7 @@ fn add_const_impl(
         if let Some(source) = const_.source(ctx.db) {
             let snippet = make_const_compl_syntax(&source.value);
 
-            let range =
-                TextRange::new(const_def_node.text_range().start(), ctx.source_range().end());
-
+            let range = replacement_range(ctx, const_def_node);
             let mut item =
                 CompletionItem::new(CompletionKind::Magic, ctx.source_range(), snippet.clone());
             item.text_edit(TextEdit::replace(range, snippet))
@@ -242,6 +238,17 @@ fn make_const_compl_syntax(const_: &ast::Const) -> String {
     format!("{} = ", syntax.trim_end())
 }
 
+fn replacement_range(ctx: &CompletionContext, item: &SyntaxNode) -> TextRange {
+    let first_child = item
+        .children_with_tokens()
+        .find(|child| {
+            !matches!(child.kind(), SyntaxKind::COMMENT | SyntaxKind::WHITESPACE | SyntaxKind::ATTR)
+        })
+        .unwrap_or_else(|| SyntaxElement::Node(item.clone()));
+
+    TextRange::new(first_child.text_range().start(), ctx.source_range().end())
+}
+
 #[cfg(test)]
 mod tests {
     use expect_test::{expect, Expect};
@@ -734,4 +741,50 @@ impl Test for T {{
             test("CONST", "const $0", "const CONST: u16 = ", next_sibling);
         }
     }
+
+    #[test]
+    fn snippet_does_not_overwrite_comment_or_attr() {
+        let test = |completion: &str, hint: &str, completed: &str| {
+            check_edit(
+                completion,
+                &format!(
+                    r#"
+trait Foo {{
+    type Type;
+    fn function();
+    const CONST: i32 = 0;
+}}
+struct T;
+
+impl Foo for T {{
+    // Comment
+    #[bar]
+    {}
+}}
+"#,
+                    hint
+                ),
+                &format!(
+                    r#"
+trait Foo {{
+    type Type;
+    fn function();
+    const CONST: i32 = 0;
+}}
+struct T;
+
+impl Foo for T {{
+    // Comment
+    #[bar]
+    {}
+}}
+"#,
+                    completed
+                ),
+            )
+        };
+        test("function", "fn f$0", "fn function() {\n    $0\n}");
+        test("Type", "type T$0", "type Type = ");
+        test("CONST", "const C$0", "const CONST: i32 = ");
+    }
 }
diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs
index 1b8b063e7..7875500c1 100644
--- a/crates/ide_completion/src/completions/unqualified_path.rs
+++ b/crates/ide_completion/src/completions/unqualified_path.rs
@@ -17,8 +17,10 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC
         return;
     }
 
-    if let Some(ty) = &ctx.expected_type {
-        super::complete_enum_variants(acc, ctx, ty, |acc, ctx, variant, path| {
+    if let Some(hir::Adt::Enum(e)) =
+        ctx.expected_type.as_ref().and_then(|ty| ty.strip_references().as_adt())
+    {
+        super::complete_enum_variants(acc, ctx, e, |acc, ctx, variant, path| {
             acc.add_qualified_enum_variant(ctx, variant, path)
         });
     }
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index f3fcb712c..62ef40818 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -381,7 +381,7 @@ impl<'a> CompletionContext<'a> {
                         let def = self.sema.to_def(&it);
                         (def.map(|def| def.ret_type(self.db)), None)
                     },
-                    ast::Stmt(it) => (None, None),
+                    ast::Stmt(_it) => (None, None),
                     _ => {
                         match node.parent() {
                             Some(n) => {