1 files changed, 91 insertions, 408 deletions

diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index a2a4d61db..c480ed352 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -9,19 +9,13 @@ use hir_def::{
 };
 use hir_expand::name;
 use rustc_hash::FxHashSet;
-use syntax::{ast, AstPtr};
 
 use crate::{
     db::HirDatabase,
-    diagnostics::{
-        match_check::{
-            self,
-            usefulness::{compute_match_usefulness, expand_pattern, MatchCheckCtx, PatternArena},
-        },
-        MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
-        MissingPatFields, RemoveThisSemicolon,
+    diagnostics::match_check::{
+        self,
+        usefulness::{compute_match_usefulness, expand_pattern, MatchCheckCtx, PatternArena},
     },
-    diagnostics_sink::DiagnosticSink,
     AdtId, InferenceResult, Interner, TyExt, TyKind,
 };
 
@@ -31,38 +25,81 @@ pub(crate) use hir_def::{
     LocalFieldId, VariantId,
 };
 
-use super::ReplaceFilterMapNextWithFindMap;
+pub enum BodyValidationDiagnostic {
+    RecordLiteralMissingFields {
+        record_expr: ExprId,
+        variant: VariantId,
+        missed_fields: Vec<LocalFieldId>,
+    },
+    RecordPatMissingFields {
+        record_pat: PatId,
+        variant: VariantId,
+        missed_fields: Vec<LocalFieldId>,
+    },
+    ReplaceFilterMapNextWithFindMap {
+        method_call_expr: ExprId,
+    },
+    MismatchedArgCount {
+        call_expr: ExprId,
+        expected: usize,
+        found: usize,
+    },
+    RemoveThisSemicolon {
+        expr: ExprId,
+    },
+    MissingOkOrSomeInTailExpr {
+        expr: ExprId,
+        required: String,
+    },
+    MissingMatchArms {
+        match_expr: ExprId,
+    },
+    InternalBailedOut {
+        pat: PatId,
+    },
+}
 
-pub(super) struct ExprValidator<'a, 'b: 'a> {
+impl BodyValidationDiagnostic {
+    pub fn collect(
+        db: &dyn HirDatabase,
+        owner: DefWithBodyId,
+        internal_diagnostics: bool,
+    ) -> Vec<BodyValidationDiagnostic> {
+        let _p = profile::span("BodyValidationDiagnostic::collect");
+        let infer = db.infer(owner);
+        let mut validator = ExprValidator::new(owner, infer.clone());
+        validator.internal_diagnostics = internal_diagnostics;
+        validator.validate_body(db);
+        validator.diagnostics
+    }
+}
+
+struct ExprValidator {
     owner: DefWithBodyId,
     infer: Arc<InferenceResult>,
-    sink: &'a mut DiagnosticSink<'b>,
+    pub(super) diagnostics: Vec<BodyValidationDiagnostic>,
+    internal_diagnostics: bool,
 }
 
-impl<'a, 'b> ExprValidator<'a, 'b> {
-    pub(super) fn new(
-        owner: DefWithBodyId,
-        infer: Arc<InferenceResult>,
-        sink: &'a mut DiagnosticSink<'b>,
-    ) -> ExprValidator<'a, 'b> {
-        ExprValidator { owner, infer, sink }
+impl ExprValidator {
+    fn new(owner: DefWithBodyId, infer: Arc<InferenceResult>) -> ExprValidator {
+        ExprValidator { owner, infer, diagnostics: Vec::new(), internal_diagnostics: false }
     }
 
-    pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) {
+    fn validate_body(&mut self, db: &dyn HirDatabase) {
         self.check_for_filter_map_next(db);
 
         let body = db.body(self.owner);
 
         for (id, expr) in body.exprs.iter() {
-            if let Some((variant_def, missed_fields, true)) =
+            if let Some((variant, missed_fields, true)) =
                 record_literal_missing_fields(db, &self.infer, id, expr)
             {
-                self.create_record_literal_missing_fields_diagnostic(
-                    id,
-                    db,
-                    variant_def,
+                self.diagnostics.push(BodyValidationDiagnostic::RecordLiteralMissingFields {
+                    record_expr: id,
+                    variant,
                     missed_fields,
-                );
+                });
             }
 
             match expr {
@@ -76,15 +113,14 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
             }
         }
         for (id, pat) in body.pats.iter() {
-            if let Some((variant_def, missed_fields, true)) =
+            if let Some((variant, missed_fields, true)) =
                 record_pattern_missing_fields(db, &self.infer, id, pat)
             {
-                self.create_record_pattern_missing_fields_diagnostic(
-                    id,
-                    db,
-                    variant_def,
+                self.diagnostics.push(BodyValidationDiagnostic::RecordPatMissingFields {
+                    record_pat: id,
+                    variant,
                     missed_fields,
-                );
+                });
             }
         }
         let body_expr = &body[body.body_expr];
@@ -92,71 +128,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
             if let Some(t) = tail {
                 self.validate_results_in_tail_expr(body.body_expr, *t, db);
             } else if let Some(Statement::Expr { expr: id, .. }) = statements.last() {
-                self.validate_missing_tail_expr(body.body_expr, *id, db);
-            }
-        }
-    }
-
-    fn create_record_literal_missing_fields_diagnostic(
-        &mut self,
-        id: ExprId,
-        db: &dyn HirDatabase,
-        variant_def: VariantId,
-        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.owner);
-
-        if let Ok(source_ptr) = source_map.expr_syntax(id) {
-            let root = source_ptr.file_syntax(db.upcast());
-            if let ast::Expr::RecordExpr(record_expr) = &source_ptr.value.to_node(&root) {
-                if let Some(_) = record_expr.record_expr_field_list() {
-                    let variant_data = variant_def.variant_data(db.upcast());
-                    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_parent: AstPtr::new(&record_expr),
-                        field_list_parent_path: record_expr.path().map(|path| AstPtr::new(&path)),
-                        missed_fields,
-                    })
-                }
-            }
-        }
-    }
-
-    fn create_record_pattern_missing_fields_diagnostic(
-        &mut self,
-        id: PatId,
-        db: &dyn HirDatabase,
-        variant_def: VariantId,
-        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.owner);
-
-        if let Ok(source_ptr) = source_map.pat_syntax(id) {
-            if let Some(expr) = source_ptr.value.as_ref().left() {
-                let root = source_ptr.file_syntax(db.upcast());
-                if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) {
-                    if let Some(_) = record_pat.record_pat_field_list() {
-                        let variant_data = variant_def.variant_data(db.upcast());
-                        let missed_fields = missed_fields
-                            .into_iter()
-                            .map(|idx| variant_data.fields()[idx].name.clone())
-                            .collect();
-                        self.sink.push(MissingPatFields {
-                            file: source_ptr.file_id,
-                            field_list_parent: AstPtr::new(&record_pat),
-                            field_list_parent_path: record_pat
-                                .path()
-                                .map(|path| AstPtr::new(&path)),
-                            missed_fields,
-                        })
-                    }
-                }
+                self.validate_missing_tail_expr(body.body_expr, *id);
             }
         }
     }
