Skip to content

Commit 8d3de56

Browse files
UrgauUrgau
committed
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
1 parent 3d23477 commit 8d3de56

File tree

9 files changed

+184
-55
lines changed

9 files changed

+184
-55
lines changed

‎compiler/rustc_attr/src/builtin.rs‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,8 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
462462
true
463463
}
464464
MetaItemKind::NameValue(..) | MetaItemKind::Word => {
465-
let name = cfg.ident().expect("multi-segment cfg predicate").name;
465+
let ident = cfg.ident().expect("multi-segment cfg predicate");
466+
let name = ident.name;
466467
let value = cfg.value_str();
467468
if let Some(names_valid) = &sess.check_config.names_valid {
468469
if !names_valid.contains(&name) {
@@ -471,30 +472,24 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
471472
cfg.span,
472473
CRATE_NODE_ID,
473474
"unexpected `cfg` condition name",
474-
BuiltinLintDiagnostics::UnexpectedCfg(
475-
cfg.ident().unwrap().span,
476-
name,
477-
None,
478-
),
475+
BuiltinLintDiagnostics::UnexpectedCfg(ident.span, name, None),
479476
);
480477
}
481478
}
482-
if let Some(val) = value {
483-
if let Some(values_valid) = &sess.check_config.values_valid {
484-
if let Some(values) = values_valid.get(&name) {
485-
if !values.contains(&val) {
486-
sess.buffer_lint_with_diagnostic(
487-
UNEXPECTED_CFGS,
488-
cfg.span,
489-
CRATE_NODE_ID,
490-
"unexpected `cfg` condition value",
491-
BuiltinLintDiagnostics::UnexpectedCfg(
492-
cfg.name_value_literal_span().unwrap(),
493-
name,
494-
Some(val),
495-
),
496-
);
497-
}
479+
if let Some(value) = value {
480+
if let Some(values) = &sess.check_config.values_valid.get(&name) {
481+
if !values.contains(&value) {
482+
sess.buffer_lint_with_diagnostic(
483+
UNEXPECTED_CFGS,
484+
cfg.span,
485+
CRATE_NODE_ID,
486+
"unexpected `cfg` condition value",
487+
BuiltinLintDiagnostics::UnexpectedCfg(
488+
cfg.name_value_literal_span().unwrap(),
489+
name,
490+
Some(value),
491+
),
492+
);
498493
}
499494
}
500495
}

‎compiler/rustc_interface/src/interface.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -183,12 +183,10 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
183183
} else if meta_item.has_name(sym::values) {
184184
if let Some((name, values)) = args.split_first() {
185185
if name.is_word() && name.ident().is_some() {
186-
let values_valid = cfg
187-
.values_valid
188-
.get_or_insert_with(|| FxHashMap::default());
189186
let ident = name.ident().expect("multi-segment cfg key");
190-
let ident_values = values_valid
191-
.entry(ident.to_string())
187+
let ident_values = cfg
188+
.values_valid
189+
.entry(ident.name.to_string())
192190
.or_insert_with(|| FxHashSet::default());
193191

194192
for val in values {
@@ -225,10 +223,8 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
225223
);
226224
}
227225

228-
if let Some(values_valid) = &cfg.values_valid {
229-
if let Some(names_valid) = &mut cfg.names_valid {
230-
names_valid.extend(values_valid.keys().cloned());
231-
}
226+
if let Some(names_valid) = &mut cfg.names_valid {
227+
names_valid.extend(cfg.values_valid.keys().cloned());
232228
}
233229
cfg
234230
})

‎compiler/rustc_lint/src/context.rs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -768,35 +768,36 @@ pub trait LintContext: Sized {
768768
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");
769769
},
770770
BuiltinLintDiagnostics::UnexpectedCfg(span, name, value) => {
771-
let mut possibilities: Vec<Symbol> = if value.is_some() {
772-
let Some(values_valid) = &sess.parse_sess.check_config.values_valid else {
773-
bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable");
774-
};
775-
let Some(values) = values_valid.get(&name) else {
771+
let possibilities: Vec<Symbol> = if value.is_some() {
772+
let Some(values) = &sess.parse_sess.check_config.values_valid.get(&name) else {
776773
bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values");
777774
};
778775
values.iter().map(|&s| s).collect()
779776
} else {
780777
let Some(names_valid) = &sess.parse_sess.check_config.names_valid else {
781-
bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable");
778+
bug!("it shouldn't be possible to have a diagnostic on a name if name checking is not enabled");
782779
};
783780
names_valid.iter().map(|s| *s).collect()
784781
};
785782

786783
// Show the full list if all possible values for a given name, but don't do it
787784
// for names as the possibilities could be very long
788785
if value.is_some() {
789-
// Sorting can take some time, so we only do it if required
790-
possibilities.sort();
786+
if !possibilities.is_empty() {
787+
let mut possibilities = possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>();
788+
possibilities.sort();
791789

792-
let possibilities = possibilities.iter().map(Symbol::as_str).intersperse(", ").collect::<String>();
793-
db.note(&format!("possible values for `{name}` are: {possibilities}"));
790+
let possibilities = possibilities.join(", ");
791+
db.note(&format!("expected values for `{name}` are: {possibilities}"));
792+
} else {
793+
db.note(&format!("no expected value for `{name}`"));
794+
}
794795
}
795796

796797
// Suggest the most probable if we found one
797798
if let Some(best_match) = find_best_match_for_name(&possibilities, value.unwrap_or(name), None) {
798-
let ponctuation = if value.is_some() { "\"" } else { "" };
799-
db.span_suggestion(span, "did you mean", format!("{ponctuation}{best_match}{ponctuation}"), Applicability::MaybeIncorrect);
799+
let punctuation = if value.is_some() { "\"" } else { "" };
800+
db.span_suggestion(span, "did you mean", format!("{punctuation}{best_match}{punctuation}"), Applicability::MaybeIncorrect);
800801
}
801802
},
802803
}

‎compiler/rustc_session/src/config.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,10 +1023,10 @@ pub fn to_crate_config(cfg: FxHashSet<(String, Option<String>)>) -> CrateConfig
10231023

10241024
/// The parsed `--check-cfg` options
10251025
pub struct CheckCfg<T = String> {
1026-
/// The set of all `names()`, if none no names checking is performed
1026+
/// The set of all `names()`, if None no name checking is performed
10271027
pub names_valid: Option<FxHashSet<T>>,
1028-
/// The set of all `values()`, if none no values chcking is performed
1029-
pub values_valid: Option<FxHashMap<T, FxHashSet<T>>>,
1028+
/// The set of all `values()`
1029+
pub values_valid: FxHashMap<T, FxHashSet<T>>,
10301030
}
10311031

10321032
impl<T> Default for CheckCfg<T> {
@@ -1042,9 +1042,11 @@ impl<T> CheckCfg<T> {
10421042
.names_valid
10431043
.as_ref()
10441044
.map(|names_valid| names_valid.iter().map(|a| f(a)).collect()),
1045-
values_valid: self.values_valid.as_ref().map(|values_valid| {
1046-
values_valid.iter().map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect())).collect()
1047-
}),
1045+
values_valid: self
1046+
.values_valid
1047+
.iter()
1048+
.map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect()))
1049+
.collect(),
10481050
}
10491051
}
10501052
}
@@ -1098,11 +1100,9 @@ impl CrateCheckConfig {
10981100
names_valid.insert(k);
10991101
}
11001102
if let Some(v) = v {
1101-
if let Some(values_valid) = &mut self.values_valid {
1102-
values_valid.entry(k).and_modify(|values| {
1103-
values.insert(v);
1104-
});
1105-
}
1103+
self.values_valid.entry(k).and_modify(|values| {
1104+
values.insert(v);
1105+
});
11061106
}
11071107
}
11081108
}

