7 files changed, 149 insertions, 138 deletions

diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs
index 5b0dda634..3623b8569 100644
--- a/crates/ra_hir_ty/src/diagnostics.rs
+++ b/crates/ra_hir_ty/src/diagnostics.rs
@@ -1,13 +1,30 @@
 //! FIXME: write short doc here
+mod expr;
+mod match_check;
+mod unsafe_check;
 
 use std::any::Any;
 
+use hir_def::DefWithBodyId;
+use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
 use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile};
+use ra_prof::profile;
 use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
 use stdx::format_to;
 
-pub use hir_def::{diagnostics::UnresolvedModule, expr::MatchArm, path::Path};
-pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
+use crate::db::HirDatabase;
+
+pub use crate::diagnostics::expr::{record_literal_missing_fields, record_pattern_missing_fields};
+
+pub fn validate_body(db: &dyn HirDatabase, owner: DefWithBodyId, sink: &mut DiagnosticSink<'_>) {
+    let _p = profile("validate_body");
+    let infer = db.infer(owner);
+    infer.add_diagnostics(db, owner, sink);
+    let mut validator = expr::ExprValidator::new(owner, infer.clone(), sink);
+    validator.validate_body(db);
+    let mut validator = unsafe_check::UnsafeValidator::new(owner, infer, sink);
+    validator.validate_body(db);
+}
 
 #[derive(Debug)]
 pub struct NoSuchField {
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/diagnostics/expr.rs
index d44562b22..239be779f 100644
--- a/crates/ra_hir_ty/src/expr.rs
+++ b/crates/ra_hir_ty/src/diagnostics/expr.rs
@@ -2,7 +2,7 @@
 
 use std::sync::Arc;
 
-use hir_def::{path::path, resolver::HasResolver, AdtId, FunctionId};
+use hir_def::{path::path, resolver::HasResolver, AdtId, DefWithBodyId};
 use hir_expand::diagnostics::DiagnosticSink;
 use ra_syntax::{ast, AstPtr};
 use rustc_hash::FxHashSet;
@@ -10,9 +10,9 @@ use rustc_hash::FxHashSet;
 use crate::{
     db::HirDatabase,
     diagnostics::{
+        match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness},
         MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields,
     },
-    match_checking::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness},
     utils::variant_data,
     ApplicationTy, InferenceResult, Ty, TypeCtor,
 };
@@ -30,23 +30,23 @@ pub use hir_def::{
     LocalFieldId, Lookup, VariantId,
 };
 
