Auto merge of rust-lang#12863 - lowr:fix/missing-fields-on-destructuring-assignment, r=Veykril

Fix missing fields check on destructuring assignment

Fixes rust-lang#12838

When checking if the record literal in question is an assignee expression or not, the new fn `is_assignee_record_literal` iterates over its ancestors until it is sure. This isn't super efficient, as we don't cache anything and does the iteration for every record literal during missing fields check. Alternatively, we may want to have a field like `assignee` on `hir_def::Expr::{RecordLit, Array, Tuple, Call}` to tell if it's an assignee expression, which would be O(1) when checking later but have some memory overhead for the field.

Author: bors

crates/hir-def/src/body/lower.rs

@@ -96,6 +96,7 @@ pub(super) fn lower(
         expander,
         name_to_pat_grouping: Default::default(),
         is_lowering_inside_or_pat: false,
+        is_lowering_assignee_expr: false,
     }
     .collect(params, body)
 }
@@ -109,6 +110,7 @@ struct ExprCollector<'a> {
     // a poor-mans union-find?
     name_to_pat_grouping: FxHashMap<Name, Vec<PatId>>,
     is_lowering_inside_or_pat: bool,
+    is_lowering_assignee_expr: bool,
 }
 
 impl ExprCollector<'_> {
@@ -283,7 +285,10 @@ impl ExprCollector<'_> {
                 } else {
                     Box::default()
                 };
-                self.alloc_expr(Expr::Call { callee, args }, syntax_ptr)
+                self.alloc_expr(
+                    Expr::Call { callee, args, is_assignee_expr: self.is_lowering_assignee_expr },
+                    syntax_ptr,
+                )
             }
             ast::Expr::MethodCallExpr(e) => {
                 let receiver = self.collect_expr_opt(e.receiver());
@@ -359,6 +364,7 @@ impl ExprCollector<'_> {
             ast::Expr::RecordExpr(e) => {
                 let path =
                     e.path().and_then(|path| self.expander.parse_path(self.db, path)).map(Box::new);
+                let is_assignee_expr = self.is_lowering_assignee_expr;
                 let record_lit = if let Some(nfl) = e.record_expr_field_list() {
                     let fields = nfl
                         .fields()
@@ -378,9 +384,16 @@ impl ExprCollector<'_> {
                         })
                         .collect();
                     let spread = nfl.spread().map(|s| self.collect_expr(s));
-                    Expr::RecordLit { path, fields, spread }
+                    let ellipsis = nfl.dotdot_token().is_some();
+                    Expr::RecordLit { path, fields, spread, ellipsis, is_assignee_expr }
                 } else {
-                    Expr::RecordLit { path, fields: Box::default(), spread: None }
+                    Expr::RecordLit {
+                        path,
+                        fields: Box::default(),
+                        spread: None,
+                        ellipsis: false,
+                        is_assignee_expr,
+                    }
                 };
 
                 self.alloc_expr(record_lit, syntax_ptr)
