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

Add way to express that no values are expected with check-cfg #119930

Merged
merged 1 commit into from
Jan 17, 2024
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
4 changes: 3 additions & 1 deletion compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,9 @@ pub(crate) fn parse_check_cfg(dcx: &DiagCtxt, specs: Vec<String>) -> CheckCfg {
}
}

if values.is_empty() && !values_any_specified && !any_specified {
if !values_specified && !any_specified {
// `cfg(name)` is equivalent to `cfg(name, values(none()))` so add
// an implicit `none()`
values.insert(None);
} else if !values.is_empty() && values_any_specified {
error!(
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,15 @@ pub(super) fn builtin(
Applicability::MaybeIncorrect,
);
}
} else {
db.note(format!("no expected values for `{name}`"));

let sp = if let Some((_value, value_span)) = value {
name_span.to(value_span)
} else {
name_span
};
db.span_suggestion(sp, "remove the condition", "", Applicability::MaybeIncorrect);
}

// We don't want to suggest adding values to well known names
Expand All @@ -373,6 +382,8 @@ pub(super) fn builtin(
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("consider defining some features in `Cargo.toml`");
}
} else if !is_cfg_a_well_know_name {
db.help(format!("consider using a Cargo feature instead or adding `println!(\"cargo:rustc-check-cfg={inst}\");` to the top of a `build.rs`"));
Expand Down
25 changes: 13 additions & 12 deletions src/doc/unstable-book/src/compiler-flags/check-cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,20 @@ To enable checking of values, but to provide an *none*/empty set of expected val

```bash
rustc --check-cfg 'cfg(name)'
rustc --check-cfg 'cfg(name, values())'
rustc --check-cfg 'cfg(name, values(none()))'
```

To enable checking of name but not values (i.e. unknown expected values), use this form:
To enable checking of name but not values, use one of these forms:

```bash
rustc --check-cfg 'cfg(name, values(any()))'
```
- No expected values (_will lint on every value_):
```bash
rustc --check-cfg 'cfg(name, values())'
```

- Unknown expected values (_will never lint_):
```bash
rustc --check-cfg 'cfg(name, values(any()))'
```

To avoid repeating the same set of values, use this form:

Expand Down Expand Up @@ -114,18 +119,14 @@ as argument to `--check-cfg`.
This table describe the equivalence of a `--cfg` argument to a `--check-cfg` argument.

| `--cfg` | `--check-cfg` |
|-----------------------------|----------------------------------------------------------|
|-------------------------------|------------------------------------------------------------|
| *nothing* | *nothing* or `--check-cfg=cfg()` (to enable the checking) |
| `--cfg foo` | `--check-cfg=cfg(foo), --check-cfg=cfg(foo, values())` or `--check-cfg=cfg(foo, values(none()))` |
| `--cfg foo` | `--check-cfg=cfg(foo)` or `--check-cfg=cfg(foo, values(none()))` |
| `--cfg foo=""` | `--check-cfg=cfg(foo, values(""))` |
| `--cfg foo="bar"` | `--check-cfg=cfg(foo, values("bar"))` |
| `--cfg foo="1" --cfg foo="2"` | `--check-cfg=cfg(foo, values("1", "2"))` |
| `--cfg foo="1" --cfg bar="2"` | `--check-cfg=cfg(foo, values("1")) --check-cfg=cfg(bar, values("2"))` |
| `--cfg foo --cfg foo="bar"` | `--check-cfg=cfg(foo) --check-cfg=cfg(foo, values("bar"))` |

NOTE: There is (currently) no way to express that a condition name is expected but no (!= none)
values are expected. Passing an empty `values()` means *(none)* in the sense of `#[cfg(foo)]`
with no value. Users are expected to NOT pass a `--check-cfg` with that condition name.
| `--cfg foo --cfg foo="bar"` | `--check-cfg=cfg(foo, values(none(), "bar"))` |

### Example: Cargo-like `feature` example

Expand Down
20 changes: 11 additions & 9 deletions tests/ui/check-cfg/cargo-feature.none.stderr
Original file line number Diff line number Diff line change
@@ -1,34 +1,36 @@
warning: unexpected `cfg` condition name: `feature`
--> $DIR/cargo-feature.rs:13:7
warning: unexpected `cfg` condition value: `serde`
--> $DIR/cargo-feature.rs:14:7
|
LL | #[cfg(feature = "serde")]
| ^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^ help: remove the condition
|
= help: consider defining some features in `Cargo.toml`
= note: no expected values for `feature`
= 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: `feature`
warning: unexpected `cfg` condition value: (none)
--> $DIR/cargo-feature.rs:18:7
|
LL | #[cfg(feature)]
| ^^^^^^^
| ^^^^^^^ help: remove the condition
|
= note: no expected values for `feature`
= 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

warning: unexpected `cfg` condition name: `tokio_unstable`
--> $DIR/cargo-feature.rs:23:7
--> $DIR/cargo-feature.rs:22: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: expected names are: `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 name: `CONFIG_NVME`
--> $DIR/cargo-feature.rs:27:7
--> $DIR/cargo-feature.rs:26:7
|
LL | #[cfg(CONFIG_NVME = "m")]
| ^^^^^^^^^^^^^^^^^
Expand Down
9 changes: 4 additions & 5 deletions tests/ui/check-cfg/cargo-feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,18 @@
// check-pass
// revisions: some none
// rustc-env:CARGO=/usr/bin/cargo
// compile-flags: --check-cfg=cfg() -Z unstable-options
// compile-flags: -Z unstable-options
// [none]compile-flags: --check-cfg=cfg(feature,values())
// [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")]
//[none]~^ WARNING unexpected `cfg` condition name
//[some]~^^ WARNING unexpected `cfg` condition value
//~^ WARNING unexpected `cfg` condition value
fn ser() {}

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

#[cfg(tokio_unstable)]
Expand Down
7 changes: 4 additions & 3 deletions tests/ui/check-cfg/cargo-feature.some.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition value: `serde`
--> $DIR/cargo-feature.rs:13:7
--> $DIR/cargo-feature.rs:14:7
|
LL | #[cfg(feature = "serde")]
| ^^^^^^^^^^^^^^^^^
Expand All @@ -16,10 +16,11 @@ LL | #[cfg(feature)]
| ^^^^^^^- help: specify a config value: `= "bitcode"`
|
= note: expected values for `feature` are: `bitcode`
= 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

warning: unexpected `cfg` condition name: `tokio_unstable`
--> $DIR/cargo-feature.rs:23:7
--> $DIR/cargo-feature.rs:22:7
|
LL | #[cfg(tokio_unstable)]
| ^^^^^^^^^^^^^^
Expand All @@ -29,7 +30,7 @@ LL | #[cfg(tokio_unstable)]
= 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:27:7
--> $DIR/cargo-feature.rs:26:7
|
LL | #[cfg(CONFIG_NVME = "m")]
| ^^^^^^^^^^^^^^---
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/check-cfg/concat-values.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// check-pass
// compile-flags: -Z unstable-options
// compile-flags: --check-cfg=cfg(my_cfg,values("foo")) --check-cfg=cfg(my_cfg,values("bar"))
// compile-flags: --check-cfg=cfg(my_cfg,values())

#[cfg(my_cfg)]
//~^ WARNING unexpected `cfg` condition value
Expand All @@ -10,4 +11,7 @@ fn my_cfg() {}
//~^ WARNING unexpected `cfg` condition value
fn my_cfg() {}

#[cfg(any(my_cfg = "foo", my_cfg = "bar"))]
fn foo_and_bar() {}

fn main() {}
4 changes: 2 additions & 2 deletions tests/ui/check-cfg/concat-values.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition value: (none)
--> $DIR/concat-values.rs:5:7
--> $DIR/concat-values.rs:6:7
|
LL | #[cfg(my_cfg)]
| ^^^^^^
Expand All @@ -10,7 +10,7 @@ LL | #[cfg(my_cfg)]
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `unk`
--> $DIR/concat-values.rs:9:7
--> $DIR/concat-values.rs:10:7
|
LL | #[cfg(my_cfg = "unk")]
| ^^^^^^^^^^^^^^
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/check-cfg/empty-values.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Check that we detect unexpected value when none are allowed
//
// check-pass
// compile-flags: --check-cfg=cfg(foo,values()) -Zunstable-options

#[cfg(foo = "foo")]
//~^ WARNING unexpected `cfg` condition value
fn do_foo() {}

#[cfg(foo)]
//~^ WARNING unexpected `cfg` condition value
fn do_foo() {}

fn main() {}
23 changes: 23 additions & 0 deletions tests/ui/check-cfg/empty-values.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
warning: unexpected `cfg` condition value: `foo`
--> $DIR/empty-values.rs:6:7
|
LL | #[cfg(foo = "foo")]
| ^^^^^^^^^^^ help: remove the condition
|
= note: no expected values for `foo`
= help: to expect this configuration use `--check-cfg=cfg(foo, values("foo"))`
= 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: (none)
--> $DIR/empty-values.rs:10:7
|
LL | #[cfg(foo)]
| ^^^ help: remove the condition
|
= note: no expected values for `foo`
= help: to expect this configuration use `--check-cfg=cfg(foo)`
= 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

Loading