Merge #9248

9248: internal: refactor unresolved macro call diagnostic r=matklad a=matklad

bors r+
🤖

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

7 files changed, 220 insertions, 186 deletions

diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index ec0a8fe41..718c86b3a 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -17,7 +17,7 @@ pub use crate::diagnostics_sink::{
 };
 
 macro_rules! diagnostics {
-    ($($diag:ident),*) => {
+    ($($diag:ident,)*) => {
         pub enum AnyDiagnostic {$(
             $diag(Box<$diag>),
         )*}
@@ -32,7 +32,13 @@ macro_rules! diagnostics {
     };
 }
 
-diagnostics![UnresolvedModule, UnresolvedExternCrate, MissingFields];
+diagnostics![
+    UnresolvedModule,
+    UnresolvedExternCrate,
+    UnresolvedImport,
+    UnresolvedMacroCall,
+    MissingFields,
+];
 
 #[derive(Debug)]
 pub struct UnresolvedModule {
@@ -47,61 +53,15 @@ pub struct UnresolvedExternCrate {
 
 #[derive(Debug)]
 pub struct UnresolvedImport {
-    pub file: HirFileId,
-    pub node: AstPtr<ast::UseTree>,
-}
-
-impl Diagnostic for UnresolvedImport {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("unresolved-import")
-    }
-    fn message(&self) -> String {
-        "unresolved import".to_string()
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile::new(self.file, self.node.clone().into())
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-    fn is_experimental(&self) -> bool {
-        // This currently results in false positives in the following cases:
-        // - `cfg_if!`-generated code in libstd (we don't load the sysroot correctly)
-        // - `core::arch` (we don't handle `#[path = "../<path>"]` correctly)
-        // - proc macros and/or proc macro generated code
-        true
-    }
+    pub decl: InFile<AstPtr<ast::UseTree>>,
 }
 
-// Diagnostic: unresolved-macro-call
-//
-// This diagnostic is triggered if rust-analyzer is unable to resolve the path to a
-// macro in a macro invocation.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct UnresolvedMacroCall {
-    pub file: HirFileId,
-    pub node: AstPtr<ast::MacroCall>,
+    pub macro_call: InFile<AstPtr<ast::MacroCall>>,
     pub path: ModPath,
 }
 
-impl Diagnostic for UnresolvedMacroCall {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("unresolved-macro-call")
-    }
-    fn message(&self) -> String {
-        format!("unresolved macro `{}!`", self.path)
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile::new(self.file, self.node.clone().into())
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-    fn is_experimental(&self) -> bool {
-        true
-    }
-}
-
 // Diagnostic: inactive-code
 //
 // This diagnostic is shown for code with inactive `#[cfg]` attributes.
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index f7883c469..0a9414013 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -498,7 +498,10 @@ impl Module {
                     let import = &item_tree[id.value];
 
                     let use_tree = import.use_tree_to_ast(db.upcast(), file_id, *index);
-                    sink.push(UnresolvedImport { file: file_id, node: AstPtr::new(&use_tree) });
+                    acc.push(
+                        UnresolvedImport { decl: InFile::new(file_id, AstPtr::new(&use_tree)) }
+                            .into(),
+                    );
                 }
 
                 DefDiagnosticKind::UnconfiguredCode { ast, cfg, opts } => {
@@ -577,11 +580,13 @@ impl Module {
 
                 DefDiagnosticKind::UnresolvedMacroCall { ast, path } => {
                     let node = ast.to_node(db.upcast());
-                    sink.push(UnresolvedMacroCall {
-                        file: ast.file_id,
-                        node: AstPtr::new(&node),
-                        path: path.clone(),
-                    });
+                    acc.push(
+                        UnresolvedMacroCall {
+                            macro_call: InFile::new(ast.file_id, AstPtr::new(&node)),
+                            path: path.clone(),
+                        }
+                        .into(),
+                    );
                 }
 
                 DefDiagnosticKind::MacroError { ast, message } => {
@@ -1057,13 +1062,9 @@ impl Function {
                     precise_location: None,
                     macro_name: None,
                 }),
-                BodyDiagnostic::UnresolvedMacroCall { node, path } => {
-                    sink.push(UnresolvedMacroCall {
-                        file: node.file_id,
-                        node: node.value.clone(),
-                        path: path.clone(),
-                    })
-                }
+                BodyDiagnostic::UnresolvedMacroCall { node, path } => acc.push(
+                    UnresolvedMacroCall { macro_call: node.clone(), path: path.clone() }.into(),
+                ),
             }
         }
 
diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs
index 84d6fdc93..5a088b6e5 100644
--- a/crates/hir_def/src/nameres/tests/diagnostics.rs
+++ b/crates/hir_def/src/nameres/tests/diagnostics.rs
@@ -13,43 +13,6 @@ fn check_no_diagnostics(ra_fixture: &str) {
 }
 
 #[test]
-fn unresolved_import() {
-    check_diagnostics(
-        r"
-        use does_exist;
-        use does_not_exist;
-      //^^^^^^^^^^^^^^^^^^^ UnresolvedImport
-
-        mod does_exist {}
-        ",
-    );
-}
-
-#[test]
-fn dedup_unresolved_import_from_unresolved_crate() {
-    check_diagnostics(
-        r"
-        //- /main.rs crate:main
-        mod a {
-            extern crate doesnotexist;
-          //^^^^^^^^^^^^^^^^^^^^^^^^^^ UnresolvedExternCrate
-
-            // Should not error, since we already errored for the missing crate.
-            use doesnotexist::{self, bla, *};
-
-            use crate::doesnotexist;
-          //^^^^^^^^^^^^^^^^^^^^^^^^ UnresolvedImport
-        }
-
-        mod m {
-            use super::doesnotexist;
-          //^^^^^^^^^^^^^^^^^^^^^^^^ UnresolvedImport
-        }
-        ",
-    );
-}
-
-#[test]
 fn inactive_item() {
     // Additional tests in `cfg` crate. This only tests disabled cfgs.
 
@@ -92,37 +55,6 @@ fn inactive_via_cfg_attr() {
 }
 
 #[test]
-fn unresolved_legacy_scope_macro() {
-    check_diagnostics(
-        r#"
-        //- /lib.rs
-          macro_rules! m { () => {} }
-
-          m!();
-          m2!();
-        //^^^^^^ UnresolvedMacroCall
-        "#,
-    );
-}
-
-#[test]
-fn unresolved_module_scope_macro() {
-    check_diagnostics(
-        r#"
-        //- /lib.rs
-          mod mac {
-            #[macro_export]
-            macro_rules! m { () => {} }
-          }
-
-          self::m!();
-          self::m2!();
-        //^^^^^^^^^^^^ UnresolvedMacroCall
-        "#,
-    );
-}
-
-#[test]
 fn builtin_macro_fails_expansion() {
     check_diagnostics(
         r#"
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs
index 1fbb7131d..1a4800832 100644
--- a/crates/ide/src/diagnostics.rs
+++ b/crates/ide/src/diagnostics.rs
@@ -6,6 +6,8 @@
 
 mod unresolved_module;
 mod unresolved_extern_crate;
+mod unresolved_import;
+mod unresolved_macro_call;
 mod missing_fields;
 
 mod fixes;
@@ -15,9 +17,8 @@ mod unlinked_file;
 use std::cell::RefCell;
 
 use hir::{
-    db::AstDatabase,
     diagnostics::{AnyDiagnostic, Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder},
-    InFile, Semantics,
+    Semantics,
 };
 use ide_assists::AssistResolveStrategy;
 use ide_db::{base_db::SourceDatabase, RootDatabase};
@@ -43,17 +44,39 @@ pub struct Diagnostic {
     pub fixes: Option<Vec<Assist>>,
     pub unused: bool,
     pub code: Option<DiagnosticCode>,
+    pub experimental: bool,
 }
 
 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 }
+        Self {
+            message,
+            range,
+            severity: Severity::Error,
+            fixes: None,
+            unused: false,
+            code,
+            experimental: false,
+        }
+    }
+
+    fn experimental(mut self) -> Diagnostic {
+        self.experimental = true;
+        self
     }
 
     fn error(range: TextRange, message: String) -> Self {
-        Self { message, range, severity: Severity::Error, fixes: None, unused: false, code: None }
+        Self {
+            message,
+            range,
+            severity: Severity::Error,
+            fixes: None,
+            unused: false,
+            code: None,
+            experimental: false,
+        }
     }
 
     fn hint(range: TextRange, message: String) -> Self {
@@ -64,6 +87,7 @@ impl Diagnostic {
             fixes: None,
             unused: false,
             code: None,
+            experimental: false,
         }
     }
 
@@ -179,20 +203,6 @@ pub(crate) fn diagnostics(
             res.borrow_mut()
                 .push(Diagnostic::hint(display_range, d.message()).with_code(Some(d.code())));
         })
-        .on::<hir::diagnostics::UnresolvedMacroCall, _>(|d| {
-            let last_path_segment = sema.db.parse_or_expand(d.file).and_then(|root| {
-                d.node
-                    .to_node(&root)
-                    .path()
-                    .and_then(|it| it.segment())
-                    .and_then(|it| it.name_ref())
-                    .map(|it| InFile::new(d.file, SyntaxNodePtr::new(it.syntax())))
-            });
-            let diagnostics = last_path_segment.unwrap_or_else(|| d.display_source());
-            let display_range = sema.diagnostics_display_range(diagnostics).range;
-            res.borrow_mut()
-                .push(Diagnostic::error(display_range, d.message()).with_code(Some(d.code())));
-        })
         .on::<hir::diagnostics::UnimplementedBuiltinMacro, _>(|d| {
             let display_range = sema.diagnostics_display_range(d.display_source()).range;
             res.borrow_mut()
@@ -234,6 +244,8 @@ pub(crate) fn diagnostics(
         let d = match diag {
             AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d),
             AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d),
+            AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d),
+            AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d),
             AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d),
         };
         if let Some(code) = d.code {
@@ -241,6 +253,9 @@ pub(crate) fn diagnostics(
                 continue;
             }
         }
+        if ctx.config.disable_experimental && d.experimental {
+            continue;
+        }
         res.push(d)
     }
 
