]> git.proxmox.com Git - cargo.git/commitdiff
Warn instead of error
authorBenedikt Werner <1benediktwerner@gmail.com>
Sat, 7 Dec 2019 08:10:06 +0000 (09:10 +0100)
committerBenedikt Werner <1benediktwerner@gmail.com>
Sat, 7 Dec 2019 08:10:06 +0000 (09:10 +0100)
crates/cargo-platform/src/cfg.rs
crates/cargo-platform/src/error.rs
crates/cargo-platform/src/lib.rs
crates/cargo-platform/tests/test_cfg.rs
src/cargo/util/toml/mod.rs
src/doc/src/reference/specifying-dependencies.md

index efa3919d06f706eed165dae7ba466b4214f635f7..662c7f97bee04f4e25b20b5d298d73e217ab8a39 100644 (file)
@@ -1,4 +1,4 @@
-use crate::error::{ParseError, ParseErrorKind, ParseErrorKind::*};
+use crate::error::{ParseError, ParseErrorKind::*};
 use std::fmt;
 use std::iter;
 use std::str::{self, FromStr};
@@ -41,21 +41,6 @@ struct Parser<'a> {
     t: Tokenizer<'a>,
 }
 
-impl Cfg {
-    pub(crate) fn validate_as_target(&self) -> Result<(), ParseErrorKind> {
-        match self {
-            Cfg::Name(name) => match name.as_str() {
-                "test" | "debug_assertions" | "proc_macro" => Err(InvalidCfgName(name.to_string())),
-                _ => Ok(()),
-            },
-            Cfg::KeyPair(name, _) => match name.as_str() {
-                "feature" => Err(InvalidCfgKey(name.to_string())),
-                _ => Ok(()),
-            },
-        }
-    }
-}
-
 impl FromStr for Cfg {
     type Err = ParseError;
 
@@ -104,19 +89,6 @@ impl CfgExpr {
             CfgExpr::Value(ref e) => cfg.contains(e),
         }
     }
-
-    pub(crate) fn validate_as_target(&self) -> Result<(), ParseErrorKind> {
-        match *self {
-            CfgExpr::Not(ref e) => e.validate_as_target()?,
-            CfgExpr::All(ref e) | CfgExpr::Any(ref e) => {
-                for e in e {
-                    e.validate_as_target()?;
-                }
-            }
-            CfgExpr::Value(ref e) => e.validate_as_target()?,
-        }
-        Ok(())
-    }
 }
 
 impl FromStr for CfgExpr {
@@ -345,39 +317,3 @@ impl<'a> Token<'a> {
         }
     }
 }
-
-#[test]
-fn cfg_validate_as_target() {
-    fn p(s: &str) -> CfgExpr {
-        s.parse().unwrap()
-    }
-
-    assert!(p("unix").validate_as_target().is_ok());
-    assert!(p("windows").validate_as_target().is_ok());
-    assert!(p("any(not(unix), windows)").validate_as_target().is_ok());
-    assert!(p("foo").validate_as_target().is_ok());
-
-    assert!(p("target_arch = \"abc\"").validate_as_target().is_ok());
-    assert!(p("target_feature = \"abc\"").validate_as_target().is_ok());
-    assert!(p("target_os = \"abc\"").validate_as_target().is_ok());
-    assert!(p("target_family = \"abc\"").validate_as_target().is_ok());
-    assert!(p("target_env = \"abc\"").validate_as_target().is_ok());
-    assert!(p("target_endian = \"abc\"").validate_as_target().is_ok());
-    assert!(p("target_pointer_width = \"abc\"")
-        .validate_as_target()
-        .is_ok());
-    assert!(p("target_vendor = \"abc\"").validate_as_target().is_ok());
-    assert!(p("bar = \"def\"").validate_as_target().is_ok());
-
-    assert!(p("test").validate_as_target().is_err());
-    assert!(p("debug_assertions").validate_as_target().is_err());
-    assert!(p("proc_macro").validate_as_target().is_err());
-    assert!(p("any(not(debug_assertions), windows)")
-        .validate_as_target()
-        .is_err());
-
-    assert!(p("feature = \"abc\"").validate_as_target().is_err());
-    assert!(p("any(not(feature = \"def\"), target_arch = \"abc\")")
-        .validate_as_target()
-        .is_err());
-}
index ce0fb49f3508a88a39e3298aadfd2f29b9bc8373..19a15a1e132150bc79ceaad5611a38c45b812402 100644 (file)
@@ -17,8 +17,6 @@ pub enum ParseErrorKind {
     IncompleteExpr(&'static str),
     UnterminatedExpression(String),
     InvalidTarget(String),
-    InvalidCfgName(String),
-    InvalidCfgKey(String),
 
     #[doc(hidden)]
     __Nonexhaustive,
@@ -55,8 +53,6 @@ impl fmt::Display for ParseErrorKind {
                 write!(f, "unexpected content `{}` found after cfg expression", s)
             }
             InvalidTarget(s) => write!(f, "invalid target specifier: {}", s),
-            InvalidCfgName(name) => write!(f, "invalid name in target cfg: {}", name),
-            InvalidCfgKey(name) => write!(f, "invalid key in target cfg: {}", name),
             __Nonexhaustive => unreachable!(),
         }
     }
