Add future-incompat warning against keywords in cfgs and add raw-idents (#14671)

### What does this PR try to resolve?

This PR tries to address this thread
#14649 (comment) in
#14649 regarding `cfg(true)`/`cfg(false)` (and keywords more generally)
which are wrongly accepted[^1] as ident.

To address this, this PR does two things:
1. it introduce a future-incompatibility warning against those (wrongly)
accepted keywords as ident
2. it add parsing for raw-idents (`r#true`) add suggest-it in the
warning

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

This PR should be reviewed commit-by-commit.
Tests are included in preliminary commits.

### Additional information

I added a new struct for representing `Ident`s which is rawness aware.
Which implied updating `cargo-platform` to `0.2.0` due to the API
changes.

r? @epage

[^1]:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ccfb9c894dbf14e791c8ae7e4798efd0

Author: weihanglo

Cargo.lock

@@ -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" }

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

crates/cargo-platform/Cargo.toml

@@ -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.
+///
+/// <https://doc.rust-lang.org/reference/keywords.html>
+pub(crate) const KEYWORDS: &[&str; 2] = &["true", "false"];
+
 #[derive(Clone)]
 struct Tokenizer<'a> {
     s: iter::Peekable<str::CharIndices<'a>>,
@@ -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<H: Hasher>(&self, state: &mut H) {
+        self.name.hash(state);
+    }
+}
+
+impl PartialEq<str> 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<Ident> 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<CfgExpr, ParseError> {
         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<Cfg, ParseError> {
         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))));

crates/cargo-platform/src/cfg.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<String>, path: &Path) {
+        fn check_cfg_expr(expr: &CfgExpr, warnings: &mut Vec<String>, 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 <https://github.com/rust-lang/rust/issues/131204> 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 {

crates/cargo-platform/src/lib.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::<Cfg>("r# foo", "unexpected character");
+    bad::<Cfg>("r #foo", "unexpected content");
+    bad::<Cfg>("r#\"foo\"", "unexpected character");
+    bad::<Cfg>("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)]));

crates/cargo-platform/tests/test_cfg.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(","));
     }