Fix pattern field completions not working for unions

5 files changed, 118 insertions, 94 deletions

diff --git a/crates/ide-completion/src/completions.rs b/crates/ide-completion/src/completions.rs
index 72579e6026a..55c3e28392a 100644
--- a/crates/ide-completion/src/completions.rs
+++ b/crates/ide-completion/src/completions.rs
@@ -617,7 +617,6 @@ pub(super) fn complete_name_ref(
 
                     dot::complete_undotted_self(acc, ctx, path_ctx, expr_ctx);
                     item_list::complete_item_list_in_expr(acc, ctx, path_ctx, expr_ctx);
-                    record::complete_record_expr_func_update(acc, ctx, path_ctx, expr_ctx);
                     snippet::complete_expr_snippet(acc, ctx, path_ctx, expr_ctx);
                 }
                 PathKind::Type { location } => {
diff --git a/crates/ide-completion/src/completions/expr.rs b/crates/ide-completion/src/completions/expr.rs
index 5d0ddaaf2a2..e6c4bdf5206 100644
--- a/crates/ide-completion/src/completions/expr.rs
+++ b/crates/ide-completion/src/completions/expr.rs
@@ -1,8 +1,10 @@
 //! Completion of names from the current scope in expression position.
 
 use hir::ScopeDef;
+use syntax::ast;
 
 use crate::{
+    completions::record::add_default_update,
     context::{ExprCtx, PathCompletionCtx, Qualified},
     CompletionContext, Completions,
 };
@@ -219,60 +221,76 @@ pub(crate) fn complete_expr_path(
                 _ => (),
             });
 