@@ -199,13 +171,11 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
                 if function_id == *next_function_id {
                     if let Some(filter_map_id) = prev {
                         if *receiver == filter_map_id {
-                            let (_, source_map) = db.body_with_source_map(self.owner);
-                            if let Ok(next_source_ptr) = source_map.expr_syntax(id) {
-                                self.sink.push(ReplaceFilterMapNextWithFindMap {
-                                    file: next_source_ptr.file_id,
-                                    next_expr: next_source_ptr.value,
-                                });
-                            }
+                            self.diagnostics.push(
+                                BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap {
+                                    method_call_expr: id,
+                                },
+                            );
                         }
                     }
                 }
@@ -266,19 +236,15 @@ 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.owner);
-            if let Ok(source_ptr) = source_map.expr_syntax(call_id) {
-                if is_method_call {
-                    param_count -= 1;
-                    arg_count -= 1;
-                }
-                self.sink.push(MismatchedArgCount {
-                    file: source_ptr.file_id,
-                    call_expr: source_ptr.value,
-                    expected: param_count,
-                    found: arg_count,
-                });
+            if is_method_call {
+                param_count -= 1;
+                arg_count -= 1;
             }
+            self.diagnostics.push(BodyValidationDiagnostic::MismatchedArgCount {
+                call_expr: call_id,
+                expected: param_count,
+                found: arg_count,
+            });
         }
     }
 
