Merge pull request #18674 from Veykril/push-lrxotqknvxvr

Show expansion errors in expand_macro feature

Author: Veykril

src/tools/rust-analyzer/crates/hir-expand/src/lib.rs

@@ -28,6 +28,7 @@ use rustc_hash::FxHashMap;
 use stdx::TupleExt;
 use triomphe::Arc;
 
+use core::fmt;
 use std::hash::Hash;
 
 use base_db::{ra_salsa::InternValueTrivial, CrateId};
@@ -147,6 +148,10 @@ impl ExpandError {
     pub fn span(&self) -> Span {
         self.inner.1
     }
+
+    pub fn render_to_string(&self, db: &dyn ExpandDatabase) -> RenderedExpandError {
+        self.inner.0.render_to_string(db)
+    }
 }
 
 #[derive(Debug, PartialEq, Eq, Clone, Hash)]
@@ -164,18 +169,18 @@ pub enum ExpandErrorKind {
     ProcMacroPanic(Box<str>),
 }
 
-impl ExpandError {
-    pub fn render_to_string(&self, db: &dyn ExpandDatabase) -> RenderedExpandError {
-        self.inner.0.render_to_string(db)
-    }
-}
-
 pub struct RenderedExpandError {
     pub message: String,
     pub error: bool,
     pub kind: &'static str,
 }
 
+impl fmt::Display for RenderedExpandError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+
 impl RenderedExpandError {
     const GENERAL_KIND: &str = "macro-error";
 }

src/tools/rust-analyzer/crates/hir/src/semantics.rs

@@ -28,7 +28,7 @@ use hir_expand::{
     hygiene::SyntaxContextExt as _,
     inert_attr_macro::find_builtin_attr_idx,
     name::AsName,
-    FileRange, InMacroFile, MacroCallId, MacroFileId, MacroFileIdExt,
+    ExpandResult, FileRange, InMacroFile, MacroCallId, MacroFileId, MacroFileIdExt,
 };
 use intern::Symbol;
 use itertools::Itertools;
@@ -381,7 +381,13 @@ impl<'db> SemanticsImpl<'db> {
         node
     }
 
