feat: add attributes support on struct fields and method #3870

Signed-off-by: Benjamin Coenen <[email protected]>

3 files changed, 151 insertions, 83 deletions

diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs
index f29a25505..9e9a9d047 100644
--- a/crates/ra_hir_ty/src/_match.rs
+++ b/crates/ra_hir_ty/src/_match.rs
@@ -235,7 +235,10 @@ impl From<PatId> for PatIdOrWild {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq)]
-pub struct MatchCheckNotImplemented;
+pub enum MatchCheckErr {
+    NotImplemented,
+    MalformedMatchArm,
+}
 
 /// The return type of `is_useful` is either an indication of usefulness
 /// of the match arm, or an error in the case the match statement
@@ -244,7 +247,7 @@ pub struct MatchCheckNotImplemented;
 ///
 /// The `std::result::Result` type is used here rather than a custom enum
 /// to allow the use of `?`.
-pub type MatchCheckResult<T> = Result<T, MatchCheckNotImplemented>;
+pub type MatchCheckResult<T> = Result<T, MatchCheckErr>;
 
 #[derive(Debug)]
 /// A row in a Matrix.
@@ -335,12 +338,12 @@ impl PatStack {
                     Expr::Literal(Literal::Bool(_)) => None,
                     // perhaps this is actually unreachable given we have
                     // already checked that these match arms have the appropriate type?
-                    _ => return Err(MatchCheckNotImplemented),
+                    _ => return Err(MatchCheckErr::NotImplemented),
                 }
             }
             (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)?),
             (Pat::Path(_), Constructor::Enum(constructor)) => {
-                // enums with no associated data become `Pat::Path`
+                // unit enum variants become `Pat::Path`
                 let pat_id = self.head().as_id().expect("we know this isn't a wild");
                 if !enum_variant_matches(cx, pat_id, *constructor) {
                     None
@@ -348,16 +351,23 @@ impl PatStack {
                     Some(self.to_tail())
                 }
             }
-            (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(constructor)) => {
+            (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(enum_constructor)) => {
                 let pat_id = self.head().as_id().expect("we know this isn't a wild");
-                if !enum_variant_matches(cx, pat_id, *constructor) {
+                if !enum_variant_matches(cx, pat_id, *enum_constructor) {
                     None
                 } else {
+                    // If the enum variant matches, then we need to confirm
+                    // that the number of patterns aligns with the expected
+                    // number of patterns for that enum variant.
+                    if pat_ids.len() != constructor.arity(cx)? {
+                        return Err(MatchCheckErr::MalformedMatchArm);
+                    }
+
                     Some(self.replace_head_with(pat_ids))
                 }
             }
-            (Pat::Or(_), _) => return Err(MatchCheckNotImplemented),
-            (_, _) => return Err(MatchCheckNotImplemented),
+            (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented),
+            (_, _) => return Err(MatchCheckErr::NotImplemented),
         };
 
         Ok(result)
@@ -514,7 +524,7 @@ pub(crate) fn is_useful(
         return if any_useful {
             Ok(Usefulness::Useful)
         } else if found_unimplemented {
-            Err(MatchCheckNotImplemented)
+            Err(MatchCheckErr::NotImplemented)
         } else {
             Ok(Usefulness::NotUseful)
         };
@@ -567,7 +577,7 @@ pub(crate) fn is_useful(
                 }
 
                 if found_unimplemented {
-                    Err(MatchCheckNotImplemented)
+                    Err(MatchCheckErr::NotImplemented)
                 } else {
                     Ok(Usefulness::NotUseful)
                 }
@@ -604,7 +614,7 @@ impl Constructor {
                 match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() {
                     VariantData::Tuple(struct_field_data) => struct_field_data.len(),
                     VariantData::Unit => 0,
-                    _ => return Err(MatchCheckNotImplemented),
+                    _ => return Err(MatchCheckErr::NotImplemented),
                 }
             }
         };
@@ -637,20 +647,20 @@ fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Opt
         Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }),
         Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] {
             Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)),
-            _ => return Err(MatchCheckNotImplemented),
+            _ => return Err(MatchCheckErr::NotImplemented),
         },
         Pat::TupleStruct { .. } | Pat::Path(_) => {
             let pat_id = pat.as_id().expect("we already know this pattern is not a wild");
             let variant_id =
-                cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckNotImplemented)?;
+                cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckErr::NotImplemented)?;
             match variant_id {
                 VariantId::EnumVariantId(enum_variant_id) => {
                     Some(Constructor::Enum(enum_variant_id))
                 }
-                _ => return Err(MatchCheckNotImplemented),
+                _ => return Err(MatchCheckErr::NotImplemented),
             }
         }