‎src/test/ui/check-cfg/invalid-cfg-value.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | #[cfg(feature = "sedre")]
55
| ^^^^^^^^^^^^^^^^^
66
|
77
= note: `#[warn(unexpected_cfgs)]` on by default
8-
= note: possible values for `feature` are: rand, serde, full
8+
= note: expected values for `feature` are: full, rand, serde
99

1010
warning: 1 warning emitted
1111

‎src/test/ui/check-cfg/mix.rs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// This test checks the combination of well known names, their activation via names(), the usage of
2+
// partial values() with a --cfg and test that we also correctly lint on the `cfg!` macro and
3+
// `cfg_attr` attribute.
4+
//
5+
// check-pass
6+
// compile-flags: --check-cfg=names() --check-cfg=values(feature,"foo") --cfg feature="bar" -Z unstable-options
7+
8+
#[cfg(windows)]
9+
fn do_windows_stuff() {}
10+
11+
#[cfg(widnows)]
12+
//~^ WARNING unexpected `cfg` condition name
13+
fn do_windows_stuff() {}
14+
15+
#[cfg(feature = "foo")]
16+
fn use_foo() {}
17+
18+
#[cfg(feature = "bar")]
19+
fn use_bar() {}
20+
21+
#[cfg(feature = "zebra")]
22+
//~^ WARNING unexpected `cfg` condition value
23+
fn use_zebra() {}
24+
25+
#[cfg_attr(uu, test)]
26+
//~^ WARNING unexpected `cfg` condition name
27+
fn do_test() {}
28+
29+
#[cfg_attr(feature = "foo", no_mangle)]
30+
fn do_test_foo() {}
31+
32+
fn test_cfg_macro() {
33+
cfg!(windows);
34+
cfg!(widnows);
35+
//~^ WARNING unexpected `cfg` condition name
36+
cfg!(feature = "foo");
37+
cfg!(feature = "bar");
38+
cfg!(feature = "zebra");
39+
//~^ WARNING unexpected `cfg` condition value
40+
cfg!(xxx = "foo");
41+
//~^ WARNING unexpected `cfg` condition name
42+
cfg!(xxx);
43+
//~^ WARNING unexpected `cfg` condition name
44+
cfg!(any(xxx, windows));
45+
//~^ WARNING unexpected `cfg` condition name
46+
cfg!(any(feature = "bad", windows));
47+
//~^ WARNING unexpected `cfg` condition value
48+
}
49+
50+
fn main() {}

