internal: unified missing fields diagnostic

5 files changed, 94 insertions, 142 deletions

diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index a5e982b75..158626dc0 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -6,6 +6,7 @@
 use std::any::Any;
 
 use cfg::{CfgExpr, CfgOptions, DnfExpr};
+use either::Either;
 use hir_def::path::ModPath;
 use hir_expand::{name::Name, HirFileId, InFile};
 use stdx::format_to;
@@ -324,60 +325,11 @@ impl Diagnostic for MissingUnsafe {
 #[derive(Debug)]
 pub struct MissingFields {
     pub file: HirFileId,
-    pub field_list_parent: AstPtr<ast::RecordExpr>,
+    pub field_list_parent: Either<AstPtr<ast::RecordExpr>, AstPtr<ast::RecordPat>>,
     pub field_list_parent_path: Option<AstPtr<ast::Path>>,
     pub missed_fields: Vec<Name>,
 }
 
-// Diagnostic: missing-pat-fields
-//
-// This diagnostic is triggered if pattern lacks some fields that exist in the corresponding structure.
-//
-// Example:
-//
-// ```rust
-// struct A { a: u8, b: u8 }
-//
-// let a = A { a: 10, b: 20 };
-//
-// if let A { a } = a {
-//     // ...
-// }
-// ```
-#[derive(Debug)]
-pub struct MissingPatFields {
-    pub file: HirFileId,
-    pub field_list_parent: AstPtr<ast::RecordPat>,
-    pub field_list_parent_path: Option<AstPtr<ast::Path>>,
-    pub missed_fields: Vec<Name>,
-}
-
-impl Diagnostic for MissingPatFields {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("missing-pat-fields")
-    }
-    fn message(&self) -> String {
-        let mut buf = String::from("Missing structure fields:\n");
-        for field in &self.missed_fields {
-            format_to!(buf, "- {}\n", field);
-        }
-        buf
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile {
-            file_id: self.file,
-            value: self
-                .field_list_parent_path
-                .clone()
-                .map(SyntaxNodePtr::from)
-                .unwrap_or_else(|| self.field_list_parent.clone().into()),
-        }
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-}
-
 // Diagnostic: replace-filter-map-next-with-find-map
 //
 // This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`.
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 583d92f20..373134f62 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -88,9 +88,9 @@ pub use crate::{
     diagnostics::{
         AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError,
         MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
-        MissingPatFields, MissingUnsafe, NoSuchField, RemoveThisSemicolon,
-        ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate,
-        UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro,
+        MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap,
+        UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
+        UnresolvedModule, UnresolvedProcMacro,
     },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope},
@@ -1098,67 +1098,70 @@ impl Function {
             BodyValidationDiagnostic::collect(db, self.id.into(), internal_diagnostics)
         {
             match diagnostic {
-                BodyValidationDiagnostic::RecordLiteralMissingFields {
-                    record_expr,
+                BodyValidationDiagnostic::RecordMissingFields {
+                    record,
                     variant,
                     missed_fields,
-                } => match source_map.expr_syntax(record_expr) {
-                    Ok(source_ptr) => {
-                        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.variant_data(db.upcast());
-                                let missed_fields = missed_fields
-                                    .into_iter()
-                                    .map(|idx| variant_data.fields()[idx].name.clone())
-                                    .collect();
-                                acc.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,
+                } => {
+                    let variant_data = variant.variant_data(db.upcast());
+                    let missed_fields = missed_fields
+                        .into_iter()
+                        .map(|idx| variant_data.fields()[idx].name.clone())
+                        .collect();
+
+                    match record {
+                        Either::Left(record_expr) => match source_map.expr_syntax(record_expr) {
+                            Ok(source_ptr) => {
+                                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() {
+                                        acc.push(
+                                            MissingFields {
+                                                file: source_ptr.file_id,
+                                                field_list_parent: Either::Left(AstPtr::new(
+                                                    record_expr,
+                                                )),
+                                                field_list_parent_path: record_expr
+                                                    .path()
+                                                    .map(|path| AstPtr::new(&path)),
+                                                missed_fields,
+                                            }
+                                            .into(),
+                                        )
                                     }
-                                    .into(),
-                                )
+                                }
                             }
-                        }
-                    }
-                    Err(SyntheticSyntax) => (),
-                },
-                BodyValidationDiagnostic::RecordPatMissingFields {
-                    record_pat,
-                    variant,
-                    missed_fields,
-                } => match source_map.pat_syntax(record_pat) {
-                    Ok(source_ptr) => {
-                        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.variant_data(db.upcast());
-                                    let missed_fields = missed_fields
-                                        .into_iter()
-                                        .map(|idx| variant_data.fields()[idx].name.clone())
-                                        .collect();
-                                    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,
-                                    })
+                            Err(SyntheticSyntax) => (),
+                        },
+                        Either::Right(record_pat) => match source_map.pat_syntax(record_pat) {
+                            Ok(source_ptr) => {
+                                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() {
+                                            acc.push(
+                                                MissingFields {
+                                                    file: source_ptr.file_id,
+                                                    field_list_parent: Either::Right(AstPtr::new(
+                                                        &record_pat,
+                                                    )),
+                                                    field_list_parent_path: record_pat
+                                                        .path()
+                                                        .map(|path| AstPtr::new(&path)),
+                                                    missed_fields,
+                                                }
+                                                .into(),
+                                            )
+                                        }
+                                    }
                                 }
                             }
-                        }
+                            Err(SyntheticSyntax) => (),
+                        },
                     }
