diff --git a/Cargo.lock b/Cargo.lock index 2a167643abe..a9c95951a82 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.2.0", "cargo-test-support", "cargo-util", "cargo-util-schemas", @@ -444,7 +444,7 @@ dependencies = [ [[package]] name = "cargo-platform" -version = "0.1.9" +version = "0.2.0" dependencies = [ "serde", ] @@ -3026,7 +3026,7 @@ name = "resolver-tests" version = "0.0.0" dependencies = [ "cargo", - "cargo-platform 0.1.9", + "cargo-platform 0.2.0", "cargo-util", "cargo-util-schemas", "proptest", diff --git a/Cargo.toml b/Cargo.toml index 196507a25fa..7e75ceaebd6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ cargo-credential = { version = "0.4.2", path = "credential/cargo-credential" } cargo-credential-libsecret = { version = "0.4.7", path = "credential/cargo-credential-libsecret" } cargo-credential-macos-keychain = { version = "0.4.7", path = "credential/cargo-credential-macos-keychain" } cargo-credential-wincred = { version = "0.4.7", path = "credential/cargo-credential-wincred" } -cargo-platform = { path = "crates/cargo-platform", version = "0.1.5" } +cargo-platform = { path = "crates/cargo-platform", version = "0.2.0" } cargo-test-macro = { version = "0.3.0", path = "crates/cargo-test-macro" } cargo-test-support = { version = "0.6.0", path = "crates/cargo-test-support" } cargo-util = { version = "0.2.14", path = "crates/cargo-util" } diff --git a/crates/cargo-platform/Cargo.toml b/crates/cargo-platform/Cargo.toml index 02dd6da8232..778a7cf6742 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.2.0" edition.workspace = true license.workspace = true rust-version.workspace = true diff --git a/crates/cargo-platform/src/cfg.rs b/crates/cargo-platform/src/cfg.rs index c3ddb69bc88..79aaa02c23f 100644 --- a/crates/cargo-platform/src/cfg.rs +++ b/crates/cargo-platform/src/cfg.rs @@ -1,5 +1,6 @@ use crate::error::{ParseError, ParseErrorKind::*}; use std::fmt; +use std::hash::{Hash, Hasher}; use std::iter; use std::str::{self, FromStr}; @@ -16,21 +17,41 @@ pub enum CfgExpr { #[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)] pub enum Cfg { /// A named cfg value, like `unix`. - Name(String), + Name(Ident), /// A key/value cfg pair, like `target_os = "linux"`. - KeyPair(String, String), + KeyPair(Ident, String), +} + +/// A identifier +#[derive(Eq, Ord, PartialOrd, Clone, Debug)] +pub struct Ident { + /// The identifier + pub name: String, + /// Is this a raw ident: `r#async` + /// + /// It's mainly used for display and doesn't take + /// part in the hash or equality (`foo` == `r#foo`). + pub raw: bool, } #[derive(PartialEq)] enum Token<'a> { LeftParen, RightParen, - Ident(&'a str), + Ident(bool, &'a str), Comma, Equals, 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>, @@ -41,6 +62,45 @@ struct Parser<'a> { t: Tokenizer<'a>, } +impl Ident { + pub fn as_str(&self) -> &str { + &self.name + } +} + +impl Hash for Ident { + fn hash(&self, state: &mut H) { + self.name.hash(state); + } +} + +impl PartialEq for Ident { + fn eq(&self, other: &str) -> bool { + self.name == other + } +} + +impl PartialEq<&str> for Ident { + fn eq(&self, other: &&str) -> bool { + self.name == *other + } +} + +impl PartialEq for Ident { + fn eq(&self, other: &Ident) -> bool { + self.name == other.name + } +} + +impl fmt::Display for Ident { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.raw { + f.write_str("r#")?; + } + f.write_str(&*self.name) + } +} + impl FromStr for Cfg { type Err = ParseError; @@ -144,7 +204,8 @@ impl<'a> Parser<'a> { fn expr(&mut self) -> Result { match self.peek() { - Some(Ok(Token::Ident(op @ "all"))) | Some(Ok(Token::Ident(op @ "any"))) => { + Some(Ok(Token::Ident(false, op @ "all"))) + | Some(Ok(Token::Ident(false, op @ "any"))) => { self.t.next(); let mut e = Vec::new(); self.eat(&Token::LeftParen)?; @@ -161,7 +222,7 @@ impl<'a> Parser<'a> { Ok(CfgExpr::Any(e)) } } - Some(Ok(Token::Ident("not"))) => { + Some(Ok(Token::Ident(false, "not"))) => { self.t.next(); self.eat(&Token::LeftParen)?; let e = self.expr()?; @@ -179,7 +240,7 @@ impl<'a> Parser<'a> { fn cfg(&mut self) -> Result { match self.t.next() { - Some(Ok(Token::Ident(name))) => { + Some(Ok(Token::Ident(raw, name))) => { let e = if self.r#try(&Token::Equals) { let val = match self.t.next() { Some(Ok(Token::String(s))) => s, @@ -197,9 +258,18 @@ impl<'a> Parser<'a> { return Err(ParseError::new(self.t.orig, IncompleteExpr("a string"))) } }; - Cfg::KeyPair(name.to_string(), val.to_string()) + Cfg::KeyPair( + Ident { + name: name.to_string(), + raw, + }, + val.to_string(), + ) } else { - Cfg::Name(name.to_string()) + Cfg::Name(Ident { + name: name.to_string(), + raw, + }) }; Ok(e) } @@ -279,14 +349,44 @@ impl<'a> Iterator for Tokenizer<'a> { return Some(Err(ParseError::new(self.orig, UnterminatedString))); } Some((start, ch)) if is_ident_start(ch) => { + let (start, raw) = if ch == 'r' { + if let Some(&(_pos, '#')) = self.s.peek() { + // starts with `r#` is a raw ident + self.s.next(); + if let Some((start, ch)) = self.s.next() { + if is_ident_start(ch) { + (start, true) + } else { + // not a starting ident character + return Some(Err(ParseError::new( + self.orig, + UnexpectedChar(ch), + ))); + } + } else { + // not followed by a ident, error out + return Some(Err(ParseError::new( + self.orig, + IncompleteExpr("identifier"), + ))); + } + } else { + // starts with `r` but not does continue with `#` + // cannot be a raw ident + (start, false) + } + } else { + // do not start with `r`, cannot be a raw ident + (start, false) + }; while let Some(&(end, ch)) = self.s.peek() { if !is_ident_rest(ch) { - return Some(Ok(Token::Ident(&self.orig[start..end]))); + return Some(Ok(Token::Ident(raw, &self.orig[start..end]))); } else { self.s.next(); } } - return Some(Ok(Token::Ident(&self.orig[start..]))); + return Some(Ok(Token::Ident(raw, &self.orig[start..]))); } Some((_, ch)) => { return Some(Err(ParseError::new(self.orig, UnexpectedChar(ch)))); diff --git a/crates/cargo-platform/src/lib.rs b/crates/cargo-platform/src/lib.rs index 71e9140bae9..f91b61708bb 100644 --- a/crates/cargo-platform/src/lib.rs +++ b/crates/cargo-platform/src/lib.rs @@ -11,13 +11,14 @@ //! //! [`Platform`]: enum.Platform.html -use std::fmt; use std::str::FromStr; +use std::{fmt, path::Path}; mod cfg; mod error; -pub use cfg::{Cfg, CfgExpr}; +use cfg::KEYWORDS; +pub use cfg::{Cfg, CfgExpr, Ident}; pub use error::{ParseError, ParseErrorKind}; /// Platform definition. @@ -104,6 +105,50 @@ 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 !name.raw && 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.\n \ + |\n \ + | help: use raw-idents instead: `cfg(r#{name})`", + 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!\n \ + |\n \ + | help: use raw-idents instead: `cfg(r#{name})`", + path.display() + )); + } + } + } + }, + } + } + + if let Platform::Cfg(cfg) = self { + check_cfg_expr(cfg, warnings, path); + } + } } impl serde::Serialize for Platform { diff --git a/crates/cargo-platform/tests/test_cfg.rs b/crates/cargo-platform/tests/test_cfg.rs index dd99d9a79b8..0fdf307fab7 100644 --- a/crates/cargo-platform/tests/test_cfg.rs +++ b/crates/cargo-platform/tests/test_cfg.rs @@ -1,13 +1,37 @@ -use cargo_platform::{Cfg, CfgExpr, Platform}; +use cargo_platform::{Cfg, CfgExpr, Ident, Platform}; use std::fmt; use std::str::FromStr; macro_rules! c { ($a:ident) => { - Cfg::Name(stringify!($a).to_string()) + Cfg::Name(Ident { + name: stringify!($a).to_string(), + raw: false, + }) + }; + (r # $a:ident) => { + Cfg::Name(Ident { + name: stringify!($a).to_string(), + raw: true, + }) }; ($a:ident = $e:expr) => { - Cfg::KeyPair(stringify!($a).to_string(), $e.to_string()) + Cfg::KeyPair( + Ident { + name: stringify!($a).to_string(), + raw: false, + }, + $e.to_string(), + ) + }; + (r # $a:ident = $e:expr) => { + Cfg::KeyPair( + Ident { + name: stringify!($a).to_string(), + raw: true, + }, + $e.to_string(), + ) }; } @@ -56,10 +80,13 @@ fn cfg_syntax() { good("_bar", c!(_bar)); good(" foo", c!(foo)); good(" foo ", c!(foo)); + good("r#foo", c!(r # foo)); good(" foo = \"bar\"", c!(foo = "bar")); good("foo=\"\"", c!(foo = "")); + good("r#foo=\"\"", c!(r # foo = "")); good(" foo=\"3\" ", c!(foo = "3")); good("foo = \"3 e\"", c!(foo = "3 e")); + good(" r#foo = \"3 e\"", c!(r # foo = "3 e")); } #[test] @@ -78,6 +105,10 @@ fn cfg_syntax_bad() { "foo, bar", "unexpected content `, bar` found after cfg expression", ); + bad::("r# foo", "unexpected character"); + bad::("r #foo", "unexpected content"); + bad::("r#\"foo\"", "unexpected character"); + bad::("foo = r#\"\"", "unexpected character"); } #[test] @@ -126,6 +157,9 @@ fn cfg_matches() { assert!(e!(not(foo)).matches(&[])); assert!(e!(any((not(foo)), (all(foo, bar)))).matches(&[c!(bar)])); assert!(e!(any((not(foo)), (all(foo, bar)))).matches(&[c!(foo), c!(bar)])); + assert!(e!(foo).matches(&[c!(r # foo)])); + assert!(e!(r # foo).matches(&[c!(foo)])); + assert!(e!(r # foo).matches(&[c!(r # foo)])); assert!(!e!(foo).matches(&[])); assert!(!e!(foo).matches(&[c!(bar)])); diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 95126f1f2e9..67e385688c0 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -353,7 +353,9 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul // That is because Cargo queries rustc without any profile settings. continue; } - let k = format!("CARGO_CFG_{}", super::envify(&k)); + // FIXME: We should handle raw-idents somehow instead of predenting they + // don't exist here + let k = format!("CARGO_CFG_{}", super::envify(k.as_str())); cmd.env(&k, v.join(",")); } diff --git a/src/cargo/util/context/target.rs b/src/cargo/util/context/target.rs index f306ecc1731..3de3d826a59 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 bd1fcf142c9..d8bf63199ac 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1312,6 +1312,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 7e90b51a626..9b5cd240765 100644 --- a/tests/testsuite/cfg.rs +++ b/tests/testsuite/cfg.rs @@ -521,3 +521,141 @@ error[E0463]: can't find crate for `bar` "#]]) .run(); } + +#[cargo_test] +fn cfg_raw_idents() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [target.'cfg(any(r#true, r#all, r#target_os = "<>"))'.dependencies] + b = { path = "b/" } + "#, + ) + .file("src/lib.rs", "") + .file("b/Cargo.toml", &basic_manifest("b", "0.0.1")) + .file("b/src/lib.rs", "pub fn foo() {}") + .build(); + + p.cargo("check") + .with_stderr_data(str![[r#" +[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(); +} + +#[cargo_test] +fn cfg_raw_idents_empty() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [target.'cfg(r#))'.dependencies] + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + failed to parse `r#)` as a cfg expression: unexpected character `)` in cfg, expected parens, a comma, an identifier, or a string + +"#]]) + .run(); +} + +#[cargo_test] +fn cfg_raw_idents_not_really() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [target.'cfg(r#11))'.dependencies] + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + failed to parse `r#11)` as a cfg expression: unexpected character `1` in cfg, expected parens, a comma, an identifier, or a string + +"#]]) + .run(); +} + +#[cargo_test] +fn cfg_keywords() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [target.'cfg(any(async, fn, const, return, true))'.dependencies] + b = { path = "b/" } + "#, + ) + .file( + ".cargo/config.toml", + r#" + [target."cfg(any(for, match, extern, crate, false))"] + "#, + ) + .file("src/lib.rs", "") + .file("b/Cargo.toml", &basic_manifest("b", "0.0.1")) + .file("b/src/lib.rs", "pub fn foo() {}") + .build(); + + 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. + | + | [HELP] use raw-idents instead: `cfg(r#true)` +[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. + | + | [HELP] use raw-idents instead: `cfg(r#false)` +[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(); +}