-        _ => return Err(MatchCheckNotImplemented),
+        _ => return Err(MatchCheckErr::NotImplemented),
     };
 
     Ok(res)
@@ -1325,6 +1335,40 @@ mod tests {
     }
 
     #[test]
+    fn malformed_match_arm_tuple_missing_pattern() {
+        let content = r"
+            fn test_fn() {
+                match (0) {
+                    () => (),
+                }
+            }
+        ";
+
+        // Match arms with the incorrect type are filtered out.
+        check_diagnostic(content);
+    }
+
+    #[test]
+    fn malformed_match_arm_tuple_enum_missing_pattern() {
+        let content = r"
+            enum Either {
+                A,
+                B(u32),
+            }
+            fn test_fn() {
+                match Either::A {
+                    Either::A => (),
+                    Either::B() => (),
+                }
+            }
+        ";
+
+        // We are testing to be sure we don't panic here when the match
+        // arm `Either::B` is missing its pattern.
+        check_no_diagnostic(content);
+    }
+
+    #[test]
     fn enum_not_in_scope() {
         let content = r"
             fn test_fn() {
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs
index fb779cbef..e45e9ea14 100644
--- a/crates/ra_hir_ty/src/expr.rs
+++ b/crates/ra_hir_ty/src/expr.rs
@@ -2,12 +2,8 @@
 
 use std::sync::Arc;
 
-use hir_def::{
-    path::{path, Path},
-    resolver::HasResolver,
-    AdtId, FunctionId,
-};
-use hir_expand::{diagnostics::DiagnosticSink, name::Name};
+use hir_def::{path::path, resolver::HasResolver, AdtId, FunctionId};
+use hir_expand::diagnostics::DiagnosticSink;
 use ra_syntax::{ast, AstPtr};
 use rustc_hash::FxHashSet;
 
@@ -28,7 +24,7 @@ pub use hir_def::{
         ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp,
         MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp,
     },
-    VariantId,
+    LocalStructFieldId, VariantId,
 };
 
 pub struct ExprValidator<'a, 'b: 'a> {
@@ -49,14 +45,37 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
     pub fn validate_body(&mut self, db: &dyn HirDatabase) {
         let body = db.body(self.func.into());
 
-        for e in body.exprs.iter() {
-            if let (id, Expr::RecordLit { path, fields, spread }) = e {
-                self.validate_record_literal(id, path, fields, *spread, db);
-            } else if let (id, Expr::Match { expr, arms }) = e {
+        for (id, expr) in body.exprs.iter() {
+            if let Some((variant_def, missed_fields, true)) =
+                record_literal_missing_fields(db, &self.infer, id, expr)
+            {
+                // XXX: only look at source_map if we do have missing fields
+                let (_, source_map) = db.body_with_source_map(self.func.into());
+
+                if let Ok(source_ptr) = source_map.expr_syntax(id) {
+                    if let Some(expr) = source_ptr.value.left() {
+                        let root = source_ptr.file_syntax(db.upcast());
+                        if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) {
+                            if let Some(field_list) = record_lit.record_field_list() {
+                                let variant_data = variant_data(db.upcast(), variant_def);
+                                let missed_fields = missed_fields
+                                    .into_iter()
+                                    .map(|idx| variant_data.fields()[idx].name.clone())
+                                    .collect();
+                                self.sink.push(MissingFields {
+                                    file: source_ptr.file_id,
+                                    field_list: AstPtr::new(&field_list),
+                                    missed_fields,
+                                })
+                            }
+                        }
+                    }
+                }
+            }
+            if let Expr::Match { expr, arms } = expr {
                 self.validate_match(id, *expr, arms, db, self.infer.clone());
             }
         }
-
         let body_expr = &body[body.body_expr];
         if let Expr::Block { tail: Some(t), .. } = body_expr {
             self.validate_results_in_tail_expr(body.body_expr, *t, db);
@@ -145,61 +164,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         }
     }
 
-    fn validate_record_literal(
-        &mut self,
-        id: ExprId,
-        _path: &Option<Path>,
-        fields: &[RecordLitField],
-        spread: Option<ExprId>,
-        db: &dyn HirDatabase,
-    ) {
-        if spread.is_some() {
-            return;
-        };
-        let variant_def: VariantId = match self.infer.variant_resolution_for_expr(id) {
-            Some(VariantId::UnionId(_)) | None => return,
-            Some(it) => it,
-        };
-        if let VariantId::UnionId(_) = variant_def {
-            return;
-        }
-
-        let variant_data = variant_data(db.upcast(), variant_def);
-
-        let lit_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect();
-        let missed_fields: Vec<Name> = variant_data
-            .fields()
-            .iter()
-            .filter_map(|(_f, d)| {
-                let name = d.name.clone();
-                if lit_fields.contains(&name) {
-                    None
-                } else {
-                    Some(name)
-                }
-            })
-            .collect();
-        if missed_fields.is_empty() {
-            return;
-        }
-        let (_, source_map) = db.body_with_source_map(self.func.into());
-
-        if let Ok(source_ptr) = source_map.expr_syntax(id) {
-            if let Some(expr) = source_ptr.value.left() {
-                let root = source_ptr.file_syntax(db.upcast());
-                if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) {
-                    if let Some(field_list) = record_lit.record_field_list() {
-                        self.sink.push(MissingFields {
-                            file: source_ptr.file_id,
-                            field_list: AstPtr::new(&field_list),
-                            missed_fields,
-                        })
-                    }
-                }
-            }
-        }
-    }
-
     fn validate_results_in_tail_expr(&mut self, body_id: ExprId, id: ExprId, db: &dyn HirDatabase) {
         // the mismatch will be on the whole block currently
         let mismatch = match self.infer.type_mismatch_for_expr(body_id) {
@@ -232,3 +196,63 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         }
     }
 }
