chore(remap): Make del function only take one arg

docs/reference/remap.cue

@@ -49,7 +49,7 @@ remap: {
 
 			arguments: [...#Argument] // Allow for empty list
 			return: [#RemapReturnTypes, ...#RemapReturnTypes]
-			category:    "Coerce" | "Encode" | "Enumerate" | "Event" | "Hash" | "IP" | "Map" | "Number" | "Parse" | "Random" | "String" | "Test" | "Timestamp"
+			category:    "Array" | "Coerce" | "Encode" | "Enumerate" | "Event" | "Hash" | "IP" | "Map" | "Number" | "Parse" | "Random" | "String" | "Test" | "Timestamp"
 			description: string
 			examples?: [#RemapExample, ...#RemapExample]
 			name: Name

docs/reference/remap/del.cue

@@ -3,32 +3,40 @@ package metadata
 remap: functions: del: {
 	arguments: [
 		{
-			name:        "paths"
-			description: "The paths of the fields to delete."
+			name:        "path"
+			description: "The path of the field to delete."
 			required:    true
-			multiple:    true
 			type: ["string"]
 		},
 	]
-	return: ["null"]
+	return: ["any"]
 	category: "Event"
 	description: #"""
-		Removed the fields specified by the given paths from the root `event` object. Multiple fields can be specified.
+		Removes the field specified by the given path from the event object. If the field exists,
+		the field's value is returned by the delete operation, while `null` is returned if the field
+		doesn't exist.
 		"""#
 	examples: [
 		{
 			title: "Success"
 			input: {
 				"field1": 1
 				"field2": 2
-				"field3": 3
 			}
-			source: #"""
-				del(.field1, .field3)
-				"""#
+			source: "del(.field1)"
 			output: {
 				"field2": 2
 			}
 		},
+		{
+			title: "Rename field"
+			input: {
+				old_field: "please rename me"
+			}
+			source: ".new_field = del(.old_field)"
+			output: {
+				new_field: "please rename me"
+			}
+		},
 	]
 }

lib/remap-functions/src/del.rs

@@ -9,47 +9,58 @@ impl Function for Del {
     }
 
     fn parameters(&self) -> &'static [Parameter] {
-        generate_param_list! {
-            accepts = |_| true,
-            required = false,
-            keywords = [
-                "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16",
-            ],
-        }
+        &[Parameter {
+            keyword: "path",
+            accepts: |_| true,
+            required: true,
+        }]
     }
 
     fn compile(&self, mut arguments: ArgumentList) -> Result<Box<dyn Expression>> {
-        let mut paths = vec![];
-        paths.push(arguments.required_path("1")?);
-
-        for i in 2..=16 {
-            if let Some(path) = arguments.optional_path(&format!("{}", i))? {
-                paths.push(path)
-            }
-        }
+        let path = arguments.required_path("path")?;
 
-        Ok(Box::new(DelFn { paths }))
+        Ok(Box::new(DelFn { path }))
     }
 }
 
 #[derive(Debug, Clone)]
 pub struct DelFn {
-    paths: Vec<Path>,
+    path: Path,
+}
+
+impl DelFn {
+    #[cfg(test)]
+    fn new(path: Path) -> Self {
+        Self { path }
+    }
 }
 
 impl Expression for DelFn {
     fn execute(&self, _: &mut state::Program, object: &mut dyn Object) -> Result<Value> {
-        self.paths
-            .iter()
-            .try_for_each(|path| object.remove(path.as_ref(), false))?;
-
-        Ok(Value::Null)
+        // TODO: we're silencing the result of the `remove` call here, to make
+        // this function infallible.
+        //
+        // This isn't correct though, since, while deleting Vector log
+        // fields is infallible, deleting metric fields is not.
+        //
+        // For example, if you try to delete `.name` in a metric event, the
+        // call returns an error, since this is an infallible field.
+        //
+        // After some debating, we've decided to _silently ignore_ deletions
+        // of infallible fields for now, but we'll circle back to this in
+        // the near future to potentially improve this situation.
+        //
+        // see tracking issue: <TODO>
+        Ok(object
+            .remove(self.path.as_ref(), false)
+            .ok()
+            .flatten()
+            .unwrap_or(Value::Null))
     }
 
     fn type_def(&self, _: &state::Compiler) -> TypeDef {
         TypeDef {
-            fallible: true,
-            kind: value::Kind::Null,
+            kind: value::Kind::all(),
             ..Default::default()
         }
     }
@@ -58,15 +69,58 @@ impl Expression for DelFn {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::map;
 
-    test_type_def![static_type_def {
-        expr: |_| DelFn {
-            paths: vec![Path::from("foo")]
-        },
-        def: TypeDef {
-            fallible: true,
-            kind: value::Kind::Null,
-            ..Default::default()
-        },
-    }];
+    #[test]
+    fn del() {
+        let cases = vec![
+            (
+                // String field exists
+                map!["exists": "value"],
+                Ok(value!("value")),
+                DelFn::new(Path::from("exists")),
+            ),
+            (
+                // String field doesn't exist
+                map!["exists": "value"],
+                Ok(value!(null)),
+                DelFn::new(Path::from("does_not_exist")),
+            ),
+            (
+                // Array field exists
+                map!["exists": value!([1, 2, 3])],
+                Ok(value!([1, 2, 3])),
+                DelFn::new(Path::from("exists")),
+            ),
+            (
+                // Null field exists
+                map!["exists": value!(null)],
+                Ok(value!(null)),
+                DelFn::new(Path::from("exists")),
+            ),
+            (
+                // Map field exists
+                map!["exists": map!["foo": "bar"]],
+                Ok(value!(map!["foo": "bar"])),
+                DelFn::new(Path::from("exists")),
+            ),
+            (
+                // Integer field exists
+                map!["exists": 127],
+                Ok(value!(127)),
+                DelFn::new(Path::from("exists")),
+            ),
+        ];
+
+        let mut state = state::Program::default();
+
+        for (object, exp, func) in cases {
+            let mut object: Value = object.into();
+            let got = func
+                .execute(&mut state, &mut object)
+                .map_err(|e| format!("{:#}", anyhow::anyhow!(e)));
+
+            assert_eq!(got, exp);
+        }
+    }
 }

lib/remap-functions/src/lib.rs

@@ -234,10 +234,6 @@ pub use uuid_v4::UuidV4;
 
 pub fn all() -> Vec<Box<dyn remap::Function>> {
     vec![
-        #[cfg(feature = "md5")]
-        Box::new(Md5),
-        #[cfg(feature = "sha1")]
-        Box::new(Sha1),
         #[cfg(feature = "assert")]
         Box::new(Assert),
         #[cfg(feature = "ceil")]
@@ -280,6 +276,8 @@ pub fn all() -> Vec<Box<dyn remap::Function>> {
         Box::new(Ipv6ToIpV4),
         #[cfg(feature = "log")]
         Box::new(Log),
+        #[cfg(feature = "md5")]
+        Box::new(Md5),
         #[cfg(feature = "merge")]
         Box::new(Merge),
         #[cfg(feature = "now")]
@@ -312,6 +310,8 @@ pub fn all() -> Vec<Box<dyn remap::Function>> {
         Box::new(Replace),
         #[cfg(feature = "round")]
         Box::new(Round),
+        #[cfg(feature = "sha1")]
+        Box::new(Sha1),
         #[cfg(feature = "sha2")]
         Box::new(Sha2),
         #[cfg(feature = "sha3")]

lib/remap-functions/src/only_fields.rs

@@ -46,7 +46,7 @@ impl Expression for OnlyFieldsFn {
             .iter()
             .map(|path| (path, path.to_string()))
             .filter(|(_, key)| paths.iter().find(|p| key.starts_with(p.as_str())).is_none())
-            .try_for_each(|(path, _)| object.remove(&path, true))?;
+            .try_for_each(|(path, _)| object.remove(&path, true).map(|_| ()))?;
 
         Ok(Value::Null)
     }

lib/remap-lang/src/object.rs

@@ -55,5 +55,5 @@ pub trait Object: std::fmt::Debug {
     ///
     /// If `compact` is true, after deletion, if an empty object or array is
     /// left behind, it should be removed as well.
-    fn remove(&mut self, path: &Path, compact: bool) -> Result<(), String>;
+    fn remove(&mut self, path: &Path, compact: bool) -> Result<Option<Value>, String>;
 }

lib/remap-lang/src/value/object.rs

@@ -14,9 +14,11 @@ impl Object for Value {
         self.paths().map_err(|err| err.to_string())
     }
 
-    fn remove(&mut self, path: &Path, compact: bool) -> Result<(), String> {
+    fn remove(&mut self, path: &Path, compact: bool) -> Result<Option<Value>, String> {
+        let value = self.get(path)?;
         self.remove_by_path(path, compact);
-        Ok(())
+
+        Ok(value)
     }
 }
 
@@ -183,10 +185,18 @@ mod tests {
     #[test]
     fn object_remove() {
         let cases = vec![
+            (
+                value!({foo: "bar"}),
+                vec![Field(Regular("baz".to_owned()))],
+                false,
+                None,
+                Some(value!({foo: "bar"})),
+            ),
             (
                 value!({foo: "bar"}),
                 vec![Field(Regular("foo".to_owned()))],
                 false,
+                Some(value!("bar")),
                 Some(value!({})),
             ),
             (
@@ -196,30 +206,35 @@ mod tests {
                     Regular("foo".to_owned()),
                 ])],
                 false,
+                Some(value!("bar")),
                 Some(value!({})),
             ),
             (
                 value!({foo: "bar", baz: "qux"}),
                 vec![],
                 false,
+                Some(value!({foo: "bar", baz: "qux"})),
                 Some(value!({})),
             ),
             (
                 value!({foo: "bar", baz: "qux"}),
                 vec![],
                 true,
+                Some(value!({foo: "bar", baz: "qux"})),
                 Some(value!({})),
             ),
             (
                 value!({foo: [0]}),
                 vec![Field(Regular("foo".to_owned())), Index(0)],
                 false,
+                Some(value!(0)),
                 Some(value!({foo: []})),
             ),
             (
                 value!({foo: [0]}),
                 vec![Field(Regular("foo".to_owned())), Index(0)],
                 true,
+                Some(value!(0)),
                 Some(value!({})),
             ),
             (
@@ -230,6 +245,7 @@ mod tests {
                     Index(0),
                 ],
                 false,
+                Some(value!(0)),
                 Some(value!({foo: {"bar baz": []}, bar: "baz"})),
             ),
             (
@@ -240,15 +256,16 @@ mod tests {
                     Index(0),
                 ],
                 true,
+                Some(value!(0)),
                 Some(value!({bar: "baz"})),
             ),
         ];
 
-        for (mut object, segments, compact, expect) in cases {
+        for (mut object, segments, compact, value, expect) in cases {
             let path = Path::new_unchecked(segments);
 
-            assert_eq!(Object::remove(&mut object, &path, compact), Ok(()));
-            assert_eq!(Object::get(&object, &Path::root()), Ok(expect))
+            assert_eq!(Object::remove(&mut object, &path, compact), Ok(value));
+            assert_eq!(Object::get(&object, &Path::root()), Ok(expect));
         }
     }