Auto merge of #13913 - Urgau:check-cfg-lints-sub-config, r=epage

Add special `check-cfg` lint config for the `unexpected_cfgs` lint

### What does this PR try to resolve?

This PR adds a special `check-cfg` lint config for the `unexpected_cfgs` lint, as it was decided by T-cargo (in today's meeting).

The goal of this lint config is to provide a simple and cost-less alternative to the build-script `cargo::rustc-check-cfg` instruction.

```toml
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(foo, values("bar"))'] }
```

### How should we test and review this PR?

I recommand reviewing commit by commit; and looking at all the new tests added in `check_cfg.rs`, I tried making them as exhaustive as I could, many of them are very similar to their non-config counterpart.

### Additional information

I didn't add *(actually removed from the 1st version of this PR)* the possibility to omit the `level` field if `check-cfg` is specified, https://github.com/rust-lang/cargo/pull/13913#discussion_r1600609229.

Regarding the implementation, I tried making it is as straight forward as possible, nothing over-engineered or complex.

r? `@epage` (or `@weihanglo` maybe)
This commit is contained in:
bors 2024-05-17 16:54:54 +00:00
commit 0de7f2ec6c
7 changed files with 386 additions and 37 deletions

View File

@ -1503,6 +1503,13 @@ impl TomlLint {
Self::Config(config) => config.priority,
}
}
pub fn config(&self) -> Option<&toml::Table> {
match self {
Self::Level(_) => None,
Self::Config(config) => Some(&config.config),
}
}
}
#[derive(Serialize, Deserialize, Debug, Clone)]
@ -1511,6 +1518,8 @@ pub struct TomlLintConfig {
pub level: TomlLintLevel,
#[serde(default)]
pub priority: i8,
#[serde(flatten)]
pub config: toml::Table,
}
#[derive(Serialize, Deserialize, Debug, Copy, Clone, Eq, PartialEq)]

View File

