Skip to content

Commit

Permalink
Auto merge of #118213 - Urgau:check-cfg-diagnostics-rustc-cargo, r=pe…
Browse files Browse the repository at this point in the history
…trochenkov

Add more suggestions to unexpected cfg names and values

This pull request adds more suggestion to unexpected cfg names and values diagnostics:
 - it first adds a links to the [rustc unstable book](https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html) or the [Cargo reference](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg), depending if rustc is invoked by Cargo
 - it secondly adds a suggestion on how to expect the cfg name or value:
    *excluding well known names and values*
    - for Cargo: it suggest using a feature or `cargo:rust-check-cfg` in build script
    - for rustc: it suggest using `--check-cfg` (with the correct invocation)

Those diagnostics improvements are directed towards enabling users to fix the issue if the previous suggestions weren't good enough.

r? `@petrochenkov`
  • Loading branch information
bors committed Dec 13, 2023
2 parents 2862500 + f6617d0 commit a90372c
Show file tree
Hide file tree
Showing 36 changed files with 468 additions and 326 deletions.
51 changes: 48 additions & 3 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,13 @@ pub trait LintContext {
},
BuiltinLintDiagnostics::UnexpectedCfgName((name, name_span), value) => {
let possibilities: Vec<Symbol> = sess.parse_sess.check_config.expecteds.keys().copied().collect();
let is_from_cargo = std::env::var_os("CARGO").is_some();
let mut is_feature_cfg = name == sym::feature;

if is_feature_cfg && is_from_cargo {
db.help("consider defining some features in `Cargo.toml`");
// Suggest the most probable if we found one
if let Some(best_match) = find_best_match_for_name(&possibilities, name, None) {
} else if let Some(best_match) = find_best_match_for_name(&possibilities, name, None) {
if let Some(ExpectedValues::Some(best_match_values)) =
sess.parse_sess.check_config.expecteds.get(&best_match) {
let mut possibilities = best_match_values.iter()
Expand Down Expand Up @@ -741,8 +745,8 @@ pub trait LintContext {
} else {
db.span_suggestion(name_span, "there is a config with a similar name", best_match, Applicability::MaybeIncorrect);
}
} else if name == sym::feature && std::env::var_os("CARGO").is_some() {
db.help("consider defining some features in `Cargo.toml`");

is_feature_cfg |= best_match == sym::feature;
} else if !possibilities.is_empty() {
let mut possibilities = possibilities.iter()
.map(Symbol::as_str)
Expand All @@ -756,6 +760,23 @@ pub trait LintContext {
// once.
db.help_once(format!("expected names are: `{possibilities}`"));
}

let inst = if let Some((value, _value_span)) = value {
let pre = if is_from_cargo { "\\" } else { "" };
format!("cfg({name}, values({pre}\"{value}{pre}\"))")
} else {
format!("cfg({name})")
};

if is_from_cargo {
if !is_feature_cfg {
db.help(format!("consider using a Cargo feature instead or adding `println!(\"cargo:rustc-check-cfg={inst}\");` to the top of a `build.rs`"));
}
db.note("see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration");
} else {
db.help(format!("to expect this configuration use `--check-cfg={inst}`"));
db.note("see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration");
}
},
BuiltinLintDiagnostics::UnexpectedCfgValue((name, name_span), value) => {
let Some(ExpectedValues::Some(values)) = &sess.parse_sess.check_config.expecteds.get(&name) else {
Expand All @@ -767,6 +788,7 @@ pub trait LintContext {
.copied()
.flatten()
.collect();
let is_from_cargo = std::env::var_os("CARGO").is_some();

// 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
Expand All @@ -787,6 +809,8 @@ pub trait LintContext {
db.span_suggestion(value_span, "there is a expected value with a similar name", format!("\"{best_match}\""), Applicability::MaybeIncorrect);

}
} else if name == sym::feature && is_from_cargo {
db.help(format!("consider defining `{name}` as feature in `Cargo.toml`"));
} else if let &[first_possibility] = &possibilities[..] {
db.span_suggestion(name_span.shrink_to_hi(), "specify a config value", format!(" = \"{first_possibility}\""), Applicability::MaybeIncorrect);
}
Expand All @@ -796,6 +820,27 @@ pub trait LintContext {
db.span_suggestion(name_span.shrink_to_hi().to(value_span), "remove the value", "", Applicability::MaybeIncorrect);
}
}

let inst = if let Some((value, _value_span)) = value {
let pre = if is_from_cargo { "\\" } else { "" };
format!("cfg({name}, values({pre}\"{value}{pre}\"))")
} else {
format!("cfg({name})")
};

if is_from_cargo {
if name == sym::feature {
if let Some((value, _value_span)) = value {
db.help(format!("consider adding `{value}` as a feature in `Cargo.toml`"));
}
} else {
db.help(format!("consider using a Cargo feature instead or adding `println!(\"cargo:rustc-check-cfg={inst}\");` to the top of a `build.rs`"));
}
db.note("see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration");
} else {
db.help(format!("to expect this configuration use `--check-cfg={inst}`"));
db.note("see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration");
}
},
BuiltinLintDiagnostics::DeprecatedWhereclauseLocation(new_span, suggestion) => {
db.multipart_suggestion(
Expand Down
2 changes: 2 additions & 0 deletions tests/rustdoc-ui/check-cfg/check-cfg.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ warning: unexpected `cfg` condition name: `uniz`
LL | #[cfg(uniz)]
| ^^^^ help: there is a config with a similar name: `unix`
|
= help: to expect this configuration use `--check-cfg=cfg(uniz)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
2 changes: 2 additions & 0 deletions tests/rustdoc-ui/doctest/check-cfg-test.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ LL | #[cfg(feature = "invalid")]
| ^^^^^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: `test`
= help: to expect this configuration use `--check-cfg=cfg(feature, values("invalid"))`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/check-cfg/allow-same-level.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ LL | #[cfg(FALSE)]
| ^^^^^
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `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`
= help: to expect this configuration use `--check-cfg=cfg(FALSE)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/check-cfg/cargo-feature.none.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
warning: unexpected `cfg` condition name: `feature`
--> $DIR/cargo-feature.rs:13:7
|
LL | #[cfg(feature = "serde")]
| ^^^^^^^^^^^^^^^^^
|
= help: consider defining some features in `Cargo.toml`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name: `tokio_unstable`
--> $DIR/cargo-feature.rs:18:7
|
LL | #[cfg(tokio_unstable)]
| ^^^^^^^^^^^^^^
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `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`
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(tokio_unstable)");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

warning: unexpected `cfg` condition name: `CONFIG_NVME`
--> $DIR/cargo-feature.rs:22:7
|
LL | #[cfg(CONFIG_NVME = "m")]
| ^^^^^^^^^^^^^^^^^
|
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(CONFIG_NVME, values(\"m\"))");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

warning: 3 warnings emitted

17 changes: 15 additions & 2 deletions tests/ui/check-cfg/cargo-feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,25 @@
// list of all the expected names
//
// check-pass
// revisions: some none
// rustc-env:CARGO=/usr/bin/cargo
// compile-flags: --check-cfg=cfg() -Z unstable-options
// error-pattern:Cargo.toml
// [some]compile-flags: --check-cfg=cfg(feature,values("bitcode"))
// [some]compile-flags: --check-cfg=cfg(CONFIG_NVME,values("y"))
// [none]error-pattern:Cargo.toml

#[cfg(feature = "serde")]
//~^ WARNING unexpected `cfg` condition name
//[none]~^ WARNING unexpected `cfg` condition name
//[some]~^^ WARNING unexpected `cfg` condition value
fn ser() {}

#[cfg(tokio_unstable)]
//~^ WARNING unexpected `cfg` condition name
fn tokio() {}

#[cfg(CONFIG_NVME = "m")]
//[none]~^ WARNING unexpected `cfg` condition name
//[some]~^^ WARNING unexpected `cfg` condition value
fn tokio() {}

fn main() {}
35 changes: 35 additions & 0 deletions tests/ui/check-cfg/cargo-feature.some.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
warning: unexpected `cfg` condition value: `serde`
--> $DIR/cargo-feature.rs:13:7
|
LL | #[cfg(feature = "serde")]
| ^^^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: `bitcode`
= help: consider adding `serde` as a feature in `Cargo.toml`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name: `tokio_unstable`
--> $DIR/cargo-feature.rs:18:7
|
LL | #[cfg(tokio_unstable)]
| ^^^^^^^^^^^^^^
|
= help: expected names are: `CONFIG_NVME`, `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `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`
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(tokio_unstable)");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

warning: unexpected `cfg` condition value: `m`
--> $DIR/cargo-feature.rs:22:7
|
LL | #[cfg(CONFIG_NVME = "m")]
| ^^^^^^^^^^^^^^---
| |
| help: there is a expected value with a similar name: `"y"`
|
= note: expected values for `CONFIG_NVME` are: `y`
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(CONFIG_NVME, values(\"m\"))");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

warning: 3 warnings emitted

11 changes: 0 additions & 11 deletions tests/ui/check-cfg/cargo-feature.stderr

This file was deleted.

2 changes: 2 additions & 0 deletions tests/ui/check-cfg/compact-names.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ LL | #[cfg(target(os = "linux", architecture = "arm"))]
| ^^^^^^^^^^^^^^^^^^^^
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `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`
= help: to expect this configuration use `--check-cfg=cfg(target_architecture, values("arm"))`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/check-cfg/compact-values.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ LL | #[cfg(target(os = "linux", pointer_width = "X"))]
| ^^^^^^^^^^^^^^^^^^^
|
= note: expected values for `target_pointer_width` are: `16`, `32`, `64`
= help: to expect this configuration use `--check-cfg=cfg(target_pointer_width, values("X"))`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/check-cfg/concat-values.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ LL | #[cfg(my_cfg)]
| ^^^^^^
|
= note: expected values for `my_cfg` are: `bar`, `foo`
= help: to expect this configuration use `--check-cfg=cfg(my_cfg)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `unk`
Expand All @@ -14,6 +16,8 @@ LL | #[cfg(my_cfg = "unk")]
| ^^^^^^^^^^^^^^
|
= note: expected values for `my_cfg` are: `bar`, `foo`
= help: to expect this configuration use `--check-cfg=cfg(my_cfg, values("unk"))`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration

warning: 2 warnings emitted

71 changes: 71 additions & 0 deletions tests/ui/check-cfg/diagnotics.cargo.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
warning: unexpected `cfg` condition name: `featur`
--> $DIR/diagnotics.rs:7:7
|
LL | #[cfg(featur)]
| ^^^^^^ help: there is a config with a similar name: `feature`
|
= help: expected values for `feature` are: `foo`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name: `featur`
--> $DIR/diagnotics.rs:11:7
|
LL | #[cfg(featur = "foo")]
| ^^^^^^^^^^^^^^
|
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
help: there is a config with a similar name and value
|
LL | #[cfg(feature = "foo")]
| ~~~~~~~

warning: unexpected `cfg` condition name: `featur`
--> $DIR/diagnotics.rs:15:7
|
LL | #[cfg(featur = "fo")]
| ^^^^^^^^^^^^^
|
= help: expected values for `feature` are: `foo`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
help: there is a config with a similar name and different values
|
LL | #[cfg(feature = "foo")]
| ~~~~~~~~~~~~~~~

warning: unexpected `cfg` condition name: `no_value`
--> $DIR/diagnotics.rs:22:7
|
LL | #[cfg(no_value)]
| ^^^^^^^^ help: there is a config with a similar name: `no_values`
|
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(no_value)");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

warning: unexpected `cfg` condition name: `no_value`
--> $DIR/diagnotics.rs:26:7
|
LL | #[cfg(no_value = "foo")]
| ^^^^^^^^^^^^^^^^
|
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(no_value, values(\"foo\"))");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
help: there is a config with a similar name and no value
|
LL | #[cfg(no_values)]
| ~~~~~~~~~

warning: unexpected `cfg` condition value: `bar`
--> $DIR/diagnotics.rs:30:7
|
LL | #[cfg(no_values = "bar")]
| ^^^^^^^^^--------
| |
| help: remove the value
|
= note: no expected value for `no_values`
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(no_values, values(\"bar\"))");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

warning: 6 warnings emitted

3 changes: 3 additions & 0 deletions tests/ui/check-cfg/diagnotics.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// check-pass
// revisions: cargo rustc
// [rustc]unset-rustc-env:CARGO
// [cargo]rustc-env:CARGO=/usr/bin/cargo
// compile-flags: --check-cfg=cfg(feature,values("foo")) --check-cfg=cfg(no_values) -Z unstable-options

#[cfg(featur)]
Expand Down
Loading

0 comments on commit a90372c

Please sign in to comment.