‎src/test/ui/check-cfg/mix.stderr‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
warning: unexpected `cfg` condition name
2+
--> $DIR/mix.rs:11:7
3+
|
4+
LL | #[cfg(widnows)]
5+
| ^^^^^^^ help: did you mean: `windows`
6+
|
7+
= note: `#[warn(unexpected_cfgs)]` on by default
8+
9+
warning: unexpected `cfg` condition value
10+
--> $DIR/mix.rs:21:7
11+
|
12+
LL | #[cfg(feature = "zebra")]
13+
| ^^^^^^^^^^^^^^^^^
14+
|
15+
= note: expected values for `feature` are: bar, foo
16+
17+
warning: unexpected `cfg` condition name
18+
--> $DIR/mix.rs:25:12
19+
|
20+
LL | #[cfg_attr(uu, test)]
21+
| ^^
22+
23+
warning: unexpected `cfg` condition name
24+
--> $DIR/mix.rs:34:10
25+
|
26+
LL | cfg!(widnows);
27+
| ^^^^^^^ help: did you mean: `windows`
28+
29+
warning: unexpected `cfg` condition value
30+
--> $DIR/mix.rs:38:10
31+
|
32+
LL | cfg!(feature = "zebra");
33+
| ^^^^^^^^^^^^^^^^^
34+
|
35+
= note: expected values for `feature` are: bar, foo
36+
37+
warning: unexpected `cfg` condition name
38+
--> $DIR/mix.rs:40:10
39+
|
40+
LL | cfg!(xxx = "foo");
41+
| ^^^^^^^^^^^
42+
43+
warning: unexpected `cfg` condition name
44+
--> $DIR/mix.rs:42:10
45+
|
46+
LL | cfg!(xxx);
47+
| ^^^
48+
49+
warning: unexpected `cfg` condition name
50+
--> $DIR/mix.rs:44:14
51+
|
52+
LL | cfg!(any(xxx, windows));
53+
| ^^^
54+
55+
warning: unexpected `cfg` condition value
56+
--> $DIR/mix.rs:46:14
57+
|
58+
LL | cfg!(any(feature = "bad", windows));
59+
| ^^^^^^^^^^-----
60+
| |
61+
| help: did you mean: `"bar"`
62+
|
63+
= note: expected values for `feature` are: bar, foo
64+
65+
warning: 9 warnings emitted
66+

‎src/test/ui/check-cfg/no-values.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Check that we detect unexpected value when none are allowed
2+
//
3+
// check-pass
4+
// compile-flags: --check-cfg=values(feature) -Z unstable-options
5+
6+
#[cfg(feature = "foo")]
7+
//~^ WARNING unexpected `cfg` condition value
8+
fn do_foo() {}
9+
10+
fn main() {}

‎src/test/ui/check-cfg/no-values.stderr‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
warning: unexpected `cfg` condition value
2+
--> $DIR/no-values.rs:6:7
3+
|
4+
LL | #[cfg(feature = "foo")]
5+
| ^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(unexpected_cfgs)]` on by default
8+
= note: no expected value for `feature`
9+
10+
warning: 1 warning emitted
11+

0 commit comments

Comments
 (0)