[target-spec] fix handling of unknown predicates

target-spec/CHANGELOG.md

@@ -1,6 +1,13 @@
 
 # Changelog
 
+## [1.1.0] - 2022-08-30
+
+### Fixed
+
+- Unknown predicates now evaluate to false, matching Cargo's behavior.
+  - As a result, `Error::UnknownPredicate` is no longer in use and has been deprecated.
+
 ## [1.0.2] - 2022-05-29
 
 ### Changed
@@ -161,6 +168,7 @@ This was mistakenly published and was yanked.
 ## [0.1.0] - 2020-03-20
 - Initial release.
 
+[1.1.0]: https://github.com/facebookincubator/cargo-guppy/releases/tag/target-spec-1.1.0
 [1.0.2]: https://github.com/facebookincubator/cargo-guppy/releases/tag/target-spec-1.0.2
 [1.0.1]: https://github.com/facebookincubator/cargo-guppy/releases/tag/target-spec-1.0.1
 [1.0.0]: https://github.com/facebookincubator/cargo-guppy/releases/tag/target-spec-1.0.0

target-spec/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "target-spec"
-version = "1.0.2"
+version = "1.1.0"
 description = "Evaluate Cargo.toml target specifications"
 documentation = "https://docs.rs/target-spec"
 repository = "https://github.com/facebookincubator/cargo-guppy"

target-spec/src/errors.rs

@@ -16,6 +16,9 @@ pub enum Error {
     /// The provided platform triple was unknown.
     UnknownPlatformTriple(TripleParseError),
     /// The provided `cfg()` expression parsed correctly, but it had an unknown predicate.
+    ///
+    /// This is no longer used, but is kept for backwards compatibility.
+    #[deprecated(since = "1.1.0", note = "this variant is no longer in use")]
     UnknownPredicate(String),
 }
 
@@ -27,6 +30,7 @@ impl fmt::Display for Error {
             Error::UnknownPlatformTriple(_) => {
                 write!(f, "unknown platform triple")
             }
+            #[allow(deprecated)]
             Error::UnknownPredicate(pred) => {
                 write!(f, "cfg() expression has unknown predicate: {}", pred)
             }
@@ -40,6 +44,7 @@ impl error::Error for Error {
             Error::InvalidExpression(err) => Some(err),
             Error::UnknownTargetTriple(err) => Some(err),
             Error::UnknownPlatformTriple(err) => Some(err),
+            #[allow(deprecated)]
             Error::UnknownPredicate(_) => None,
         }
     }

target-spec/src/spec.rs

@@ -94,7 +94,9 @@ impl TargetExpression {
     pub fn new(input: &str) -> Result<Self, Error> {
         let expr = Expression::parse(input)
             .map_err(|err| Error::InvalidExpression(ExpressionParseError::new(err)))?;
-        Self::verify_expr(expr)
+        Ok(Self {
+            inner: Arc::new(expr),
+        })
     }
 
     /// Returns the string that was parsed into `self`.
@@ -127,25 +129,12 @@ impl TargetExpression {
                     Some(platform.has_flag(flag))
                 }
                 Predicate::KeyValue { .. } => {
-                    unreachable!("these predicates are disallowed in TargetExpression::verify_expr")
+                    // This is always interpreted by Cargo as false.
+                    Some(false)
                 }
             }
         })
     }
-
-    /// Verify this `cfg()` expression.
-    fn verify_expr(expr: Expression) -> Result<Self, Error> {
-        // Error out on unknown key-value pairs. Everything else is recognized (though
-        // DebugAssertions/ProcMacro etc always returns false, and flags return false by default).
-        for pred in expr.predicates() {
-            if let Predicate::KeyValue { key, .. } = pred {
-                return Err(Error::UnknownPredicate(key.to_string()));
-            }
-        }
-        Ok(Self {
-            inner: Arc::new(expr),
-        })
-    }
 }
 
 impl FromStr for TargetExpression {
@@ -237,9 +226,30 @@ mod tests {
 
     #[test]
     fn test_unknown_predicate() {
-        let err =
-            TargetSpec::new("cfg(bogus_key = \"bogus_value\")").expect_err("unknown predicate");
-        assert_eq!(err, Error::UnknownPredicate("bogus_key".to_string()));
+        let expr = match TargetSpec::new("cfg(bogus_key = \"bogus_value\")")
+            .expect("unknown predicate should parse")
+        {
+            TargetSpec::Triple(triple) => {
+                panic!("expected spec, got triple: {:?}", triple)
+            }
+            TargetSpec::Expression(expr) => expr,
+        };
+        assert_eq!(
+            expr.inner.predicates().collect::<Vec<_>>(),
+            vec![Predicate::KeyValue {
+                key: "bogus_key",
+                val: "bogus_value"
+            }],
+        );
+
+        let platform = Platform::current().unwrap();
+        // This should always evaluate to false.
+        assert_eq!(expr.eval(&platform), Some(false));
+
+        let expr = TargetSpec::new("cfg(not(bogus_key = \"bogus_value\"))")
+            .expect("unknown predicate should parse");
+        // This is a cfg(not()), so it should always evaluate to true.
+        assert_eq!(expr.eval(&platform), Some(true));
     }
 
     #[test]