+
+pub fn record_literal_missing_fields(
+    db: &dyn HirDatabase,
+    infer: &InferenceResult,
+    id: ExprId,
+    expr: &Expr,
+) -> Option<(VariantId, Vec<LocalStructFieldId>, /*exhaustive*/ bool)> {
+    let (fields, exhausitve) = match expr {
+        Expr::RecordLit { path: _, fields, spread } => (fields, spread.is_none()),
+        _ => return None,
+    };
+
+    let variant_def = infer.variant_resolution_for_expr(id)?;
+    if let VariantId::UnionId(_) = variant_def {
+        return None;
+    }
+
+    let variant_data = variant_data(db.upcast(), variant_def);
+
+    let specified_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect();
+    let missed_fields: Vec<LocalStructFieldId> = variant_data
+        .fields()
+        .iter()
+        .filter_map(|(f, d)| if specified_fields.contains(&d.name) { None } else { Some(f) })
+        .collect();
+    if missed_fields.is_empty() {
+        return None;
+    }
+    Some((variant_def, missed_fields, exhausitve))
+}
+
+pub fn record_pattern_missing_fields(
+    db: &dyn HirDatabase,
+    infer: &InferenceResult,
+    id: PatId,
+    pat: &Pat,
+) -> Option<(VariantId, Vec<LocalStructFieldId>)> {
+    let fields = match pat {
+        Pat::Record { path: _, args } => args,
+        _ => return None,
+    };
+
+    let variant_def = infer.variant_resolution_for_pat(id)?;
+    if let VariantId::UnionId(_) = variant_def {
+        return None;
+    }
+
+    let variant_data = variant_data(db.upcast(), variant_def);
+
+    let specified_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect();
+    let missed_fields: Vec<LocalStructFieldId> = variant_data
+        .fields()
+        .iter()
+        .filter_map(|(f, d)| if specified_fields.contains(&d.name) { None } else { Some(f) })
+        .collect();
+    if missed_fields.is_empty() {
+        return None;
+    }
+    Some((variant_def, missed_fields))
+}
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs
index 060814e53..608408d88 100644
--- a/crates/ra_hir_ty/src/tests.rs
+++ b/crates/ra_hir_ty/src/tests.rs
@@ -336,7 +336,7 @@ fn no_such_field_with_feature_flag_diagnostics() {
             pub(crate) fn new(my_val: usize, bar: bool) -> Self {
                 Self { my_val, bar }
             }
-        
+
             #[cfg(not(feature = "foo"))]
             pub(crate) fn new(my_val: usize, _bar: bool) -> Self {
                 Self { my_val }