Auto merge of rust-lang#119930 - Urgau:check-cfg-empty-values-means-e…

…mpty, r=petrochenkov

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

This PR adds way to express no-values (no values expected) with `--check-cfg` by making empty `values()` no longer mean `values(none())` (internal: `&[None]`) and now be an empty list (internal: `&[]`).

### Context

Currently `--check-cfg` has a way to express that _any value is expected_ with `values(any())`, but has no way to do the inverse and say that _no value is expected_.

This would be particularly useful for build systems that control a config name and it's values as they could always declare a config name as expected and if in the current state they have values pass them and if not pass an empty list.

To give a more concrete example, Cargo `--check-cfg` currently needs to generate:
 - `--check-cfg=cfg(feature, values(...))` for the case with declared features
 - and `--check-cfg=cfg()` for the case without any features declared

This means that when there are no features declared, users will get an `unexpected config name` but from the point of view of Cargo the config name `feature` is expected, it's just that for now there aren't any values for it.

See [Cargo `check_cfg_args` function](https://github.com/rust-lang/cargo/blob/92395d90106b3b61bcb68bcf2069052c93771764/src/cargo/core/compiler/mod.rs#L1263-L1281) for more details.

### De-specializing *empty* `values()`

To solve this issue I propose that we "de-specialize" empty `values()` to no longer mean `values(none())` but to actually mean empty set/list. This is one of the last source of confusion for my-self and others with the `--check-cfg` syntax.

> The confusing part here is that an empty `values()` currently means the same as `values(none())`, i.e. an expected list of values with the _none_ variant (as in `#[cfg(name)]` where the value is none) instead of meaning an empty set.

Before the new `cfg()` syntax, defining the _none_ variant was only possible under certain circumstances, so in rust-lang#111068 I decided to make `values()` to mean the _none_ variant, but it is no longer necessary since rust-lang#119473 which introduced the `none()` syntax.

A simplified representation of the proposed "de-specialization" would be:

| Syntax                                  | List/set of expected values |
|-----------------------------------------|-----------------------------|
| `cfg(name)`/`cfg(name, values(none()))` | `&[None]`                   |
| `cfg(name, values())`                   | `&[]`                       |

Note that I have my-self made the mistake of using an empty `values()` as meaning empty set, see rust-lang/cargo#13011.

`@rustbot` label +F-check-cfg
r? `@petrochenkov`
cc `@epage`

compiler/rustc_interface/src/interface.rs

@@ -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!(

compiler/rustc_lint/src/context/diagnostics.rs

@@ -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
@@ -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`"));

src/doc/unstable-book/src/compiler-flags/check-cfg.md

@@ -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:
 
@@ -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
 

tests/ui/check-cfg/cargo-feature.none.stderr

@@ -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")]
    |       ^^^^^^^^^^^^^^^^^

tests/ui/check-cfg/cargo-feature.rs

@@ -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)]

tests/ui/check-cfg/cargo-feature.some.stderr

@@ -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")]
    |       ^^^^^^^^^^^^^^^^^
@@ -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)]
    |       ^^^^^^^^^^^^^^
@@ -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")]
    |       ^^^^^^^^^^^^^^---

tests/ui/check-cfg/concat-values.rs

@@ -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
@@ -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() {}

tests/ui/check-cfg/concat-values.stderr

@@ -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)]
    |       ^^^^^^
@@ -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")]
    |       ^^^^^^^^^^^^^^

tests/ui/check-cfg/empty-values.rs

@@ -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() {}

tests/ui/check-cfg/empty-values.stderr

@@ -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
+