From 33d50c9617d8326eeb31bdfe260a0a55b235b4a5 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 9 Nov 2024 14:38:37 +0100 Subject: [PATCH 01/10] Bump cargo-platform to 0.1.10 --- Cargo.lock | 6 +++--- crates/cargo-platform/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2a167643abe..9889800aabc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -318,7 +318,7 @@ dependencies = [ "cargo-credential-libsecret", "cargo-credential-macos-keychain", "cargo-credential-wincred", - "cargo-platform 0.1.9", + "cargo-platform 0.1.10", "cargo-test-support", "cargo-util", "cargo-util-schemas", @@ -444,7 +444,7 @@ dependencies = [ [[package]] name = "cargo-platform" -version = "0.1.9" +version = "0.1.10" dependencies = [ "serde", ] @@ -3026,7 +3026,7 @@ name = "resolver-tests" version = "0.0.0" dependencies = [ "cargo", - "cargo-platform 0.1.9", + "cargo-platform 0.1.10", "cargo-util", "cargo-util-schemas", "proptest", diff --git a/crates/cargo-platform/Cargo.toml b/crates/cargo-platform/Cargo.toml index 02dd6da8232..7e4cc7487da 100644 --- a/crates/cargo-platform/Cargo.toml +++ b/crates/cargo-platform/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-platform" -version = "0.1.9" +version = "0.1.10" edition.workspace = true license.workspace = true rust-version.workspace = true From 0b835eb2f5255236304bbcd6314d068e4d8786cb Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 26 Sep 2024 13:31:41 +0200 Subject: [PATCH 02/10] Add `CfgExpr::walk` utility method --- crates/cargo-platform/src/cfg.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/crates/cargo-platform/src/cfg.rs b/crates/cargo-platform/src/cfg.rs index c3ddb69bc88..f311dfb53be 100644 --- a/crates/cargo-platform/src/cfg.rs +++ b/crates/cargo-platform/src/cfg.rs @@ -89,6 +89,28 @@ impl CfgExpr { CfgExpr::Value(ref e) => cfg.contains(e), } } + + /// Walk over all the `Cfg`s of the given `CfgExpr`, recursing into `not(...)`, `all(...)`, + /// `any(...)` and stopping at the first error and returning that error. + pub fn walk(&self, mut f: impl FnMut(&Cfg) -> Result<(), E>) -> Result<(), E> { + fn walk_inner( + cfg_expr: &CfgExpr, + f: &mut impl FnMut(&Cfg) -> Result<(), E>, + ) -> Result<(), E> { + match *cfg_expr { + CfgExpr::Not(ref e) => walk_inner(e, &mut *f), + CfgExpr::All(ref e) | CfgExpr::Any(ref e) => { + for e in e { + let _ = walk_inner(e, &mut *f)?; + } + Ok(()) + } + CfgExpr::Value(ref e) => f(e), + } + } + + walk_inner(self, &mut f) + } } impl FromStr for CfgExpr { From a5fc314dfde4ccf03684afef160ff4a4b5682d8c Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 26 Sep 2024 14:36:42 +0200 Subject: [PATCH 03/10] Add `CheckCfg`/`ExpectedValues` to cargo-platform --- crates/cargo-platform/src/check_cfg.rs | 98 ++++++++++++++ crates/cargo-platform/src/lib.rs | 2 + .../tests/test_print_check_cfg.rs | 123 ++++++++++++++++++ 3 files changed, 223 insertions(+) create mode 100644 crates/cargo-platform/src/check_cfg.rs create mode 100644 crates/cargo-platform/tests/test_print_check_cfg.rs diff --git a/crates/cargo-platform/src/check_cfg.rs b/crates/cargo-platform/src/check_cfg.rs new file mode 100644 index 00000000000..d91f2ea699c --- /dev/null +++ b/crates/cargo-platform/src/check_cfg.rs @@ -0,0 +1,98 @@ +//! check-cfg + +use std::{ + collections::{HashMap, HashSet}, + error::Error, + fmt::Display, +}; + +/// Check Config (aka `--check-cfg`/`--print=check-cfg` representation) +#[derive(Debug, Default, Clone)] +pub struct CheckCfg { + /// Is `--check-cfg` activated + pub exhaustive: bool, + /// List of expected cfgs + pub expecteds: HashMap, +} + +/// List of expected check-cfg values +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ExpectedValues { + /// List of expected values + /// + /// - `#[cfg(foo)]` value is `None` + /// - `#[cfg(foo = "")]` value is `Some("")` + /// - `#[cfg(foo = "bar")]` value is `Some("bar")` + Some(HashSet>), + /// All values expected + Any, +} + +/// Error when parse a line from `--print=check-cfg` +#[derive(Debug)] +#[non_exhaustive] +pub struct PrintCheckCfgParsingError; + +impl Error for PrintCheckCfgParsingError {} + +impl Display for PrintCheckCfgParsingError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("error when parsing a `--print=check-cfg` line") + } +} + +impl CheckCfg { + /// Parse a line from `--print=check-cfg` + pub fn parse_print_check_cfg_line( + &mut self, + line: &str, + ) -> Result<(), PrintCheckCfgParsingError> { + if line == "any()=any()" || line == "any()" { + self.exhaustive = false; + return Ok(()); + } + + let mut value: HashSet> = HashSet::default(); + let mut value_any_specified = false; + let name: String; + + if let Some((n, val)) = line.split_once('=') { + name = n.to_string(); + + if val == "any()" { + value_any_specified = true; + } else if val.is_empty() { + // no value, nothing to add + } else if let Some(val) = maybe_quoted_value(val) { + value.insert(Some(val.to_string())); + } else { + // missing quotes and non-empty + return Err(PrintCheckCfgParsingError); + } + } else { + name = line.to_string(); + value.insert(None); + } + + self.expecteds + .entry(name) + .and_modify(|v| match v { + ExpectedValues::Some(_) if value_any_specified => *v = ExpectedValues::Any, + ExpectedValues::Some(v) => v.extend(value.clone()), + ExpectedValues::Any => {} + }) + .or_insert_with(|| { + if value_any_specified { + ExpectedValues::Any + } else { + ExpectedValues::Some(value) + } + }); + Ok(()) + } +} + +fn maybe_quoted_value<'a>(v: &'a str) -> Option<&'a str> { + // strip "" around the value, e.g. "linux" -> linux + v.strip_prefix('"').and_then(|v| v.strip_suffix('"')) +} diff --git a/crates/cargo-platform/src/lib.rs b/crates/cargo-platform/src/lib.rs index 71e9140bae9..7007878764e 100644 --- a/crates/cargo-platform/src/lib.rs +++ b/crates/cargo-platform/src/lib.rs @@ -15,9 +15,11 @@ use std::fmt; use std::str::FromStr; mod cfg; +mod check_cfg; mod error; pub use cfg::{Cfg, CfgExpr}; +pub use check_cfg::{CheckCfg, ExpectedValues}; pub use error::{ParseError, ParseErrorKind}; /// Platform definition. diff --git a/crates/cargo-platform/tests/test_print_check_cfg.rs b/crates/cargo-platform/tests/test_print_check_cfg.rs new file mode 100644 index 00000000000..aa49a7f3089 --- /dev/null +++ b/crates/cargo-platform/tests/test_print_check_cfg.rs @@ -0,0 +1,123 @@ +use cargo_platform::{CheckCfg, ExpectedValues}; +use std::collections::HashSet; + +#[test] +fn print_check_cfg_none() { + let mut check_cfg = CheckCfg::default(); + + check_cfg.parse_print_check_cfg_line("cfg_a").unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_a").unwrap(), + ExpectedValues::Some(HashSet::from([None])) + ); +} + +#[test] +fn print_check_cfg_empty() { + let mut check_cfg = CheckCfg::default(); + + check_cfg.parse_print_check_cfg_line("cfg_b=").unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_b").unwrap(), + ExpectedValues::Some(HashSet::from([])) + ); +} + +#[test] +fn print_check_cfg_any() { + let mut check_cfg = CheckCfg::default(); + + check_cfg.parse_print_check_cfg_line("cfg_c=any()").unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_c").unwrap(), + ExpectedValues::Any + ); + + check_cfg.parse_print_check_cfg_line("cfg_c").unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_c").unwrap(), + ExpectedValues::Any + ); +} + +#[test] +fn print_check_cfg_value() { + let mut check_cfg = CheckCfg::default(); + + check_cfg + .parse_print_check_cfg_line("cfg_d=\"test\"") + .unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_d").unwrap(), + ExpectedValues::Some(HashSet::from([Some("test".to_string())])) + ); + + check_cfg + .parse_print_check_cfg_line("cfg_d=\"tmp\"") + .unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_d").unwrap(), + ExpectedValues::Some(HashSet::from([ + Some("test".to_string()), + Some("tmp".to_string()) + ])) + ); +} + +#[test] +fn print_check_cfg_none_and_value() { + let mut check_cfg = CheckCfg::default(); + + check_cfg.parse_print_check_cfg_line("cfg").unwrap(); + check_cfg.parse_print_check_cfg_line("cfg=\"foo\"").unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg").unwrap(), + ExpectedValues::Some(HashSet::from([None, Some("foo".to_string())])) + ); +} + +#[test] +fn print_check_cfg_quote_in_value() { + let mut check_cfg = CheckCfg::default(); + + check_cfg + .parse_print_check_cfg_line("cfg_e=\"quote_in_value\"\"") + .unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_e").unwrap(), + ExpectedValues::Some(HashSet::from([Some("quote_in_value\"".to_string())])) + ); +} + +#[test] +fn print_check_cfg_value_and_any() { + let mut check_cfg = CheckCfg::default(); + + // having both a value and `any()` shouldn't be possible but better + // handle this correctly anyway + + check_cfg + .parse_print_check_cfg_line("cfg_1=\"foo\"") + .unwrap(); + check_cfg.parse_print_check_cfg_line("cfg_1=any()").unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_1").unwrap(), + ExpectedValues::Any + ); + + check_cfg.parse_print_check_cfg_line("cfg_2=any()").unwrap(); + check_cfg + .parse_print_check_cfg_line("cfg_2=\"foo\"") + .unwrap(); + assert_eq!( + *check_cfg.expecteds.get("cfg_2").unwrap(), + ExpectedValues::Any + ); +} + +#[test] +#[should_panic] +fn print_check_cfg_missing_quote_value() { + let mut check_cfg = CheckCfg::default(); + check_cfg.parse_print_check_cfg_line("foo=bar").unwrap(); +} From c0296b0811a2e32af9167ac27a6e6dcd7c57ece1 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 26 Sep 2024 15:12:32 +0200 Subject: [PATCH 04/10] Add `-Zcheck-target-cfgs` in preparation for linting --- src/cargo/core/features.rs | 2 + src/doc/src/reference/unstable.md | 26 ++++++++ tests/testsuite/cargo/z_help/stdout.term.svg | 68 ++++++++++---------- tests/testsuite/cfg.rs | 54 ++++++++++++++++ 4 files changed, 117 insertions(+), 33 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 8bce51535f8..c7a70afc3a9 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -761,6 +761,7 @@ unstable_cli_options!( build_std: Option> = ("Enable Cargo to compile the standard library itself as part of a crate graph compilation"), build_std_features: Option> = ("Configure features enabled for the standard library itself when building the standard library"), cargo_lints: bool = ("Enable the `[lints.cargo]` table"), + check_target_cfgs: bool = ("Enable unexpected cfgs checking in `[target.'cfg(...)']` tables"), checksum_freshness: bool = ("Use a checksum to determine if output is fresh rather than filesystem mtime"), codegen_backend: bool = ("Enable the `codegen-backend` option in profiles in .cargo/config.toml file"), config_include: bool = ("Enable the `include` key in config files"), @@ -1259,6 +1260,7 @@ impl CliUnstable { } "build-std-features" => self.build_std_features = Some(parse_features(v)), "cargo-lints" => self.cargo_lints = parse_empty(k, v)?, + "check-target-cfgs" => self.check_target_cfgs = parse_empty(k, v)?, "codegen-backend" => self.codegen_backend = parse_empty(k, v)?, "config-include" => self.config_include = parse_empty(k, v)?, "direct-minimal-versions" => self.direct_minimal_versions = parse_empty(k, v)?, diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index c59600a5d5e..02438bb38f5 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -121,6 +121,7 @@ Each new feature described below should explain how to use it. * [package-workspace](#package-workspace) --- Allows for packaging and publishing multiple crates in a workspace. * [native-completions](#native-completions) --- Move cargo shell completions to native completions. * [warnings](#warnings) --- controls warning behavior; options for allowing or denying warnings. + * [check-target-cfgs](#check-target-cfgs) --- Allows checking unexpected cfgs in `[target.'cfg(...)']` ## allow-features @@ -1767,6 +1768,31 @@ Controls how Cargo handles warnings. Allowed values are: * `warn`: warnings are emitted as warnings (default). * `allow`: warnings are hidden. * `deny`: if warnings are emitted, an error will be raised at the end of the operation and the process will exit with a failure exit code. + +## check-target-cfgs + +* Tracking Issue: [#00000](https://github.com/rust-lang/cargo/issues/00000) + +**WARNING: Incomplete/WIP!** + +This feature checks for unexpected cfgs in `[target.'cfg(...)']` entries, based +on `rustc --print=check-cfg`. + +```sh +cargo check -Zcheck-target-cfgs +``` + +It follows the lint Rust `unexpected_cfgs` lint configuration: + +```toml +[target.'cfg(foo)'.dependencies] +cfg-if = "1.0" + +[lints.rust.unexpected_cfgs] +level = "warn" +check-cfg = ['cfg(foo)'] +``` + # Stabilized and removed features ## Compile progress diff --git a/tests/testsuite/cargo/z_help/stdout.term.svg b/tests/testsuite/cargo/z_help/stdout.term.svg index 2c6b292a5be..64d3b66d8dc 100644 --- a/tests/testsuite/cargo/z_help/stdout.term.svg +++ b/tests/testsuite/cargo/z_help/stdout.term.svg @@ -1,4 +1,4 @@ - +