Fix missing unresolved macro diagnostic in function body

7 files changed, 75 insertions, 39 deletions

diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs
index 8bcc350ce..d2dcd6727 100644
--- a/crates/hir_def/src/body.rs
+++ b/crates/hir_def/src/body.rs
@@ -32,6 +32,7 @@ use crate::{
     path::{ModPath, Path},
     src::HasSource,
     AsMacroCall, BlockId, DefWithBodyId, HasModule, LocalModuleId, Lookup, ModuleId,
+    UnresolvedMacro,
 };
 
 /// A subset of Expander that only deals with cfg attributes. We only need it to
@@ -101,10 +102,12 @@ impl Expander {
         &mut self,
         db: &dyn DefDatabase,
         macro_call: ast::MacroCall,
-    ) -> ExpandResult<Option<(Mark, T)>> {
+    ) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {
         if self.recursion_limit + 1 > EXPANSION_RECURSION_LIMIT {
             cov_mark::hit!(your_stack_belongs_to_me);
-            return ExpandResult::str_err("reached recursion limit during macro expansion".into());
+            return Ok(ExpandResult::str_err(
+                "reached recursion limit during macro expansion".into(),
+            ));
         }
 
         let macro_call = InFile::new(self.current_file_id, &macro_call);
@@ -116,14 +119,14 @@ impl Expander {
         let call_id =
             macro_call.as_call_id_with_errors(db, self.def_map.krate(), resolver, &mut |e| {
                 err.get_or_insert(e);
-            });
+            })?;
         let call_id = match call_id {
             Some(it) => it,
             None => {
                 if err.is_none() {
                     log::warn!("no error despite `as_call_id_with_errors` returning `None`");
                 }
-                return ExpandResult { value: None, err };
+                return Ok(ExpandResult { value: None, err });
             }
         };
 
@@ -141,9 +144,9 @@ impl Expander {
                     log::warn!("no error despite `parse_or_expand` failing");
                 }
 
-                return ExpandResult::only_err(err.unwrap_or_else(|| {
+                return Ok(ExpandResult::only_err(err.unwrap_or_else(|| {
                     mbe::ExpandError::Other("failed to parse macro invocation".into())
-                }));
+                })));
             }
         };
 
@@ -151,7 +154,7 @@ impl Expander {
             Some(it) => it,
             None => {
                 // This can happen without being an error, so only forward previous errors.
-                return ExpandResult { value: None, err };
+                return Ok(ExpandResult { value: None, err });
             }
         };
 
@@ -167,7 +170,7 @@ impl Expander {
         self.current_file_id = file_id;
         self.ast_id_map = db.ast_id_map(file_id);
 
-        ExpandResult { value: Some((mark, node)), err }
+        Ok(ExpandResult { value: Some((mark, node)), err })
     }
 
     pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
diff --git a/crates/hir_def/src/body/diagnostics.rs b/crates/hir_def/src/body/diagnostics.rs
index 1de7d30e2..f6992c9a8 100644
--- a/crates/hir_def/src/body/diagnostics.rs
+++ b/crates/hir_def/src/body/diagnostics.rs
@@ -2,13 +2,14 @@
 
 use hir_expand::diagnostics::DiagnosticSink;
 
-use crate::diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro};
+use crate::diagnostics::{InactiveCode, MacroError, UnresolvedMacroCall, UnresolvedProcMacro};
 
 #[derive(Debug, Eq, PartialEq)]
 pub(crate) enum BodyDiagnostic {
     InactiveCode(InactiveCode),
     MacroError(MacroError),
     UnresolvedProcMacro(UnresolvedProcMacro),
+    UnresolvedMacroCall(UnresolvedMacroCall),
 }
 
 impl BodyDiagnostic {
@@ -23,6 +24,9 @@ impl BodyDiagnostic {
             BodyDiagnostic::UnresolvedProcMacro(diag) => {
                 sink.push(diag.clone());
             }
+            BodyDiagnostic::UnresolvedMacroCall(diag) => {
+                sink.push(diag.clone());
+            }
         }
     }
 }
diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs
index 8934ae6c9..08f0c32f4 100644
--- a/crates/hir_def/src/body/lower.rs
+++ b/crates/hir_def/src/body/lower.rs
@@ -24,7 +24,7 @@ use crate::{
     body::{Body, BodySourceMap, Expander, LabelSource, PatPtr, SyntheticSyntax},
     builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint},
     db::DefDatabase,