@@ -346,8 +312,9 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
             // fit the match expression, we skip this diagnostic. Skipping the entire
             // diagnostic rather than just not including this match arm is preferred
             // to avoid the chance of false positives.
-            #[cfg(test)]
-            match_check::tests::report_bail_out(db, self.owner, arm.pat, self.sink);
+            if self.internal_diagnostics {
+                self.diagnostics.push(BodyValidationDiagnostic::InternalBailedOut { pat: arm.pat })
+            }
             return;
         }
 
@@ -382,20 +349,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         // FIXME Report witnesses
         // eprintln!("compute_match_usefulness(..) -> {:?}", &witnesses);
         if !witnesses.is_empty() {
-            if let Ok(source_ptr) = source_map.expr_syntax(id) {
-                let root = source_ptr.file_syntax(db.upcast());
-                if let ast::Expr::MatchExpr(match_expr) = &source_ptr.value.to_node(&root) {
-                    if let (Some(match_expr), Some(arms)) =
-                        (match_expr.expr(), match_expr.match_arm_list())
-                    {
-                        self.sink.push(MissingMatchArms {
-                            file: source_ptr.file_id,
-                            match_expr: AstPtr::new(&match_expr),
-                            arms: AstPtr::new(&arms),
-                        })
-                    }
-                }
-            }
+            self.diagnostics.push(BodyValidationDiagnostic::MissingMatchArms { match_expr: id });
         }
     }
 
@@ -453,24 +407,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         if params.len(&Interner) > 0
             && params.at(&Interner, 0).ty(&Interner) == Some(&mismatch.actual)
         {
-            let (_, source_map) = db.body_with_source_map(self.owner);
-
-            if let Ok(source_ptr) = source_map.expr_syntax(id) {
-                self.sink.push(MissingOkOrSomeInTailExpr {
-                    file: source_ptr.file_id,
-                    expr: source_ptr.value,
-                    required,
-                });
-            }
+            self.diagnostics
+                .push(BodyValidationDiagnostic::MissingOkOrSomeInTailExpr { expr: id, required });
         }
     }
 
-    fn validate_missing_tail_expr(
-        &mut self,
-        body_id: ExprId,
-        possible_tail_id: ExprId,
-        db: &dyn HirDatabase,
-    ) {
+    fn validate_missing_tail_expr(&mut self, body_id: ExprId, possible_tail_id: ExprId) {
         let mismatch = match self.infer.type_mismatch_for_expr(body_id) {
             Some(m) => m,
             None => return,
@@ -485,12 +427,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
             return;
         }
 
-        let (_, source_map) = db.body_with_source_map(self.owner);
-
-        if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) {
-            self.sink
-                .push(RemoveThisSemicolon { file: source_ptr.file_id, expr: source_ptr.value });
-        }
+        self.diagnostics
+            .push(BodyValidationDiagnostic::RemoveThisSemicolon { expr: possible_tail_id });
     }
 }
 
