Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve check-cfg diagnostics #116666

Merged
merged 4 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,11 @@ pub fn cfg_matches(
UNEXPECTED_CFGS,
cfg.span,
lint_node_id,
"unexpected `cfg` condition value",
if let Some(value) = cfg.value {
format!("unexpected `cfg` condition value: `{value}`")
} else {
format!("unexpected `cfg` condition value: (none)")
},
BuiltinLintDiagnostics::UnexpectedCfgValue(
(cfg.name, cfg.name_span),
cfg.value.map(|v| (v, cfg.value_span.unwrap())),
Expand All @@ -560,7 +564,7 @@ pub fn cfg_matches(
UNEXPECTED_CFGS,
cfg.span,
lint_node_id,
"unexpected `cfg` condition name",
format!("unexpected `cfg` condition name: `{}`", cfg.name),
BuiltinLintDiagnostics::UnexpectedCfgName(
(cfg.name, cfg.name_span),
cfg.value.map(|v| (v, cfg.value_span.unwrap())),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
msg: impl Into<SubdiagnosticMessage>,
) -> &mut Self);
forward!(pub fn help(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
forward!(pub fn help_once(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
forward!(pub fn span_help(
&mut self,
sp: impl Into<MultiSpan>,
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,11 +727,14 @@ pub trait LintContext: Sized {
.collect::<Vec<_>>();
possibilities.sort();

let mut should_print_possibilities = true;
if let Some((value, value_span)) = value {
if best_match_values.contains(&Some(value)) {
db.span_suggestion(name_span, "there is a config with a similar name and value", best_match, Applicability::MaybeIncorrect);
should_print_possibilities = false;
} else if best_match_values.contains(&None) {
db.span_suggestion(name_span.to(value_span), "there is a config with a similar name and no value", best_match, Applicability::MaybeIncorrect);
should_print_possibilities = false;
} else if let Some(first_value) = possibilities.first() {
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);
} else {
Expand All @@ -741,13 +744,25 @@ pub trait LintContext: Sized {
db.span_suggestion(name_span, "there is a config with a similar name", best_match, Applicability::MaybeIncorrect);
}

if !possibilities.is_empty() {
if !possibilities.is_empty() && should_print_possibilities {
let possibilities = possibilities.join("`, `");
db.help(format!("expected values for `{best_match}` are: `{possibilities}`"));
}
} else {
db.span_suggestion(name_span, "there is a config with a similar name", best_match, Applicability::MaybeIncorrect);
}
} else if !possibilities.is_empty() {
let mut possibilities = possibilities.iter()
Urgau marked this conversation as resolved.
Show resolved Hide resolved
.map(Symbol::as_str)
.collect::<Vec<_>>();
possibilities.sort();
let possibilities = possibilities.join("`, `");

// The list of expected names can be long (even by default) and
// so the diagnostic produced can take a lot of space. To avoid
// cloging the user output we only want to print that diagnostic
// once.
db.help_once(format!("expected names are: `{possibilities}`"));
}
},
BuiltinLintDiagnostics::UnexpectedCfgValue((name, name_span), value) => {
Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc-ui/check-cfg/check-cfg.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `uniz`
--> $DIR/check-cfg.rs:5:7
|
LL | #[cfg(uniz)]
Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc-ui/doctest/check-cfg-test.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: unexpected `cfg` condition value
warning: unexpected `cfg` condition value: `invalid`
--> $DIR/check-cfg-test.rs:9:7
|
LL | #[cfg(feature = "invalid")]
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/check-cfg/allow-same-level.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `FALSE`
--> $DIR/allow-same-level.rs:7:7
|
LL | #[cfg(FALSE)]
| ^^^^^
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/check-cfg/compact-names.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `target_architecture`
--> $DIR/compact-names.rs:11:28
|
LL | #[cfg(target(os = "linux", architecture = "arm"))]
| ^^^^^^^^^^^^^^^^^^^^
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/compact-values.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: unexpected `cfg` condition value
warning: unexpected `cfg` condition value: `X`
--> $DIR/compact-values.rs:11:28
|
LL | #[cfg(target(os = "linux", arch = "X"))]
Expand Down
13 changes: 6 additions & 7 deletions tests/ui/check-cfg/diagnotics.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `featur`
--> $DIR/diagnotics.rs:4:7
|
LL | #[cfg(featur)]
Expand All @@ -7,19 +7,18 @@ LL | #[cfg(featur)]
= help: expected values for `feature` are: `foo`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `featur`
--> $DIR/diagnotics.rs:8:7
|
LL | #[cfg(featur = "foo")]
| ^^^^^^^^^^^^^^
|
= help: expected values for `feature` are: `foo`
help: there is a config with a similar name and value
|
LL | #[cfg(feature = "foo")]
| ~~~~~~~

warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `featur`
--> $DIR/diagnotics.rs:12:7
|
LL | #[cfg(featur = "fo")]
Expand All @@ -31,13 +30,13 @@ help: there is a config with a similar name and different values
LL | #[cfg(feature = "foo")]
| ~~~~~~~~~~~~~~~

warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `no_value`
--> $DIR/diagnotics.rs:19:7
|
LL | #[cfg(no_value)]
| ^^^^^^^^ help: there is a config with a similar name: `no_values`

warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `no_value`
--> $DIR/diagnotics.rs:23:7
|
LL | #[cfg(no_value = "foo")]
Expand All @@ -48,7 +47,7 @@ help: there is a config with a similar name and no value
LL | #[cfg(no_values)]
| ~~~~~~~~~

warning: unexpected `cfg` condition value
warning: unexpected `cfg` condition value: `bar`
--> $DIR/diagnotics.rs:27:7
|
LL | #[cfg(no_values = "bar")]
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/check-cfg/empty-names.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `unknown_key`
--> $DIR/empty-names.rs:6:7
|
LL | #[cfg(unknown_key = "value")]
| ^^^^^^^^^^^^^^^^^^^^^
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/empty-values.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: unexpected `cfg` condition value
warning: unexpected `cfg` condition value: `value`
--> $DIR/empty-values.rs:6:7
|
LL | #[cfg(test = "value")]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/invalid-cfg-name.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: unexpected `cfg` condition name
warning: unexpected `cfg` condition name: `widnows`
--> $DIR/invalid-cfg-name.rs:7:7
|
LL | #[cfg(widnows)]
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/check-cfg/invalid-cfg-value.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: unexpected `cfg` condition value
warning: unexpected `cfg` condition value: `sedre`
--> $DIR/invalid-cfg-value.rs:7:7
|
LL | #[cfg(feature = "sedre")]
Expand All @@ -9,7 +9,7 @@ LL | #[cfg(feature = "sedre")]
= note: expected values for `feature` are: `full`, `serde`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value
warning: unexpected `cfg` condition value: `rand`
--> $DIR/invalid-cfg-value.rs:14:7
|
LL | #[cfg(feature = "rand")]
Expand Down
Loading
Loading