From d1632c2727474c0a88b19de3718af806d42e4450 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 16 Apr 2023 17:22:06 +0200 Subject: [PATCH] Report syntax errors from item level macro expansions --- crates/hir-def/src/body.rs | 42 +++++++-------- crates/hir-def/src/body/lower.rs | 6 ++- crates/hir-def/src/data.rs | 66 +++++++++++++++-------- crates/hir-def/src/generics.rs | 2 +- crates/hir-def/src/nameres/diagnostics.rs | 23 +++++++- crates/hir-expand/src/db.rs | 11 ++-- crates/hir-expand/src/eager.rs | 23 ++++---- crates/hir-ty/src/lower.rs | 3 +- crates/hir/src/diagnostics.rs | 12 ++++- crates/hir/src/lib.rs | 22 ++++---- crates/ide-diagnostics/src/lib.rs | 13 +++++ 11 files changed, 147 insertions(+), 76 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index f4304ae7e8a..1d082d55547 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -21,7 +21,7 @@ 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 @@ pub fn enter_expand( &mut self, db: &dyn DefDatabase, macro_call: ast::MacroCall, - ) -> Result>, UnresolvedMacro> { + ) -> Result)>>, 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 @@ pub fn enter_expand_id( &mut self, db: &dyn DefDatabase, call_id: MacroCallId, - ) -> ExpandResult> { + ) -> ExpandResult)>> { self.within_limit(db, |_this| ExpandResult::ok(Some(call_id))) } fn enter_expand_inner( db: &dyn DefDatabase, call_id: MacroCallId, - mut err: Option, - ) -> ExpandResult> { - if err.is_none() { - err = db.macro_expand_error(call_id); + mut error: Option, + ) -> ExpandResult>>> { + 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 @@ fn within_limit( &mut self, db: &dyn DefDatabase, op: F, - ) -> ExpandResult> + ) -> ExpandResult)>> where F: FnOnce(&mut Self) -> ExpandResult>, { @@ -286,15 +286,15 @@ fn within_limit( }; 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::()?; 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)) }) }) } diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index b5487dda1b9..db619b97dbe 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -824,7 +824,11 @@ fn collect_macro_call( 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 diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 6d2c88660f9..95581727add 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -7,7 +7,7 @@ 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 @@ fn collect(&mut self, item_tree: &ItemTree, tree_id: TreeId, assoc_items: &[Asso continue 'attrs; } } - match self.expander.enter_expand_id::(self.db, call_id) { - ExpandResult { value: Some((mark, _)), .. } => { - self.collect_macro_items(mark); - continue 'items; - } - ExpandResult { .. } => {} - } + + let res = self.expander.enter_expand_id::(self.db, call_id); + self.collect_macro_items(res, &|| loc.kind.clone()); + continue 'items; } } @@ -641,22 +638,24 @@ fn collect(&mut self, item_tree: &ItemTree, tree_id: TreeId, assoc_items: &[Asso 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::(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::(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 @@ fn collect(&mut self, item_tree: &ItemTree, tree_id: TreeId, assoc_items: &[Asso } } - fn collect_macro_items(&mut self, mark: Mark) { + fn collect_macro_items( + &mut self, + ExpandResult { value, err }: ExpandResult)>>, + 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]> = diff --git a/crates/hir-def/src/generics.rs b/crates/hir-def/src/generics.rs index 30edaed1095..c1e20d657bd 100644 --- a/crates/hir-def/src/generics.rs +++ b/crates/hir-def/src/generics.rs @@ -350,7 +350,7 @@ pub(crate) fn fill_implicit_impl_trait_args( match expander.enter_expand::(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); } diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs index a57896e5460..6b922b5785b 100644 --- a/crates/hir-def/src/nameres/diagnostics.rs +++ b/crates/hir-def/src/nameres/diagnostics.rs @@ -4,7 +4,10 @@ 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 }, InvalidDeriveTarget { ast: AstId, id: usize }, @@ -91,7 +96,7 @@ pub(super) fn unresolved_proc_macro( 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 @@ pub(super) fn macro_error( 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, diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index e8fba15601b..1749698387d 100644 --- a/crates/hir-expand/src/db.rs +++ b/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; #[salsa::transparent] - fn parse_or_expand_with_err(&self, file_id: HirFileId) -> Option>; + fn parse_or_expand_with_err( + &self, + file_id: HirFileId, + ) -> ExpandResult>>; /// 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 Option> { +) -> ExpandResult>> { 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)) } } } diff --git a/crates/hir-expand/src/eager.rs b/crates/hir-expand/src/eager.rs index 7d00b682a93..84f391316c6 100644 --- a/crates/hir-expand/src/eager.rs +++ b/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, krate: CrateId, -) -> ExpandResult>> { +) -> ExpandResult>>> { let ast_id = db.ast_id_map(macro_call.file_id).ast_id(¯o_call.value); let expand_to = ExpandTo::from_call_site(¯o_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 } } diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index b37fe1d63d5..33dc5e2d69b 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -381,7 +381,8 @@ pub fn lower_ty_ext(&self, type_ref: &TypeRef) -> (Ty, Option) { match expander.enter_expand::(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); diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index b735decfcb3..f81f8b0b011 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -10,7 +10,7 @@ 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 @@ fn from(d: $diag) -> AnyDiagnostic { 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, + pub precise_location: Option, + pub errors: Box<[SyntaxError]>, +} + #[derive(Debug, Clone, Eq, PartialEq)] pub struct MacroDefError { pub node: InFile>, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 7e9b89db7a1..3adb484b12f 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -87,12 +87,12 @@ 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. diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 59976ecf29c..7c8cb7a4476 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/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),