-pub struct ExprValidator<'a, 'b: 'a> {
-    func: FunctionId,
+pub(super) struct ExprValidator<'a, 'b: 'a> {
+    owner: DefWithBodyId,
     infer: Arc<InferenceResult>,
     sink: &'a mut DiagnosticSink<'b>,
 }
 
 impl<'a, 'b> ExprValidator<'a, 'b> {
-    pub fn new(
-        func: FunctionId,
+    pub(super) fn new(
+        owner: DefWithBodyId,
         infer: Arc<InferenceResult>,
         sink: &'a mut DiagnosticSink<'b>,
     ) -> ExprValidator<'a, 'b> {
-        ExprValidator { func, infer, sink }
+        ExprValidator { owner, infer, sink }
     }
 
-    pub fn validate_body(&mut self, db: &dyn HirDatabase) {
-        let body = db.body(self.func.into());
+    pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) {
+        let body = db.body(self.owner.into());
 
         for (id, expr) in body.exprs.iter() {
             if let Some((variant_def, missed_fields, true)) =
@@ -96,7 +96,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         missed_fields: Vec<LocalFieldId>,
     ) {
         // XXX: only look at source_map if we do have missing fields
-        let (_, source_map) = db.body_with_source_map(self.func.into());
+        let (_, source_map) = db.body_with_source_map(self.owner.into());
 
         if let Ok(source_ptr) = source_map.expr_syntax(id) {
             let root = source_ptr.file_syntax(db.upcast());
@@ -125,7 +125,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         missed_fields: Vec<LocalFieldId>,
     ) {
         // XXX: only look at source_map if we do have missing fields
-        let (_, source_map) = db.body_with_source_map(self.func.into());
+        let (_, source_map) = db.body_with_source_map(self.owner.into());
 
         if let Ok(source_ptr) = source_map.pat_syntax(id) {
             if let Some(expr) = source_ptr.value.as_ref().left() {
@@ -181,7 +181,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         let mut arg_count = args.len();
 
         if arg_count != param_count {
-            let (_, source_map) = db.body_with_source_map(self.func.into());
+            let (_, source_map) = db.body_with_source_map(self.owner.into());
             if let Ok(source_ptr) = source_map.expr_syntax(call_id) {
                 if is_method_call {
                     param_count -= 1;
@@ -208,7 +208,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         infer: Arc<InferenceResult>,
     ) {
         let (body, source_map): (Arc<Body>, Arc<BodySourceMap>) =
-            db.body_with_source_map(self.func.into());
+            db.body_with_source_map(self.owner.into());
 
         let match_expr_ty = match infer.type_of_expr.get(match_expr) {
             Some(ty) => ty,
@@ -289,7 +289,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
 
         let core_result_path = path![core::result::Result];
 
-        let resolver = self.func.resolver(db.upcast());
+        let resolver = self.owner.resolver(db.upcast());
         let core_result_enum = match resolver.resolve_known_enum(db.upcast(), &core_result_path) {
             Some(it) => it,
             _ => return,
@@ -304,7 +304,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         };
 
         if params.len() == 2 && params[0] == mismatch.actual {
-            let (_, source_map) = db.body_with_source_map(self.func.into());
+            let (_, source_map) = db.body_with_source_map(self.owner.into());
 
             if let Ok(source_ptr) = source_map.expr_syntax(id) {
                 self.sink
diff --git a/crates/ra_hir_ty/src/match_checking.rs b/crates/ra_hir_ty/src/diagnostics/match_check.rs
index 5495ce284..899025a87 100644
--- a/crates/ra_hir_ty/src/match_checking.rs
+++ b/crates/ra_hir_ty/src/diagnostics/match_check.rs
@@ -218,15 +218,16 @@
 //!   ```
 use std::sync::Arc;
 
-use smallvec::{smallvec, SmallVec};
-
-use crate::{
-    db::HirDatabase,
-    expr::{Body, Expr, Literal, Pat, PatId},
-    ApplicationTy, InferenceResult, Ty, TypeCtor,
+use hir_def::{
+    adt::VariantData,
+    body::Body,
+    expr::{Expr, Literal, Pat, PatId},
+    AdtId, EnumVariantId, VariantId,
 };
-use hir_def::{adt::VariantData, AdtId, EnumVariantId, VariantId};
 use ra_arena::Idx;
+use smallvec::{smallvec, SmallVec};
+
+use crate::{db::HirDatabase, ApplicationTy, InferenceResult, Ty, TypeCtor};
 
 #[derive(Debug, Clone, Copy)]
 /// Either a pattern from the source code being analyzed, represented as
@@ -271,7 +272,7 @@ impl From<&PatId> for PatIdOrWild {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq)]
-pub enum MatchCheckErr {
+pub(super) enum MatchCheckErr {
     NotImplemented,
     MalformedMatchArm,
     /// Used when type inference cannot resolve the type of
@@ -286,21 +287,21 @@ pub enum MatchCheckErr {
 ///
 /// The `std::result::Result` type is used here rather than a custom enum
 /// to allow the use of `?`.
-pub type MatchCheckResult<T> = Result<T, MatchCheckErr>;
+pub(super) type MatchCheckResult<T> = Result<T, MatchCheckErr>;
 
 #[derive(Debug)]
 /// A row in a Matrix.
 ///
 /// This type is modeled from the struct of the same name in `rustc`.
-pub(crate) struct PatStack(PatStackInner);
+pub(super) struct PatStack(PatStackInner);
 type PatStackInner = SmallVec<[PatIdOrWild; 2]>;
 
 impl PatStack {
-    pub(crate) fn from_pattern(pat_id: PatId) -> PatStack {
+    pub(super) fn from_pattern(pat_id: PatId) -> PatStack {
         Self(smallvec!(pat_id.into()))
     }
 
-    pub(crate) fn from_wild() -> PatStack {
+    pub(super) fn from_wild() -> PatStack {
         Self(smallvec!(PatIdOrWild::Wild))
     }
 
@@ -509,14 +510,14 @@ impl PatStack {
 /// A collection of PatStack.
 ///
 /// This type is modeled from the struct of the same name in `rustc`.
-pub(crate) struct Matrix(Vec<PatStack>);
+pub(super) struct Matrix(Vec<PatStack>);
 
 impl Matrix {
-    pub(crate) fn empty() -> Self {
+    pub(super) fn empty() -> Self {
         Self(vec![])
     }
 
-    pub(crate) fn push(&mut self, cx: &MatchCheckCtx, row: PatStack) {
+    pub(super) fn push(&mut self, cx: &MatchCheckCtx, row: PatStack) {
         if let Some(Pat::Or(pat_ids)) = row.get_head().map(|pat_id| pat_id.as_pat(cx)) {
             // Or patterns are expanded here
             for pat_id in pat_ids {
@@ -578,16 +579,16 @@ impl Matrix {
 /// not matched by an prior match arms.
 ///
 /// We may eventually need an `Unknown` variant here.
-pub enum Usefulness {
+pub(super) enum Usefulness {
     Useful,
     NotUseful,
 }
 
-pub struct MatchCheckCtx<'a> {
-    pub match_expr: Idx<Expr>,
-    pub body: Arc<Body>,
-    pub infer: Arc<InferenceResult>,
-    pub db: &'a dyn HirDatabase,
+pub(super) struct MatchCheckCtx<'a> {
+    pub(super) match_expr: Idx<Expr>,
+    pub(super) body: Arc<Body>,
+    pub(super) infer: Arc<InferenceResult>,
+    pub(super) db: &'a dyn HirDatabase,
 }
 
 /// Given a set of patterns `matrix`, and pattern to consider `v`, determines
@@ -598,7 +599,7 @@ pub struct MatchCheckCtx<'a> {
 /// expected that you have already type checked the match arms. All patterns in
 /// matrix should be the same type as v, as well as they should all be the same
 /// type as the match expression.
-pub(crate) fn is_useful(
+pub(super) fn is_useful(
     cx: &MatchCheckCtx,
     matrix: &Matrix,
     v: &PatStack,
@@ -836,23 +837,23 @@ fn enum_variant_matches(cx: &MatchCheckCtx, pat_id: PatId, enum_variant_id: Enum
 
 #[cfg(test)]
 mod tests {
-    pub(super) use insta::assert_snapshot;
-    pub(super) use ra_db::fixture::WithFixture;
+    use insta::assert_snapshot;
+    use ra_db::fixture::WithFixture;
 
-    pub(super) use crate::{diagnostics::MissingMatchArms, test_db::TestDB};
+    use crate::{diagnostics::MissingMatchArms, test_db::TestDB};
 
-    pub(super) fn check_diagnostic_message(ra_fixture: &str) -> String {
+    fn check_diagnostic_message(ra_fixture: &str) -> String {
         TestDB::with_single_file(ra_fixture).0.diagnostic::<MissingMatchArms>().0
     }
 
-    pub(super) fn check_diagnostic(ra_fixture: &str) {
+    fn check_diagnostic(ra_fixture: &str) {
         let diagnostic_count =
             TestDB::with_single_file(ra_fixture).0.diagnostic::<MissingMatchArms>().1;
 
         assert_eq!(1, diagnostic_count, "no diagnostic reported");
     }
 
-    pub(super) fn check_no_diagnostic(ra_fixture: &str) {
+    fn check_no_diagnostic(ra_fixture: &str) {
         let (s, diagnostic_count) =
             TestDB::with_single_file(ra_fixture).0.diagnostic::<MissingMatchArms>();
 
@@ -2035,28 +2036,25 @@ mod tests {
         ",
         );
     }
-}
 
-#[cfg(test)]
-mod false_negatives {
-    //! The implementation of match checking here is a work in progress. As we roll this out, we
-    //! prefer false negatives to false positives (ideally there would be no false positives). This
-    //! test module should document known false negatives. Eventually we will have a complete
-    //! implementation of match checking and this module will be empty.
-    //!
-    //! The reasons for documenting known false negatives:
-    //!
-    //!   1. It acts as a backlog of work that can be done to improve the behavior of the system.
-    //!   2. It ensures the code doesn't panic when handling these cases.
-
-    use super::tests::*;
-
-    #[test]
-    fn integers() {
-        // This is a false negative.
-        // We don't currently check integer exhaustiveness.
-        check_no_diagnostic(
-            r"
+    mod false_negatives {
+        //! The implementation of match checking here is a work in progress. As we roll this out, we
+        //! prefer false negatives to false positives (ideally there would be no false positives). This
+        //! test module should document known false negatives. Eventually we will have a complete
+        //! implementation of match checking and this module will be empty.
+        //!
+        //! The reasons for documenting known false negatives:
+        //!
+        //!   1. It acts as a backlog of work that can be done to improve the behavior of the system.
+        //!   2. It ensures the code doesn't panic when handling these cases.
+        use super::*;
+
+        #[test]
+        fn integers() {
+            // This is a false negative.
+            // We don't currently check integer exhaustiveness.
+            check_no_diagnostic(
+                r"
             fn test_fn() {
                 match 5 {
                     10 => (),
@@ -2064,15 +2062,15 @@ mod false_negatives {
                 }
             }
         ",
-        );
-    }
+            );
+        }
 
-    #[test]
-    fn internal_or() {
-        // This is a false negative.
-        // We do not currently handle patterns with internal `or`s.
-        check_no_diagnostic(
-            r"
+        #[test]
+        fn internal_or() {
+            // This is a false negative.
+            // We do not currently handle patterns with internal `or`s.
+            check_no_diagnostic(
+                r"
             fn test_fn() {
                 enum Either {
                     A(bool),
@@ -2083,17 +2081,17 @@ mod false_negatives {
                 }
             }
         ",
-        );
-    }
+            );
+        }
 
-    #[test]
-    fn expr_loop_missing_arm() {
-        // This is a false negative.
-        // We currently infer the type of `loop { break Foo::A }` to `!`, which
-        // causes us to skip the diagnostic since `Either::A` doesn't type check
-        // with `!`.
-        check_diagnostic(
-            r"
+        #[test]
+        fn expr_loop_missing_arm() {
+            // This is a false negative.
+            // We currently infer the type of `loop { break Foo::A }` to `!`, which
+            // causes us to skip the diagnostic since `Either::A` doesn't type check
+            // with `!`.
+            check_diagnostic(
+                r"
             enum Either {
                 A,
                 B,
@@ -2104,45 +2102,45 @@ mod false_negatives {
                 }
             }
         ",
-        );
-    }
+            );
+        }
 
-    #[test]
-    fn tuple_of_bools_with_ellipsis_at_end_missing_arm() {
-        // This is a false negative.
-        // We don't currently handle tuple patterns with ellipsis.
-        check_no_diagnostic(
-            r"
+        #[test]
+        fn tuple_of_bools_with_ellipsis_at_end_missing_arm() {
+            // This is a false negative.
+            // We don't currently handle tuple patterns with ellipsis.
+            check_no_diagnostic(
+                r"
             fn test_fn() {
                 match (false, true, false) {
                     (false, ..) => {},
                 }
             }
         ",
-        );
-    }
+            );
+        }
 
-    #[test]
-    fn tuple_of_bools_with_ellipsis_at_beginning_missing_arm() {
-        // This is a false negative.
-        // We don't currently handle tuple patterns with ellipsis.
-        check_no_diagnostic(
-            r"
+        #[test]
+        fn tuple_of_bools_with_ellipsis_at_beginning_missing_arm() {
+            // This is a false negative.
+            // We don't currently handle tuple patterns with ellipsis.
+            check_no_diagnostic(
+                r"
             fn test_fn() {
                 match (false, true, false) {
                     (.., false) => {},
                 }
             }
         ",
-        );
-    }
+            );
+        }
 
-    #[test]
-    fn struct_missing_arm() {
-        // This is a false negative.
-        // We don't currently handle structs.
-        check_no_diagnostic(
-            r"
+        #[test]
+        fn struct_missing_arm() {
+            // This is a false negative.
+            // We don't currently handle structs.
+            check_no_diagnostic(
+                r"
             struct Foo {
                 a: bool,
             }
@@ -2152,6 +2150,7 @@ mod false_negatives {
                 }
             }
         ",
-        );
+            );
+        }
     }
 }
diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
index c512c4f8e..b8ff95ee1 100644
--- a/crates/ra_hir_ty/src/unsafe_validation.rs
+++ b/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
@@ -6,7 +6,7 @@ use std::sync::Arc;
 use hir_def::{
     body::Body,
     expr::{Expr, ExprId, UnaryOp},
-    DefWithBodyId, FunctionId,
+    DefWithBodyId,
 };
 use hir_expand::diagnostics::DiagnosticSink;
 
@@ -15,26 +15,29 @@ use crate::{
     InferenceResult, Ty, TypeCtor,
 };
 
-pub struct UnsafeValidator<'a, 'b: 'a> {
-    func: FunctionId,
+pub(super) struct UnsafeValidator<'a, 'b: 'a> {
+    owner: DefWithBodyId,
     infer: Arc<InferenceResult>,
     sink: &'a mut DiagnosticSink<'b>,
 }
 
 impl<'a, 'b> UnsafeValidator<'a, 'b> {
-    pub fn new(
-        func: FunctionId,
+    pub(super) fn new(
+        owner: DefWithBodyId,
         infer: Arc<InferenceResult>,
         sink: &'a mut DiagnosticSink<'b>,
     ) -> UnsafeValidator<'a, 'b> {
-        UnsafeValidator { func, infer, sink }
+        UnsafeValidator { owner, infer, sink }
     }
 
-    pub fn validate_body(&mut self, db: &dyn HirDatabase) {
-        let def = self.func.into();
+    pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) {
+        let def = self.owner.into();
         let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def);
-        let func_data = db.function_data(self.func);
-        if func_data.is_unsafe
+        let is_unsafe = match self.owner {
+            DefWithBodyId::FunctionId(it) => db.function_data(it).is_unsafe,
+            DefWithBodyId::StaticId(_) | DefWithBodyId::ConstId(_) => false,
+        };
+        if is_unsafe
             || unsafe_expressions
                 .iter()
                 .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block)
diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs
index 2ce4f65cc..28f32a0a4 100644
--- a/crates/ra_hir_ty/src/infer.rs
+++ b/crates/ra_hir_ty/src/infer.rs
@@ -168,7 +168,7 @@ impl InferenceResult {
     pub fn add_diagnostics(
         &self,
         db: &dyn HirDatabase,
-        owner: FunctionId,
+        owner: DefWithBodyId,
         sink: &mut DiagnosticSink,
     ) {
         self.diagnostics.iter().for_each(|it| it.add_to(db, owner, sink))
@@ -760,7 +760,7 @@ impl std::ops::BitOrAssign for Diverges {
 }
 
 mod diagnostics {
-    use hir_def::{expr::ExprId, FunctionId};
+    use hir_def::{expr::ExprId, DefWithBodyId};
     use hir_expand::diagnostics::DiagnosticSink;
 
     use crate::{
@@ -778,17 +778,17 @@ mod diagnostics {
         pub(super) fn add_to(
             &self,
             db: &dyn HirDatabase,
-            owner: FunctionId,
+            owner: DefWithBodyId,
             sink: &mut DiagnosticSink,
         ) {
             match self {
                 InferenceDiagnostic::NoSuchField { expr, field } => {
-                    let (_, source_map) = db.body_with_source_map(owner.into());
+                    let (_, source_map) = db.body_with_source_map(owner);
                     let field = source_map.field_syntax(*expr, *field);
                     sink.push(NoSuchField { file: field.file_id, field: field.value })
                 }
                 InferenceDiagnostic::BreakOutsideOfLoop { expr } => {
-                    let (_, source_map) = db.body_with_source_map(owner.into());
+                    let (_, source_map) = db.body_with_source_map(owner);
                     let ptr = source_map
                         .expr_syntax(*expr)
                         .expect("break outside of loop in synthetic syntax");
diff --git a/crates/ra_hir_ty/src/lib.rs b/crates/ra_hir_ty/src/lib.rs
index 2652d200f..d54568e67 100644
--- a/crates/ra_hir_ty/src/lib.rs
+++ b/crates/ra_hir_ty/src/lib.rs
@@ -12,15 +12,12 @@ pub mod traits;
 pub mod method_resolution;
 mod op;
 mod lower;
-mod match_checking;
 pub(crate) mod infer;
 pub(crate) mod utils;
 
 pub mod display;
 pub mod db;
 pub mod diagnostics;
-pub mod expr;
-pub mod unsafe_validation;
 
 #[cfg(test)]
 mod tests;
diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs
index dc447955f..ddafd0ea1 100644
--- a/crates/ra_hir_ty/src/test_db.rs
+++ b/crates/ra_hir_ty/src/test_db.rs
@@ -6,17 +6,17 @@ use std::{
 };
 
 use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId, ModuleId};
-use hir_expand::{db::AstDatabase, diagnostics::DiagnosticSink};
+use hir_expand::{
+    db::AstDatabase,
+    diagnostics::{Diagnostic, DiagnosticSink},
+};
 use ra_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, SourceDatabase, Upcast};
 use ra_syntax::TextRange;
 use rustc_hash::{FxHashMap, FxHashSet};
 use stdx::format_to;
 use test_utils::extract_annotations;
 
-use crate::{
-    db::HirDatabase, diagnostics::Diagnostic, expr::ExprValidator,
-    unsafe_validation::UnsafeValidator,
-};
+use crate::diagnostics::validate_body;
 
 #[salsa::database(
     ra_db::SourceDatabaseExtStorage,
@@ -118,13 +118,8 @@ impl TestDB {
             }
 
             for f in fns {
-                let infer = self.infer(f.into());
                 let mut sink = DiagnosticSink::new(&mut cb);
-                infer.add_diagnostics(self, f, &mut sink);
-                let mut validator = ExprValidator::new(f, infer.clone(), &mut sink);
-                validator.validate_body(self);
-                let mut validator = UnsafeValidator::new(f, infer, &mut sink);
-                validator.validate_body(self);
+                validate_body(self, f.into(), &mut sink);
             }
         }
     }