internal: Refactor our record pat/expr handling in completion context

crates/ide-completion/src/completions/expr.rs

@@ -5,7 +5,7 @@
 use syntax::T;
 
 use crate::{
-    context::{PathCompletionCtx, PathKind, PathQualifierCtx},
+    context::{NameRefContext, PathCompletionCtx, PathKind, PathQualifierCtx},
     CompletionContext, Completions,
 };
 
@@ -15,14 +15,25 @@ pub(crate) fn complete_expr_path(acc: &mut Completions, ctx: &CompletionContext)
         return;
     }
 
-    let (is_absolute_path, qualifier, in_block_expr, in_loop_body, in_functional_update) =
+    let (is_absolute_path, qualifier, in_block_expr, in_loop_body, is_func_update) =
-        match ctx.path_context() {
+        match ctx.nameref_ctx() {
-            Some(&PathCompletionCtx {
+            Some(NameRefContext {
-                kind: PathKind::Expr { in_block_expr, in_loop_body, in_functional_update },
+                path_ctx:
-                is_absolute_path,
+                    Some(PathCompletionCtx {
-                ref qualifier,
+                        kind: PathKind::Expr { in_block_expr, in_loop_body },
+                        is_absolute_path,
+                        qualifier,
+                        ..
+                    }),
+                record_expr,
                 ..
-            }) => (is_absolute_path, qualifier, in_block_expr, in_loop_body, in_functional_update),
+            }) => (
+                *is_absolute_path,
+                qualifier,
+                *in_block_expr,
+                *in_loop_body,
+                record_expr.as_ref().map_or(false, |&(_, it)| it),
+            ),
             _ => return,
         };
 
@@ -165,7 +176,7 @@ pub(crate) fn complete_expr_path(acc: &mut Completions, ctx: &CompletionContext)
                 }
             });
 
