From 6577c4fda94b57321ea1f8d812884cf61e9b6f03 Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 11 Oct 2024 10:34:24 +0200 Subject: [PATCH] Add future-incompatibility warning against some keyword-as-ident --- crates/cargo-platform/src/cfg.rs | 8 ++++++ crates/cargo-platform/src/lib.rs | 43 +++++++++++++++++++++++++++++++- src/cargo/util/context/target.rs | 9 ++++++- src/cargo/util/toml/mod.rs | 1 + tests/testsuite/cfg.rs | 13 +++++++++- 5 files changed, 71 insertions(+), 3 deletions(-) diff --git a/crates/cargo-platform/src/cfg.rs b/crates/cargo-platform/src/cfg.rs index c3ddb69bc88c..5753813a5e0b 100644 --- a/crates/cargo-platform/src/cfg.rs +++ b/crates/cargo-platform/src/cfg.rs @@ -31,6 +31,14 @@ enum Token<'a> { String(&'a str), } +/// The list of keywords. +/// +/// We should consider all the keywords, but some are conditional on +/// the edition so for now we just consider true/false. +/// +/// +pub(crate) const KEYWORDS: &[&str; 2] = &["true", "false"]; + #[derive(Clone)] struct Tokenizer<'a> { s: iter::Peekable>, diff --git a/crates/cargo-platform/src/lib.rs b/crates/cargo-platform/src/lib.rs index 71e9140bae9c..c1b42880438b 100644 --- a/crates/cargo-platform/src/lib.rs +++ b/crates/cargo-platform/src/lib.rs @@ -11,12 +11,13 @@ //! //! [`Platform`]: enum.Platform.html -use std::fmt; use std::str::FromStr; +use std::{fmt, path::Path}; mod cfg; mod error; +use cfg::KEYWORDS; pub use cfg::{Cfg, CfgExpr}; pub use error::{ParseError, ParseErrorKind}; @@ -104,6 +105,46 @@ impl Platform { check_cfg_expr(cfg, warnings); } } + + pub fn check_cfg_keywords(&self, warnings: &mut Vec, path: &Path) { + fn check_cfg_expr(expr: &CfgExpr, warnings: &mut Vec, path: &Path) { + match *expr { + CfgExpr::Not(ref e) => check_cfg_expr(e, warnings, path), + CfgExpr::All(ref e) | CfgExpr::Any(ref e) => { + for e in e { + check_cfg_expr(e, warnings, path); + } + } + CfgExpr::Value(ref e) => match e { + Cfg::Name(name) | Cfg::KeyPair(name, _) => { + if KEYWORDS.contains(&name.as_str()) { + if name.as_str() == "true" || name.as_str() == "false" { + warnings.push(format!( + "[{}] future-incompatibility: the meaning of `cfg({e})` will change in the future\n \ + | Cargo is erroneously allowing `cfg(true)` and `cfg(false)`, but both forms are interpreted as false unless manually overridden with `--cfg`.\n \ + | In the future these will be built-in defines that will have the corresponding true/false value.\n \ + | It is recommended to avoid using these configs until they are properly supported.\n \ + | See for more information.", + path.display() + )); + } else { + warnings.push(format!( + "[{}] future-incompatibility: `cfg({e})` is deprecated as `{name}` is a keyword \ + and not an identifier and should not have have been accepted in this position.\n \ + | this was previously accepted by Cargo but is being phased out; it will become a hard error in a future release!", + path.display() + )); + } + } + } + }, + } + } + + if let Platform::Cfg(cfg) = self { + check_cfg_expr(cfg, warnings, path); + } + } } impl serde::Serialize for Platform { diff --git a/src/cargo/util/context/target.rs b/src/cargo/util/context/target.rs index f306ecc1731b..3de3d826a59e 100644 --- a/src/cargo/util/context/target.rs +++ b/src/cargo/util/context/target.rs @@ -3,7 +3,7 @@ use crate::core::compiler::{BuildOutput, LinkArgTarget}; use crate::util::CargoResult; use serde::Deserialize; use std::collections::{BTreeMap, HashMap}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::rc::Rc; /// Config definition of a `[target.'cfg(…)']` table. @@ -53,6 +53,13 @@ pub(super) fn load_target_cfgs( let target: BTreeMap = gctx.get("target")?; tracing::debug!("Got all targets {:#?}", target); for (key, cfg) in target { + if let Ok(platform) = key.parse::() { + let mut warnings = Vec::new(); + platform.check_cfg_keywords(&mut warnings, &Path::new(".cargo/config.toml")); + for w in warnings { + gctx.shell().warn(w)?; + } + } if key.starts_with("cfg(") { // Unfortunately this is not able to display the location of the // unused key. Using config::Value doesn't work. One diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index edbfe996292b..d8e6a5bfe806 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1326,6 +1326,7 @@ pub fn to_real_manifest( for (name, platform) in original_toml.target.iter().flatten() { let platform_kind: Platform = name.parse()?; platform_kind.check_cfg_attributes(warnings); + platform_kind.check_cfg_keywords(warnings, manifest_file); let platform_kind = Some(platform_kind); validate_dependencies( platform.dependencies.as_ref(), diff --git a/tests/testsuite/cfg.rs b/tests/testsuite/cfg.rs index eb4ae73bb2a0..9e58b9711884 100644 --- a/tests/testsuite/cfg.rs +++ b/tests/testsuite/cfg.rs @@ -550,9 +550,20 @@ fn cfg_keywords() { p.cargo("check") .with_stderr_data(str![[r#" +[WARNING] [[ROOT]/foo/Cargo.toml] future-incompatibility: the meaning of `cfg(true)` will change in the future + | Cargo is erroneously allowing `cfg(true)` and `cfg(false)`, but both forms are interpreted as false unless manually overridden with `--cfg`. + | In the future these will be built-in defines that will have the corresponding true/false value. + | It is recommended to avoid using these configs until they are properly supported. + | See for more information. +[WARNING] [.cargo/config.toml] future-incompatibility: the meaning of `cfg(false)` will change in the future + | Cargo is erroneously allowing `cfg(true)` and `cfg(false)`, but both forms are interpreted as false unless manually overridden with `--cfg`. + | In the future these will be built-in defines that will have the corresponding true/false value. + | It is recommended to avoid using these configs until they are properly supported. + | See for more information. [LOCKING] 1 package to latest compatible version [CHECKING] foo v0.1.0 ([ROOT]/foo) -... +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + "#]]) .run(); }