index 9b3033da4a5951f639c9c5cd7e8cfa4e4fffeed6..e647643ca1655a3a4b4f61d35971237e23421c40 100644 (file)
@@ -61,6 +61,48 @@ impl Platform {
         }
         Ok(())
     }
+
+    pub fn check_cfg_attributes(&self, warnings: &mut Vec<String>) {
+        fn check_cfg_expr(expr: &CfgExpr, warnings: &mut Vec<String>) {
+            match *expr {
+                CfgExpr::Not(ref e) => check_cfg_expr(e, warnings),
+                CfgExpr::All(ref e) | CfgExpr::Any(ref e) => {
+                    for e in e {
+                        check_cfg_expr(e, warnings);
+                    }
+                }
+                CfgExpr::Value(ref e) => match e {
+                    Cfg::Name(name) => match name.as_str() {
+                        "test" | "debug_assertions" | "proc_macro" =>
+                            warnings.push(format!(
+                                "Found `{}` in `target.'cfg(...)'.dependencies`. \
+                                 This value is not supported for selecting dependencies \
+                                 and will not work as expected. \
+                                 To learn more visit \
+                                 https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies",
+                                 name
+                            )),
+                        _ => (),
+                    },
+                    Cfg::KeyPair(name, _) => match name.as_str() {
+                        "feature" =>
+                            warnings.push(String::from(
+                                "Found `feature = ...` in `target.'cfg(...)'.dependencies`. \
+                                 This key is not supported for selecting dependencies \
+                                 and will not work as expected. \
+                                 Use the [features] section instead: \
+                                 https://doc.rust-lang.org/cargo/reference/manifest.html#the-features-section"
+                            )),
+                        _ => (),
+                    },
+                }
+            }
+        }
+
+        if let Platform::Cfg(cfg) = self {
+            check_cfg_expr(cfg, warnings);
+        }
+    }
 }
 
 impl serde::Serialize for Platform {
@@ -88,10 +130,7 @@ impl FromStr for Platform {
     fn from_str(s: &str) -> Result<Platform, ParseError> {
         if s.starts_with("cfg(") && s.ends_with(')') {
             let s = &s[4..s.len() - 1];
-            let cfg: CfgExpr = s.parse()?;
-            cfg.validate_as_target()
-                .map_err(|kind| ParseError::new(s, kind))?;
-            Ok(Platform::Cfg(cfg))
+            s.parse().map(Platform::Cfg)
         } else {
             Platform::validate_named_platform(s)?;
             Ok(Platform::Name(s.to_string()))
index 703acc908d3d9d74d14f6b06c06cede48af42e9d..dd99d9a79b85f35da92bee0cad636757e8f8e6ce 100644 (file)
@@ -155,16 +155,6 @@ fn bad_target_name() {
         "failed to parse `!foo` as a cfg expression: \
          invalid target specifier: unexpected character ! in target name",
     );
-    bad::<Platform>(
-        "cfg(debug_assertions)",
-        "failed to parse `debug_assertions` as a cfg expression: \
-         invalid name in target cfg: debug_assertions",
-    );
-    bad::<Platform>(
-        "cfg(feature = \"abc\")",
-        "failed to parse `feature = \"abc\"` as a cfg expression: \
-         invalid key in target cfg: feature",
-    );
 }
 
 #[test]
@@ -186,3 +176,76 @@ fn round_trip_platform() {
          all(target_os = \"freebsd\", target_arch = \"x86_64\")))",
     );
 }
+
+#[test]
+fn check_cfg_attributes() {
+    fn ok(s: &str) {
+        let p = Platform::Cfg(s.parse().unwrap());
+        let mut warnings = Vec::new();
+        p.check_cfg_attributes(&mut warnings);
+        assert!(
+            warnings.is_empty(),
+            "Expected no warnings but got: {:?}",
+            warnings,
+        );
+    }
+
+    fn warn(s: &str, names: &[&str]) {
+        let p = Platform::Cfg(s.parse().unwrap());
+        let mut warnings = Vec::new();
+        p.check_cfg_attributes(&mut warnings);
+        assert_eq!(
+            warnings.len(),
+            names.len(),
+            "Expecter warnings about {:?} but got {:?}",
+            names,
+            warnings,
+        );
+        for (name, warning) in names.iter().zip(warnings.iter()) {
+            assert!(
+                warning.contains(name),
+                "Expected warning about '{}' but got: {}",
+                name,
+                warning,
+            );
+        }
+    }
+
+    ok("unix");
+    ok("windows");
+    ok("any(not(unix), windows)");
+    ok("foo");
+
+    ok("target_arch = \"abc\"");
+    ok("target_feature = \"abc\"");
+    ok("target_os = \"abc\"");
+    ok("target_family = \"abc\"");
+    ok("target_env = \"abc\"");
+    ok("target_endian = \"abc\"");
+    ok("target_pointer_width = \"abc\"");
+    ok("target_vendor = \"abc\"");
+    ok("bar = \"def\"");
+
+    warn("test", &["test"]);
+    warn("debug_assertions", &["debug_assertions"]);
+    warn("proc_macro", &["proc_macro"]);
+    warn("feature = \"abc\"", &["feature"]);
+
+    warn("any(not(debug_assertions), windows)", &["debug_assertions"]);
+    warn(
+        "any(not(feature = \"def\"), target_arch = \"abc\")",
+        &["feature"],
+    );
+    warn(
+        "any(not(target_os = \"windows\"), proc_macro)",
+        &["proc_macro"],
+    );
+    warn(
+        "any(not(feature = \"windows\"), proc_macro)",
+        &["feature", "proc_macro"],
+    );
+    warn(
+        "all(not(debug_assertions), any(windows, proc_macro))",
+        &["debug_assertions", "proc_macro"],
+    );
+}
index 1553553010d39ebf96314db58a219e43457a5385..8ce11f22ece0cae4427a0fbcda08d328f8ceb5d5 100644 (file)
@@ -1085,7 +1085,11 @@ impl TomlManifest {
             process_dependencies(&mut cx, build_deps, Some(Kind::Build))?;
 
             for (name, platform) in me.target.iter().flatten() {
-                cx.platform = Some(name.parse()?);
+                cx.platform = {
+                    let platform: Platform = name.parse()?;
+                    platform.check_cfg_attributes(&mut cx.warnings);
+                    Some(platform)
+                };
                 process_dependencies(&mut cx, platform.dependencies.as_ref(), None)?;
                 let build_deps = platform
                     .build_dependencies
index 38488fc94ee794550922c573d6eab1085666afc6..aa2881df00b46eceee934500deda18bcba8135cc 100644 (file)
@@ -477,6 +477,8 @@ Use [the `[features]` section](manifest.md#the-features-section)
 instead.
 
 The same applies to `cfg(debug_assertions)`, `cfg(test)` and `cfg(prog_macro)`.
+These values will not work as expected and will always have the default value
+returned by `rustc --print=cfg`.
 There is currently no way to add dependencies based on these configuration values.
 
 In addition to `#[cfg]` syntax, Cargo also supports listing out the full target