internal: start new diagnostics API

At the moment, this moves only a single diagnostic, but the idea is reafactor the rest to use the same pattern. We are going to have a single file per diagnostic. This file will define diagnostics code, rendering range and fixes, if any. It'll also have all of the tests. This is similar to how we deal with assists. After we refactor all diagnostics to follow this pattern, we'll probably move them to a new `ide_diagnostics` crate. Not that we intentionally want to test all diagnostics on this layer, despite the fact that they are generally emitted in the guts on the compiler. Diagnostics care to much about the end presentation details/fixes to be worth-while "unit" testing. So, we'll unit-test only the primary output of compilation process (types and name res tables), and will use integrated UI tests for diagnostics.
2024-12-25 04:23:25 +00:00 · 2021-06-13 14:41:19 +03:00 · 2021-06-13 14:41:19 +03:00 · efa069d288
commit efa069d288
parent 546be18e3a
7 changed files with 124 additions and 84 deletions
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@ -15,29 +15,28 @@ 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.
-#[derive(Debug)]
-pub struct UnresolvedModule {
-    pub file: HirFileId,
-    pub decl: AstPtr<ast::Module>,
-    pub candidate: String,
+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))
+                }
+            }
+        )*
+    };
 }

-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
-    }
+diagnostics![UnresolvedModule];
+
+#[derive(Debug)]
+pub struct UnresolvedModule {
+    pub decl: InFile<AstPtr<ast::Module>>,
+    pub candidate: String,
 }

 // Diagnostic: unresolved-extern-crate
--- a/crates/hir/src/lib.rs
+++ b/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> {
--- a/crates/hir_def/src/nameres/tests/diagnostics.rs
+++ b/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.
--- a/crates/ide/src/diagnostics.rs
+++ b/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>(
--- a/crates/ide/src/diagnostics/fixes.rs
+++ b/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};
--- a/crates/ide/src/diagnostics/fixes/unresolved_module.rs
+++ b/crates/ide/src/diagnostics/fixes/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() {
--- a/xtask/src/tidy.rs
+++ b/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());