Merge #9245

9245: internal: start new diagnostics API r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

Author: bors[bot]

crates/hir/src/diagnostics.rs

@@ -15,31 +15,30 @@ pub use crate::diagnostics_sink::{
     Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder,
 };
 
-// Diagnostic: unresolved-module
-//
-// This diagnostic is triggered if rust-analyzer is unable to discover referred module.
+macro_rules! diagnostics {
+    ($($diag:ident)*) => {
+        pub enum AnyDiagnostic {$(
+            $diag(Box<$diag>),
+        )*}
+
+        $(
+            impl From<$diag> for AnyDiagnostic {
+                fn from(d: $diag) -> AnyDiagnostic {
+                    AnyDiagnostic::$diag(Box::new(d))
+                }
+            }
+        )*
+    };
+}
+
+diagnostics![UnresolvedModule];
+
 #[derive(Debug)]
 pub struct UnresolvedModule {
-    pub file: HirFileId,
-    pub decl: AstPtr<ast::Module>,
+    pub decl: InFile<AstPtr<ast::Module>>,
     pub candidate: String,
 }
 
-impl Diagnostic for UnresolvedModule {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("unresolved-module")
-    }
-    fn message(&self) -> String {
-        "unresolved module".to_string()
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile::new(self.file, self.decl.clone().into())
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-}
-
 // Diagnostic: unresolved-extern-crate
 //
 // This diagnostic is triggered if rust-analyzer is unable to discover referred extern crate.

crates/hir/src/lib.rs

@@ -80,18 +80,18 @@ use tt::{Ident, Leaf, Literal, TokenTree};
 
 use crate::{
     db::{DefDatabase, HirDatabase},
-    diagnostics::{
-        BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, MismatchedArgCount,
-        MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, MissingPatFields,
-        MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap,
-        UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
-        UnresolvedModule, UnresolvedProcMacro,
-    },
     diagnostics_sink::DiagnosticSink,
 };
 
 pub use crate::{
     attrs::{HasAttrs, Namespace},
+    diagnostics::{
+        AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError,
+        MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
+        MissingPatFields, MissingUnsafe, NoSuchField, RemoveThisSemicolon,
+        ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate,
+        UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro,
+    },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope},
 };
@@ -460,10 +460,11 @@ impl Module {
         db: &dyn HirDatabase,
         sink: &mut DiagnosticSink,
         internal_diagnostics: bool,
-    ) {
+    ) -> 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 {
@@ -473,11 +474,13 @@ impl Module {
             match &diag.kind {
                 DefDiagnosticKind::UnresolvedModule { ast: declaration, candidate } => {
                     let decl = declaration.to_node(db.upcast());
-                    sink.push(UnresolvedModule {
-                        file: declaration.file_id,
-                        decl: AstPtr::new(&decl),
-                        candidate: candidate.clone(),
-                    })
+                    acc.push(
+                        UnresolvedModule {
+                            decl: InFile::new(declaration.file_id, AstPtr::new(&decl)),
+                            candidate: candidate.clone(),
+                        }
+                        .into(),
+                    )
                 }
                 DefDiagnosticKind::UnresolvedExternCrate { ast } => {
                     let item = ast.to_node(db.upcast());
@@ -610,7 +613,7 @@ impl Module {
                 crate::ModuleDef::Module(m) => {
                     // Only add diagnostics from inline modules
                     if def_map[m.id.local_id].origin.is_inline() {
-                        m.diagnostics(db, sink, internal_diagnostics)
+                        acc.extend(m.diagnostics(db, sink, internal_diagnostics))
                     }
                 }
                 _ => {
@@ -626,6 +629,7 @@ impl Module {
                 }
             }
         }
+        acc
     }
 
     pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> {

crates/hir_def/src/nameres/tests/diagnostics.rs

@@ -78,20 +78,6 @@ fn dedup_unresolved_import_from_unresolved_crate() {
     );
 }
 
-#[test]
-fn unresolved_module() {
-    check_diagnostics(
-        r"
-        //- /lib.rs
-        mod foo;
-          mod bar;
-        //^^^^^^^^ UnresolvedModule
-        mod baz {}
-        //- /foo.rs
-        ",
-    );
-}
-
 #[test]
 fn inactive_item() {
     // Additional tests in `cfg` crate. This only tests disabled cfgs.

crates/ide/src/diagnostics.rs

@@ -4,6 +4,8 @@
 //! macro-expanded files, but we need to present them to the users in terms of
 //! original files. So we need to map the ranges.
 
+mod unresolved_module;
+
 mod fixes;
 mod field_shorthand;
 mod unlinked_file;
@@ -12,7 +14,7 @@ use std::cell::RefCell;
 
 use hir::{
     db::AstDatabase,
-    diagnostics::{Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder},
+    diagnostics::{AnyDiagnostic, Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder},
     InFile, Semantics,
 };
 use ide_assists::AssistResolveStrategy;
@@ -42,6 +44,12 @@ pub struct Diagnostic {
 }
 
 impl Diagnostic {
+    fn new(code: &'static str, message: impl Into<String>, range: TextRange) -> Diagnostic {
+        let message = message.into();
+        let code = Some(DiagnosticCode(code));
+        Self { message, range, severity: Severity::Error, fixes: None, unused: false, code }
+    }
+
     fn error(range: TextRange, message: String) -> Self {
         Self { message, range, severity: Severity::Error, fixes: None, unused: false, code: None }
     }
@@ -82,6 +90,13 @@ pub struct DiagnosticsConfig {
     pub disabled: FxHashSet<String>,
 }
 
+struct DiagnosticsContext<'a> {
+    config: &'a DiagnosticsConfig,
+    sema: Semantics<'a, RootDatabase>,
+    #[allow(unused)]
+    resolve: &'a AssistResolveStrategy,
+}
+
 pub(crate) fn diagnostics(
     db: &RootDatabase,
     config: &DiagnosticsConfig,
@@ -108,9 +123,6 @@ pub(crate) fn diagnostics(
     }
     let res = RefCell::new(res);
     let sink_builder = DiagnosticSinkBuilder::new()
-        .on::<hir::diagnostics::UnresolvedModule, _>(|d| {
-            res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
-        })
         .on::<hir::diagnostics::MissingFields, _>(|d| {
             res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
         })
@@ -204,16 +216,33 @@ pub(crate) fn diagnostics(
             );
         });
 
+    let mut diags = Vec::new();
     let internal_diagnostics = cfg!(test);
     match sema.to_module_def(file_id) {
-        Some(m) => m.diagnostics(db, &mut sink, internal_diagnostics),
+        Some(m) => diags = m.diagnostics(db, &mut sink, internal_diagnostics),
         None => {
             sink.push(UnlinkedFile { file_id, node: SyntaxNodePtr::new(parse.tree().syntax()) });
         }
     }
 
     drop(sink);
-    res.into_inner()
+
+    let mut res = res.into_inner();
+
+    let ctx = DiagnosticsContext { config, sema, resolve };
+    for diag in diags {
+        let d = match diag {
+            AnyDiagnostic::UnresolvedModule(d) => unresolved_module::render(&ctx, &d),
+        };
+        if let Some(code) = d.code {
+            if ctx.config.disabled.contains(code.as_str()) {
+                continue;
+            }
+        }
+        res.push(d)
+    }
+
+    res
 }
 
 fn diagnostic_with_fix<D: DiagnosticWithFixes>(

crates/ide/src/diagnostics/fixes.rs

@@ -5,7 +5,6 @@ mod create_field;
 mod fill_missing_fields;
 mod remove_semicolon;
 mod replace_with_find_map;
-mod unresolved_module;
 mod wrap_tail_expr;
 
 use hir::{diagnostics::Diagnostic, Semantics};

crates/ide/src/diagnostics/fixes/unresolved_module.rs renamed to crates/ide/src/diagnostics/unresolved_module.rs

@@ -1,39 +1,59 @@
-use hir::{db::AstDatabase, diagnostics::UnresolvedModule, Semantics};
-use ide_assists::{Assist, AssistResolveStrategy};
-use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit, RootDatabase};
+use hir::db::AstDatabase;
+use ide_assists::Assist;
+use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit};
 use syntax::AstNode;
 
-use crate::diagnostics::{fix, DiagnosticWithFixes};
+use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext};
 