-    pub fn expand(&self, macro_call: &ast::MacroCall) -> Option<SyntaxNode> {
+    pub fn expand(&self, file_id: MacroFileId) -> ExpandResult<SyntaxNode> {
+        let res = self.db.parse_macro_expansion(file_id).map(|it| it.0.syntax_node());
+        self.cache(res.value.clone(), file_id.into());
+        res
+    }
+
+    pub fn expand_macro_call(&self, macro_call: &ast::MacroCall) -> Option<SyntaxNode> {
         let sa = self.analyze_no_infer(macro_call.syntax())?;
 
         let macro_call = InFile::new(sa.file_id, macro_call);
@@ -412,7 +418,10 @@ impl<'db> SemanticsImpl<'db> {
 
     /// Expands the macro if it isn't one of the built-in ones that expand to custom syntax or dummy
     /// expansions.
-    pub fn expand_allowed_builtins(&self, macro_call: &ast::MacroCall) -> Option<SyntaxNode> {
+    pub fn expand_allowed_builtins(
+        &self,
+        macro_call: &ast::MacroCall,
+    ) -> Option<ExpandResult<SyntaxNode>> {
         let sa = self.analyze_no_infer(macro_call.syntax())?;
 
         let macro_call = InFile::new(sa.file_id, macro_call);
@@ -448,15 +457,15 @@ impl<'db> SemanticsImpl<'db> {
             return None;
         }
 
-        let node = self.parse_or_expand(file_id.into());
+        let node = self.expand(file_id);
         Some(node)
     }
 
     /// If `item` has an attribute macro attached to it, expands it.
-    pub fn expand_attr_macro(&self, item: &ast::Item) -> Option<SyntaxNode> {
+    pub fn expand_attr_macro(&self, item: &ast::Item) -> Option<ExpandResult<SyntaxNode>> {
         let src = self.wrap_node_infile(item.clone());
         let macro_call_id = self.with_ctx(|ctx| ctx.item_to_macro_call(src.as_ref()))?;
-        Some(self.parse_or_expand(macro_call_id.as_file()))
+        Some(self.expand(macro_call_id.as_macro_file()))
     }
 
     pub fn expand_derive_as_pseudo_attr_macro(&self, attr: &ast::Attr) -> Option<SyntaxNode> {
@@ -480,15 +489,16 @@ impl<'db> SemanticsImpl<'db> {
         })
     }
 
-    pub fn expand_derive_macro(&self, attr: &ast::Attr) -> Option<Vec<SyntaxNode>> {
+    pub fn expand_derive_macro(&self, attr: &ast::Attr) -> Option<Vec<ExpandResult<SyntaxNode>>> {
         let res: Vec<_> = self
             .derive_macro_calls(attr)?
             .into_iter()
             .flat_map(|call| {
-                let file_id = call?.as_file();
-                let node = self.db.parse_or_expand(file_id);
-                self.cache(node.clone(), file_id);
-                Some(node)
+                let file_id = call?.as_macro_file();
+                let ExpandResult { value, err } = self.db.parse_macro_expansion(file_id);
+                let root_node = value.0.syntax_node();
+                self.cache(root_node.clone(), file_id.into());
+                Some(ExpandResult { value: root_node, err })
             })
             .collect();
         Some(res)
@@ -556,7 +566,7 @@ impl<'db> SemanticsImpl<'db> {
 
     /// Expand the macro call with a different token tree, mapping the `token_to_map` down into the
     /// expansion. `token_to_map` should be a token from the `speculative args` node.
-    pub fn speculative_expand(
+    pub fn speculative_expand_macro_call(
         &self,
         actual_macro_call: &ast::MacroCall,
         speculative_args: &ast::TokenTree,

src/tools/rust-analyzer/crates/ide-completion/src/context/analysis.rs

@@ -1,7 +1,7 @@
 //! Module responsible for analyzing the code surrounding the cursor for completion.
 use std::iter;
 
-use hir::{Semantics, Type, TypeInfo, Variant};
+use hir::{ExpandResult, Semantics, Type, TypeInfo, Variant};
 use ide_db::{active_parameter::ActiveParameter, RootDatabase};
 use itertools::Either;
 use syntax::{
@@ -104,7 +104,10 @@ fn expand(
                 // maybe parent items have attributes, so continue walking the ancestors
                 (None, None) => continue 'ancestors,
                 // successful expansions
-                (Some(actual_expansion), Some((fake_expansion, fake_mapped_token))) => {
+                (
+                    Some(ExpandResult { value: actual_expansion, err: _ }),
+                    Some((fake_expansion, fake_mapped_token)),
+                ) => {
                     let new_offset = fake_mapped_token.text_range().start();
                     if new_offset + relative_offset > actual_expansion.text_range().end() {
                         // offset outside of bounds from the original expansion,
@@ -239,8 +242,8 @@ fn expand(
             };
 
             match (
-                sema.expand(&actual_macro_call),
-                sema.speculative_expand(
+                sema.expand_macro_call(&actual_macro_call),
+                sema.speculative_expand_macro_call(
                     &actual_macro_call,
                     &speculative_args,
                     fake_ident_token.clone(),

src/tools/rust-analyzer/crates/ide-ssr/src/lib.rs

@@ -286,7 +286,7 @@ impl<'db> MatchFinder<'db> {
                     });
                 }
             } else if let Some(macro_call) = ast::MacroCall::cast(node.clone()) {
-                if let Some(expanded) = self.sema.expand(&macro_call) {
+                if let Some(expanded) = self.sema.expand_macro_call(&macro_call) {
                     if let Some(tt) = macro_call.token_tree() {
                         self.output_debug_for_nodes_at_range(
                             &expanded,

src/tools/rust-analyzer/crates/ide-ssr/src/search.rs

@@ -189,7 +189,7 @@ impl MatchFinder<'_> {
         // If we've got a macro call, we already tried matching it pre-expansion, which is the only
         // way to match the whole macro, now try expanding it and matching the expansion.
         if let Some(macro_call) = ast::MacroCall::cast(code.clone()) {
-            if let Some(expanded) = self.sema.expand(&macro_call) {
+            if let Some(expanded) = self.sema.expand_macro_call(&macro_call) {
                 if let Some(tt) = macro_call.token_tree() {
                     // When matching within a macro expansion, we only want to allow matches of
                     // nodes that originated entirely from within the token tree of the macro call.

src/tools/rust-analyzer/crates/ide/src/expand_macro.rs

@@ -1,10 +1,11 @@
 use hir::db::ExpandDatabase;
-use hir::{InFile, MacroFileIdExt, Semantics};
+use hir::{ExpandResult, InFile, MacroFileIdExt, Semantics};
 use ide_db::base_db::CrateId;
 use ide_db::{
     helpers::pick_best_token, syntax_helpers::prettify_macro_expansion, FileId, RootDatabase,
 };
 use span::{Edition, SpanMap, SyntaxContextId, TextRange, TextSize};
+use stdx::format_to;
 use syntax::{ast, ted, AstNode, NodeOrToken, SyntaxKind, SyntaxNode, T};
 
 use crate::FilePosition;
@@ -63,17 +64,23 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
             .take_while(|it| it != &token)
             .filter(|it| it.kind() == T![,])
             .count();
-        let expansion = expansions.get(idx)?.clone();
+        let ExpandResult { err, value: expansion } = expansions.get(idx)?.clone();
         let expansion_file_id = sema.hir_file_for(&expansion).macro_file()?;
         let expansion_span_map = db.expansion_span_map(expansion_file_id);
-        let expansion = format(
+        let mut expansion = format(
             db,
             SyntaxKind::MACRO_ITEMS,
             position.file_id,
             expansion,
             &expansion_span_map,
             krate,
         );
+        if let Some(err) = err {
+            expansion.insert_str(
+                0,
+                &format!("Expansion had errors: {}\n\n", err.render_to_string(sema.db)),
+            );
+        }
         Some(ExpandedMacro { name, expansion })
     });
 
@@ -83,6 +90,7 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
 
     let mut anc = tok.parent_ancestors();
     let mut span_map = SpanMap::empty();
+    let mut error = String::new();
     let (name, expanded, kind) = loop {
         let node = anc.next()?;
 
@@ -97,7 +105,7 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
                                 .unwrap_or(Edition::CURRENT),
                         )
                         .to_string(),
-                    expand_macro_recur(&sema, &item, &mut span_map, TextSize::new(0))?,
+                    expand_macro_recur(&sema, &item, &mut error, &mut span_map, TextSize::new(0))?,
                     SyntaxKind::MACRO_ITEMS,
                 );
             }
@@ -112,6 +120,7 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
                 expand_macro_recur(
                     &sema,
                     &ast::Item::MacroCall(mac),
+                    &mut error,
                     &mut span_map,
                     TextSize::new(0),
                 )?,
@@ -123,24 +132,31 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
     // FIXME:
     // macro expansion may lose all white space information
     // But we hope someday we can use ra_fmt for that
-    let expansion = format(db, kind, position.file_id, expanded, &span_map, krate);
+    let mut expansion = format(db, kind, position.file_id, expanded, &span_map, krate);
 
+    if !error.is_empty() {
+        expansion.insert_str(0, &format!("Expansion had errors:{error}\n\n"));
+    }
     Some(ExpandedMacro { name, expansion })
 }
 
 fn expand_macro_recur(
     sema: &Semantics<'_, RootDatabase>,
     macro_call: &ast::Item,
+    error: &mut String,
     result_span_map: &mut SpanMap<SyntaxContextId>,
     offset_in_original_node: TextSize,
 ) -> Option<SyntaxNode> {
-    let expanded = match macro_call {
-        item @ ast::Item::MacroCall(macro_call) => sema
-            .expand_attr_macro(item)
-            .or_else(|| sema.expand_allowed_builtins(macro_call))?
-            .clone_for_update(),
-        item => sema.expand_attr_macro(item)?.clone_for_update(),
+    let ExpandResult { value: expanded, err } = match macro_call {
+        item @ ast::Item::MacroCall(macro_call) => {
+            sema.expand_attr_macro(item).or_else(|| sema.expand_allowed_builtins(macro_call))?
+        }
+        item => sema.expand_attr_macro(item)?,
     };
+    let expanded = expanded.clone_for_update();
+    if let Some(err) = err {
+        format_to!(error, "\n{}", err.render_to_string(sema.db));
+    }
     let file_id =
         sema.hir_file_for(&expanded).macro_file().expect("expansion must produce a macro file");
     let expansion_span_map = sema.db.expansion_span_map(file_id);
@@ -149,12 +165,13 @@ fn expand_macro_recur(
         expanded.text_range().len(),
         &expansion_span_map,
     );
-    Some(expand(sema, expanded, result_span_map, u32::from(offset_in_original_node) as i32))
+    Some(expand(sema, expanded, error, result_span_map, u32::from(offset_in_original_node) as i32))
 }
 
 fn expand(
     sema: &Semantics<'_, RootDatabase>,
     expanded: SyntaxNode,
+    error: &mut String,
     result_span_map: &mut SpanMap<SyntaxContextId>,
     mut offset_in_original_node: i32,
 ) -> SyntaxNode {
@@ -165,6 +182,7 @@ fn expand(
         if let Some(new_node) = expand_macro_recur(
             sema,
             &child,
+            error,
             result_span_map,
             TextSize::new(
                 (offset_in_original_node + (u32::from(child.syntax().text_range().start()) as i32))
@@ -495,6 +513,9 @@ fn main() {
 "#,
             expect![[r#"
                 foo!
+                Expansion had errors:
+                expected ident: `BAD`
+
             "#]],
         );
     }

src/tools/rust-analyzer/crates/ide/src/highlight_related.rs

@@ -608,7 +608,7 @@ impl<'a> WalkExpandedExprCtx<'a> {
 
                     if let ast::Expr::MacroExpr(expr) = expr {
                         if let Some(expanded) =
-                            expr.macro_call().and_then(|call| self.sema.expand(&call))
+                            expr.macro_call().and_then(|call| self.sema.expand_macro_call(&call))
                         {
                             match_ast! {
                                 match expanded {