@@ -453,43 +468,6 @@ mod tests {
     }
 
     #[test]
-    fn test_unresolved_macro_range() {
-        check_diagnostics(
-            r#"
-foo::bar!(92);
-   //^^^ unresolved macro `foo::bar!`
-"#,
-        );
-    }
-
-    #[test]
-    fn unresolved_import_in_use_tree() {
-        // Only the relevant part of a nested `use` item should be highlighted.
-        check_diagnostics(
-            r#"
-use does_exist::{Exists, DoesntExist};
-                       //^^^^^^^^^^^ unresolved import
-
-use {does_not_exist::*, does_exist};
-   //^^^^^^^^^^^^^^^^^ unresolved import
-
-use does_not_exist::{
-    a,
-  //^ unresolved import
-    b,
-  //^ unresolved import
-    c,
-  //^ unresolved import
-};
-
-mod does_exist {
-    pub struct Exists;
-}
-"#,
-        );
-    }
-
-    #[test]
     fn range_mapping_out_of_macros() {
         // FIXME: this is very wrong, but somewhat tricky to fix.
         check_fix(
diff --git a/crates/ide/src/diagnostics/unresolved_import.rs b/crates/ide/src/diagnostics/unresolved_import.rs
new file mode 100644
index 000000000..1cbf96ba1
--- /dev/null
+++ b/crates/ide/src/diagnostics/unresolved_import.rs
@@ -0,0 +1,90 @@
+use crate::diagnostics::{Diagnostic, DiagnosticsContext};
+
+// Diagnostic: unresolved-import
+//
+// This diagnostic is triggered if rust-analyzer is unable to resolve a path in
+// a `use` declaration.
+pub(super) fn unresolved_import(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::UnresolvedImport,
+) -> Diagnostic {
+    Diagnostic::new(
+        "unresolved-import",
+        "unresolved import",
+        ctx.sema.diagnostics_display_range(d.decl.clone().map(|it| it.into())).range,
+    )
+    // This currently results in false positives in the following cases:
+    // - `cfg_if!`-generated code in libstd (we don't load the sysroot correctly)
+    // - `core::arch` (we don't handle `#[path = "../<path>"]` correctly)
+    // - proc macros and/or proc macro generated code
+    .experimental()
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::diagnostics::tests::check_diagnostics;
+
+    #[test]
+    fn unresolved_import() {
+        check_diagnostics(
+            r#"
+use does_exist;
+use does_not_exist;
+  //^^^^^^^^^^^^^^ unresolved import
+
+mod does_exist {}
+"#,
+        );
+    }
+
+    #[test]
+    fn unresolved_import_in_use_tree() {
+        // Only the relevant part of a nested `use` item should be highlighted.
+        check_diagnostics(
+            r#"
+use does_exist::{Exists, DoesntExist};
+                       //^^^^^^^^^^^ unresolved import
+
+use {does_not_exist::*, does_exist};
+   //^^^^^^^^^^^^^^^^^ unresolved import
+
+use does_not_exist::{
+    a,
+  //^ unresolved import
+    b,
+  //^ unresolved import
+    c,
+  //^ unresolved import
+};
+
+mod does_exist {
+    pub struct Exists;
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn dedup_unresolved_import_from_unresolved_crate() {
+        check_diagnostics(
+            r#"
+//- /main.rs crate:main
+mod a {
+    extern crate doesnotexist;
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^ unresolved extern crate
+
+    // Should not error, since we already errored for the missing crate.
+    use doesnotexist::{self, bla, *};
+
+    use crate::doesnotexist;
+      //^^^^^^^^^^^^^^^^^^^ unresolved import
+}
+
+mod m {
+    use super::doesnotexist;
+      //^^^^^^^^^^^^^^^^^^^ unresolved import
+}
+"#,
+        );
+    }
+}
diff --git a/crates/ide/src/diagnostics/unresolved_macro_call.rs b/crates/ide/src/diagnostics/unresolved_macro_call.rs
new file mode 100644
index 000000000..a3af332a4
--- /dev/null
+++ b/crates/ide/src/diagnostics/unresolved_macro_call.rs
@@ -0,0 +1,72 @@
+use hir::{db::AstDatabase, InFile};
+use syntax::{AstNode, SyntaxNodePtr};
+
+use crate::diagnostics::{Diagnostic, DiagnosticsContext};
+
+// Diagnostic: unresolved-macro-call
+//
+// This diagnostic is triggered if rust-analyzer is unable to resolve the path
+// to a macro in a macro invocation.
+pub(super) fn unresolved_macro_call(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::UnresolvedMacroCall,
+) -> Diagnostic {
+    let last_path_segment = ctx.sema.db.parse_or_expand(d.macro_call.file_id).and_then(|root| {
+        d.macro_call
+            .value
+            .to_node(&root)
+            .path()
+            .and_then(|it| it.segment())
+            .and_then(|it| it.name_ref())
+            .map(|it| InFile::new(d.macro_call.file_id, SyntaxNodePtr::new(it.syntax())))
+    });
+    let diagnostics = last_path_segment.unwrap_or_else(|| d.macro_call.clone().map(|it| it.into()));
+
+    Diagnostic::new(
+        "unresolved-macro-call",
+        format!("unresolved macro `{}!`", d.path),
+        ctx.sema.diagnostics_display_range(diagnostics).range,
+    )
+    .experimental()
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::diagnostics::tests::check_diagnostics;
+
+    #[test]
+    fn test_unresolved_macro_range() {
+        check_diagnostics(
+            r#"
+foo::bar!(92);
+   //^^^ unresolved macro `foo::bar!`
+"#,
+        );
+    }
+
+    #[test]
+    fn unresolved_legacy_scope_macro() {
+        check_diagnostics(
+            r#"
+macro_rules! m { () => {} }
+
+m!(); m2!();
+    //^^ unresolved macro `self::m2!`
+"#,
+        );
+    }
+
+    #[test]
+    fn unresolved_module_scope_macro() {
+        check_diagnostics(
+            r#"
+mod mac {
+#[macro_export]
+macro_rules! m { () => {} } }
+
+self::m!(); self::m2!();
+                //^^ unresolved macro `self::m2!`
+"#,
+        );
+    }
+}
diff --git a/crates/ide/src/diagnostics/unresolved_module.rs b/crates/ide/src/diagnostics/unresolved_module.rs
index 4c8c74ff7..b1da8f0e1 100644
--- a/crates/ide/src/diagnostics/unresolved_module.rs
+++ b/crates/ide/src/diagnostics/unresolved_module.rs
@@ -104,6 +104,7 @@ mod baz {}
                                 "unresolved-module",
                             ),
                         ),
+                        experimental: false,
                     },
                 ]
             "#]],