Repository: clarkmcc/cel-rust (language: rust)

Issue:
Defaulting to features for a compliant CEL interpreter? 
With the fix of #90, some "standard" features of CEL are now disabled by default. While I agree with the reason and think that providing this fine grained control over dependencies et al is real great to have in any crate, I wonder if such functionality now behind a feature flag, should be `default` and be an opt-out rather an opt-in. So that anyone grabbing the crate, gets a "std compatible experience" wrt CEL... more so here, as this would be a "slow killer", i.e. only becoming visible at runtime when actually executing some otherwise perfectly fine `Expression`. wdyt? cc/ @Caellian

<details>

i.e.
```diff
diff --git a/interpreter/Cargo.toml b/interpreter/Cargo.toml
index fa70cce..c9fb30e 100644
--- a/interpreter/Cargo.toml
+++ b/interpreter/Cargo.toml
@@ -28,6 +28,7 @@ name = "runtime"
 harness = false
 
 [features]
+default = ["regex", "chrono"]
 json = ["dep:base64", "dep:serde_json"]
 regex = ["dep:regex"]
 chrono = ["dep:chrono"]
```

</details>


Discussion / hints:
Also, unsure about `json`, unlike `regex` & `chrono`, I _think_ this would become visible during development using the crate... so less opinionated whether it should be opt-in or -out... Tho technically JSON (& protobuf) interop are meant to be standard based on my reading of the spec... 

Fix patch:
diff --git a/example/Cargo.toml b/example/Cargo.toml
index a0720ef..5751eee 100644
--- a/example/Cargo.toml
+++ b/example/Cargo.toml
@@ -3,48 +3,58 @@ name = "example"
 version = "0.1.0"
 edition = "2021"
 
+[features]
+axum = ["dep:axum", "dep:tokio", "dep:thiserror"]
+json = ["dep:serde_json", "cel-interpreter/json"]
+chrono = ["dep:chrono", "cel-interpreter/chrono"]
+
 [dependencies]
+cel-interpreter = { path = "../interpreter", default-features = false }
+
+chrono = { version = "0.4", optional = true }
+
+serde = { version = "1.0", features = ["derive"] }
+serde_json = { version = "1.0", optional = true }
+
 axum = { version = "0.7.5", default-features = false, features = [
     "http1",
     "json",
     "tokio",
-] }
-cel-interpreter = { path = "../interpreter", features = ["json", "chrono", "regex"] }
-chrono = "0.4.26"
-serde = { version = "1.0.196", features = ["derive"] }
-serde_json = "1.0.124"
-thiserror = { version = "1.0.61", default-features = false }
+], optional = true }
 tokio = { version = "1.38.0", default-features = false, features = [
     "macros",
     "net",
     "rt-multi-thread",
-] }
+], optional = true }
+thiserror = { version = "1.0", optional = true }
 
 [[bin]]
-name = "simple"
+name = "example-simple"
 path = "src/simple.rs"
 
 [[bin]]
-name = "variables"
+name = "example-variables"
 path = "src/variables.rs"
 
 [[bin]]
-name = "functions"
+name = "example-functions"
 path = "src/functions.rs"
+required-features = ["chrono"]
 
 [[bin]]
-name = "threads"
+name = "example-threads"
 path = "src/threads.rs"
 
 [[bin]]
-name = "serde"
+name = "example-serde"
 path = "src/serde.rs"
 
 [[bin]]
-name = "axum"
+name = "example-axum"
 path = "src/axum.rs"
+required-features = ["axum"]
 
 [[bin]]
-name = "json"
+name = "example-json"
 path = "src/json.rs"
-
+required-features = ["json"]
diff --git a/interpreter/Cargo.toml b/interpreter/Cargo.toml
index fa70cce..45a1379 100644
--- a/interpreter/Cargo.toml
+++ b/interpreter/Cargo.toml
@@ -10,15 +10,18 @@ categories = ["compilers"]
 
 [dependencies]
 cel-parser = { path = "../parser", version = "0.8.0" }
