From 0103f5df8fff2ccdbfb03adfe432b69c7840cf42 Mon Sep 17 00:00:00 2001 From: Brandon Date: Tue, 16 Mar 2021 00:46:57 -0700 Subject: Fix missing unresolved macro diagnostic in function body --- crates/hir_def/src/body.rs | 19 +++++++++-------- crates/hir_def/src/body/diagnostics.rs | 6 +++++- crates/hir_def/src/body/lower.rs | 15 ++++++++++++-- crates/hir_def/src/body/tests.rs | 12 +++++++++++ crates/hir_def/src/data.rs | 37 ++++++++++++++++++---------------- crates/hir_def/src/diagnostics.rs | 2 +- crates/hir_def/src/lib.rs | 23 ++++++++++++--------- 7 files changed, 75 insertions(+), 39 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs index 8bcc350ce..d2dcd6727 100644 --- a/crates/hir_def/src/body.rs +++ b/crates/hir_def/src/body.rs @@ -32,6 +32,7 @@ use crate::{ path::{ModPath, Path}, src::HasSource, AsMacroCall, BlockId, DefWithBodyId, HasModule, LocalModuleId, Lookup, ModuleId, + UnresolvedMacro, }; /// A subset of Expander that only deals with cfg attributes. We only need it to @@ -101,10 +102,12 @@ impl Expander { &mut self, db: &dyn DefDatabase, macro_call: ast::MacroCall, - ) -> ExpandResult> { + ) -> Result>, UnresolvedMacro> { if self.recursion_limit + 1 > EXPANSION_RECURSION_LIMIT { cov_mark::hit!(your_stack_belongs_to_me); - return ExpandResult::str_err("reached recursion limit during macro expansion".into()); + return Ok(ExpandResult::str_err( + "reached recursion limit during macro expansion".into(), + )); } let macro_call = InFile::new(self.current_file_id, ¯o_call); @@ -116,14 +119,14 @@ impl Expander { let call_id = macro_call.as_call_id_with_errors(db, self.def_map.krate(), resolver, &mut |e| { err.get_or_insert(e); - }); + })?; let call_id = match call_id { Some(it) => it, None => { if err.is_none() { log::warn!("no error despite `as_call_id_with_errors` returning `None`"); } - return ExpandResult { value: None, err }; + return Ok(ExpandResult { value: None, err }); } }; @@ -141,9 +144,9 @@ impl Expander { log::warn!("no error despite `parse_or_expand` failing"); } - return ExpandResult::only_err(err.unwrap_or_else(|| { + return Ok(ExpandResult::only_err(err.unwrap_or_else(|| { mbe::ExpandError::Other("failed to parse macro invocation".into()) - })); + }))); } }; @@ -151,7 +154,7 @@ impl Expander { Some(it) => it, None => { // This can happen without being an error, so only forward previous errors. - return ExpandResult { value: None, err }; + return Ok(ExpandResult { value: None, err }); } }; @@ -167,7 +170,7 @@ impl Expander { self.current_file_id = file_id; self.ast_id_map = db.ast_id_map(file_id); - ExpandResult { value: Some((mark, node)), err } + Ok(ExpandResult { value: Some((mark, node)), err }) } pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) { diff --git a/crates/hir_def/src/body/diagnostics.rs b/crates/hir_def/src/body/diagnostics.rs index 1de7d30e2..f6992c9a8 100644 --- a/crates/hir_def/src/body/diagnostics.rs +++ b/crates/hir_def/src/body/diagnostics.rs @@ -2,13 +2,14 @@ use hir_expand::diagnostics::DiagnosticSink; -use crate::diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro}; +use crate::diagnostics::{InactiveCode, MacroError, UnresolvedMacroCall, UnresolvedProcMacro}; #[derive(Debug, Eq, PartialEq)] pub(crate) enum BodyDiagnostic { InactiveCode(InactiveCode), MacroError(MacroError), UnresolvedProcMacro(UnresolvedProcMacro), + UnresolvedMacroCall(UnresolvedMacroCall), } impl BodyDiagnostic { @@ -23,6 +24,9 @@ impl BodyDiagnostic { BodyDiagnostic::UnresolvedProcMacro(diag) => { sink.push(diag.clone()); } + BodyDiagnostic::UnresolvedMacroCall(diag) => { + sink.push(diag.clone()); + } } } } diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs index 8934ae6c9..08f0c32f4 100644 --- a/crates/hir_def/src/body/lower.rs +++ b/crates/hir_def/src/body/lower.rs @@ -24,7 +24,7 @@ use crate::{ body::{Body, BodySourceMap, Expander, LabelSource, PatPtr, SyntheticSyntax}, builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, db::DefDatabase, - diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro}, + diagnostics::{InactiveCode, MacroError, UnresolvedMacroCall, UnresolvedProcMacro}, expr::{ dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Label, LabelId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, @@ -33,7 +33,7 @@ use crate::{ item_scope::BuiltinShadowMode, path::{GenericArgs, Path}, type_ref::{Mutability, Rawness, TypeRef}, - AdtId, BlockLoc, ModuleDefId, + AdtId, BlockLoc, ModuleDefId, UnresolvedMacro, }; use super::{diagnostics::BodyDiagnostic, ExprSource, PatSource}; @@ -542,6 +542,17 @@ impl ExprCollector<'_> { let macro_call = self.expander.to_source(AstPtr::new(&e)); let res = self.expander.enter_expand(self.db, e); + let res = match res { + Ok(res) => res, + Err(UnresolvedMacro) => { + self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedMacroCall( + UnresolvedMacroCall { file: outer_file, node: syntax_ptr.cast().unwrap() }, + )); + collector(self, None); + return; + } + }; + match &res.err { Some(ExpandError::UnresolvedProcMacro) => { self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro( diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs index 991a32b15..f8e6f70e8 100644 --- a/crates/hir_def/src/body/tests.rs +++ b/crates/hir_def/src/body/tests.rs @@ -174,6 +174,18 @@ fn f() { ); } +#[test] +fn unresolved_macro_diag() { + check_diagnostics( + r#" +fn f() { + m!(); + //^^^^ unresolved macro call +} + "#, + ); +} + #[test] fn dollar_crate_in_builtin_macro() { check_diagnostics( diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs index aea53d527..e3bb9de08 100644 --- a/crates/hir_def/src/data.rs +++ b/crates/hir_def/src/data.rs @@ -261,23 +261,26 @@ fn collect_items( let ast_id_map = db.ast_id_map(file_id); let root = db.parse_or_expand(file_id).unwrap(); let call = ast_id_map.get(call.ast_id).to_node(&root); - - if let Some((mark, mac)) = expander.enter_expand(db, call).value { - let src: InFile = expander.to_source(mac); - let item_tree = db.item_tree(src.file_id); - let iter = - item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item); - items.extend(collect_items( - db, - module, - expander, - iter, - src.file_id, - container, - limit - 1, - )); - - expander.exit(db, mark); + let res = expander.enter_expand(db, call); + + if let Ok(res) = res { + if let Some((mark, mac)) = res.value { + let src: InFile = expander.to_source(mac); + let item_tree = db.item_tree(src.file_id); + let iter = + item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item); + items.extend(collect_items( + db, + module, + expander, + iter, + src.file_id, + container, + limit - 1, + )); + + expander.exit(db, mark); + } } } } diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index 7188bb299..97abf8653 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -99,7 +99,7 @@ impl Diagnostic for UnresolvedImport { // // This diagnostic is triggered if rust-analyzer is unable to resolve the path to a // macro in a macro invocation. -#[derive(Debug)] +#[derive(Debug, Clone, Eq, PartialEq)] pub struct UnresolvedMacroCall { pub file: HirFileId, pub node: AstPtr, diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs index c6655c5fb..4b5cdd7a1 100644 --- a/crates/hir_def/src/lib.rs +++ b/crates/hir_def/src/lib.rs @@ -583,7 +583,7 @@ pub trait AsMacroCall { krate: CrateId, resolver: impl Fn(path::ModPath) -> Option, ) -> Option { - self.as_call_id_with_errors(db, krate, resolver, &mut |_| ()) + self.as_call_id_with_errors(db, krate, resolver, &mut |_| ()).ok()? } fn as_call_id_with_errors( @@ -592,7 +592,7 @@ pub trait AsMacroCall { krate: CrateId, resolver: impl Fn(path::ModPath) -> Option, error_sink: &mut dyn FnMut(mbe::ExpandError), - ) -> Option; + ) -> Result, UnresolvedMacro>; } impl AsMacroCall for InFile<&ast::MacroCall> { @@ -602,24 +602,27 @@ impl AsMacroCall for InFile<&ast::MacroCall> { krate: CrateId, resolver: impl Fn(path::ModPath) -> Option, error_sink: &mut dyn FnMut(mbe::ExpandError), - ) -> Option { + ) -> Result, UnresolvedMacro> { let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value)); let h = Hygiene::new(db.upcast(), self.file_id); let path = self.value.path().and_then(|path| path::ModPath::from_src(path, &h)); - if path.is_none() { - error_sink(mbe::ExpandError::Other("malformed macro invocation".into())); - } + let path = match path { + None => { + error_sink(mbe::ExpandError::Other("malformed macro invocation".into())); + return Ok(None); + } + Some(path) => path, + }; macro_call_as_call_id( - &AstIdWithPath::new(ast_id.file_id, ast_id.value, path?), + &AstIdWithPath::new(ast_id.file_id, ast_id.value, path), db, krate, resolver, error_sink, ) - .ok()? - .ok() + .map(Result::ok) } } @@ -636,7 +639,7 @@ impl AstIdWithPath { } } -struct UnresolvedMacro; +pub struct UnresolvedMacro; fn macro_call_as_call_id( call: &AstIdWithPath, -- cgit v1.2.3