@@ -458,14 +471,21 @@ impl ExprCollector<'_> {
                 )
             }
             ast::Expr::BinExpr(e) => {
+                let op = e.op_kind();
+                if let Some(ast::BinaryOp::Assignment { op: None }) = op {
+                    self.is_lowering_assignee_expr = true;
+                }
                 let lhs = self.collect_expr_opt(e.lhs());
+                self.is_lowering_assignee_expr = false;
                 let rhs = self.collect_expr_opt(e.rhs());
-                let op = e.op_kind();
                 self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr)
             }
             ast::Expr::TupleExpr(e) => {
                 let exprs = e.fields().map(|expr| self.collect_expr(expr)).collect();
-                self.alloc_expr(Expr::Tuple { exprs }, syntax_ptr)
+                self.alloc_expr(
+                    Expr::Tuple { exprs, is_assignee_expr: self.is_lowering_assignee_expr },
+                    syntax_ptr,
+                )
             }
             ast::Expr::BoxExpr(e) => {
                 let expr = self.collect_expr_opt(e.expr());
@@ -477,8 +497,14 @@ impl ExprCollector<'_> {
 
                 match kind {
                     ArrayExprKind::ElementList(e) => {
-                        let exprs = e.map(|expr| self.collect_expr(expr)).collect();
-                        self.alloc_expr(Expr::Array(Array::ElementList(exprs)), syntax_ptr)
+                        let elements = e.map(|expr| self.collect_expr(expr)).collect();
+                        self.alloc_expr(
+                            Expr::Array(Array::ElementList {
+                                elements,
+                                is_assignee_expr: self.is_lowering_assignee_expr,
+                            }),
+                            syntax_ptr,
+                        )
                     }
                     ArrayExprKind::Repeat { initializer, repeat } => {
                         let initializer = self.collect_expr_opt(initializer);

crates/hir-def/src/expr.rs

@@ -110,6 +110,7 @@ pub enum Expr {
     Call {
         callee: ExprId,
         args: Box<[ExprId]>,
+        is_assignee_expr: bool,
     },
     MethodCall {
         receiver: ExprId,
@@ -138,6 +139,8 @@ pub enum Expr {
         path: Option<Box<Path>>,
         fields: Box<[RecordLitField]>,
         spread: Option<ExprId>,
+        ellipsis: bool,
+        is_assignee_expr: bool,
     },
     Field {
         expr: ExprId,
@@ -196,6 +199,7 @@ pub enum Expr {
     },
     Tuple {
         exprs: Box<[ExprId]>,
+        is_assignee_expr: bool,
     },
     Unsafe {
         body: ExprId,
@@ -211,7 +215,7 @@ pub enum Expr {
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum Array {
-    ElementList(Box<[ExprId]>),
+    ElementList { elements: Box<[ExprId]>, is_assignee_expr: bool },
     Repeat { initializer: ExprId, repeat: ExprId },
 }
 
@@ -285,7 +289,7 @@ impl Expr {
                 f(*iterable);
                 f(*body);
             }
-            Expr::Call { callee, args } => {
+            Expr::Call { callee, args, .. } => {
                 f(*callee);
                 args.iter().copied().for_each(f);
             }
@@ -339,9 +343,9 @@ impl Expr {
             | Expr::Box { expr } => {
                 f(*expr);
             }
-            Expr::Tuple { exprs } => exprs.iter().copied().for_each(f),
+            Expr::Tuple { exprs, .. } => exprs.iter().copied().for_each(f),
             Expr::Array(a) => match a {
-                Array::ElementList(exprs) => exprs.iter().copied().for_each(f),
+                Array::ElementList { elements, .. } => elements.iter().copied().for_each(f),
                 Array::Repeat { initializer, repeat } => {
                     f(*initializer);
                     f(*repeat)

crates/hir-ty/src/diagnostics/expr.rs

@@ -305,7 +305,10 @@ pub fn record_literal_missing_fields(
     expr: &Expr,
 ) -> Option<(VariantId, Vec<LocalFieldId>, /*exhaustive*/ bool)> {
     let (fields, exhaustive) = match expr {
-        Expr::RecordLit { path: _, fields, spread } => (fields, spread.is_none()),
+        Expr::RecordLit { fields, spread, ellipsis, is_assignee_expr, .. } => {
+            let exhaustive = if *is_assignee_expr { !*ellipsis } else { spread.is_none() };
+            (fields, exhaustive)
+        }
         _ => return None,
     };
 

crates/hir-ty/src/infer/expr.rs

@@ -276,7 +276,7 @@ impl<'a> InferenceContext<'a> {
 
                 closure_ty
             }
-            Expr::Call { callee, args } => {
+            Expr::Call { callee, args, .. } => {
                 let callee_ty = self.infer_expr(*callee, &Expectation::none());
                 let mut derefs = Autoderef::new(&mut self.table, callee_ty.clone());
                 let mut res = None;
@@ -421,7 +421,7 @@ impl<'a> InferenceContext<'a> {
                 }
                 TyKind::Never.intern(Interner)
             }
-            Expr::RecordLit { path, fields, spread } => {
+            Expr::RecordLit { path, fields, spread, .. } => {
                 let (ty, def_id) = self.resolve_variant(path.as_deref(), false);
                 if let Some(variant) = def_id {
                     self.write_variant_resolution(tgt_expr.into(), variant);
@@ -693,7 +693,7 @@ impl<'a> InferenceContext<'a> {
                     self.err_ty()
                 }
             }
-            Expr::Tuple { exprs } => {
+            Expr::Tuple { exprs, .. } => {
                 let mut tys = match expected
                     .only_has_type(&mut self.table)
                     .as_ref()
@@ -724,12 +724,12 @@ impl<'a> InferenceContext<'a> {
 
                 let expected = Expectation::has_type(elem_ty.clone());
                 let len = match array {
-                    Array::ElementList(items) => {
-                        for &expr in items.iter() {
+                    Array::ElementList { elements, .. } => {
+                        for &expr in elements.iter() {
                             let cur_elem_ty = self.infer_expr_inner(expr, &expected);
                             coerce.coerce(self, Some(expr), &cur_elem_ty);
                         }
-                        consteval::usize_const(Some(items.len() as u128))
+                        consteval::usize_const(Some(elements.len() as u128))
                     }
                     &Array::Repeat { initializer, repeat } => {
                         self.infer_expr_coerce(initializer, &Expectation::has_type(elem_ty));
@@ -850,15 +850,15 @@ impl<'a> InferenceContext<'a> {
         let rhs_ty = self.resolve_ty_shallow(rhs_ty);
 
         let ty = match &self.body[lhs] {
-            Expr::Tuple { exprs } => {
+            Expr::Tuple { exprs, .. } => {
                 // We don't consider multiple ellipses. This is analogous to
                 // `hir_def::body::lower::ExprCollector::collect_tuple_pat()`.
                 let ellipsis = exprs.iter().position(|e| is_rest_expr(*e));
                 let exprs: Vec<_> = exprs.iter().filter(|e| !is_rest_expr(**e)).copied().collect();
 
                 self.infer_tuple_pat_like(&rhs_ty, (), ellipsis, &exprs)
             }
-            Expr::Call { callee, args } => {
+            Expr::Call { callee, args, .. } => {
                 // Tuple structs
                 let path = match &self.body[*callee] {
                     Expr::Path(path) => Some(path),
@@ -872,7 +872,7 @@ impl<'a> InferenceContext<'a> {
 
                 self.infer_tuple_struct_pat_like(path, &rhs_ty, (), lhs, ellipsis, &args)
             }
-            Expr::Array(Array::ElementList(elements)) => {
+            Expr::Array(Array::ElementList { elements, .. }) => {
                 let elem_ty = match rhs_ty.kind(Interner) {
                     TyKind::Array(st, _) => st.clone(),
                     _ => self.err_ty(),

crates/ide-diagnostics/src/handlers/missing_fields.rs

@@ -292,6 +292,37 @@ fn x(a: S) {
         )
     }
 
+    #[test]
+    fn missing_record_expr_in_assignee_expr() {
+        check_diagnostics(
+            r"
+struct S { s: usize, t: usize }
+struct S2 { s: S, t: () }
+struct T(S);
+fn regular(a: S) {
+    let s;
+    S { s, .. } = a;
+}
+fn nested(a: S2) {
+    let s;
+    S2 { s: S { s, .. }, .. } = a;
+}
+fn in_tuple(a: (S,)) {
+    let s;
+    (S { s, .. },) = a;
+}
+fn in_array(a: [S;1]) {
+    let s;
+    [S { s, .. },] = a;
+}
+fn in_tuple_struct(a: T) {
+    let s;
+    T(S { s, .. }) = a;
+}
+            ",
+        );
+    }
+
     #[test]
     fn range_mapping_out_of_macros() {
         check_fix(