@@ -568,258 +506,3 @@ fn types_of_subpatterns_do_match(pat: PatId, body: &Body, infer: &InferenceResul
     walk(pat, body, infer, &mut has_type_mismatches);
     !has_type_mismatches
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::diagnostics::tests::check_diagnostics;
-
-    #[test]
-    fn simple_free_fn_zero() {
-        check_diagnostics(
-            r#"
-fn zero() {}
-fn f() { zero(1); }
-       //^^^^^^^ Expected 0 arguments, found 1
-"#,
-        );
-
-        check_diagnostics(
-            r#"
-fn zero() {}
-fn f() { zero(); }
-"#,
-        );
-    }
-
-    #[test]
-    fn simple_free_fn_one() {
-        check_diagnostics(
-            r#"
-fn one(arg: u8) {}
-fn f() { one(); }
-       //^^^^^ Expected 1 argument, found 0
-"#,
-        );
-
-        check_diagnostics(
-            r#"
-fn one(arg: u8) {}
-fn f() { one(1); }
-"#,
-        );
-    }
-
-    #[test]
-    fn method_as_fn() {
-        check_diagnostics(
-            r#"
-struct S;
-impl S { fn method(&self) {} }
-
-fn f() {
-    S::method();
-} //^^^^^^^^^^^ Expected 1 argument, found 0
-"#,
-        );
-
-        check_diagnostics(
-            r#"
-struct S;
-impl S { fn method(&self) {} }
-
-fn f() {
-    S::method(&S);
-    S.method();
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn method_with_arg() {
-        check_diagnostics(
-            r#"
-struct S;
-impl S { fn method(&self, arg: u8) {} }
-
-            fn f() {
-                S.method();
-            } //^^^^^^^^^^ Expected 1 argument, found 0
-            "#,
-        );
-
-        check_diagnostics(
-            r#"
-struct S;
-impl S { fn method(&self, arg: u8) {} }
-
-fn f() {
-    S::method(&S, 0);
-    S.method(1);
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn method_unknown_receiver() {
-        // note: this is incorrect code, so there might be errors on this in the
-        // future, but we shouldn't emit an argument count diagnostic here
-        check_diagnostics(
-            r#"
-trait Foo { fn method(&self, arg: usize) {} }
-
-fn f() {
-    let x;
-    x.method();
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn tuple_struct() {
-        check_diagnostics(
-            r#"
-struct Tup(u8, u16);
-fn f() {
-    Tup(0);
-} //^^^^^^ Expected 2 arguments, found 1
-"#,
-        )
-    }
-
-    #[test]
-    fn enum_variant() {
-        check_diagnostics(
-            r#"
-enum En { Variant(u8, u16), }
-fn f() {
-    En::Variant(0);
-} //^^^^^^^^^^^^^^ Expected 2 arguments, found 1
-"#,
-        )
-    }
-
-    #[test]
-    fn enum_variant_type_macro() {
-        check_diagnostics(
-            r#"
-macro_rules! Type {
-    () => { u32 };
-}
-enum Foo {
-    Bar(Type![])
-}
-impl Foo {
-    fn new() {
-        Foo::Bar(0);
-        Foo::Bar(0, 1);
-      //^^^^^^^^^^^^^^ Expected 1 argument, found 2
-        Foo::Bar();
-      //^^^^^^^^^^ Expected 1 argument, found 0
-    }
-}
-        "#,
-        );
-    }
-
-    #[test]
-    fn varargs() {
-        check_diagnostics(
-            r#"
-extern "C" {
-    fn fixed(fixed: u8);
-    fn varargs(fixed: u8, ...);
-    fn varargs2(...);
-}
-
-fn f() {
-    unsafe {
-        fixed(0);
-        fixed(0, 1);
-      //^^^^^^^^^^^ Expected 1 argument, found 2
-        varargs(0);
-        varargs(0, 1);
-        varargs2();
-        varargs2(0);
-        varargs2(0, 1);
-    }
-}
-        "#,
-        )
-    }
-
-    #[test]
-    fn arg_count_lambda() {
-        check_diagnostics(
-            r#"
-fn main() {
-    let f = |()| ();
-    f();
-  //^^^ Expected 1 argument, found 0
-    f(());
-    f((), ());
-  //^^^^^^^^^ Expected 1 argument, found 2
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn cfgd_out_call_arguments() {
-        check_diagnostics(
-            r#"
-struct C(#[cfg(FALSE)] ());
-impl C {
-    fn new() -> Self {
-        Self(
-            #[cfg(FALSE)]
-            (),
-        )
-    }
-
-    fn method(&self) {}
-}
-
-fn main() {
-    C::new().method(#[cfg(FALSE)] 0);
-}
-            "#,
-        );
-    }
-
-    #[test]
-    fn cfgd_out_fn_params() {
-        check_diagnostics(
-            r#"
-fn foo(#[cfg(NEVER)] x: ()) {}
-
-struct S;
-
-impl S {
-    fn method(#[cfg(NEVER)] self) {}
-    fn method2(#[cfg(NEVER)] self, arg: u8) {}
-    fn method3(self, #[cfg(NEVER)] arg: u8) {}
-}
-
-extern "C" {
-    fn fixed(fixed: u8, #[cfg(NEVER)] ...);
-    fn varargs(#[cfg(not(NEVER))] ...);
-}
-
-fn main() {
-    foo();
-    S::method();
-    S::method2(0);
-    S::method3(S);
-    S.method3();
-    unsafe {
-        fixed(0);
-        varargs(1, 2, 3);
-    }
-}
-            "#,
-        )
-    }
-}