From 3421b645e6f7d15ddad0e8e526d8a7db09b72516 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 22 Oct 2020 19:19:18 +0200 Subject: Emit better #[cfg] diagnostics --- crates/hir_def/src/attr.rs | 14 ++++- crates/hir_def/src/diagnostics.rs | 14 ++++- crates/hir_def/src/nameres.rs | 16 +++-- crates/hir_def/src/nameres/collector.rs | 29 +++++---- crates/hir_def/src/nameres/tests/diagnostics.rs | 22 +++++++ crates/ide/src/hover.rs | 4 +- crates/ide/src/runnables.rs | 84 +++++++++++++------------ crates/rust-analyzer/src/cargo_target_spec.rs | 4 +- crates/rust-analyzer/src/to_proto.rs | 2 +- 9 files changed, 124 insertions(+), 65 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index dea552a60..b2ce7ca3c 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -125,12 +125,20 @@ impl Attrs { AttrQuery { attrs: self, key } } - pub fn cfg(&self) -> impl Iterator + '_ { + pub fn cfg(&self) -> Option { // FIXME: handle cfg_attr :-) - self.by_key("cfg").tt_values().map(CfgExpr::parse) + let mut cfgs = self.by_key("cfg").tt_values().map(CfgExpr::parse).collect::>(); + match cfgs.len() { + 0 => None, + 1 => Some(cfgs.pop().unwrap()), + _ => Some(CfgExpr::All(cfgs)), + } } pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> bool { - self.cfg().all(|cfg| cfg_options.check(&cfg) != Some(false)) + match self.cfg() { + None => true, + Some(cfg) => cfg_options.check(&cfg) != Some(false), + } } } diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index c9c08b01f..34a6a8d4b 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -1,7 +1,9 @@ //! Diagnostics produced by `hir_def`. use std::any::Any; +use std::fmt::Write; +use cfg::{CfgExpr, CfgOptions, DnfExpr}; use hir_expand::diagnostics::{Diagnostic, DiagnosticCode}; use syntax::{ast, AstPtr, SyntaxNodePtr}; @@ -94,6 +96,8 @@ impl Diagnostic for UnresolvedImport { pub struct InactiveCode { pub file: HirFileId, pub node: SyntaxNodePtr, + pub cfg: CfgExpr, + pub opts: CfgOptions, } impl Diagnostic for InactiveCode { @@ -101,8 +105,14 @@ impl Diagnostic for InactiveCode { DiagnosticCode("inactive-code") } fn message(&self) -> String { - // FIXME: say *why* it is configured out - "code is inactive due to #[cfg] directives".to_string() + let inactive = DnfExpr::new(self.cfg.clone()).why_inactive(&self.opts); + let mut buf = "code is inactive due to #[cfg] directives".to_string(); + + if let Some(inactive) = inactive { + write!(buf, ": {}", inactive).unwrap(); + } + + buf } fn display_source(&self) -> InFile { InFile::new(self.file, self.node.clone()) diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index 01a28aeeb..eb41d324e 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -283,6 +283,7 @@ pub enum ModuleSource { } mod diagnostics { + use cfg::{CfgExpr, CfgOptions}; use hir_expand::diagnostics::DiagnosticSink; use hir_expand::hygiene::Hygiene; use hir_expand::InFile; @@ -299,7 +300,7 @@ mod diagnostics { UnresolvedImport { ast: AstId, index: usize }, - UnconfiguredCode { ast: InFile }, + UnconfiguredCode { ast: InFile, cfg: CfgExpr, opts: CfgOptions }, } #[derive(Debug, PartialEq, Eq)] @@ -341,8 +342,10 @@ mod diagnostics { pub(super) fn unconfigured_code( container: LocalModuleId, ast: InFile, + cfg: CfgExpr, + opts: CfgOptions, ) -> Self { - Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast } } + Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast, cfg, opts } } } pub(super) fn add_to( @@ -395,8 +398,13 @@ mod diagnostics { } } - DiagnosticKind::UnconfiguredCode { ast } => { - sink.push(InactiveCode { file: ast.file_id, node: ast.value.clone() }); + DiagnosticKind::UnconfiguredCode { ast, cfg, opts } => { + sink.push(InactiveCode { + file: ast.file_id, + node: ast.value.clone(), + cfg: cfg.clone(), + opts: opts.clone(), + }); } } } diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index bff8edb62..f30172d90 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -6,7 +6,7 @@ use std::iter; use base_db::{CrateId, FileId, ProcMacroId}; -use cfg::CfgOptions; +use cfg::{CfgExpr, CfgOptions}; use hir_expand::InFile; use hir_expand::{ ast_id_map::FileAstId, @@ -900,7 +900,8 @@ impl ModCollector<'_, '_> { // `#[macro_use] extern crate` is hoisted to imports macros before collecting // any other items. for item in items { - if self.is_cfg_enabled(self.item_tree.attrs((*item).into())) { + let attrs = self.item_tree.attrs((*item).into()); + if attrs.cfg().map_or(true, |cfg| self.is_cfg_enabled(&cfg)) { if let ModItem::ExternCrate(id) = item { let import = self.item_tree[*id].clone(); if import.is_macro_use { @@ -912,9 +913,11 @@ impl ModCollector<'_, '_> { for &item in items { let attrs = self.item_tree.attrs(item.into()); - if !self.is_cfg_enabled(attrs) { - self.emit_unconfigured_diagnostic(item); - continue; + if let Some(cfg) = attrs.cfg() { + if !self.is_cfg_enabled(&cfg) { + self.emit_unconfigured_diagnostic(item, &cfg); + continue; + } } let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: self.module_id }; @@ -1321,20 +1324,22 @@ impl ModCollector<'_, '_> { } } - fn is_cfg_enabled(&self, attrs: &Attrs) -> bool { - attrs.is_cfg_enabled(self.def_collector.cfg_options) + fn is_cfg_enabled(&self, cfg: &CfgExpr) -> bool { + self.def_collector.cfg_options.check(cfg) != Some(false) } - fn emit_unconfigured_diagnostic(&mut self, item: ModItem) { + fn emit_unconfigured_diagnostic(&mut self, item: ModItem, cfg: &CfgExpr) { let ast_id = item.ast_id(self.item_tree); let id_map = self.def_collector.db.ast_id_map(self.file_id); let syntax_ptr = id_map.get(ast_id).syntax_node_ptr(); let ast_node = InFile::new(self.file_id, syntax_ptr); - self.def_collector - .def_map - .diagnostics - .push(DefDiagnostic::unconfigured_code(self.module_id, ast_node)); + self.def_collector.def_map.diagnostics.push(DefDiagnostic::unconfigured_code( + self.module_id, + ast_node, + cfg.clone(), + self.def_collector.cfg_options.clone(), + )); } } diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs index 576b813d2..5972248de 100644 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs @@ -129,3 +129,25 @@ fn unresolved_module() { ", ); } + +#[test] +fn inactive_item() { + // Additional tests in `cfg` crate. This only tests disabled cfgs. + + check_diagnostics( + r#" + //- /lib.rs + #[cfg(no)] pub fn f() {} + //^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled + + #[cfg(no)] #[cfg(no2)] mod m; + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no and no2 are disabled + + #[cfg(all(not(a), b))] enum E {} + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: b is disabled + + #[cfg(feature = "std")] use std; + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: feature = "std" is disabled + "#, + ); +} diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 6466422c5..0332c7be0 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -2128,7 +2128,7 @@ fn foo_<|>test() {} ignore: false, }, }, - cfg_exprs: [], + cfg: None, }, ), ] @@ -2166,7 +2166,7 @@ mod tests<|> { kind: TestMod { path: "tests", }, - cfg_exprs: [], + cfg: None, }, ), ] diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index 752ef2f21..eb82456ad 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -15,7 +15,7 @@ use crate::{display::ToNav, FileId, NavigationTarget}; pub struct Runnable { pub nav: NavigationTarget, pub kind: RunnableKind, - pub cfg_exprs: Vec, + pub cfg: Option, } #[derive(Debug, Clone)] @@ -168,7 +168,7 @@ fn runnable_fn( }; let attrs = Attrs::from_attrs_owner(sema.db, InFile::new(HirFileId::from(file_id), &fn_def)); - let cfg_exprs = attrs.cfg().collect(); + let cfg = attrs.cfg(); let nav = if let RunnableKind::DocTest { .. } = kind { NavigationTarget::from_doc_commented( @@ -179,7 +179,7 @@ fn runnable_fn( } else { NavigationTarget::from_named(sema.db, InFile::new(file_id.into(), &fn_def)) }; - Some(Runnable { nav, kind, cfg_exprs }) + Some(Runnable { nav, kind, cfg }) } #[derive(Debug, Copy, Clone)] @@ -255,9 +255,9 @@ fn runnable_mod( .join("::"); let attrs = Attrs::from_attrs_owner(sema.db, InFile::new(HirFileId::from(file_id), &module)); - let cfg_exprs = attrs.cfg().collect(); + let cfg = attrs.cfg(); let nav = module_def.to_nav(sema.db); - Some(Runnable { nav, kind: RunnableKind::TestMod { path }, cfg_exprs }) + Some(Runnable { nav, kind: RunnableKind::TestMod { path }, cfg }) } // We could create runnables for modules with number_of_test_submodules > 0, @@ -348,7 +348,7 @@ fn bench() {} docs: None, }, kind: Bin, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -373,7 +373,7 @@ fn bench() {} ignore: false, }, }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -398,7 +398,7 @@ fn bench() {} ignore: true, }, }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -420,7 +420,7 @@ fn bench() {} "bench", ), }, - cfg_exprs: [], + cfg: None, }, ] "#]], @@ -507,7 +507,7 @@ fn should_have_no_runnable_6() {} docs: None, }, kind: Bin, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -527,7 +527,7 @@ fn should_have_no_runnable_6() {} "should_have_runnable", ), }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -547,7 +547,7 @@ fn should_have_no_runnable_6() {} "should_have_runnable_1", ), }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -567,7 +567,7 @@ fn should_have_no_runnable_6() {} "should_have_runnable_2", ), }, - cfg_exprs: [], + cfg: None, }, ] "#]], @@ -609,7 +609,7 @@ impl Data { docs: None, }, kind: Bin, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -629,7 +629,7 @@ impl Data { "Data::foo", ), }, - cfg_exprs: [], + cfg: None, }, ] "#]], @@ -668,7 +668,7 @@ mod test_mod { kind: TestMod { path: "test_mod", }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -693,7 +693,7 @@ mod test_mod { ignore: false, }, }, - cfg_exprs: [], + cfg: None, }, ] "#]], @@ -748,7 +748,7 @@ mod root_tests { kind: TestMod { path: "root_tests::nested_tests_0", }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -768,7 +768,7 @@ mod root_tests { kind: TestMod { path: "root_tests::nested_tests_0::nested_tests_1", }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -793,7 +793,7 @@ mod root_tests { ignore: false, }, }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -818,7 +818,7 @@ mod root_tests { ignore: false, }, }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -838,7 +838,7 @@ mod root_tests { kind: TestMod { path: "root_tests::nested_tests_0::nested_tests_2", }, - cfg_exprs: [], + cfg: None, }, Runnable { nav: NavigationTarget { @@ -863,7 +863,7 @@ mod root_tests { ignore: false, }, }, - cfg_exprs: [], + cfg: None, }, ] "#]], @@ -906,12 +906,14 @@ fn test_foo1() {} ignore: false, }, }, - cfg_exprs: [ - KeyValue { - key: "feature", - value: "foo", - }, - ], + cfg: Some( + Atom( + KeyValue { + key: "feature", + value: "foo", + }, + ), + ), }, ] "#]], @@ -954,20 +956,24 @@ fn test_foo1() {} ignore: false, }, }, - cfg_exprs: [ + cfg: Some( All( [ - KeyValue { - key: "feature", - value: "foo", - }, - KeyValue { - key: "feature", - value: "bar", - }, + Atom( + KeyValue { + key: "feature", + value: "foo", + }, + ), + Atom( + KeyValue { + key: "feature", + value: "bar", + }, + ), ], ), - ], + ), }, ] "#]], diff --git a/crates/rust-analyzer/src/cargo_target_spec.rs b/crates/rust-analyzer/src/cargo_target_spec.rs index 7da935464..1ab72bd91 100644 --- a/crates/rust-analyzer/src/cargo_target_spec.rs +++ b/crates/rust-analyzer/src/cargo_target_spec.rs @@ -24,7 +24,7 @@ impl CargoTargetSpec { snap: &GlobalStateSnapshot, spec: Option, kind: &RunnableKind, - cfgs: &[CfgExpr], + cfg: &Option, ) -> Result<(Vec, Vec)> { let mut args = Vec::new(); let mut extra_args = Vec::new(); @@ -87,7 +87,7 @@ impl CargoTargetSpec { args.push("--all-features".to_string()); } else { let mut features = Vec::new(); - for cfg in cfgs { + if let Some(cfg) = cfg.as_ref() { required_features(cfg, &mut features); } for feature in &snap.config.cargo.features { diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index aeacde0f7..121357a5a 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -745,7 +745,7 @@ pub(crate) fn runnable( let workspace_root = spec.as_ref().map(|it| it.workspace_root.clone()); let target = spec.as_ref().map(|s| s.target.clone()); let (cargo_args, executable_args) = - CargoTargetSpec::runnable_args(snap, spec, &runnable.kind, &runnable.cfg_exprs)?; + CargoTargetSpec::runnable_args(snap, spec, &runnable.kind, &runnable.cfg)?; let label = runnable.label(target); let location = location_link(snap, None, runnable.nav)?; -- cgit v1.2.3