-            if !in_functional_update {
+            if !is_func_update {
                 let mut add_keyword =
                     |kw, snippet| super::keyword::add_keyword(acc, ctx, kw, snippet);
 

crates/ide-completion/src/completions/keyword.rs

@@ -5,12 +5,12 @@
 use syntax::T;
 
 use crate::{
-    context::PathKind, patterns::ImmediateLocation, CompletionContext, CompletionItem,
+    context::{NameRefContext, PathKind},
-    CompletionItemKind, Completions,
+    CompletionContext, CompletionItem, CompletionItemKind, Completions,
 };
 
 pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionContext) {
-    if matches!(ctx.completion_location, Some(ImmediateLocation::RecordExpr(_))) {
+    if matches!(ctx.nameref_ctx(), Some(NameRefContext { record_expr: Some(_), .. })) {
         cov_mark::hit!(no_keyword_completion_in_record_lit);
         return;
     }

crates/ide-completion/src/completions/pattern.rs

@@ -15,7 +15,6 @@ pub(crate) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) {
         Some(ctx) => ctx,
         _ => return,
     };
-    let refutable = patctx.refutability == PatternRefutability::Refutable;
 
     if let Some(path_ctx) = ctx.path_context() {
         pattern_path_completion(acc, ctx, path_ctx);
@@ -47,6 +46,11 @@ pub(crate) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) {
         }
     }
 
+    if patctx.record_pat.is_some() {
+        return;
+    }
+
+    let refutable = patctx.refutability == PatternRefutability::Refutable;
     let single_variant_enum = |enum_: hir::Enum| ctx.db.enum_data(enum_.into()).variants.len() == 1;
 
     if let Some(hir::Adt::Enum(e)) =

crates/ide-completion/src/completions/record.rs

@@ -3,67 +3,65 @@
 use syntax::{ast::Expr, T};
 
 use crate::{
-    patterns::ImmediateLocation, CompletionContext, CompletionItem, CompletionItemKind,
+    context::{NameRefContext, PatternContext},
-    CompletionRelevance, CompletionRelevancePostfixMatch, Completions,
+    CompletionContext, CompletionItem, CompletionItemKind, CompletionRelevance,
+    CompletionRelevancePostfixMatch, Completions,
 };
 
 pub(crate) fn complete_record(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> {
-    let missing_fields = match &ctx.completion_location {
+    let missing_fields = if let Some(PatternContext { record_pat: Some(record_pat), .. }) =
-        Some(
+        &ctx.pattern_ctx
-            ImmediateLocation::RecordExpr(record_expr)
+    {
-            | ImmediateLocation::RecordExprUpdate(record_expr),
+        ctx.sema.record_pattern_missing_fields(record_pat)
-        ) => {
+    } else if let Some(NameRefContext { record_expr: Some((record_expr, _)), .. }) =
-            let ty = ctx.sema.type_of_expr(&Expr::RecordExpr(record_expr.clone()));
+        ctx.nameref_ctx()
+    {
+        let ty = ctx.sema.type_of_expr(&Expr::RecordExpr(record_expr.clone()));
 
-            if let Some(hir::Adt::Union(un)) = ty.as_ref().and_then(|t| t.original.as_adt()) {
+        if let Some(hir::Adt::Union(un)) = ty.as_ref().and_then(|t| t.original.as_adt()) {
-                // ctx.sema.record_literal_missing_fields will always return
+            // ctx.sema.record_literal_missing_fields will always return
-                // an empty Vec on a union literal. This is normally
+            // an empty Vec on a union literal. This is normally
-                // reasonable, but here we'd like to present the full list
+            // reasonable, but here we'd like to present the full list
-                // of fields if the literal is empty.
+            // of fields if the literal is empty.
-                let were_fields_specified = record_expr
+            let were_fields_specified =
-                    .record_expr_field_list()
+                record_expr.record_expr_field_list().and_then(|fl| fl.fields().next()).is_some();
-                    .and_then(|fl| fl.fields().next())
-                    .is_some();
 
-                match were_fields_specified {
+            match were_fields_specified {
-                    false => un.fields(ctx.db).into_iter().map(|f| (f, f.ty(ctx.db))).collect(),
+                false => un.fields(ctx.db).into_iter().map(|f| (f, f.ty(ctx.db))).collect(),
-                    true => vec![],
+                true => vec![],
-                }
-            } else {
-                let missing_fields = ctx.sema.record_literal_missing_fields(record_expr);
-
-                let default_trait = ctx.famous_defs().core_default_Default();
-                let impl_default_trait =
-                    default_trait.zip(ty.as_ref()).map_or(false, |(default_trait, ty)| {
-                        ty.original.impls_trait(ctx.db, default_trait, &[])
-                    });
-
-                if impl_default_trait && !missing_fields.is_empty() && ctx.path_qual().is_none() {
-                    let completion_text = "..Default::default()";
-                    let mut item =
-                        CompletionItem::new(SymbolKind::Field, ctx.source_range(), completion_text);
-                    let completion_text =
-                        completion_text.strip_prefix(ctx.token.text()).unwrap_or(completion_text);
-                    item.insert_text(completion_text).set_relevance(CompletionRelevance {
-                        postfix_match: Some(CompletionRelevancePostfixMatch::Exact),
-                        ..Default::default()
-                    });
-                    item.add_to(acc);
-                }
-                if ctx.previous_token_is(T![.]) {
-                    let mut item =
-                        CompletionItem::new(CompletionItemKind::Snippet, ctx.source_range(), "..");
-                    item.insert_text(".");
-                    item.add_to(acc);
-                    return None;
-                }
-                missing_fields
             }
+        } else {
+            let missing_fields = ctx.sema.record_literal_missing_fields(record_expr);
+
+            let default_trait = ctx.famous_defs().core_default_Default();
+            let impl_default_trait =
+                default_trait.zip(ty.as_ref()).map_or(false, |(default_trait, ty)| {
+                    ty.original.impls_trait(ctx.db, default_trait, &[])
+                });
+
+            if impl_default_trait && !missing_fields.is_empty() && ctx.path_qual().is_none() {
+                let completion_text = "..Default::default()";
+                let mut item =
+                    CompletionItem::new(SymbolKind::Field, ctx.source_range(), completion_text);
+                let completion_text =
+                    completion_text.strip_prefix(ctx.token.text()).unwrap_or(completion_text);
+                item.insert_text(completion_text).set_relevance(CompletionRelevance {
+                    postfix_match: Some(CompletionRelevancePostfixMatch::Exact),
+                    ..Default::default()
+                });
+                item.add_to(acc);
+            }
+            if ctx.previous_token_is(T![.]) {
+                let mut item =
+                    CompletionItem::new(CompletionItemKind::Snippet, ctx.source_range(), "..");
+                item.insert_text(".");
+                item.add_to(acc);
+                return None;
+            }
+            missing_fields
         }
-        Some(ImmediateLocation::RecordPat(record_pat)) => {
+    } else {
-            ctx.sema.record_pattern_missing_fields(record_pat)
+        return None;
-        }
-        _ => return None,
     };
 
     for (field, ty) in missing_fields {

crates/ide-completion/src/context.rs

@@ -48,7 +48,6 @@ pub(super) enum PathKind {
     Expr {
         in_block_expr: bool,
         in_loop_body: bool,
-        in_functional_update: bool,
     },
     Type,
     Attr {
@@ -115,6 +114,8 @@ pub(super) struct PatternContext {
     pub(super) parent_pat: Option<ast::Pat>,
     pub(super) ref_token: Option<SyntaxToken>,
     pub(super) mut_token: Option<SyntaxToken>,
+    /// The record pattern this name or ref is a field of
+    pub(super) record_pat: Option<ast::RecordPat>,
 }
 
 #[derive(Debug)]
@@ -166,8 +167,11 @@ pub(super) enum NameKind {
 pub(super) struct NameRefContext {
     /// NameRef syntax in the original file
     pub(super) nameref: Option<ast::NameRef>,
+    // FIXME: these fields are actually disjoint -> enum
     pub(super) dot_access: Option<DotAccess>,
     pub(super) path_ctx: Option<PathCompletionCtx>,
+    /// The record expression this nameref is a field of
+    pub(super) record_expr: Option<(ast::RecordExpr, bool)>,
 }
 
 #[derive(Debug)]
@@ -375,14 +379,15 @@ pub(crate) fn after_if(&self) -> bool {
     pub(crate) fn is_path_disallowed(&self) -> bool {
         self.previous_token_is(T![unsafe])
             || matches!(self.prev_sibling, Some(ImmediatePrevSibling::Visibility))
-            || matches!(
-                self.completion_location,
-                Some(ImmediateLocation::RecordPat(_) | ImmediateLocation::RecordExpr(_))
-            )
             || matches!(
                 self.name_ctx(),
                 Some(NameContext { kind: NameKind::Module(_) | NameKind::Rename, .. })
             )
+            || matches!(self.pattern_ctx, Some(PatternContext { record_pat: Some(_), .. }))
+            || matches!(
+                self.nameref_ctx(),
+                Some(NameRefContext { record_expr: Some((_, false)), .. })
+            )
     }
 
     pub(crate) fn path_context(&self) -> Option<&PathCompletionCtx> {
@@ -1023,14 +1028,13 @@ fn classify_name(
                 ast::Enum(_) => NameKind::Enum,
                 ast::Fn(_) => NameKind::Function,
                 ast::IdentPat(bind_pat) => {
-                    let is_name_in_field_pat = bind_pat
+                    pat_ctx = Some({
-                        .syntax()
+                        let mut pat_ctx = pattern_context_for(original_file, bind_pat.into());
-                        .parent()
+                        if let Some(record_field) = ast::RecordPatField::for_field_name(&name) {
-                        .and_then(ast::RecordPatField::cast)
+                            pat_ctx.record_pat = find_node_in_file_compensated(original_file, &record_field.parent_record_pat());
-                        .map_or(false, |pat_field| pat_field.name_ref().is_none());
+                        }
-                    if !is_name_in_field_pat {
+                        pat_ctx
-                        pat_ctx = Some(pattern_context_for(original_file, bind_pat.into()));
+                    });
-                    }
 
                     NameKind::IdentPat
                 },
@@ -1062,7 +1066,33 @@ fn classify_name_ref(
     ) -> (NameRefContext, Option<PatternContext>) {
         let nameref = find_node_at_offset(&original_file, name_ref.syntax().text_range().start());
 
-        let mut nameref_ctx = NameRefContext { dot_access: None, path_ctx: None, nameref };
+        let mut nameref_ctx =
+            NameRefContext { dot_access: None, path_ctx: None, nameref, record_expr: None };
+
+        if let Some(record_field) = ast::RecordExprField::for_field_name(&name_ref) {
+            nameref_ctx.record_expr =
+                find_node_in_file_compensated(original_file, &record_field.parent_record_lit())
+                    .zip(Some(false));
+            return (nameref_ctx, None);
+        }
+        if let Some(record_field) = ast::RecordPatField::for_field_name_ref(&name_ref) {
+            let pat_ctx =
+                pattern_context_for(original_file, record_field.parent_record_pat().clone().into());
+            return (
+                nameref_ctx,
+                Some(PatternContext {
+                    param_ctx: None,
+                    has_type_ascription: false,
+                    ref_token: None,
+                    mut_token: None,
+                    record_pat: find_node_in_file_compensated(
+                        original_file,
+                        &record_field.parent_record_pat(),
+                    ),
+                    ..pat_ctx
+                }),
+            );
+        }
 
         let segment = match_ast! {
             match parent {
@@ -1115,8 +1145,11 @@ fn classify_name_ref(
                 })
                 .unwrap_or(false)
         };
-        let is_in_func_update = |it: &SyntaxNode| {
+        let mut fill_record_expr = |syn: &SyntaxNode| {
-            it.parent().map_or(false, |it| ast::RecordExprFieldList::can_cast(it.kind()))
+            if let Some(record_expr) = syn.ancestors().nth(2).and_then(ast::RecordExpr::cast) {
+                nameref_ctx.record_expr =
+                    find_node_in_file_compensated(original_file, &record_expr).zip(Some(true));
+            }
         };
 
         let kind = path.syntax().ancestors().find_map(|it| {
@@ -1125,11 +1158,12 @@ fn classify_name_ref(
                 match it {
                     ast::PathType(_) => Some(PathKind::Type),
                     ast::PathExpr(it) => {
+                        fill_record_expr(it.syntax());
+
                         path_ctx.has_call_parens = it.syntax().parent().map_or(false, |it| ast::CallExpr::can_cast(it.kind()));
                         let in_block_expr = is_in_block(it.syntax());
                         let in_loop_body = is_in_loop_body(it.syntax());
-                        let in_functional_update = is_in_func_update(it.syntax());
+                        Some(PathKind::Expr { in_block_expr, in_loop_body })
-                        Some(PathKind::Expr { in_block_expr, in_loop_body, in_functional_update })
                     },
                     ast::TupleStructPat(it) => {
                         path_ctx.has_call_parens = true;
@@ -1163,8 +1197,8 @@ fn classify_name_ref(
                                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());
-                                    let in_functional_update = is_in_func_update(it.syntax());
+                                    fill_record_expr(it.syntax());
-                                    PathKind::Expr { in_block_expr, in_loop_body, in_functional_update }
+                                    PathKind::Expr { in_block_expr, in_loop_body }
                                 }));
                             },
                         }
@@ -1299,6 +1333,7 @@ fn pattern_context_for(original_file: &SyntaxNode, pat: ast::Pat) -> PatternCont
         parent_pat: pat.syntax().parent().and_then(ast::Pat::cast),
         mut_token,
         ref_token,
+        record_pat: None,
     }
 }
 

crates/ide-completion/src/patterns.rs

@@ -53,19 +53,6 @@ pub(crate) enum ImmediateLocation {
     // Only set from a type arg
     /// Original file ast node
     GenericArgList(ast::GenericArgList),
-    /// The record expr of the field name we are completing
-    ///
-    /// Original file ast node
-    RecordExpr(ast::RecordExpr),
-    /// The record expr of the functional update syntax we are completing
-    ///
-    /// Original file ast node
-    RecordExprUpdate(ast::RecordExpr),
-    /// The record pat of the field name we are completing
-    ///
-    /// Original file ast node
-    // FIXME: This should be moved to pattern_ctx
-    RecordPat(ast::RecordPat),
 }
 
 pub(crate) fn determine_prev_sibling(name_like: &ast::NameLike) -> Option<ImmediatePrevSibling> {
@@ -136,27 +123,8 @@ pub(crate) fn determine_location(
     name_like: &ast::NameLike,
 ) -> Option<ImmediateLocation> {
     let node = match name_like {
-        ast::NameLike::NameRef(name_ref) => {
+        ast::NameLike::NameRef(name_ref) => maximize_name_ref(name_ref),
-            if ast::RecordExprField::for_field_name(name_ref).is_some() {
+        ast::NameLike::Name(name) => name.syntax().clone(),
-                return sema
-                    .find_node_at_offset_with_macros(original_file, offset)
-                    .map(ImmediateLocation::RecordExpr);
-            }
-            if ast::RecordPatField::for_field_name_ref(name_ref).is_some() {
-                return sema
-                    .find_node_at_offset_with_macros(original_file, offset)
-                    .map(ImmediateLocation::RecordPat);
-            }
-            maximize_name_ref(name_ref)
-        }
-        ast::NameLike::Name(name) => {
-            if ast::RecordPatField::for_field_name(name).is_some() {
-                return sema
-                    .find_node_at_offset_with_macros(original_file, offset)
-                    .map(ImmediateLocation::RecordPat);
-            }
-            name.syntax().clone()
-        }
         ast::NameLike::Lifetime(lt) => lt.syntax().clone(),
     };
 
@@ -199,9 +167,6 @@ pub(crate) fn determine_location(
             ast::SourceFile(_) => ImmediateLocation::ItemList,
             ast::ItemList(_) => ImmediateLocation::ItemList,
             ast::RefExpr(_) => ImmediateLocation::RefExpr,
-            ast::RecordExprFieldList(_) => sema
-                .find_node_at_offset_with_macros(original_file, offset)
-                .map(ImmediateLocation::RecordExprUpdate)?,
             ast::TupleField(_) => ImmediateLocation::TupleField,
             ast::TupleFieldList(_) => ImmediateLocation::TupleField,
             ast::TypeBound(_) => ImmediateLocation::TypeBound,

crates/ide-completion/src/tests/pattern.rs

@@ -338,7 +338,10 @@ struct Foo { bar: Bar }
 struct Bar(u32);
 fn outer(Foo { bar$0 }: Foo) {}
 "#,
-        expect![[r#""#]],
+        expect![[r#"
+            kw mut
+            kw ref
+        "#]],
     )
 }
 

crates/ide-completion/src/tests/record.rs

@@ -41,6 +41,8 @@ fn foo(s: Struct) {
 "#,
         expect![[r#"
             fd bar u32
+            kw mut
+            kw ref
         "#]],
     );
 }
@@ -58,6 +60,8 @@ fn foo(e: Enum) {
 "#,
         expect![[r#"
             fd bar u32
+            kw mut
+            kw ref
         "#]],
     );
 }
@@ -93,6 +97,8 @@ fn foo(f: Struct) {
 ",
         expect![[r#"
             fd field u32
+            kw mut
+            kw ref
         "#]],
     );
 }

crates/syntax/src/ast/node_ext.rs

@@ -510,6 +510,10 @@ pub fn for_field_name(field_name: &ast::Name) -> Option<ast::RecordPatField> {
         }
     }
 
+    pub fn parent_record_pat(&self) -> ast::RecordPat {
+        self.syntax().ancestors().find_map(ast::RecordPat::cast).unwrap()
+    }
+
     /// Deals with field init shorthand
     pub fn field_name(&self) -> Option<NameOrNameRef> {
         if let Some(name_ref) = self.name_ref() {