-                    Err(SyntheticSyntax) => (),
-                },
-
+                }
                 BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap { method_call_expr } => {
                     if let Ok(next_source_ptr) = source_map.expr_syntax(method_call_expr) {
                         sink.push(ReplaceFilterMapNextWithFindMap {
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index c480ed352..2a211fd8e 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -8,6 +8,7 @@ use hir_def::{
     expr::Statement, path::path, resolver::HasResolver, AssocItemId, DefWithBodyId, HasModule,
 };
 use hir_expand::name;
+use itertools::Either;
 use rustc_hash::FxHashSet;
 
 use crate::{
@@ -26,13 +27,8 @@ pub(crate) use hir_def::{
 };
 
 pub enum BodyValidationDiagnostic {
-    RecordLiteralMissingFields {
-        record_expr: ExprId,
-        variant: VariantId,
-        missed_fields: Vec<LocalFieldId>,
-    },
-    RecordPatMissingFields {
-        record_pat: PatId,
+    RecordMissingFields {
+        record: Either<ExprId, PatId>,
         variant: VariantId,
         missed_fields: Vec<LocalFieldId>,
     },
@@ -95,8 +91,8 @@ impl ExprValidator {
             if let Some((variant, missed_fields, true)) =
                 record_literal_missing_fields(db, &self.infer, id, expr)
             {
-                self.diagnostics.push(BodyValidationDiagnostic::RecordLiteralMissingFields {
-                    record_expr: id,
+                self.diagnostics.push(BodyValidationDiagnostic::RecordMissingFields {
+                    record: Either::Left(id),
                     variant,
                     missed_fields,
                 });
@@ -116,8 +112,8 @@ impl ExprValidator {
             if let Some((variant, missed_fields, true)) =
                 record_pattern_missing_fields(db, &self.infer, id, pat)
             {
-                self.diagnostics.push(BodyValidationDiagnostic::RecordPatMissingFields {
-                    record_pat: id,
+                self.diagnostics.push(BodyValidationDiagnostic::RecordMissingFields {
+                    record: Either::Right(id),
                     variant,
                     missed_fields,
                 });
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs
index ef8c8044c..3307e240b 100644
--- a/crates/ide/src/diagnostics.rs
+++ b/crates/ide/src/diagnostics.rs
@@ -1056,20 +1056,6 @@ fn main() {
     }
 
     #[test]
-    fn missing_record_pat_field_diagnostic() {
-        check_diagnostics(
-            r#"
-struct S { foo: i32, bar: () }
-fn baz(s: S) {
-    let S { foo: _ } = s;
-      //^ Missing structure fields:
-      //| - bar
-}
-"#,
-        );
-    }
-
-    #[test]
     fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() {
         check_diagnostics(
             r"
diff --git a/crates/ide/src/diagnostics/missing_fields.rs b/crates/ide/src/diagnostics/missing_fields.rs
index aee780972..66575f713 100644
--- a/crates/ide/src/diagnostics/missing_fields.rs
+++ b/crates/ide/src/diagnostics/missing_fields.rs
@@ -1,3 +1,4 @@
+use either::Either;
 use hir::{db::AstDatabase, InFile};
 use ide_assists::Assist;
 use ide_db::source_change::SourceChange;
@@ -7,7 +8,7 @@ use text_edit::TextEdit;
 
 use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext};
 
-// Diagnostic: missing-structure-fields
+// Diagnostic: missing-fields
 //
 // This diagnostic is triggered if record lacks some fields that exist in the corresponding structure.
 //
@@ -29,15 +30,11 @@ pub(super) fn missing_fields(ctx: &DiagnosticsContext<'_>, d: &hir::MissingField
         d.field_list_parent_path
             .clone()
             .map(SyntaxNodePtr::from)
-            .unwrap_or_else(|| d.field_list_parent.clone().into()),
+            .unwrap_or_else(|| d.field_list_parent.clone().either(|it| it.into(), |it| it.into())),
     );
 
-    Diagnostic::new(
-        "missing-structure-fields",
-        message,
-        ctx.sema.diagnostics_display_range(ptr).range,
-    )
-    .with_fixes(fixes(ctx, d))
+    Diagnostic::new("missing-fields", message, ctx.sema.diagnostics_display_range(ptr).range)
+        .with_fixes(fixes(ctx, d))
 }
 
 fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Assist>> {
@@ -51,7 +48,11 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
     }
 
     let root = ctx.sema.db.parse_or_expand(d.file)?;
-    let field_list_parent = d.field_list_parent.to_node(&root);
+    let field_list_parent = match &d.field_list_parent {
+        Either::Left(record_expr) => record_expr.to_node(&root),
+        // FIXE: patterns should be fixable as well.
+        Either::Right(_) => return None,
+    };
     let old_field_list = field_list_parent.record_expr_field_list()?;
     let new_field_list = old_field_list.clone_for_update();
     for f in d.missed_fields.iter() {
@@ -76,7 +77,21 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
 
 #[cfg(test)]
 mod tests {
-    use crate::diagnostics::tests::{check_fix, check_no_diagnostics};
+    use crate::diagnostics::tests::{check_diagnostics, check_fix, check_no_diagnostics};
+
+    #[test]
+    fn missing_record_pat_field_diagnostic() {
+        check_diagnostics(
+            r#"
+struct S { foo: i32, bar: () }
+fn baz(s: S) {
+    let S { foo: _ } = s;
+      //^ Missing structure fields:
+      //| - bar
+}
+"#,
+        );
+    }
 
     #[test]
     fn test_fill_struct_fields_empty() {