-thiserror = "1.0.40"
-chrono = { version = "0.4.26", default-features = false, features = ["alloc"], optional = true }
+
 nom = "7.1.3"
-paste = "1.0.14"
-serde = "1.0.196"
+
+chrono = { version = "0.4", default-features = false, features = ["alloc"], optional = true }
 regex = { version = "1.10.5", optional = true }
-serde_json = { version = "1.0.124", optional = true }
+serde = "1.0"
+serde_json = { version = "1.0", optional = true }
 base64 = { version = "0.22.1", optional = true }
 
+thiserror = "1.0"
+paste = "1.0"
+
 [dev-dependencies]
 criterion = { version = "0.5.1", features = ["html_reports"] }
 serde_bytes = "0.11.14"
@@ -28,6 +31,7 @@ name = "runtime"
 harness = false
 
 [features]
-json = ["dep:base64", "dep:serde_json"]
+default = ["regex", "chrono"]
+json = ["dep:serde_json", "dep:base64"]
 regex = ["dep:regex"]
 chrono = ["dep:chrono"]
diff --git a/interpreter/src/functions.rs b/interpreter/src/functions.rs
index a63cb38..457010f 100644
--- a/interpreter/src/functions.rs
+++ b/interpreter/src/functions.rs
@@ -645,10 +645,7 @@ pub fn max(Arguments(args): Arguments) -> Result<Value> {
 #[cfg(test)]
 mod tests {
     use crate::context::Context;
-    use crate::testing::test_script;
-    #[cfg(feature = "regex")]
-    use crate::ExecutionError::FunctionError;
-    use std::collections::HashMap;
+    use crate::tests::test_script;
 
     fn assert_script(input: &(&str, &str)) {
         assert_eq!(test_script(input.1, None), Ok(true.into()), "{}", input.0);
@@ -679,7 +676,7 @@ mod tests {
 
         for (name, script) in tests {
             let mut ctx = Context::default();
-            ctx.add_variable_from_value("foo", HashMap::from([("bar", 1)]));
+            ctx.add_variable_from_value("foo", std::collections::HashMap::from([("bar", 1)]));
             assert_eq!(test_script(script, Some(ctx)), Ok(true.into()), "{}", name);
         }
     }
@@ -943,7 +940,7 @@ mod tests {
             test_script(
                 "'foobar'.matches('(foo') == true", None),
             Err(
-                FunctionError {
+                crate::ExecutionError::FunctionError {
                     function: "matches".to_string(),
                     message: "'(foo' not a valid regex:\nregex parse error:\n    (foo\n    ^\nerror: unclosed group".to_string()
                 }
diff --git a/interpreter/src/lib.rs b/interpreter/src/lib.rs
index e307f55..b11ff47 100644
--- a/interpreter/src/lib.rs
+++ b/interpreter/src/lib.rs
@@ -13,19 +13,19 @@ pub use cel_parser::Expression;
 pub use context::Context;
 pub use functions::FunctionContext;
 pub use objects::{ResolveResult, Value};
-#[cfg(feature = "chrono")]
-mod duration;
 pub mod functions;
 mod magic;
 pub mod objects;
 mod resolvers;
+
+#[cfg(feature = "chrono")]
+mod duration;
+
 mod ser;
 pub use ser::to_value;
 
 #[cfg(feature = "json")]
 mod json;
-#[cfg(test)]
-mod testing;
 
 use magic::FromContext;
 
@@ -173,11 +173,16 @@ impl TryFrom<&str> for Program {
 mod tests {
     use crate::context::Context;
     use crate::objects::{ResolveResult, Value};
-    use crate::testing::test_script;
     use crate::{ExecutionError, Program};
     use std::collections::HashMap;
     use std::convert::TryInto;
 
+    /// Tests the provided script and returns the result. An optional context can be provided.
+    pub(crate) fn test_script(script: &str, ctx: Option<Context>) -> ResolveResult {
+        let program = Program::compile(script).unwrap();
+        program.execute(&ctx.unwrap_or_default())
+    }
+
     #[test]
     fn parse() {
         Program::compile("1 + 1").unwrap();
diff --git a/interpreter/src/objects.rs b/interpreter/src/objects.rs
index 50708d8..6c8ccc2 100644
--- a/interpreter/src/objects.rs
+++ b/interpreter/src/objects.rs
@@ -1,15 +1,12 @@
 use crate::context::Context;
 use crate::functions::FunctionContext;
-use crate::ser::SerializationError;
-use crate::ExecutionError::NoSuchKey;
-use crate::{to_value, ExecutionError};
-use cel_parser::{ArithmeticOp, Atom, Expression, Member, RelationOp, UnaryOp};
-use core::ops;
-use serde::{Serialize, Serializer};
+use crate::ExecutionError;
+use cel_parser::ast::*;
 use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::convert::{Infallible, TryFrom, TryInto};
 use std::fmt::{Display, Formatter};
+use std::ops;
 use std::sync::Arc;
 
 #[derive(Debug, PartialEq, Clone)]
@@ -84,10 +81,10 @@ impl From<u64> for Key {
     }
 }
 
-impl Serialize for Key {
+impl serde::Serialize for Key {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
-        S: Serializer,
+        S: serde::Serializer,
     {
         match self {
             Key::Int(v) => v.serialize(serializer),
@@ -143,13 +140,12 @@ pub trait TryIntoValue {
     fn try_into_value(self) -> Result<Value, Self::Error>;
 }
 
-impl<T: Serialize> TryIntoValue for T {
-    type Error = SerializationError;
+impl<T: serde::Serialize> TryIntoValue for T {
+    type Error = crate::ser::SerializationError;
     fn try_into_value(self) -> Result<Value, Self::Error> {
-        to_value(self)
+        crate::ser::to_value(self)
     }
 }
-
 impl TryIntoValue for Value {
     type Error = Infallible;
     fn try_into_value(self) -> Result<Value, Self::Error> {
@@ -629,7 +625,7 @@ impl<'a> Value {
                 // give priority to the property. Maybe we can implement lookahead
                 // to see if the next token is a function call?
                 match (child, ctx.has_function(&***name)) {
-                    (None, false) => NoSuchKey(name.clone()).into(),
+                    (None, false) => ExecutionError::NoSuchKey(name.clone()).into(),
                     (Some(child), _) => child.into(),
                     (None, true) => Value::Function(name.clone(), Some(self.into())).into(),
                 }
@@ -960,4 +956,20 @@ mod tests {
         let result = program.execute(&context);
         assert_eq!(result.unwrap(), Value::Null);
     }
+
+    #[test]
+    fn reference_to_value() {
+        let test = "example".to_string();
+        let direct: Value = test.as_str().into();
+        assert_eq!(direct, Value::String(Arc::new(String::from("example"))));
+
+        let vec = vec![test.as_str()];
+        let indirect: Value = vec.into();
+        assert_eq!(
+            indirect,
+            Value::List(Arc::new(vec![Value::String(Arc::new(String::from(
+                "example"
+            )))]))
+        );
+    }
 }


Test patch:
diff --git a/interpreter/src/testing.rs b/interpreter/src/testing.rs
deleted file mode 100644
index f814652..0000000
--- a/interpreter/src/testing.rs
+++ /dev/null
@@ -1,9 +0,0 @@
-use crate::context::Context;
-use crate::objects::ResolveResult;
-use crate::Program;
-
-/// Tests the provided script and returns the result. An optional context can be provided.
-pub(crate) fn test_script(script: &str, ctx: Option<Context>) -> ResolveResult {
-    let program = Program::compile(script).unwrap();
-    program.execute(&ctx.unwrap_or_default())
-}