-            if is_func_update.is_none() {
-                let mut add_keyword =
-                    |kw, snippet| acc.add_keyword_snippet_expr(ctx, incomplete_let, kw, snippet);
+            match is_func_update {
+                Some(record_expr) => {
+                    let ty = ctx.sema.type_of_expr(&ast::Expr::RecordExpr(record_expr.clone()));
 
-                if !in_block_expr {
-                    add_keyword("unsafe", "unsafe {\n    $0\n}");
-                }
-                add_keyword("match", "match $1 {\n    $0\n}");
-                add_keyword("while", "while $1 {\n    $0\n}");
-                add_keyword("while let", "while let $1 = $2 {\n    $0\n}");
-                add_keyword("loop", "loop {\n    $0\n}");
-                if in_match_guard {
-                    add_keyword("if", "if $0");
-                } else {
-                    add_keyword("if", "if $1 {\n    $0\n}");
+                    match ty.as_ref().and_then(|t| t.original.as_adt()) {
+                        Some(hir::Adt::Union(_)) => (),
+                        _ => {
+                            cov_mark::hit!(functional_update);
+                            let missing_fields =
+                                ctx.sema.record_literal_missing_fields(record_expr);
+                            add_default_update(acc, ctx, ty, &missing_fields);
+                        }
+                    };
                 }
-                add_keyword("if let", "if let $1 = $2 {\n    $0\n}");
-                add_keyword("for", "for $1 in $2 {\n    $0\n}");
-                add_keyword("true", "true");
-                add_keyword("false", "false");
+                None => {
+                    let mut add_keyword = |kw, snippet| {
+                        acc.add_keyword_snippet_expr(ctx, incomplete_let, kw, snippet)
+                    };
 
-                if in_condition || in_block_expr {
-                    add_keyword("let", "let");
-                }
+                    if !in_block_expr {
+                        add_keyword("unsafe", "unsafe {\n    $0\n}");
+                    }
+                    add_keyword("match", "match $1 {\n    $0\n}");
+                    add_keyword("while", "while $1 {\n    $0\n}");
+                    add_keyword("while let", "while let $1 = $2 {\n    $0\n}");
+                    add_keyword("loop", "loop {\n    $0\n}");
+                    if in_match_guard {
+                        add_keyword("if", "if $0");
+                    } else {
+                        add_keyword("if", "if $1 {\n    $0\n}");
+                    }
+                    add_keyword("if let", "if let $1 = $2 {\n    $0\n}");
+                    add_keyword("for", "for $1 in $2 {\n    $0\n}");
+                    add_keyword("true", "true");
+                    add_keyword("false", "false");
 
-                if after_if_expr {
-                    add_keyword("else", "else {\n    $0\n}");
-                    add_keyword("else if", "else if $1 {\n    $0\n}");
-                }
+                    if in_condition || in_block_expr {
+                        add_keyword("let", "let");
+                    }
 
-                if wants_mut_token {
-                    add_keyword("mut", "mut ");
-                }
+                    if after_if_expr {
+                        add_keyword("else", "else {\n    $0\n}");
+                        add_keyword("else if", "else if $1 {\n    $0\n}");
+                    }
 
-                if in_loop_body {
-                    if in_block_expr {
-                        add_keyword("continue", "continue;");
-                        add_keyword("break", "break;");
-                    } else {
-                        add_keyword("continue", "continue");
-                        add_keyword("break", "break");
+                    if wants_mut_token {
+                        add_keyword("mut", "mut ");
+                    }
+
+                    if in_loop_body {
+                        if in_block_expr {
+                            add_keyword("continue", "continue;");
+                            add_keyword("break", "break;");
+                        } else {
+                            add_keyword("continue", "continue");
+                            add_keyword("break", "break");
+                        }
                     }
-                }
 
-                if let Some(ty) = innermost_ret_ty {
-                    add_keyword(
-                        "return",
-                        match (in_block_expr, ty.is_unit()) {
-                            (true, true) => "return ;",
-                            (true, false) => "return;",
-                            (false, true) => "return $0",
-                            (false, false) => "return",
-                        },
-                    );
+                    if let Some(ty) = innermost_ret_ty {
+                        add_keyword(
+                            "return",
+                            match (in_block_expr, ty.is_unit()) {
+                                (true, true) => "return ;",
+                                (true, false) => "return;",
+                                (false, true) => "return $0",
+                                (false, false) => "return",
+                            },
+                        );
+                    }
                 }
             }
         }
diff --git a/crates/ide-completion/src/completions/record.rs b/crates/ide-completion/src/completions/record.rs
index 1c9042390d3..3f85b45a13c 100644
--- a/crates/ide-completion/src/completions/record.rs
+++ b/crates/ide-completion/src/completions/record.rs
@@ -3,7 +3,7 @@ use ide_db::SymbolKind;
 use syntax::ast::{self, Expr};
 
 use crate::{
-    context::{DotAccess, DotAccessKind, ExprCtx, PathCompletionCtx, PatternContext, Qualified},
+    context::{DotAccess, DotAccessKind, PatternContext},
     CompletionContext, CompletionItem, CompletionItemKind, CompletionRelevance,
     CompletionRelevancePostfixMatch, Completions,
 };
@@ -14,7 +14,24 @@ pub(crate) fn complete_record_pattern_fields(
     pattern_ctx: &PatternContext,
 ) {
     if let PatternContext { record_pat: Some(record_pat), .. } = pattern_ctx {
-        complete_fields(acc, ctx, ctx.sema.record_pattern_missing_fields(record_pat));
+        let ty = ctx.sema.type_of_pat(&ast::Pat::RecordPat(record_pat.clone()));
+        let missing_fields = match ty.as_ref().and_then(|t| t.original.as_adt()) {
+            Some(hir::Adt::Union(un)) => {
+                // ctx.sema.record_pat_missing_fields will always return
+                // an empty Vec on a union literal. This is normally
+                // reasonable, but here we'd like to present the full list
+                // of fields if the literal is empty.
+                let were_fields_specified =
+                    record_pat.record_pat_field_list().and_then(|fl| fl.fields().next()).is_some();
+
+                match were_fields_specified {
+                    false => un.fields(ctx.db).into_iter().map(|f| (f, f.ty(ctx.db))).collect(),
+                    true => return,
+                }
+            }
+            _ => ctx.sema.record_pattern_missing_fields(record_pat),
+        };
+        complete_fields(acc, ctx, missing_fields);
     }
 }
 
@@ -44,6 +61,7 @@ pub(crate) fn complete_record_expr_fields(
             let missing_fields = ctx.sema.record_literal_missing_fields(record_expr);
             add_default_update(acc, ctx, ty, &missing_fields);
             if dot_prefix {
+                cov_mark::hit!(functional_update_one_dot);
                 let mut item =
                     CompletionItem::new(CompletionItemKind::Snippet, ctx.source_range(), "..");
                 item.insert_text(".");
@@ -56,30 +74,7 @@ pub(crate) fn complete_record_expr_fields(
     complete_fields(acc, ctx, missing_fields);
 }
 
-// FIXME: This should probably be part of complete_path_expr
-pub(crate) fn complete_record_expr_func_update(
-    acc: &mut Completions,
-    ctx: &CompletionContext<'_>,
-    path_ctx: &PathCompletionCtx,
-    expr_ctx: &ExprCtx,
-) {
-    if !matches!(path_ctx.qualified, Qualified::No) {
-        return;
-    }
-    if let ExprCtx { is_func_update: Some(record_expr), .. } = expr_ctx {
-        let ty = ctx.sema.type_of_expr(&Expr::RecordExpr(record_expr.clone()));
-
-        match ty.as_ref().and_then(|t| t.original.as_adt()) {
-            Some(hir::Adt::Union(_)) => (),
-            _ => {
-                let missing_fields = ctx.sema.record_literal_missing_fields(record_expr);
-                add_default_update(acc, ctx, ty, &missing_fields);
-            }
-        };
-    }
-}
-
-fn add_default_update(
+pub(crate) fn add_default_update(
     acc: &mut Completions,
     ctx: &CompletionContext<'_>,
     ty: Option<hir::TypeInfo>,
diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs
index e35f79d2b69..759742d3472 100644
--- a/crates/ide-completion/src/context.rs
+++ b/crates/ide-completion/src/context.rs
@@ -134,6 +134,7 @@ pub(crate) struct ExprCtx {
     pub(crate) in_condition: bool,
     pub(crate) incomplete_let: bool,
     pub(crate) ref_expr_parent: Option<ast::RefExpr>,
+    /// The surrounding RecordExpression we are completing a functional update
     pub(crate) is_func_update: Option<ast::RecordExpr>,
     pub(crate) self_param: Option<hir::SelfParam>,
     pub(crate) innermost_ret_ty: Option<hir::Type>,
diff --git a/crates/ide-completion/src/tests/record.rs b/crates/ide-completion/src/tests/record.rs
index f6accc68e5e..e9ee5161179 100644
--- a/crates/ide-completion/src/tests/record.rs
+++ b/crates/ide-completion/src/tests/record.rs
@@ -103,8 +103,9 @@ fn foo(f: Struct) {
 }
 
 #[test]
-fn functional_update() {
-    // FIXME: This should filter out all completions that do not have the type `Foo`
+fn in_functional_update() {
+    cov_mark::check!(functional_update);
+
     check(
         r#"
 //- minicore:default
@@ -116,13 +117,21 @@ impl Default for Foo {
 fn main() {
     let thing = 1;
     let foo = Foo { foo1: 0, foo2: 0 };
-    let foo2 = Foo { thing, $0 }
+    let foo2 = Foo { thing, ..$0 }
 }
 "#,
         expect![[r#"
             fd ..Default::default()
-            fd foo1                 u32
-            fd foo2                 u32
+            fn main()               fn()
+            lc foo                  Foo
+            lc thing                i32
+            md core
+            st Foo
+            st Foo {…}              Foo { foo1: u32, foo2: u32 }
+            tt Default
+            bt u32
+            kw crate::
+            kw self::
         "#]],
     );
     check(
@@ -136,14 +145,18 @@ impl Default for Foo {
 fn main() {
     let thing = 1;
     let foo = Foo { foo1: 0, foo2: 0 };
-    let foo2 = Foo { thing, .$0 }
+    let foo2 = Foo { thing, ..Default::$0 }
 }
 "#,
         expect![[r#"
-            fd ..Default::default()
-            sn ..
+            fn default() (as Default) fn() -> Self
         "#]],
     );
+}
+
+#[test]
+fn functional_update_no_dot() {
+    // FIXME: This should filter out all completions that do not have the type `Foo`
     check(
         r#"
 //- minicore:default
@@ -155,23 +168,20 @@ impl Default for Foo {
 fn main() {
     let thing = 1;
     let foo = Foo { foo1: 0, foo2: 0 };
-    let foo2 = Foo { thing, ..$0 }
+    let foo2 = Foo { thing, $0 }
 }
 "#,
         expect![[r#"
             fd ..Default::default()
-            fn main()               fn()
-            lc foo                  Foo
-            lc thing                i32
-            md core
-            st Foo
-            st Foo {…}              Foo { foo1: u32, foo2: u32 }
-            tt Default
-            bt u32
-            kw crate::
-            kw self::
+            fd foo1                 u32
+            fd foo2                 u32
         "#]],
     );
+}
+
+#[test]
+fn functional_update_one_dot() {
+    cov_mark::check!(functional_update_one_dot);
     check(
         r#"
 //- minicore:default
@@ -183,11 +193,12 @@ impl Default for Foo {
 fn main() {
     let thing = 1;
     let foo = Foo { foo1: 0, foo2: 0 };
-    let foo2 = Foo { thing, ..Default::$0 }
+    let foo2 = Foo { thing, .$0 }
 }
 "#,
         expect![[r#"
-            fn default() (as Default) fn() -> Self
+            fd ..Default::default()
+            sn ..
         "#]],
     );
 }