Skip to content

Commit 5364784

Browse files
UrgauUrgau
committed
Improve check-cfg diagnostics (part 2)
1 parent a5f8dba commit 5364784

File tree

12 files changed

+192
-47
lines changed

12 files changed

+192
-47
lines changed

‎compiler/rustc_attr/src/builtin.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ pub fn cfg_matches(
589589
cfg.span,
590590
lint_node_id,
591591
"unexpected `cfg` condition value",
592-
BuiltinLintDiagnostics::UnexpectedCfg(
592+
BuiltinLintDiagnostics::UnexpectedCfgValue(
593593
(cfg.name, cfg.name_span),
594594
cfg.value.map(|v| (v, cfg.value_span.unwrap())),
595595
),
@@ -601,7 +601,10 @@ pub fn cfg_matches(
601601
cfg.span,
602602
lint_node_id,
603603
"unexpected `cfg` condition name",
604-
BuiltinLintDiagnostics::UnexpectedCfg((cfg.name, cfg.name_span), None),
604+
BuiltinLintDiagnostics::UnexpectedCfgName(
605+
(cfg.name, cfg.name_span),
606+
cfg.value.map(|v| (v, cfg.value_span.unwrap())),
607+
),
605608
);
606609
}
607610
_ => { /* not unexpected */ }

‎compiler/rustc_lint/src/context.rs

+45-9
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ use rustc_middle::middle::stability;
3636
use rustc_middle::ty::layout::{LayoutError, LayoutOfHelpers, TyAndLayout};
3737
use rustc_middle::ty::print::with_no_trimmed_paths;
3838
use rustc_middle::ty::{self, print::Printer, subst::GenericArg, RegisteredTools, Ty, TyCtxt};
39+
use rustc_session::config::ExpectedValues;
3940
use rustc_session::lint::{BuiltinLintDiagnostics, LintExpectationId};
4041
use rustc_session::lint::{FutureIncompatibleInfo, Level, Lint, LintBuffer, LintId};
4142
use rustc_session::Session;
@@ -768,23 +769,51 @@ pub trait LintContext: Sized {
768769
db.help(help);
769770
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");
770771
},
771-
BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), None) => {
772+
BuiltinLintDiagnostics::UnexpectedCfgName((name, name_span), value) => {
772773
let possibilities: Vec<Symbol> = sess.parse_sess.check_config.expecteds.keys().map(|s| *s).collect();
773774

774775
// Suggest the most probable if we found one
775776
if let Some(best_match) = find_best_match_for_name(&possibilities, name, None) {
776-
db.span_suggestion(name_span, "there is an config with a similar name", best_match, Applicability::MaybeIncorrect);
777+
if let Some(ExpectedValues::Some(best_match_values)) =
778+
sess.parse_sess.check_config.expecteds.get(&best_match) {
779+
let mut possibilities = best_match_values.iter()
780+
.flatten()
781+
.map(Symbol::as_str)
782+
.collect::<Vec<_>>();
783+
possibilities.sort();
784+
785+
if let Some((value, value_span)) = value {
786+
if best_match_values.contains(&Some(value)) {
787+
db.span_suggestion(name_span, "there is a config with a similar name and value", best_match, Applicability::MaybeIncorrect);
788+
} else if best_match_values.contains(&None) {
789+
db.span_suggestion(name_span.to(value_span), "there is a config with a similar name and no value", best_match, Applicability::MaybeIncorrect);
790+
} else if let Some(first_value) = possibilities.first() {
791+
db.span_suggestion(name_span.to(value_span), "there is a config with a similar name and different values", format!("{best_match} = \"{first_value}\""), Applicability::MaybeIncorrect);
792+
} else {
793+
db.span_suggestion(name_span.to(value_span), "there is a config with a similar name and different values", best_match, Applicability::MaybeIncorrect);
794+
};
795+
} else {
796+
db.span_suggestion(name_span, "there is a config with a similar name", best_match, Applicability::MaybeIncorrect);
797+
}
798+
799+
if !possibilities.is_empty() {
800+
let possibilities = possibilities.join("`, `");
801+
db.help(format!("expected values for `{best_match}` are: `{possibilities}`"));
802+
}
803+
} else {
804+
db.span_suggestion(name_span, "there is a config with a similar name", best_match, Applicability::MaybeIncorrect);
805+
}
777806
}
778807
},
779-
BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), Some((value, value_span))) => {
780-
let Some(rustc_session::config::ExpectedValues::Some(values)) = &sess.parse_sess.check_config.expecteds.get(&name) else {
808+
BuiltinLintDiagnostics::UnexpectedCfgValue((name, name_span), value) => {
809+
let Some(ExpectedValues::Some(values)) = &sess.parse_sess.check_config.expecteds.get(&name) else {
781810
bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values");
782811
};
783812
let mut have_none_possibility = false;
784813
let possibilities: Vec<Symbol> = values.iter()
785814
.inspect(|a| have_none_possibility |= a.is_none())
786815
.copied()
787-
.filter_map(std::convert::identity)
816+
.flatten()
788817
.collect();
789818

790819
// Show the full list if all possible values for a given name, but don't do it
@@ -800,13 +829,20 @@ pub trait LintContext: Sized {
800829
db.note(format!("expected values for `{name}` are: {none}`{possibilities}`"));
801830
}
802831

803-
// Suggest the most probable if we found one
804-
if let Some(best_match) = find_best_match_for_name(&possibilities, value, None) {
805-
db.span_suggestion(value_span, "there is an expected value with a similar name", format!("\"{best_match}\""), Applicability::MaybeIncorrect);
832+
if let Some((value, value_span)) = value {
833+
// Suggest the most probable if we found one
834+
if let Some(best_match) = find_best_match_for_name(&possibilities, value, None) {
835+
db.span_suggestion(value_span, "there is a expected value with a similar name", format!("\"{best_match}\""), Applicability::MaybeIncorrect);
836+
837+
}
838+
} else if let &[first_possibility] = &possibilities[..] {
839+
db.span_suggestion(name_span.shrink_to_hi(), "specify a config value", format!(" = \"{first_possibility}\""), Applicability::MaybeIncorrect);
806840
}
807841
} else if have_none_possibility {
808842
db.note(format!("no expected value for `{name}`"));
809-
db.span_suggestion(name_span.shrink_to_hi().to(value_span), "remove the value", "", Applicability::MaybeIncorrect);
843+
if let Some((_value, value_span)) = value {
844+
db.span_suggestion(name_span.shrink_to_hi().to(value_span), "remove the value", "", Applicability::MaybeIncorrect);
845+
}
810846
}
811847
},
812848
BuiltinLintDiagnostics::DeprecatedWhereclauseLocation(new_span, suggestion) => {

‎compiler/rustc_lint_defs/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,8 @@ pub enum BuiltinLintDiagnostics {
496496
BreakWithLabelAndLoop(Span),
497497
NamedAsmLabel(String),
498498
UnicodeTextFlow(Span, String),
499-
UnexpectedCfg((Symbol, Span), Option<(Symbol, Span)>),
499+
UnexpectedCfgName((Symbol, Span), Option<(Symbol, Span)>),
500+
UnexpectedCfgValue((Symbol, Span), Option<(Symbol, Span)>),
500501
DeprecatedWhereclauseLocation(Span, String),
501502
SingleUseLifetime {
502503
/// Span of the parameter which declares this lifetime.

‎tests/ui/check-cfg/diagnotics.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// check-pass
2+
// compile-flags: --check-cfg=names() --check-cfg=values(feature,"foo") --check-cfg=values(no_values) -Z unstable-options
3+
4+
#[cfg(featur)]
5+
//~^ WARNING unexpected `cfg` condition name
6+
fn feature() {}
7+
8+
#[cfg(featur = "foo")]
9+
//~^ WARNING unexpected `cfg` condition name
10+
fn feature() {}
11+
12+
#[cfg(featur = "fo")]
13+
//~^ WARNING unexpected `cfg` condition name
14+
fn feature() {}
15+
16+
#[cfg(feature = "foo")]
17+
fn feature() {}
18+
19+
#[cfg(no_value)]
20+
//~^ WARNING unexpected `cfg` condition name
21+
fn no_values() {}
22+
23+
#[cfg(no_value = "foo")]
24+
//~^ WARNING unexpected `cfg` condition name
25+
fn no_values() {}
26+
27+
#[cfg(no_values = "bar")]
28+
//~^ WARNING unexpected `cfg` condition value
29+
fn no_values() {}
30+
31+
fn main() {}

‎tests/ui/check-cfg/diagnotics.stderr

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
warning: unexpected `cfg` condition name
2+
--> $DIR/diagnotics.rs:4:7
3+
|
4+
LL | #[cfg(featur)]
5+
| ^^^^^^ help: there is a config with a similar name: `feature`
6+
|
7+
= help: expected values for `feature` are: `foo`
8+
= note: `#[warn(unexpected_cfgs)]` on by default
9+
10+
warning: unexpected `cfg` condition name
11+
--> $DIR/diagnotics.rs:8:7
12+
|
13+
LL | #[cfg(featur = "foo")]
14+
| ^^^^^^^^^^^^^^
15+
|
16+
= help: expected values for `feature` are: `foo`
17+
help: there is a config with a similar name and value
18+
|
19+
LL | #[cfg(feature = "foo")]
20+
| ~~~~~~~
21+
22+
warning: unexpected `cfg` condition name
23+
--> $DIR/diagnotics.rs:12:7
24+
|
25+
LL | #[cfg(featur = "fo")]
26+
| ^^^^^^^^^^^^^
27+
|
28+
= help: expected values for `feature` are: `foo`
29+
help: there is a config with a similar name and different values
30+
|
31+
LL | #[cfg(feature = "foo")]
32+
| ~~~~~~~~~~~~~~~
33+
34+
warning: unexpected `cfg` condition name
35+
--> $DIR/diagnotics.rs:19:7
36+
|
37+
LL | #[cfg(no_value)]
38+
| ^^^^^^^^ help: there is a config with a similar name: `no_values`
39+
40+
warning: unexpected `cfg` condition name
41+
--> $DIR/diagnotics.rs:23:7
42+
|
43+
LL | #[cfg(no_value = "foo")]
44+
| ^^^^^^^^^^^^^^^^
45+
|
46+
help: there is a config with a similar name and no value
47+
|
48+
LL | #[cfg(no_values)]
49+
| ~~~~~~~~~
50+
51+
warning: unexpected `cfg` condition value
52+
--> $DIR/diagnotics.rs:27:7
53+
|
54+
LL | #[cfg(no_values = "bar")]
55+
| ^^^^^^^^^--------
56+
| |
57+
| help: remove the value
58+
|
59+
= note: no expected value for `no_values`
60+
61+
warning: 6 warnings emitted
62+

‎tests/ui/check-cfg/invalid-cfg-name.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ warning: unexpected `cfg` condition name
22
--> $DIR/invalid-cfg-name.rs:7:7
33
|
44
LL | #[cfg(widnows)]
5-
| ^^^^^^^ help: there is an config with a similar name: `windows`
5+
| ^^^^^^^ help: there is a config with a similar name: `windows`
66
|
77
= note: `#[warn(unexpected_cfgs)]` on by default
88

‎tests/ui/check-cfg/invalid-cfg-value.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ warning: unexpected `cfg` condition value
44
LL | #[cfg(feature = "sedre")]
55
| ^^^^^^^^^^-------
66
| |
7-
| help: there is an expected value with a similar name: `"serde"`
7+
| help: there is a expected value with a similar name: `"serde"`
88
|
99
= note: expected values for `feature` are: `full`, `serde`
1010
= note: `#[warn(unexpected_cfgs)]` on by default

‎tests/ui/check-cfg/mix.rs

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ fn do_windows_stuff() {}
1212
//~^ WARNING unexpected `cfg` condition name
1313
fn do_windows_stuff() {}
1414

15+
#[cfg(feature)]
16+
//~^ WARNING unexpected `cfg` condition value
17+
fn no_feature() {}
18+
1519
#[cfg(feature = "foo")]
1620
fn use_foo() {}
1721

0 commit comments

Comments
 (0)