Add tests for get_data_by_key (#2658)

* Add test for get_data_by_key

* Apply same order for ValuExt impl

* Nothing helper for tests

* Use get_data_by_key from ValueExt

crates/nu-cli/src/commands/help.rs

@@ -6,7 +6,7 @@ use nu_data::command::signature_dict;
 use nu_errors::ShellError;
 use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, TaggedDictBuilder, UntaggedValue, Value};
 use nu_source::{SpannedItem, Tagged};
-use nu_value_ext::get_data_by_key;
+use nu_value_ext::ValueExt;
 
 pub struct Help;
 
@@ -96,7 +96,8 @@ async fn help(args: CommandArgs, registry: &CommandRegistry) -> Result<OutputStr
                 dict.insert_untagged("name", cmd_name);
                 dict.insert_untagged(
                     "description",
-                    get_data_by_key(&value, "usage".spanned_unknown())
+                    value
+                        .get_data_by_key("usage".spanned_unknown())
                         .ok_or_else(|| {
                             ShellError::labeled_error(
                                 "Expected a usage key",

crates/nu-cli/src/commands/pivot.rs

@@ -5,7 +5,7 @@ use nu_protocol::{
     merge_descriptors, ReturnSuccess, Signature, SyntaxShape, TaggedDictBuilder, UntaggedValue,
 };
 use nu_source::{SpannedItem, Tagged};
-use nu_value_ext::get_data_by_key;
+use nu_value_ext::ValueExt;
 
 pub struct Pivot;
 
@@ -79,7 +79,7 @@ pub async fn pivot(
     if args.header_row {
         for i in input.clone() {
             if let Some(desc) = descs.get(0) {
-                match get_data_by_key(&i, desc[..].spanned_unknown()) {
+                match &i.get_data_by_key(desc[..].spanned_unknown()) {
                     Some(x) => {
                         if let Ok(s) = x.as_string() {
                             headers.push(s.to_string());
@@ -136,7 +136,7 @@ pub async fn pivot(
         }
 
         for i in input.clone() {
-            match get_data_by_key(&i, desc[..].spanned_unknown()) {
+            match &i.get_data_by_key(desc[..].spanned_unknown()) {
                 Some(x) => {
                     dict.insert_value(headers[column_num].clone(), x.clone());
                 }

crates/nu-cli/src/commands/sort_by.rs

@@ -4,7 +4,7 @@ use nu_data::base::coerce_compare;
 use nu_errors::ShellError;
 use nu_protocol::{Signature, SyntaxShape, UntaggedValue, Value};
 use nu_source::Tagged;
-use nu_value_ext::get_data_by_key;
+use nu_value_ext::ValueExt;
 
 pub struct SortBy;
 
@@ -118,8 +118,8 @@ pub fn sort(
     }
 
     for sort_arg in keys.iter() {
-        let match_test = get_data_by_key(&vec[0], sort_arg.borrow_spanned());
+        let match_test = &vec[0].get_data_by_key(sort_arg.borrow_spanned());
-        if match_test == None {
+        if match_test.is_none() {
             return Err(ShellError::labeled_error(
                 "Can not find column to sort by",
                 "invalid column",
@@ -168,7 +168,7 @@ pub fn sort(
             let calc_key = |item: &Value| {
                 keys.iter()
                     .map(|f| {
-                        let mut value_option = get_data_by_key(item, f.borrow_spanned());
+                        let mut value_option = item.get_data_by_key(f.borrow_spanned());
 
                         if insensitive {
                             if let Some(value) = &value_option {

crates/nu-cli/src/commands/to_delimited_data.rs

@@ -4,7 +4,7 @@ use indexmap::{indexset, IndexSet};
 use nu_errors::ShellError;
 use nu_protocol::{Primitive, ReturnSuccess, UntaggedValue, Value};
 use nu_source::Spanned;
-use nu_value_ext::{as_string, get_data_by_key};
+use nu_value_ext::{as_string, ValueExt};
 
 fn from_value_to_delimited_string(
     tagged_value: &Value,
@@ -66,7 +66,7 @@ fn from_value_to_delimited_string(
                 for l in list {
                     let mut row = vec![];
                     for desc in &merged_descriptors {
-                        row.push(match get_data_by_key(l, desc.borrow_spanned()) {
+                        row.push(match l.get_data_by_key(desc.borrow_spanned()) {
                             Some(s) => to_string_tagged_value(&s)?,
                             None => String::new(),
                         });

crates/nu-test-support/src/value.rs

@@ -22,6 +22,10 @@ pub fn row(entries: IndexMap<String, Value>) -> Value {
     UntaggedValue::row(entries).into_untagged_value()
 }
 
+pub fn nothing() -> Value {
+    UntaggedValue::nothing().into_untagged_value()
+}
+
 pub fn table(list: &[Value]) -> Value {
     UntaggedValue::table(list).into_untagged_value()
 }

crates/nu-value-ext/src/lib.rs

@@ -97,14 +97,6 @@ impl ValueExt for Value {
         insert_data_at_member(self, member, new_value)
     }
 
-    fn insert_data_at_column_path(
-        &self,
-        split_path: &ColumnPath,
-        new_value: Value,
-    ) -> Result<Value, ShellError> {
-        insert_data_at_column_path(self, split_path, new_value)
-    }
-
     fn forgiving_insert_data_at_column_path(
         &self,
         split_path: &ColumnPath,
@@ -113,6 +105,14 @@ impl ValueExt for Value {
         forgiving_insert_data_at_column_path(self, split_path, new_value)
     }
 
+    fn insert_data_at_column_path(
+        &self,
+        split_path: &ColumnPath,
+        new_value: Value,
+    ) -> Result<Value, ShellError> {
+        insert_data_at_column_path(self, split_path, new_value)
+    }
+
     fn replace_data_at_column_path(
         &self,
         split_path: &ColumnPath,
@@ -911,4 +911,43 @@ mod tests {
             *string("Arepas de Yehuda")
         );
     }
+
+    #[test]
+    fn get_row_data_by_key() {
+        let row = row(indexmap! {
+                "lines".to_string() => int(0),
+                "words".to_string() => int(7),
+        });
+        assert_eq!(
+            row.get_data_by_key("lines".spanned_unknown()).unwrap(),
+            int(0)
+        );
+        assert!(row.get_data_by_key("chars".spanned_unknown()).is_none());
+    }
+
+    #[test]
+    fn get_table_data_by_key() {
+        let row1 = row(indexmap! {
+                "lines".to_string() => int(0),
+                "files".to_string() => int(10),
+        });
+
+        let row2 = row(indexmap! {
+                "files".to_string() => int(1)
+        });
+
+        let table_value = table(&[row1, row2]);
+        assert_eq!(
+            table_value
+                .get_data_by_key("files".spanned_unknown())
+                .unwrap(),
+            table(&[int(10), int(1)])
+        );
+        assert_eq!(
+            table_value
+                .get_data_by_key("chars".spanned_unknown())
+                .unwrap(),
+            table(&[nothing(), nothing()])
+        );
+    }
 }