Merge #9256

9256: internal: kill diagnostic sink r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

Author: bors[bot]

crates/hir/src/diagnostics.rs

@@ -3,18 +3,12 @@
 //!
 //! This probably isn't the best way to do this -- ideally, diagnistics should
 //! be expressed in terms of hir types themselves.
-use std::any::Any;
-
 use cfg::{CfgExpr, CfgOptions};
 use either::Either;
 use hir_def::path::ModPath;
 use hir_expand::{name::Name, HirFileId, InFile};
 use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange};
 
-pub use crate::diagnostics_sink::{
-    Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder,
-};
-
 macro_rules! diagnostics {
     ($($diag:ident,)*) => {
         pub enum AnyDiagnostic {$(
@@ -38,6 +32,7 @@ diagnostics![
     MacroError,
     MismatchedArgCount,
     MissingFields,
+    MissingMatchArms,
     MissingOkOrSomeInTailExpr,
     MissingUnsafe,
     NoSuchField,
@@ -149,50 +144,11 @@ pub struct MissingOkOrSomeInTailExpr {
     pub required: String,
 }
 
-// Diagnostic: missing-match-arm
-//
-// This diagnostic is triggered if `match` block is missing one or more match arms.
 #[derive(Debug)]
 pub struct MissingMatchArms {
     pub file: HirFileId,
     pub match_expr: AstPtr<ast::Expr>,
     pub arms: AstPtr<ast::MatchArmList>,
 }
 
-impl Diagnostic for MissingMatchArms {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("missing-match-arm")
-    }
-    fn message(&self) -> String {
-        String::from("Missing match arm")
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.match_expr.clone().into() }
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-}
-
-#[derive(Debug)]
-pub struct InternalBailedOut {
-    pub file: HirFileId,
-    pub pat_syntax_ptr: SyntaxNodePtr,
-}
-
-impl Diagnostic for InternalBailedOut {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("internal:match-check-bailed-out")
-    }
-    fn message(&self) -> String {
-        format!("Internal: match check bailed out")
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.pat_syntax_ptr.clone() }
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-}
-
 pub use hir_ty::diagnostics::IncorrectCase;

crates/hir/src/diagnostics_sink.rs

@@ -27,7 +27,6 @@ mod attrs;
 mod has_source;
 
 pub mod diagnostics;
-pub mod diagnostics_sink;
 pub mod db;
 
 mod display;
@@ -78,16 +77,13 @@ use syntax::{
 };
 use tt::{Ident, Leaf, Literal, TokenTree};
 
-use crate::{
-    db::{DefDatabase, HirDatabase},
-    diagnostics_sink::DiagnosticSink,
-};
+use crate::db::{DefDatabase, HirDatabase};
 
 pub use crate::{
     attrs::{HasAttrs, Namespace},
     diagnostics::{
-        AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InternalBailedOut,
-        MacroError, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
+        AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, MacroError,
+        MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
         MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap,
         UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
         UnresolvedModule, UnresolvedProcMacro,
@@ -457,16 +453,10 @@ impl Module {
         self.id.def_map(db.upcast())[self.id.local_id].scope.visibility_of((*def).into())
     }
 
-    pub fn diagnostics(
-        self,
-        db: &dyn HirDatabase,
-        sink: &mut DiagnosticSink,
-        internal_diagnostics: bool,
-    ) -> Vec<AnyDiagnostic> {
+    pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
         let _p = profile::span("Module::diagnostics").detail(|| {
             format!("{:?}", self.name(db).map_or("<unknown>".into(), |name| name.to_string()))
         });
-        let mut acc: Vec<AnyDiagnostic> = Vec::new();
         let def_map = self.id.def_map(db.upcast());
         for diag in def_map.diagnostics() {
             if diag.in_module != self.id.local_id {
@@ -619,11 +609,11 @@ impl Module {
         }
         for decl in self.declarations(db) {
             match decl {
-                ModuleDef::Function(f) => acc.extend(f.diagnostics(db, sink, internal_diagnostics)),
+                ModuleDef::Function(f) => f.diagnostics(db, acc),
                 ModuleDef::Module(m) => {
                     // Only add diagnostics from inline modules
                     if def_map[m.id.local_id].origin.is_inline() {
-                        acc.extend(m.diagnostics(db, sink, internal_diagnostics))
+                        m.diagnostics(db, acc)
                     }
                 }
                 _ => acc.extend(decl.diagnostics(db)),
@@ -633,11 +623,10 @@ impl Module {
         for impl_def in self.impl_defs(db) {
             for item in impl_def.items(db) {
                 if let AssocItem::Function(f) = item {
-                    acc.extend(f.diagnostics(db, sink, internal_diagnostics));
+                    f.diagnostics(db, acc);
                 }
             }
         }
-        acc
     }
 
     pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> {
@@ -1036,13 +1025,7 @@ impl Function {
         db.function_data(self.id).is_async()
     }
 
-    pub fn diagnostics(
-        self,
-        db: &dyn HirDatabase,
-        sink: &mut DiagnosticSink,
-        internal_diagnostics: bool,
-    ) -> Vec<AnyDiagnostic> {
-        let mut acc: Vec<AnyDiagnostic> = Vec::new();
+    pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
         let krate = self.module(db).id.krate();
 
         let source_map = db.body_with_source_map(self.id.into()).1;
@@ -1100,9 +1083,7 @@ impl Function {
             }
         }
 
-        for diagnostic in
-            BodyValidationDiagnostic::collect(db, self.id.into(), internal_diagnostics)
-        {
+        for diagnostic in BodyValidationDiagnostic::collect(db, self.id.into()) {
             match diagnostic {
                 BodyValidationDiagnostic::RecordMissingFields {
                     record,
@@ -1209,36 +1190,26 @@ impl Function {
                                 if let (Some(match_expr), Some(arms)) =
                                     (match_expr.expr(), match_expr.match_arm_list())
                                 {
-                                    sink.push(MissingMatchArms {
-                                        file: source_ptr.file_id,
-                                        match_expr: AstPtr::new(&match_expr),
-                                        arms: AstPtr::new(&arms),
-                                    })
+                                    acc.push(
+                                        MissingMatchArms {
+                                            file: source_ptr.file_id,
+                                            match_expr: AstPtr::new(&match_expr),
+                                            arms: AstPtr::new(&arms),
+                                        }
+                                        .into(),
+                                    )
                                 }
                             }
                         }
                         Err(SyntheticSyntax) => (),
                     }
                 }
-                BodyValidationDiagnostic::InternalBailedOut { pat } => {
-                    match source_map.pat_syntax(pat) {
-                        Ok(source_ptr) => {
-                            let pat_syntax_ptr = source_ptr.value.either(Into::into, Into::into);
-                            sink.push(InternalBailedOut {
-                                file: source_ptr.file_id,
-                                pat_syntax_ptr,
-                            });
-                        }
-                        Err(SyntheticSyntax) => (),
-                    }
-                }
             }
         }
 
         for diag in hir_ty::diagnostics::validate_module_item(db, krate, self.id.into()) {
             acc.push(diag.into())
         }
-        acc
     }
 
     /// Whether this function declaration has a definition.

crates/hir/src/lib.rs

@@ -50,21 +50,13 @@ pub enum BodyValidationDiagnostic {
     MissingMatchArms {
         match_expr: ExprId,
     },
-    InternalBailedOut {
-        pat: PatId,
-    },
 }
 
 impl BodyValidationDiagnostic {
-    pub fn collect(
-        db: &dyn HirDatabase,
-        owner: DefWithBodyId,
-        internal_diagnostics: bool,
-    ) -> Vec<BodyValidationDiagnostic> {
+    pub fn collect(db: &dyn HirDatabase, owner: DefWithBodyId) -> 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
     }
@@ -74,12 +66,11 @@ struct ExprValidator {
     owner: DefWithBodyId,
     infer: Arc<InferenceResult>,
     pub(super) diagnostics: Vec<BodyValidationDiagnostic>,
-    internal_diagnostics: bool,
 }
 
 impl ExprValidator {
     fn new(owner: DefWithBodyId, infer: Arc<InferenceResult>) -> ExprValidator {
-        ExprValidator { owner, infer, diagnostics: Vec::new(), internal_diagnostics: false }
+        ExprValidator { owner, infer, diagnostics: Vec::new() }
     }
 
     fn validate_body(&mut self, db: &dyn HirDatabase) {
@@ -308,9 +299,7 @@ impl ExprValidator {
             // 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.
-            if self.internal_diagnostics {
-                self.diagnostics.push(BodyValidationDiagnostic::InternalBailedOut { pat: arm.pat })
-            }
+            cov_mark::hit!(validate_match_bailed_out);
             return;
         }