Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #14583 - Veykril:macro-def-err, r=Veykril
internal: Report macro definition errors on the definition
We still report them on the call site as well for the time being, and the diagnostic doesn't know where the error in the definition comes from, but that can be done later on
| -rw-r--r-- | crates/hir-def/src/body.rs | 5 | ||||
| -rw-r--r-- | crates/hir-def/src/data.rs | 7 | ||||
| -rw-r--r-- | crates/hir-def/src/nameres/collector.rs | 2 | ||||
| -rw-r--r-- | crates/hir-def/src/nameres/diagnostics.rs | 2 | ||||
| -rw-r--r-- | crates/hir-expand/src/db.rs | 16 | ||||
| -rw-r--r-- | crates/hir-expand/src/eager.rs | 5 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 8 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 52 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/macro_error.rs | 29 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 3 |
10 files changed, 115 insertions, 14 deletions
diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index 6b887bb1b3..928aadfbcc 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -190,8 +190,9 @@ impl Expander { let file_id = call_id.as_file(); - let raw_node = match db.parse_or_expand(file_id) { - Some(it) => it, + let raw_node = match db.parse_or_expand_with_err(file_id) { + // FIXME: report parse errors + Some(it) => it.syntax_node(), None => { // Only `None` if the macro expansion produced no usable AST. if err.is_none() { diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 98cf69c168..6d2c88660f 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -641,7 +641,12 @@ impl<'a> AssocItemCollector<'a> { self.items.push((item.name.clone(), def.into())); } AssocItem::MacroCall(call) => { - if let Some(root) = self.db.parse_or_expand(self.expander.current_file_id()) { + 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(); + let call = &item_tree[call]; let ast_id_map = self.db.ast_id_map(self.expander.current_file_id()); diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 84c7a80e15..1d625fa3c7 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1374,6 +1374,8 @@ impl DefCollector<'_> { // Then, fetch and process the item tree. This will reuse the expansion result from above. let item_tree = self.db.file_item_tree(file_id); + // FIXME: report parse errors for the macro expansion here + let mod_dir = self.mod_dirs[&module_id].clone(); ModCollector { def_collector: &mut *self, diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs index b024d7c677..a57896e546 100644 --- a/crates/hir-def/src/nameres/diagnostics.rs +++ b/crates/hir-def/src/nameres/diagnostics.rs @@ -34,6 +34,8 @@ pub enum DefDiagnosticKind { InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize }, MalformedDerive { ast: AstId<ast::Adt>, id: usize }, + + MacroDefError { ast: AstId<ast::Macro>, message: String }, } #[derive(Debug, PartialEq, Eq)] diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 37c3661540..d93f3b08d3 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -99,6 +99,8 @@ pub trait ExpandDatabase: SourceDatabase { /// file or a macro expansion. #[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>>; /// Implementation for the macro case. fn parse_macro_expansion( &self, @@ -252,13 +254,23 @@ fn parse_or_expand(db: &dyn ExpandDatabase, file_id: HirFileId) -> Option<Syntax match file_id.repr() { HirFileIdRepr::FileId(file_id) => Some(db.parse(file_id).tree().syntax().clone()), HirFileIdRepr::MacroFile(macro_file) => { - // FIXME: Note how we convert from `Parse` to `SyntaxNode` here, - // forgetting about parse errors. db.parse_macro_expansion(macro_file).value.map(|(it, _)| it.syntax_node()) } } } +fn parse_or_expand_with_err( + db: &dyn ExpandDatabase, + file_id: HirFileId, +) -> Option<Parse<SyntaxNode>> { + match file_id.repr() { + HirFileIdRepr::FileId(file_id) => Some(db.parse(file_id).to_syntax()), + HirFileIdRepr::MacroFile(macro_file) => { + db.parse_macro_expansion(macro_file).value.map(|(parse, _)| parse) + } + } +} + fn parse_macro_expansion( db: &dyn ExpandDatabase, macro_file: MacroFile, diff --git a/crates/hir-expand/src/eager.rs b/crates/hir-expand/src/eager.rs index aca41b11f9..6b646f5e4f 100644 --- a/crates/hir-expand/src/eager.rs +++ b/crates/hir-expand/src/eager.rs @@ -187,7 +187,10 @@ fn lazy_expand( ); let err = db.macro_expand_error(id); - let value = db.parse_or_expand(id.as_file()).map(|node| InFile::new(id.as_file(), node)); + 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 } } diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index f756832f0f..b735decfcb 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -39,6 +39,7 @@ diagnostics![ InvalidDeriveTarget, IncoherentImpl, MacroError, + MacroDefError, MalformedDerive, MismatchedArgCount, MissingFields, @@ -131,6 +132,13 @@ pub struct MacroError { pub message: String, } +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct MacroDefError { + pub node: InFile<AstPtr<ast::Macro>>, + pub message: String, + pub name: Option<TextRange>, +} + #[derive(Debug)] pub struct UnimplementedBuiltinMacro { pub node: InFile<SyntaxNodePtr>, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 3e568a4021..7e9b89db7a 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -46,6 +46,7 @@ use hir_def::{ item_tree::ItemTreeNode, lang_item::{LangItem, LangItemTarget}, layout::ReprOptions, + macro_id_to_def_id, nameres::{self, diagnostics::DefDiagnostic, ModuleOrigin}, per_ns::PerNs, resolver::{HasResolver, Resolver}, @@ -86,12 +87,12 @@ pub use crate::{ attrs::{HasAttrs, Namespace}, diagnostics::{ AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncoherentImpl, - IncorrectCase, InvalidDeriveTarget, 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, 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}, @@ -563,6 +564,7 @@ impl Module { } emit_def_diagnostic(db, acc, diag); } + for decl in self.declarations(db) { match decl { ModuleDef::Module(m) => { @@ -601,9 +603,11 @@ impl Module { } acc.extend(decl.diagnostics(db)) } + ModuleDef::Macro(m) => emit_macro_def_diagnostics(db, acc, m), _ => acc.extend(decl.diagnostics(db)), } } + self.legacy_macros(db).into_iter().for_each(|m| emit_macro_def_diagnostics(db, acc, m)); let inherent_impls = db.inherent_impls_in_crate(self.id.krate()); @@ -685,8 +689,31 @@ impl Module { } } +fn emit_macro_def_diagnostics(db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>, m: Macro) { + let id = macro_id_to_def_id(db.upcast(), m.id); + if let Err(e) = db.macro_def(id) { + let Some(ast) = id.ast_id().left() else { + never!("MacroDefError for proc-macro: {:?}", e); + return; + }; + emit_def_diagnostic_( + db, + acc, + &DefDiagnosticKind::MacroDefError { ast, message: e.to_string() }, + ); + } +} + fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>, diag: &DefDiagnostic) { - match &diag.kind { + emit_def_diagnostic_(db, acc, &diag.kind) +} + +fn emit_def_diagnostic_( + db: &dyn HirDatabase, + acc: &mut Vec<AnyDiagnostic>, + diag: &DefDiagnosticKind, +) { + match diag { DefDiagnosticKind::UnresolvedModule { ast: declaration, candidates } => { let decl = declaration.to_node(db.upcast()); acc.push( @@ -794,6 +821,17 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>, diag: None => stdx::never!("derive diagnostic on item without derive attribute"), } } + DefDiagnosticKind::MacroDefError { ast, message } => { + let node = ast.to_node(db.upcast()); + acc.push( + MacroDefError { + node: InFile::new(ast.file_id, AstPtr::new(&node)), + name: node.name().map(|it| it.syntax().text_range()), + message: message.clone(), + } + .into(), + ); + } } } diff --git a/crates/ide-diagnostics/src/handlers/macro_error.rs b/crates/ide-diagnostics/src/handlers/macro_error.rs index 870c78d1f1..af74015cf9 100644 --- a/crates/ide-diagnostics/src/handlers/macro_error.rs +++ b/crates/ide-diagnostics/src/handlers/macro_error.rs @@ -9,6 +9,16 @@ pub(crate) fn macro_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroError) -> Diagnostic::new("macro-error", d.message.clone(), display_range).experimental() } +// Diagnostic: macro-error +// +// This diagnostic is shown for macro expansion errors. +pub(crate) fn macro_def_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroDefError) -> Diagnostic { + // Use more accurate position if available. + let display_range = + ctx.resolve_precise_location(&d.node.clone().map(|it| it.syntax_node_ptr()), d.name); + Diagnostic::new("macro-def-error", d.message.clone(), display_range).experimental() +} + #[cfg(test)] mod tests { use crate::{ @@ -188,6 +198,7 @@ fn f() { "#, ); } + #[test] fn dollar_crate_in_builtin_macro() { check_diagnostics( @@ -212,4 +223,22 @@ fn f() { "#, ) } + + #[test] + fn def_diagnostic() { + check_diagnostics( + r#" +macro_rules! foo { + //^^^ error: expected subtree + f => {}; +} + +fn f() { + foo!(); + //^^^ error: invalid macro definition: expected subtree + +} +"#, + ) + } } diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 70116f15a7..59976ecf29 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -249,7 +249,7 @@ pub fn diagnostics( let mut diags = Vec::new(); if let Some(m) = module { - m.diagnostics(db, &mut diags) + m.diagnostics(db, &mut diags); } for diag in diags { @@ -263,6 +263,7 @@ pub fn diagnostics( AnyDiagnostic::IncoherentImpl(d) => handlers::incoherent_impl::incoherent_impl(&ctx, &d), AnyDiagnostic::IncorrectCase(d) => handlers::incorrect_case::incorrect_case(&ctx, &d), 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::MalformedDerive(d) => handlers::malformed_derive::malformed_derive(&ctx, &d), AnyDiagnostic::MismatchedArgCount(d) => handlers::mismatched_arg_count::mismatched_arg_count(&ctx, &d), |