-impl DiagnosticWithFixes for UnresolvedModule {
-    fn fixes(
-        &self,
-        sema: &Semantics<RootDatabase>,
-        _resolve: &AssistResolveStrategy,
-    ) -> Option<Vec<Assist>> {
-        let root = sema.db.parse_or_expand(self.file)?;
-        let unresolved_module = self.decl.to_node(&root);
-        Some(vec![fix(
-            "create_module",
-            "Create module",
-            FileSystemEdit::CreateFile {
-                dst: AnchoredPathBuf {
-                    anchor: self.file.original_file(sema.db),
-                    path: self.candidate.clone(),
-                },
-                initial_contents: "".to_string(),
-            }
-            .into(),
-            unresolved_module.syntax().text_range(),
-        )])
-    }
+// Diagnostic: unresolved-module
+//
+// This diagnostic is triggered if rust-analyzer is unable to discover referred module.
+pub(super) fn render(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedModule) -> Diagnostic {
+    Diagnostic::new(
+        "unresolved-module",
+        "unresolved module",
+        ctx.sema.diagnostics_display_range(d.decl.clone().map(|it| it.into())).range,
+    )
+    .with_fixes(fixes(ctx, d))
+}
+
+fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedModule) -> Option<Vec<Assist>> {
+    let root = ctx.sema.db.parse_or_expand(d.decl.file_id)?;
+    let unresolved_module = d.decl.value.to_node(&root);
+    Some(vec![fix(
+        "create_module",
+        "Create module",
+        FileSystemEdit::CreateFile {
+            dst: AnchoredPathBuf {
+                anchor: d.decl.file_id.original_file(ctx.sema.db),
+                path: d.candidate.clone(),
+            },
+            initial_contents: "".to_string(),
+        }
+        .into(),
+        unresolved_module.syntax().text_range(),
+    )])
 }
 
 #[cfg(test)]
 mod tests {
     use expect_test::expect;
 
-    use crate::diagnostics::tests::check_expect;
+    use crate::diagnostics::tests::{check_diagnostics, check_expect};
+
+    #[test]
+    fn unresolved_module() {
+        check_diagnostics(
+            r#"
+//- /lib.rs
+mod foo;
+  mod bar;
+//^^^^^^^^ unresolved module
+mod baz {}
+//- /foo.rs
+"#,
+        );
+    }
 
     #[test]
     fn test_unresolved_module_diagnostic() {

xtask/src/tidy.rs

@@ -372,7 +372,10 @@ impl TidyDocs {
                 self.contains_fixme.push(path.to_path_buf());
             }
         } else {
-            if text.contains("// Feature:") || text.contains("// Assist:") {
+            if text.contains("// Feature:")
+                || text.contains("// Assist:")
+                || text.contains("// Diagnostic:")
+            {
                 return;
             }
             self.missing_docs.push(path.display().to_string());