Report syntax errors from item level macro expansions

crates/hir-def/src/body.rs

@@ -21,7 +21,7 @@ use limit::Limit;
 use once_cell::unsync::OnceCell;
 use profile::Count;
 use rustc_hash::FxHashMap;
-use syntax::{ast, AstPtr, SyntaxNode, SyntaxNodePtr};
+use syntax::{ast, AstPtr, Parse, SyntaxNode, SyntaxNodePtr};
 
 use crate::{
     attr::Attrs,
@@ -137,7 +137,7 @@ impl Expander {
         &mut self,
         db: &dyn DefDatabase,
         macro_call: ast::MacroCall,
-    ) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {
+    ) -> Result<ExpandResult<Option<(Mark, Parse<T>)>>, UnresolvedMacro> {
         // FIXME: within_limit should support this, instead of us having to extract the error
         let mut unresolved_macro_err = None;
 
@@ -167,37 +167,37 @@ impl Expander {
         &mut self,
         db: &dyn DefDatabase,
         call_id: MacroCallId,
-    ) -> ExpandResult<Option<(Mark, T)>> {
+    ) -> ExpandResult<Option<(Mark, Parse<T>)>> {
         self.within_limit(db, |_this| ExpandResult::ok(Some(call_id)))
     }
 
     fn enter_expand_inner(
         db: &dyn DefDatabase,
         call_id: MacroCallId,
-        mut err: Option<ExpandError>,
-    ) -> ExpandResult<Option<(HirFileId, SyntaxNode)>> {
-        if err.is_none() {
-            err = db.macro_expand_error(call_id);
+        mut error: Option<ExpandError>,
+    ) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
+        let file_id = call_id.as_file();
+        let ExpandResult { value, err } = db.parse_or_expand_with_err(file_id);
+
+        if error.is_none() {
+            error = err;
         }
 
-        let file_id = call_id.as_file();
-
-        let raw_node = match db.parse_or_expand_with_err(file_id) {
-            // FIXME: report parse errors
-            Some(it) => it.syntax_node(),
+        let parse = match value {
+            Some(it) => it,
             None => {
                 // Only `None` if the macro expansion produced no usable AST.
-                if err.is_none() {
+                if error.is_none() {
                     tracing::warn!("no error despite `parse_or_expand` failing");
                 }
 
-                return ExpandResult::only_err(err.unwrap_or_else(|| {
+                return ExpandResult::only_err(error.unwrap_or_else(|| {
                     ExpandError::Other("failed to parse macro invocation".into())
                 }));
             }
         };
 
-        ExpandResult { value: Some((file_id, raw_node)), err }
+        ExpandResult { value: Some(InFile::new(file_id, parse)), err: error }
     }
 
     pub fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
@@ -259,7 +259,7 @@ impl Expander {
         &mut self,
         db: &dyn DefDatabase,
         op: F,
-    ) -> ExpandResult<Option<(Mark, T)>>
+    ) -> ExpandResult<Option<(Mark, Parse<T>)>>
     where
         F: FnOnce(&mut Self) -> ExpandResult<Option<MacroCallId>>,
     {
@@ -286,15 +286,15 @@ impl Expander {
         };
 
         Self::enter_expand_inner(db, call_id, err).map(|value| {
-            value.and_then(|(new_file_id, node)| {
-                let node = T::cast(node)?;
+            value.and_then(|InFile { file_id, value }| {
+                let parse = value.cast::<T>()?;
 
                 self.recursion_depth += 1;
-                self.cfg_expander.hygiene = Hygiene::new(db.upcast(), new_file_id);
-                let old_file_id = std::mem::replace(&mut self.current_file_id, new_file_id);
+                self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id);
+                let old_file_id = std::mem::replace(&mut self.current_file_id, file_id);
                 let mark =
                     Mark { file_id: old_file_id, bomb: DropBomb::new("expansion mark dropped") };
-                Some((mark, node))
+                Some((mark, parse))
             })
         })
     }

crates/hir-def/src/body/lower.rs

@@ -824,7 +824,11 @@ impl ExprCollector<'_> {
                     self.db.ast_id_map(self.expander.current_file_id),
                 );
 
-                let id = collector(self, Some(expansion));
+                if record_diagnostics {
+                    // FIXME: Report parse errors here
+                }
+
+                let id = collector(self, Some(expansion.tree()));
                 self.ast_id_map = prev_ast_id_map;
                 self.expander.exit(self.db, mark);
                 id

crates/hir-def/src/data.rs

@@ -7,7 +7,7 @@ use std::sync::Arc;
 use hir_expand::{name::Name, AstId, ExpandResult, HirFileId, InFile, MacroCallId, MacroDefKind};
 use intern::Interned;
 use smallvec::SmallVec;
-use syntax::ast;
+use syntax::{ast, Parse};
 
 use crate::{
     attr::Attrs,
@@ -604,13 +604,10 @@ impl<'a> AssocItemCollector<'a> {
                             continue 'attrs;
                         }
                     }
-                    match self.expander.enter_expand_id::<ast::MacroItems>(self.db, call_id) {
-                        ExpandResult { value: Some((mark, _)), .. } => {
-                            self.collect_macro_items(mark);
-                            continue 'items;
-                        }
-                        ExpandResult { .. } => {}
-                    }
+
+                    let res = self.expander.enter_expand_id::<ast::MacroItems>(self.db, call_id);
+                    self.collect_macro_items(res, &|| loc.kind.clone());
+                    continue 'items;
                 }
             }
 
@@ -641,22 +638,24 @@ impl<'a> AssocItemCollector<'a> {
                     self.items.push((item.name.clone(), def.into()));
                 }
                 AssocItem::MacroCall(call) => {
-                    if let Some(root) =
-                        self.db.parse_or_expand_with_err(self.expander.current_file_id())
-                    {
-                        // FIXME: report parse errors
-                        let root = root.syntax_node();
-
+                    // TODO: These are the wrong errors to report, report in collect_macro_items instead
+                    let file_id = self.expander.current_file_id();
+                    let root = self.db.parse_or_expand(file_id);
+                    if let Some(root) = root {
                         let call = &item_tree[call];
 
-                        let ast_id_map = self.db.ast_id_map(self.expander.current_file_id());
-                        let call = ast_id_map.get(call.ast_id).to_node(&root);
-                        let _cx =
-                            stdx::panic_context::enter(format!("collect_items MacroCall: {call}"));
-                        let res = self.expander.enter_expand::<ast::MacroItems>(self.db, call);
-
-                        if let Ok(ExpandResult { value: Some((mark, _)), .. }) = res {
-                            self.collect_macro_items(mark);
+                        let ast_id_map = self.db.ast_id_map(file_id);
+                        let macro_call = ast_id_map.get(call.ast_id).to_node(&root);
+                        let _cx = stdx::panic_context::enter(format!(
+                            "collect_items MacroCall: {macro_call}"
+                        ));
+                        if let Ok(res) =
+                            self.expander.enter_expand::<ast::MacroItems>(self.db, macro_call)
+                        {
+                            self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike {
+                                ast_id: InFile::new(file_id, call.ast_id),
+                                expand_to: hir_expand::ExpandTo::Items,
+                            });
                         }
                     }
                 }
@@ -664,7 +663,28 @@ impl<'a> AssocItemCollector<'a> {
         }
     }
 
-    fn collect_macro_items(&mut self, mark: Mark) {
+    fn collect_macro_items(
+        &mut self,
+        ExpandResult { value, err }: ExpandResult<Option<(Mark, Parse<ast::MacroItems>)>>,
+        error_call_kind: &dyn Fn() -> hir_expand::MacroCallKind,
+    ) {
+        let Some((mark, parse)) = value else { return };
+
+        if let Some(err) = err {
+            self.inactive_diagnostics.push(DefDiagnostic::macro_error(
+                self.module_id.local_id,
+                error_call_kind(),
+                err.to_string(),
+            ));
+        }
+        if let errors @ [_, ..] = parse.errors() {
+            self.inactive_diagnostics.push(DefDiagnostic::macro_expansion_parse_error(
+                self.module_id.local_id,
+                error_call_kind(),
+                errors.into(),
+            ));
+        }
+
         let tree_id = item_tree::TreeId::new(self.expander.current_file_id(), None);
         let item_tree = tree_id.item_tree(self.db);
         let iter: SmallVec<[_; 2]> =

crates/hir-def/src/generics.rs

@@ -350,7 +350,7 @@ impl GenericParams {
                 match expander.enter_expand::<ast::Type>(db, macro_call) {
                     Ok(ExpandResult { value: Some((mark, expanded)), .. }) => {
                         let ctx = expander.ctx(db);
-                        let type_ref = TypeRef::from_ast(&ctx, expanded);
+                        let type_ref = TypeRef::from_ast(&ctx, expanded.tree());
                         self.fill_implicit_impl_trait_args(db, expander, &type_ref);
                         expander.exit(db, mark);
                     }

crates/hir-def/src/nameres/diagnostics.rs

@@ -4,7 +4,10 @@ use base_db::CrateId;
 use cfg::{CfgExpr, CfgOptions};
 use hir_expand::{attrs::AttrId, MacroCallKind};
 use la_arena::Idx;
-use syntax::ast::{self, AnyHasAttrs};
+use syntax::{
+    ast::{self, AnyHasAttrs},
+    SyntaxError,
+};
 
 use crate::{
     item_tree::{self, ItemTreeId},
@@ -29,6 +32,8 @@ pub enum DefDiagnosticKind {
 
     MacroError { ast: MacroCallKind, message: String },
 
+    MacroExpansionParseError { ast: MacroCallKind, errors: Box<[SyntaxError]> },
+
     UnimplementedBuiltinMacro { ast: AstId<ast::Macro> },
 
     InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize },
@@ -91,7 +96,7 @@ impl DefDiagnostic {
         Self { in_module: container, kind: DefDiagnosticKind::UnresolvedProcMacro { ast, krate } }
     }
 
-    pub(super) fn macro_error(
+    pub(crate) fn macro_error(
         container: LocalModuleId,
         ast: MacroCallKind,
         message: String,
@@ -99,6 +104,20 @@ impl DefDiagnostic {
         Self { in_module: container, kind: DefDiagnosticKind::MacroError { ast, message } }
     }
 
+    pub(crate) fn macro_expansion_parse_error(
+        container: LocalModuleId,
+        ast: MacroCallKind,
+        errors: &[SyntaxError],
+    ) -> Self {
+        Self {
+            in_module: container,
+            kind: DefDiagnosticKind::MacroExpansionParseError {
+                ast,
+                errors: errors.to_vec().into_boxed_slice(),
+            },
+        }
+    }
+
     pub(super) fn unresolved_macro_call(
         container: LocalModuleId,
         ast: MacroCallKind,

crates/hir-expand/src/db.rs

@@ -100,7 +100,10 @@ pub trait ExpandDatabase: SourceDatabase {
     #[salsa::transparent]
     fn parse_or_expand(&self, file_id: HirFileId) -> Option<SyntaxNode>;
     #[salsa::transparent]
-    fn parse_or_expand_with_err(&self, file_id: HirFileId) -> Option<Parse<SyntaxNode>>;
+    fn parse_or_expand_with_err(
+        &self,
+        file_id: HirFileId,
+    ) -> ExpandResult<Option<Parse<SyntaxNode>>>;
     /// Implementation for the macro case.
     fn parse_macro_expansion(
         &self,
@@ -262,11 +265,11 @@ fn parse_or_expand(db: &dyn ExpandDatabase, file_id: HirFileId) -> Option<Syntax
 fn parse_or_expand_with_err(
     db: &dyn ExpandDatabase,
     file_id: HirFileId,
-) -> Option<Parse<SyntaxNode>> {
+) -> ExpandResult<Option<Parse<SyntaxNode>>> {
     match file_id.repr() {
-        HirFileIdRepr::FileId(file_id) => Some(db.parse(file_id).to_syntax()),
+        HirFileIdRepr::FileId(file_id) => ExpandResult::ok(Some(db.parse(file_id).to_syntax())),
         HirFileIdRepr::MacroFile(macro_file) => {
-            db.parse_macro_expansion(macro_file).value.map(|(parse, _)| parse)
+            db.parse_macro_expansion(macro_file).map(|it| it.map(|(parse, _)| parse))
         }
     }
 }

crates/hir-expand/src/eager.rs

@@ -21,7 +21,7 @@
 use std::sync::Arc;
 
 use base_db::CrateId;
-use syntax::{ted, SyntaxNode};
+use syntax::{ted, Parse, SyntaxNode};
 
 use crate::{
     ast::{self, AstNode},
@@ -111,7 +111,7 @@ fn lazy_expand(
     def: &MacroDefId,
     macro_call: InFile<ast::MacroCall>,
     krate: CrateId,
-) -> ExpandResult<Option<InFile<SyntaxNode>>> {
+) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
     let ast_id = db.ast_id_map(macro_call.file_id).ast_id(&macro_call.value);
 
     let expand_to = ExpandTo::from_call_site(&macro_call.value);
@@ -121,13 +121,8 @@ fn lazy_expand(
         MacroCallKind::FnLike { ast_id: macro_call.with_value(ast_id), expand_to },
     );
 
-    let err = db.macro_expand_error(id);
-    let value =
-        db.parse_or_expand_with_err(id.as_file()).map(|node| InFile::new(id.as_file(), node));
-    // FIXME: report parse errors
-    let value = value.map(|it| it.map(|it| it.syntax_node()));
-
-    ExpandResult { value, err }
+    db.parse_or_expand_with_err(id.as_file())
+        .map(|parse| parse.map(|parse| InFile::new(id.as_file(), parse)))
 }
 
 fn eager_macro_recur(
@@ -183,8 +178,14 @@ fn eager_macro_recur(
                     Some(val) => {
                         // replace macro inside
                         let hygiene = Hygiene::new(db, val.file_id);
-                        let ExpandResult { value, err: error } =
-                            eager_macro_recur(db, &hygiene, val, krate, macro_resolver)?;
+                        let ExpandResult { value, err: error } = eager_macro_recur(
+                            db,
+                            &hygiene,
+                            // FIXME: We discard parse errors here
+                            val.map(|it| it.syntax_node()),
+                            krate,
+                            macro_resolver,
+                        )?;
                         let err = if err.is_none() { error } else { err };
                         ExpandResult { value, err }
                     }

crates/hir-ty/src/lower.rs

@@ -381,7 +381,8 @@ impl<'a> TyLoweringContext<'a> {
                     match expander.enter_expand::<ast::Type>(self.db.upcast(), macro_call) {
                         Ok(ExpandResult { value: Some((mark, expanded)), .. }) => {
                             let ctx = expander.ctx(self.db.upcast());
-                            let type_ref = TypeRef::from_ast(&ctx, expanded);
+                            // FIXME: Report syntax errors in expansion here
+                            let type_ref = TypeRef::from_ast(&ctx, expanded.tree());
 
                             drop(expander);
                             let ty = self.lower_ty(&type_ref);

crates/hir/src/diagnostics.rs

@@ -10,7 +10,7 @@ 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};
+use syntax::{ast, AstPtr, SyntaxError, SyntaxNodePtr, TextRange};
 
 use crate::{AssocItem, Field, Local, MacroKind, Type};
 
@@ -38,8 +38,9 @@ diagnostics![
     IncorrectCase,
     InvalidDeriveTarget,
     IncoherentImpl,
-    MacroError,
     MacroDefError,
+    MacroError,
+    MacroExpansionParseError,
     MalformedDerive,
     MismatchedArgCount,
     MissingFields,
@@ -132,6 +133,13 @@ pub struct MacroError {
     pub message: String,
 }
 
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct MacroExpansionParseError {
+    pub node: InFile<SyntaxNodePtr>,
+    pub precise_location: Option<TextRange>,
+    pub errors: Box<[SyntaxError]>,
+}
+
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct MacroDefError {
     pub node: InFile<AstPtr<ast::Macro>>,

crates/hir/src/lib.rs

@@ -87,12 +87,12 @@ pub use crate::{
     attrs::{HasAttrs, Namespace},
     diagnostics::{
         AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncoherentImpl,
-        IncorrectCase, InvalidDeriveTarget, MacroDefError, MacroError, MalformedDerive,
-        MismatchedArgCount, MissingFields, MissingMatchArms, MissingUnsafe, NeedMut, NoSuchField,
-        PrivateAssocItem, PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch,
-        UndeclaredLabel, UnimplementedBuiltinMacro, UnreachableLabel, UnresolvedExternCrate,
-        UnresolvedField, UnresolvedImport, UnresolvedMacroCall, UnresolvedMethodCall,
-        UnresolvedModule, UnresolvedProcMacro, UnusedMut,
+        IncorrectCase, InvalidDeriveTarget, MacroDefError, MacroError, MacroExpansionParseError,
+        MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms, MissingUnsafe,
+        NeedMut, NoSuchField, PrivateAssocItem, PrivateField, ReplaceFilterMapNextWithFindMap,
+        TypeMismatch, UndeclaredLabel, UnimplementedBuiltinMacro, UnreachableLabel,
+        UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall,
+        UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, UnusedMut,
     },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits},
@@ -753,7 +753,6 @@ fn emit_def_diagnostic_(
                 .into(),
             );
         }
-
         DefDiagnosticKind::UnresolvedProcMacro { ast, krate } => {
             let (node, precise_location, macro_name, kind) = precise_macro_call_location(ast, db);
             acc.push(
@@ -761,7 +760,6 @@ fn emit_def_diagnostic_(
                     .into(),
             );
         }
-
         DefDiagnosticKind::UnresolvedMacroCall { ast, path } => {
             let (node, precise_location, _, _) = precise_macro_call_location(ast, db);
             acc.push(
@@ -774,12 +772,16 @@ fn emit_def_diagnostic_(
                 .into(),
             );
         }
-
         DefDiagnosticKind::MacroError { ast, message } => {
             let (node, precise_location, _, _) = precise_macro_call_location(ast, db);
             acc.push(MacroError { node, precise_location, message: message.clone() }.into());
         }
-
+        DefDiagnosticKind::MacroExpansionParseError { ast, errors } => {
+            let (node, precise_location, _, _) = precise_macro_call_location(ast, db);
+            acc.push(
+                MacroExpansionParseError { node, precise_location, errors: errors.clone() }.into(),
+            );
+        }
         DefDiagnosticKind::UnimplementedBuiltinMacro { ast } => {
             let node = ast.to_node(db.upcast());
             // Must have a name, otherwise we wouldn't emit it.

crates/ide-diagnostics/src/lib.rs

@@ -265,6 +265,19 @@ pub fn diagnostics(
             AnyDiagnostic::InvalidDeriveTarget(d) => handlers::invalid_derive_target::invalid_derive_target(&ctx, &d),
             AnyDiagnostic::MacroDefError(d) => handlers::macro_error::macro_def_error(&ctx, &d),
             AnyDiagnostic::MacroError(d) => handlers::macro_error::macro_error(&ctx, &d),
+            AnyDiagnostic::MacroExpansionParseError(d) => {
+                res.extend(d.errors.iter().take(32).map(|err| {
+                    {
+                        Diagnostic::new(
+                            "syntax-error",
+                            format!("Syntax Error in Expansion: {err}"),
+                            ctx.resolve_precise_location(&d.node.clone(), d.precise_location),
+                        )
+                    }
+                    .experimental()
+                }));
+                continue;
+            },
             AnyDiagnostic::MalformedDerive(d) => handlers::malformed_derive::malformed_derive(&ctx, &d),
             AnyDiagnostic::MismatchedArgCount(d) => handlers::mismatched_arg_count::mismatched_arg_count(&ctx, &d),
             AnyDiagnostic::MissingFields(d) => handlers::missing_fields::missing_fields(&ctx, &d),