@ -246,7 +246,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
let mut args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
args.extend(compiler::lto_args(&self, unit));
args.extend(compiler::features_args(unit));
args.extend(compiler::check_cfg_args(&self, unit));
args.extend(compiler::check_cfg_args(&self, unit)?);
let script_meta = self.find_build_script_metadata(unit);
if let Some(meta) = script_meta {

View File

@ -63,7 +63,7 @@ use std::io::{BufRead, Write};
use std::path::{Path, PathBuf};
use std::sync::Arc;
use anyhow::{Context as _, Error};
use anyhow::{bail, Context as _, Error};
use lazycell::LazyCell;
use tracing::{debug, trace};
@ -732,7 +732,7 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu
let doc_dir = build_runner.files().out_dir(unit);
rustdoc.arg("-o").arg(&doc_dir);
rustdoc.args(&features_args(unit));
rustdoc.args(&check_cfg_args(build_runner, unit));
rustdoc.args(&check_cfg_args(build_runner, unit)?);
add_error_format_and_color(build_runner, &mut rustdoc);
add_allow_features(build_runner, &mut rustdoc);
@ -1125,7 +1125,7 @@ fn build_base_args(
}
cmd.args(&features_args(unit));
cmd.args(&check_cfg_args(build_runner, unit));
cmd.args(&check_cfg_args(build_runner, unit)?);
let meta = build_runner.files().metadata(unit);
cmd.arg("-C").arg(&format!("metadata={}", meta));
@ -1310,7 +1310,7 @@ fn trim_paths_args(
}
/// Generates the `--check-cfg` arguments for the `unit`.
fn check_cfg_args(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> Vec<OsString> {
fn check_cfg_args(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResult<Vec<OsString>> {
if build_runner
.bcx
.target_data
@ -1353,14 +1353,39 @@ fn check_cfg_args(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> Vec<OsStri
// Cargo and docs.rs than rustc and docs.rs. In particular, all users of docs.rs use
// Cargo, but not all users of rustc (like Rust-for-Linux) use docs.rs.
vec![
let mut args = vec![
OsString::from("--check-cfg"),
OsString::from("cfg(docsrs)"),
OsString::from("--check-cfg"),
arg_feature,
]
];
// Also include the custom arguments specified in `[lints.rust.unexpected_cfgs.check_cfg]`
if let Ok(Some(lints)) = unit.pkg.manifest().resolved_toml().resolved_lints() {
if let Some(rust_lints) = lints.get("rust") {
if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") {
if let Some(config) = unexpected_cfgs.config() {
if let Some(check_cfg) = config.get("check-cfg") {
if let Ok(check_cfgs) =
toml::Value::try_into::<Vec<String>>(check_cfg.clone())
{
for check_cfg in check_cfgs {
args.push(OsString::from("--check-cfg"));
args.push(OsString::from(check_cfg));
}
// error about `check-cfg` not being a list-of-string
} else {
bail!("`lints.rust.unexpected_cfgs.check-cfg` must be a list of string");
}
}
}
}
}
}
Ok(args)
} else {
Vec::new()
Ok(Vec::new())
}
}

View File

@ -2265,7 +2265,7 @@ supported tools: {}",
if tool == "cargo" && !gctx.cli_unstable().cargo_lints {
warn_for_cargo_lint_feature(gctx, warnings);
}
for name in lints.keys() {
for (name, config) in lints {
if let Some((prefix, suffix)) = name.split_once("::") {
if tool == prefix {
anyhow::bail!(
@ -2278,6 +2278,19 @@ supported tools: {}",
} else {
anyhow::bail!("`lints.{tool}.{name}` is not a valid lint name")
}
} else if let Some(config) = config.config() {
for config_name in config.keys() {
// manually report unused manifest key warning since we collect all the "extra"
// keys and values inside the config table
//
// except for `rust.unexpected_cfgs.check-cfg` which is used by rustc/rustdoc
if !(tool == "rust" && name == "unexpected_cfgs" && config_name == "check-cfg")
{
let message =
format!("unused manifest key: `lints.{tool}.{name}.{config_name}`");
warnings.push(message);
}
}
}
}
}

View File

@ -292,32 +292,6 @@ if foo_bar_condition {
}
```
##### Example of local-only `build.rs`/build script
Build scripts can impose costs on downstream users, and crate authors who wish to avoid
this can use "local-only" build scripts, which are active locally but not packaged in the
distributed package. Completly removing the cost from downstream users.
The way to achieved this, is by excluding the `build.rs` in the `Cargo.toml` with the
`exclude` key:
```diff
[package]
name = "foo"
version = "0.1.0"
+ exclude = ["build.rs"]
```
*`build.rs`:*
```rust
fn main() {
// Warning: build.rs is not published to crates.io.
println!("cargo::rerun-if-changed=build.rs");
println!("cargo::rustc-check-cfg=cfg(foo)");
}
```
For a more complete example see in the [build script examples][build-script-examples] page
the [conditional compilation][conditional-compilation-example] example.

View File

@ -496,3 +496,331 @@ fn build_script_test() {
.with_stdout_contains_n("test [..] ... ok", 3)
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_simple() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(has_foo)", "cfg(has_bar, values(\"yes\", \"no\"))"] }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("check -v")
.with_stderr_contains(x!("rustc" => "cfg" of "has_foo"))
.with_stderr_contains(x!("rustc" => "cfg" of "has_bar" with "yes" "no"))
.with_stderr_does_not_contain("[..]unused manifest key[..]")
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_workspace() {
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["foo/"]
[workspace.lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(has_foo)"] }
"#,
)
.file(
"foo/Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints]
workspace = true
"#,
)
.file("foo/src/main.rs", "fn main() {}")
.build();
p.cargo("check -v")
.with_stderr_contains(x!("rustc" => "cfg" of "has_foo"))
.with_stderr_does_not_contain("unexpected_cfgs")
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_workspace_not_inherited() {
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["foo/"]
[workspace.lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(has_foo)"] }
"#,
)
.file(
"foo/Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
"#,
)
.file("foo/src/main.rs", "fn main() {}")
.build();
p.cargo("check -v")
.with_stderr_does_not_contain(x!("rustc" => "cfg" of "has_foo"))
.with_stderr_does_not_contain("unexpected_cfgs")
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_invalid_position() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints.rust]
use_bracket = { level = "warn", check-cfg = ["cfg(has_foo)"] }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("check -v")
.with_stderr_contains("[..]unused manifest key: `lints.rust.use_bracket.check-cfg`[..]")
.with_stderr_does_not_contain(x!("rustc" => "cfg" of "has_foo"))
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_invalid_empty() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints.rust]
unexpected_cfgs = { }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("check")
.with_stderr_contains("[..]missing field `level`[..]")
.run_expect_error();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_invalid_not_list() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = "cfg()" }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("check")
.with_status(101)
.with_stderr_contains(
"[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string",
)
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_invalid_not_list_string() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = [12] }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("check")
.with_status(101)
.with_stderr_contains(
"[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string",
)
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_and_features() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[features]
my_feature = []
alloc = []
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(has_foo)", "cfg(has_bar, values(\"yes\", \"no\"))"] }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("check -v")
.with_stderr_contains(x!("rustc" => "cfg" of "has_foo"))
.with_stderr_contains(x!("rustc" => "cfg" of "has_bar" with "yes" "no"))
.with_stderr_contains(x!("rustc" => "cfg" of "feature" with "alloc" "my_feature"))
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_with_cargo_doc() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(has_foo)"] }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("doc -v")
.with_stderr_contains(x!("rustdoc" => "cfg" of "has_foo"))
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_with_cargo_test() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(has_foo)"] }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("test -v")
.with_stderr_contains(x!("rustc" => "cfg" of "has_foo"))
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_and_build_script() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2021"
build = "build.rs"
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(bar)"] }
"#,
)
.file("src/main.rs", "fn main() {}")
.file(
"build.rs",
r#"fn main() { println!("cargo::rustc-check-cfg=cfg(foo)"); }"#,
)
.build();
p.cargo("check -v")
.with_stderr_contains(x!("rustc" => "cfg" of "foo")) // from build.rs
.with_stderr_contains(x!("rustc" => "cfg" of "bar")) // from config
.run();
}
#[cargo_test(>=1.79, reason = "--check-cfg was stabilized in Rust 1.79")]
fn config_features_and_build_script() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2021"
build = "build.rs"
[features]
serde = []
json = []
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ["cfg(bar)"] }
"#,
)
.file("src/main.rs", "fn main() {}")
.file(
"build.rs",
r#"fn main() { println!("cargo::rustc-check-cfg=cfg(foo)"); }"#,
)
.build();
p.cargo("check -v")
.with_stderr_contains(x!("rustc" => "cfg" of "foo")) // from build.rs
.with_stderr_contains(x!("rustc" => "cfg" of "bar")) // from config
.with_stderr_contains(x!("rustc" => "cfg" of "feature" with "json" "serde")) // features
.with_stderr_contains(x!("rustc" => "cfg" of "docsrs")) // Cargo well known
.run();
}

View File

@ -172,8 +172,8 @@ fn warn_on_unused_key() {
foo.cargo("check")
.with_stderr(
"\
[WARNING] [CWD]/Cargo.toml: unused manifest key: lints.rust.rust-2018-idioms.unused
[WARNING] [CWD]/Cargo.toml: unused manifest key: workspace.lints.rust.rust-2018-idioms.unused
[WARNING][..]unused manifest key: `lints.rust.rust-2018-idioms.unused`
[WARNING][..]unused manifest key: `lints.rust.rust-2018-idioms.unused`
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]s
",