From 28fbecff4273b9ead96e76d1111799e880b2c230 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 12 Dec 2024 17:08:17 +0100 Subject: [PATCH] Show expansion errors in expand_macro feature --- crates/hir-expand/src/lib.rs | 17 ++++--- crates/hir/src/semantics.rs | 34 +++++++++----- crates/ide-completion/src/context/analysis.rs | 11 +++-- crates/ide-ssr/src/lib.rs | 2 +- crates/ide-ssr/src/search.rs | 2 +- crates/ide/src/expand_macro.rs | 45 ++++++++++++++----- crates/ide/src/highlight_related.rs | 2 +- 7 files changed, 76 insertions(+), 37 deletions(-) diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 2ee598dfbf..5aafe6db52 100644 --- a/crates/hir-expand/src/lib.rs +++ b/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), } -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"; } diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index f9d3f9d07e..8d3f24dd11 100644 --- a/crates/hir/src/semantics.rs +++ b/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 { + pub fn expand(&self, file_id: MacroFileId) -> ExpandResult { + 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 { 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 { + pub fn expand_allowed_builtins( + &self, + macro_call: &ast::MacroCall, + ) -> Option> { let sa = self.analyze_no_infer(macro_call.syntax())?; let macro_call = InFile::new(sa.file_id, macro_call); @@ -447,15 +456,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 { + pub fn expand_attr_macro(&self, item: &ast::Item) -> Option> { 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 { @@ -479,15 +488,16 @@ impl<'db> SemanticsImpl<'db> { }) } - pub fn expand_derive_macro(&self, attr: &ast::Attr) -> Option> { + pub fn expand_derive_macro(&self, attr: &ast::Attr) -> Option>> { 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) @@ -555,7 +565,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, diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs index 4a678963b9..3b7898b9e8 100644 --- a/crates/ide-completion/src/context/analysis.rs +++ b/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(), diff --git a/crates/ide-ssr/src/lib.rs b/crates/ide-ssr/src/lib.rs index eaca95d98c..6b654f8934 100644 --- a/crates/ide-ssr/src/lib.rs +++ b/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(¯o_call) { + if let Some(expanded) = self.sema.expand_macro_call(¯o_call) { if let Some(tt) = macro_call.token_tree() { self.output_debug_for_nodes_at_range( &expanded, diff --git a/crates/ide-ssr/src/search.rs b/crates/ide-ssr/src/search.rs index 241de10b44..b1cade3926 100644 --- a/crates/ide-ssr/src/search.rs +++ b/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(¯o_call) { + if let Some(expanded) = self.sema.expand_macro_call(¯o_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. diff --git a/crates/ide/src/expand_macro.rs b/crates/ide/src/expand_macro.rs index 79fdf75b7f..10a73edd51 100644 --- a/crates/ide/src/expand_macro.rs +++ b/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,10 +64,10 @@ 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, @@ -74,6 +75,12 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option< &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, offset_in_original_node: TextSize, ) -> Option { - 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, 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` + "#]], ); } diff --git a/crates/ide/src/highlight_related.rs b/crates/ide/src/highlight_related.rs index fc29ba06da..4690416e05 100644 --- a/crates/ide/src/highlight_related.rs +++ b/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 {