-    diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro},
+    diagnostics::{InactiveCode, MacroError, UnresolvedMacroCall, UnresolvedProcMacro},
     expr::{
         dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Label,
         LabelId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField,
@@ -33,7 +33,7 @@ use crate::{
     item_scope::BuiltinShadowMode,
     path::{GenericArgs, Path},
     type_ref::{Mutability, Rawness, TypeRef},
-    AdtId, BlockLoc, ModuleDefId,
+    AdtId, BlockLoc, ModuleDefId, UnresolvedMacro,
 };
 
 use super::{diagnostics::BodyDiagnostic, ExprSource, PatSource};
@@ -542,6 +542,17 @@ impl ExprCollector<'_> {
         let macro_call = self.expander.to_source(AstPtr::new(&e));
         let res = self.expander.enter_expand(self.db, e);
 
+        let res = match res {
+            Ok(res) => res,
+            Err(UnresolvedMacro) => {
+                self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedMacroCall(
+                    UnresolvedMacroCall { file: outer_file, node: syntax_ptr.cast().unwrap() },
+                ));
+                collector(self, None);
+                return;
+            }
+        };
+
         match &res.err {
             Some(ExpandError::UnresolvedProcMacro) => {
                 self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro(
diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs
index 991a32b15..f8e6f70e8 100644
--- a/crates/hir_def/src/body/tests.rs
+++ b/crates/hir_def/src/body/tests.rs
@@ -175,6 +175,18 @@ fn f() {
 }
 
 #[test]
+fn unresolved_macro_diag() {
+    check_diagnostics(
+        r#"
+fn f() {
+    m!();
+  //^^^^ unresolved macro call
+}
+      "#,
+    );
+}
+
+#[test]
 fn dollar_crate_in_builtin_macro() {
     check_diagnostics(
         r#"
diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs
index aea53d527..e3bb9de08 100644
--- a/crates/hir_def/src/data.rs
+++ b/crates/hir_def/src/data.rs
@@ -261,23 +261,26 @@ fn collect_items(
                 let ast_id_map = db.ast_id_map(file_id);
                 let root = db.parse_or_expand(file_id).unwrap();
                 let call = ast_id_map.get(call.ast_id).to_node(&root);
-
-                if let Some((mark, mac)) = expander.enter_expand(db, call).value {
-                    let src: InFile<ast::MacroItems> = expander.to_source(mac);
-                    let item_tree = db.item_tree(src.file_id);
-                    let iter =
-                        item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item);
-                    items.extend(collect_items(
-                        db,
-                        module,
-                        expander,
-                        iter,
-                        src.file_id,
-                        container,
-                        limit - 1,
-                    ));
-
-                    expander.exit(db, mark);
+                let res = expander.enter_expand(db, call);
+
+                if let Ok(res) = res {
+                    if let Some((mark, mac)) = res.value {
+                        let src: InFile<ast::MacroItems> = expander.to_source(mac);
+                        let item_tree = db.item_tree(src.file_id);
+                        let iter =
+                            item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item);
+                        items.extend(collect_items(
+                            db,
+                            module,
+                            expander,
+                            iter,
+                            src.file_id,
+                            container,
+                            limit - 1,
+                        ));
+
+                        expander.exit(db, mark);
+                    }
                 }
             }
         }
diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs
index 7188bb299..97abf8653 100644
--- a/crates/hir_def/src/diagnostics.rs
+++ b/crates/hir_def/src/diagnostics.rs
@@ -99,7 +99,7 @@ impl Diagnostic for UnresolvedImport {
 //
 // This diagnostic is triggered if rust-analyzer is unable to resolve the path to a
 // macro in a macro invocation.
-#[derive(Debug)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct UnresolvedMacroCall {
     pub file: HirFileId,
     pub node: AstPtr<ast::MacroCall>,
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index c6655c5fb..4b5cdd7a1 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -583,7 +583,7 @@ pub trait AsMacroCall {
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
     ) -> Option<MacroCallId> {
-        self.as_call_id_with_errors(db, krate, resolver, &mut |_| ())
+        self.as_call_id_with_errors(db, krate, resolver, &mut |_| ()).ok()?
     }
 
     fn as_call_id_with_errors(
@@ -592,7 +592,7 @@ pub trait AsMacroCall {
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
         error_sink: &mut dyn FnMut(mbe::ExpandError),
-    ) -> Option<MacroCallId>;
+    ) -> Result<Option<MacroCallId>, UnresolvedMacro>;
 }
 
 impl AsMacroCall for InFile<&ast::MacroCall> {
@@ -602,24 +602,27 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
         error_sink: &mut dyn FnMut(mbe::ExpandError),
-    ) -> Option<MacroCallId> {
+    ) -> Result<Option<MacroCallId>, UnresolvedMacro> {
         let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
         let h = Hygiene::new(db.upcast(), self.file_id);
         let path = self.value.path().and_then(|path| path::ModPath::from_src(path, &h));
 
-        if path.is_none() {
-            error_sink(mbe::ExpandError::Other("malformed macro invocation".into()));
-        }
+        let path = match path {
+            None => {
+                error_sink(mbe::ExpandError::Other("malformed macro invocation".into()));
+                return Ok(None);
+            }
+            Some(path) => path,
+        };
 
         macro_call_as_call_id(
-            &AstIdWithPath::new(ast_id.file_id, ast_id.value, path?),
+            &AstIdWithPath::new(ast_id.file_id, ast_id.value, path),
             db,
             krate,
             resolver,
             error_sink,
         )
-        .ok()?
-        .ok()
+        .map(Result::ok)
     }
 }
 
@@ -636,7 +639,7 @@ impl<T: ast::AstNode> AstIdWithPath<T> {
     }
 }
 
-struct UnresolvedMacro;
+pub struct UnresolvedMacro;
 
 fn macro_call_as_call_id(
     call: &AstIdWithPath<ast::MacroCall>,