From 8b3ec12aac755822f9a281d6d453c6a7d0f69b90 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 28 Jun 2022 10:41:10 +0200 Subject: [PATCH] fix: Report proc macro errors in expressions correctly as well They didn't have a krate before, resulting in the generic "proc macro not found" error. Also improve error messages a bit more. --- crates/hir-def/src/body.rs | 2 +- crates/hir-def/src/body/lower.rs | 3 +- crates/hir-def/src/nameres/collector.rs | 14 ++-- crates/hir-def/src/nameres/diagnostics.rs | 4 +- crates/hir-expand/src/lib.rs | 4 +- crates/hir-expand/src/proc_macro.rs | 2 +- crates/hir/src/diagnostics.rs | 2 +- crates/hir/src/lib.rs | 4 +- .../src/handlers/unresolved_proc_macro.rs | 6 +- crates/project-model/src/tests.rs | 74 +++++++++---------- crates/project-model/src/workspace.rs | 4 +- crates/rust-analyzer/src/reload.rs | 19 ++--- 12 files changed, 69 insertions(+), 69 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index 57dd3a3502f..c7566b06d09 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -312,7 +312,7 @@ pub struct SyntheticSyntax; pub enum BodyDiagnostic { InactiveCode { node: InFile, cfg: CfgExpr, opts: CfgOptions }, MacroError { node: InFile>, message: String }, - UnresolvedProcMacro { node: InFile> }, + UnresolvedProcMacro { node: InFile>, krate: CrateId }, UnresolvedMacroCall { node: InFile>, path: ModPath }, } diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 5d7a1100cd5..31e3877ea93 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -572,9 +572,10 @@ impl ExprCollector<'_> { if record_diagnostics { match &res.err { - Some(ExpandError::UnresolvedProcMacro) => { + Some(ExpandError::UnresolvedProcMacro(krate)) => { self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro { node: InFile::new(outer_file, syntax_ptr), + krate: *krate, }); } Some(err) => { diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 9901df2af08..ca0cb99b371 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -22,6 +22,7 @@ use itertools::Itertools; use la_arena::Idx; use limit::Limit; use rustc_hash::{FxHashMap, FxHashSet}; +use stdx::always; use syntax::{ast, SmolStr}; use crate::{ @@ -1234,7 +1235,7 @@ impl DefCollector<'_> { self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( directive.module_id, loc.kind, - Some(loc.def.krate), + loc.def.krate, )); return recollect_without(self); } @@ -1258,7 +1259,7 @@ impl DefCollector<'_> { self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( directive.module_id, loc.kind, - Some(loc.def.krate), + loc.def.krate, )); return recollect_without(self); @@ -1308,13 +1309,10 @@ impl DefCollector<'_> { let err = self.db.macro_expand_error(macro_call_id); if let Some(err) = err { let diag = match err { - hir_expand::ExpandError::UnresolvedProcMacro => { + hir_expand::ExpandError::UnresolvedProcMacro(krate) => { + always!(krate == loc.def.krate); // Missing proc macros are non-fatal, so they are handled specially. - DefDiagnostic::unresolved_proc_macro( - module_id, - loc.kind.clone(), - Some(loc.def.krate), - ) + DefDiagnostic::unresolved_proc_macro(module_id, loc.kind.clone(), loc.def.krate) } _ => DefDiagnostic::macro_error(module_id, loc.kind.clone(), err.to_string()), }; diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs index 3ebc5629d7c..0d01f6d0aba 100644 --- a/crates/hir-def/src/nameres/diagnostics.rs +++ b/crates/hir-def/src/nameres/diagnostics.rs @@ -24,7 +24,7 @@ pub enum DefDiagnosticKind { UnconfiguredCode { ast: AstId, cfg: CfgExpr, opts: CfgOptions }, - UnresolvedProcMacro { ast: MacroCallKind, krate: Option }, + UnresolvedProcMacro { ast: MacroCallKind, krate: CrateId }, UnresolvedMacroCall { ast: MacroCallKind, path: ModPath }, @@ -85,7 +85,7 @@ impl DefDiagnostic { pub(super) fn unresolved_proc_macro( container: LocalModuleId, ast: MacroCallKind, - krate: Option, + krate: CrateId, ) -> Self { Self { in_module: container, kind: DefDiagnosticKind::UnresolvedProcMacro { ast, krate } } } diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index bc5372754d6..0844e1f6254 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -43,7 +43,7 @@ pub type ExpandResult = ValueResult; #[derive(Debug, PartialEq, Eq, Clone)] pub enum ExpandError { - UnresolvedProcMacro, + UnresolvedProcMacro(CrateId), Mbe(mbe::ExpandError), Other(Box), } @@ -57,7 +57,7 @@ impl From for ExpandError { impl fmt::Display for ExpandError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - ExpandError::UnresolvedProcMacro => f.write_str("unresolved proc-macro"), + ExpandError::UnresolvedProcMacro(_) => f.write_str("unresolved proc-macro"), ExpandError::Mbe(it) => it.fmt(f), ExpandError::Other(it) => f.write_str(it), } diff --git a/crates/hir-expand/src/proc_macro.rs b/crates/hir-expand/src/proc_macro.rs index 29d78e21ba3..5afdcc0e66d 100644 --- a/crates/hir-expand/src/proc_macro.rs +++ b/crates/hir-expand/src/proc_macro.rs @@ -75,7 +75,7 @@ impl ProcMacroExpander { }, } } - None => ExpandResult::only_err(ExpandError::UnresolvedProcMacro), + None => ExpandResult::only_err(ExpandError::UnresolvedProcMacro(self.krate)), } } } diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 6a06c6b10f5..7ecfb0cd6f2 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -90,7 +90,7 @@ pub struct UnresolvedProcMacro { pub macro_name: Option, pub kind: MacroKind, /// The crate id of the proc-macro this macro belongs to, or `None` if the proc-macro can't be found. - pub krate: Option, + pub krate: CrateId, } #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 7a31a8a4173..f24edebfd0b 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1159,13 +1159,13 @@ impl DefWithBody { } .into(), ), - BodyDiagnostic::UnresolvedProcMacro { node } => acc.push( + BodyDiagnostic::UnresolvedProcMacro { node, krate } => acc.push( UnresolvedProcMacro { node: node.clone().map(|it| it.into()), precise_location: None, macro_name: None, kind: MacroKind::ProcMacro, - krate: None, + krate: *krate, } .into(), ), diff --git a/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs b/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs index 5ebfe33dab1..fde3901a730 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs @@ -32,13 +32,13 @@ pub(crate) fn unresolved_proc_macro( None => "proc macro not expanded".to_string(), }; let severity = if config_enabled { Severity::Error } else { Severity::WeakWarning }; - let def_map = d.krate.map(|krate| ctx.sema.db.crate_def_map(krate)); + let def_map = ctx.sema.db.crate_def_map(d.krate); let message = format!( "{message}: {}", if config_enabled { - match def_map.as_ref().and_then(|def_map| def_map.proc_macro_loading_error()) { + match def_map.proc_macro_loading_error() { Some(e) => e, - None => "proc macro not found", + None => "proc macro not found in the built dylib", } } else { match d.kind { diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index 2e8e93c926c..ddfea0ce4c4 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -173,7 +173,7 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -248,7 +248,7 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -313,7 +313,7 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { }, dependencies: [], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: Some( @@ -390,7 +390,7 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -465,7 +465,7 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -555,7 +555,7 @@ fn cargo_hello_world_project_model_with_selective_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -632,7 +632,7 @@ fn cargo_hello_world_project_model_with_selective_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -697,7 +697,7 @@ fn cargo_hello_world_project_model_with_selective_overrides() { }, dependencies: [], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: Some( @@ -776,7 +776,7 @@ fn cargo_hello_world_project_model_with_selective_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -853,7 +853,7 @@ fn cargo_hello_world_project_model_with_selective_overrides() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -934,7 +934,7 @@ fn cargo_hello_world_project_model() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -1011,7 +1011,7 @@ fn cargo_hello_world_project_model() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -1076,7 +1076,7 @@ fn cargo_hello_world_project_model() { }, dependencies: [], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: Some( @@ -1155,7 +1155,7 @@ fn cargo_hello_world_project_model() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -1232,7 +1232,7 @@ fn cargo_hello_world_project_model() { }, ], proc_macro: Err( - "no build data", + "crate has not (yet) been built", ), origin: CratesIo { repo: None, @@ -1288,8 +1288,8 @@ fn rust_project_hello_world_project_model() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Alloc, @@ -1322,8 +1322,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Other, @@ -1356,8 +1356,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Other, @@ -1400,8 +1400,8 @@ fn rust_project_hello_world_project_model() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Other, @@ -1434,8 +1434,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Core, @@ -1539,8 +1539,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Other, @@ -1573,8 +1573,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Other, @@ -1607,8 +1607,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Other, @@ -1641,8 +1641,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Test, @@ -1757,8 +1757,8 @@ fn rust_project_hello_world_project_model() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Std, @@ -1791,8 +1791,8 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro loaded for sysroot crate", ), origin: Lang( Other, diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 18c9c2c8106..de2db0e1da7 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -873,7 +873,7 @@ fn add_target_crate_root( let proc_macro = match build_data.as_ref().map(|it| it.proc_macro_dylib_path.as_ref()) { Some(Some(it)) => load_proc_macro(it), Some(None) => Err("no proc macro dylib present".into()), - None => Err("no build data".into()), + None => Err("crate has not (yet) been built".into()), }; let display_name = CrateDisplayName::from_canonical_name(cargo_name.to_string()); @@ -929,7 +929,7 @@ fn sysroot_to_crate_graph( cfg_options.clone(), cfg_options.clone(), env, - Ok(Vec::new()), + Err("no proc macro loaded for sysroot crate".into()), false, CrateOrigin::Lang(LangCrateOrigin::from(&*sysroot[krate].name)), ); diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 3f129efd95b..13bcb7dfa23 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -540,17 +540,18 @@ pub(crate) fn load_proc_macro( path: &AbsPath, dummy_replace: &[Box], ) -> ProcMacroLoadResult { - let res: Result<_, String> = (|| { + let res: Result, String> = (|| { let dylib = MacroDylib::new(path.to_path_buf()) .map_err(|io| format!("Proc-macro dylib loading failed: {io}"))?; - Ok(if let Some(it) = server { - let vec = it.load_dylib(dylib).map_err(|e| format!("{e}"))?; - vec.into_iter() - .map(|expander| expander_to_proc_macro(expander, dummy_replace)) - .collect() - } else { - Vec::new() - }) + let server = server.ok_or_else(|| format!("Proc-macro server not started"))?; + let vec = server.load_dylib(dylib).map_err(|e| format!("{e}"))?; + if vec.is_empty() { + return Err("proc macro library returned no proc macros".to_string()); + } + Ok(vec + .into_iter() + .map(|expander| expander_to_proc_macro(expander, dummy_replace)) + .collect()) })(); return match res { Ok(proc_macros) => {