Continue improvements on the --check-cfg implementation

- Test the combinations of --check-cfg with partial values() and --cfg
- Test that we detect unexpected value when none are expected
This commit is contained in:
Loïc BRANSTETT 2022-02-20 01:26:52 +01:00
parent 3d234770b1
commit 8d3de56da1
9 changed files with 184 additions and 55 deletions

View File

@ -462,7 +462,8 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
true
}
MetaItemKind::NameValue(..) | MetaItemKind::Word => {
let name = cfg.ident().expect("multi-segment cfg predicate").name;
let ident = cfg.ident().expect("multi-segment cfg predicate");
let name = ident.name;
let value = cfg.value_str();
if let Some(names_valid) = &sess.check_config.names_valid {
if !names_valid.contains(&name) {
@ -471,18 +472,13 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
cfg.span,
CRATE_NODE_ID,
"unexpected `cfg` condition name",
BuiltinLintDiagnostics::UnexpectedCfg(
cfg.ident().unwrap().span,
name,
None,
),
BuiltinLintDiagnostics::UnexpectedCfg(ident.span, name, None),
);
}
}
if let Some(val) = value {
if let Some(values_valid) = &sess.check_config.values_valid {
if let Some(values) = values_valid.get(&name) {
if !values.contains(&val) {
if let Some(value) = value {
if let Some(values) = &sess.check_config.values_valid.get(&name) {
if !values.contains(&value) {
sess.buffer_lint_with_diagnostic(
UNEXPECTED_CFGS,
cfg.span,
@ -491,13 +487,12 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
BuiltinLintDiagnostics::UnexpectedCfg(
cfg.name_value_literal_span().unwrap(),
name,
Some(val),
Some(value),
),
);
}
}
}
}
sess.config.contains(&(name, value))
}
}

View File

@ -183,12 +183,10 @@ macro_rules! error {
} else if meta_item.has_name(sym::values) {
if let Some((name, values)) = args.split_first() {
if name.is_word() && name.ident().is_some() {
let values_valid = cfg
.values_valid
.get_or_insert_with(|| FxHashMap::default());
let ident = name.ident().expect("multi-segment cfg key");
let ident_values = values_valid
.entry(ident.to_string())
let ident_values = cfg
.values_valid
.entry(ident.name.to_string())
.or_insert_with(|| FxHashSet::default());
for val in values {
@ -225,10 +223,8 @@ macro_rules! error {
);
}
if let Some(values_valid) = &cfg.values_valid {
if let Some(names_valid) = &mut cfg.names_valid {
names_valid.extend(values_valid.keys().cloned());
}
names_valid.extend(cfg.values_valid.keys().cloned());
}
cfg
})

View File

@ -768,17 +768,14 @@ fn lookup_with_diagnostics(
db.note("see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information");
},
BuiltinLintDiagnostics::UnexpectedCfg(span, name, value) => {
let mut possibilities: Vec<Symbol> = if value.is_some() {
let Some(values_valid) = &sess.parse_sess.check_config.values_valid else {
bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable");
};
let Some(values) = values_valid.get(&name) else {
let possibilities: Vec<Symbol> = if value.is_some() {
let Some(values) = &sess.parse_sess.check_config.values_valid.get(&name) else {
bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values");
};
values.iter().map(|&s| s).collect()
} else {
let Some(names_valid) = &sess.parse_sess.check_config.names_valid else {
bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable");
bug!("it shouldn't be possible to have a diagnostic on a name if name checking is not enabled");
};
names_valid.iter().map(|s| *s).collect()
};
@ -786,17 +783,21 @@ fn lookup_with_diagnostics(
// Show the full list if all possible values for a given name, but don't do it
// for names as the possibilities could be very long
if value.is_some() {
// Sorting can take some time, so we only do it if required
if !possibilities.is_empty() {
let mut possibilities = possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>();
possibilities.sort();
let possibilities = possibilities.iter().map(Symbol::as_str).intersperse(", ").collect::<String>();
db.note(&format!("possible values for `{name}` are: {possibilities}"));
let possibilities = possibilities.join(", ");
db.note(&format!("expected values for `{name}` are: {possibilities}"));
} else {
db.note(&format!("no expected value for `{name}`"));
}
}
// Suggest the most probable if we found one
if let Some(best_match) = find_best_match_for_name(&possibilities, value.unwrap_or(name), None) {
let ponctuation = if value.is_some() { "\"" } else { "" };
db.span_suggestion(span, "did you mean", format!("{ponctuation}{best_match}{ponctuation}"), Applicability::MaybeIncorrect);
let punctuation = if value.is_some() { "\"" } else { "" };
db.span_suggestion(span, "did you mean", format!("{punctuation}{best_match}{punctuation}"), Applicability::MaybeIncorrect);
}
},
}

View File

@ -1023,10 +1023,10 @@ pub fn to_crate_config(cfg: FxHashSet<(String, Option<String>)>) -> CrateConfig
/// The parsed `--check-cfg` options
pub struct CheckCfg<T = String> {
/// The set of all `names()`, if none no names checking is performed
/// The set of all `names()`, if None no name checking is performed
pub names_valid: Option<FxHashSet<T>>,
/// The set of all `values()`, if none no values chcking is performed
pub values_valid: Option<FxHashMap<T, FxHashSet<T>>>,
/// The set of all `values()`
pub values_valid: FxHashMap<T, FxHashSet<T>>,
}
impl<T> Default for CheckCfg<T> {
@ -1042,9 +1042,11 @@ fn map_data<O: Eq + Hash>(&self, f: impl Fn(&T) -> O) -> CheckCfg<O> {
.names_valid
.as_ref()
.map(|names_valid| names_valid.iter().map(|a| f(a)).collect()),
values_valid: self.values_valid.as_ref().map(|values_valid| {
values_valid.iter().map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect())).collect()
}),
values_valid: self
.values_valid
.iter()
.map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect()))
.collect(),
}
}
}
@ -1098,14 +1100,12 @@ pub fn fill_actual(&mut self, cfg: &CrateConfig) {
names_valid.insert(k);
}
if let Some(v) = v {
if let Some(values_valid) = &mut self.values_valid {
values_valid.entry(k).and_modify(|values| {
self.values_valid.entry(k).and_modify(|values| {
values.insert(v);
});
}
}
}
}
}
pub fn build_configuration(sess: &Session, mut user_cfg: CrateConfig) -> CrateConfig {

View File

@ -5,7 +5,7 @@ LL | #[cfg(feature = "sedre")]
| ^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unexpected_cfgs)]` on by default
= note: possible values for `feature` are: rand, serde, full
= note: expected values for `feature` are: full, rand, serde
warning: 1 warning emitted

View File

@ -0,0 +1,50 @@
// This test checks the combination of well known names, their activation via names(), the usage of
// partial values() with a --cfg and test that we also correctly lint on the `cfg!` macro and
// `cfg_attr` attribute.
//
// check-pass
// compile-flags: --check-cfg=names() --check-cfg=values(feature,"foo") --cfg feature="bar" -Z unstable-options
#[cfg(windows)]
fn do_windows_stuff() {}
#[cfg(widnows)]
//~^ WARNING unexpected `cfg` condition name
fn do_windows_stuff() {}
#[cfg(feature = "foo")]
fn use_foo() {}
#[cfg(feature = "bar")]
fn use_bar() {}
#[cfg(feature = "zebra")]
//~^ WARNING unexpected `cfg` condition value
fn use_zebra() {}
#[cfg_attr(uu, test)]
//~^ WARNING unexpected `cfg` condition name
fn do_test() {}
#[cfg_attr(feature = "foo", no_mangle)]
fn do_test_foo() {}
fn test_cfg_macro() {
cfg!(windows);
cfg!(widnows);
//~^ WARNING unexpected `cfg` condition name
cfg!(feature = "foo");
cfg!(feature = "bar");
cfg!(feature = "zebra");
//~^ WARNING unexpected `cfg` condition value
cfg!(xxx = "foo");
//~^ WARNING unexpected `cfg` condition name
cfg!(xxx);
//~^ WARNING unexpected `cfg` condition name
cfg!(any(xxx, windows));
//~^ WARNING unexpected `cfg` condition name
cfg!(any(feature = "bad", windows));
//~^ WARNING unexpected `cfg` condition value
}
fn main() {}

View File

@ -0,0 +1,66 @@
warning: unexpected `cfg` condition name
--> $DIR/mix.rs:11:7
|
LL | #[cfg(widnows)]
| ^^^^^^^ help: did you mean: `windows`
|
= note: `#[warn(unexpected_cfgs)]` on by default
warning: unexpected `cfg` condition value
--> $DIR/mix.rs:21:7
|
LL | #[cfg(feature = "zebra")]
| ^^^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: bar, foo
warning: unexpected `cfg` condition name
--> $DIR/mix.rs:25:12
|
LL | #[cfg_attr(uu, test)]
| ^^
warning: unexpected `cfg` condition name
--> $DIR/mix.rs:34:10
|
LL | cfg!(widnows);
| ^^^^^^^ help: did you mean: `windows`
warning: unexpected `cfg` condition value
--> $DIR/mix.rs:38:10
|
LL | cfg!(feature = "zebra");
| ^^^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: bar, foo
warning: unexpected `cfg` condition name
--> $DIR/mix.rs:40:10
|
LL | cfg!(xxx = "foo");
| ^^^^^^^^^^^
warning: unexpected `cfg` condition name
--> $DIR/mix.rs:42:10
|
LL | cfg!(xxx);
| ^^^
warning: unexpected `cfg` condition name
--> $DIR/mix.rs:44:14
|
LL | cfg!(any(xxx, windows));
| ^^^
warning: unexpected `cfg` condition value
--> $DIR/mix.rs:46:14
|
LL | cfg!(any(feature = "bad", windows));
| ^^^^^^^^^^-----
| |
| help: did you mean: `"bar"`
|
= note: expected values for `feature` are: bar, foo
warning: 9 warnings emitted

View File

@ -0,0 +1,10 @@
// Check that we detect unexpected value when none are allowed
//
// check-pass
// compile-flags: --check-cfg=values(feature) -Z unstable-options
#[cfg(feature = "foo")]
//~^ WARNING unexpected `cfg` condition value
fn do_foo() {}
fn main() {}

View File

@ -0,0 +1,11 @@
warning: unexpected `cfg` condition value
--> $DIR/no-values.rs:6:7
|
LL | #[cfg(feature = "foo")]
| ^^^^^^^^^^^^^^^
|
= note: `#[warn(unexpected_cfgs)]` on by default
= note: no expected value for `feature`
warning: 1 warning emitted