Emit better #[cfg] diagnostics

This commit is contained in:
Jonas Schievink 2020-10-22 19:19:18 +02:00
parent 978cc93649
commit 3421b645e6
9 changed files with 124 additions and 65 deletions

View File

@ -125,12 +125,20 @@ impl Attrs {
AttrQuery { attrs: self, key }
}
pub fn cfg(&self) -> impl Iterator<Item = CfgExpr> + '_ {
pub fn cfg(&self) -> Option<CfgExpr> {
// 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::<Vec<_>>();
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),
}
}
}

View File

@ -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<SyntaxNodePtr> {
InFile::new(self.file, self.node.clone())

View File

@ -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<ast::Use>, index: usize },
UnconfiguredCode { ast: InFile<SyntaxNodePtr> },
UnconfiguredCode { ast: InFile<SyntaxNodePtr>, cfg: CfgExpr, opts: CfgOptions },
}
#[derive(Debug, PartialEq, Eq)]
@ -341,8 +342,10 @@ mod diagnostics {
pub(super) fn unconfigured_code(
container: LocalModuleId,
ast: InFile<SyntaxNodePtr>,
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(),
});
}
}
}

View File

@ -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(),
));
}
}

View File

@ -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
"#,
);
}

View File

@ -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,
},
),
]

View File

@ -15,7 +15,7 @@ use crate::{display::ToNav, FileId, NavigationTarget};
pub struct Runnable {
pub nav: NavigationTarget,
pub kind: RunnableKind,
pub cfg_exprs: Vec<CfgExpr>,
pub cfg: Option<CfgExpr>,
}
#[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",
},
),
],
),
],
),
},
]
"#]],

View File

@ -24,7 +24,7 @@ impl CargoTargetSpec {
snap: &GlobalStateSnapshot,
spec: Option<CargoTargetSpec>,
kind: &RunnableKind,
cfgs: &[CfgExpr],
cfg: &Option<CfgExpr>,
) -> Result<(Vec<String>, Vec<String>)> {
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 {

View File

@ -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)?;