From da8cb14f8b50bfc063363abb35345364b4f6fd42 Mon Sep 17 00:00:00 2001
From: Reilly Wood <26268125+rgwood@users.noreply.github.com>
Date: Tue, 28 Mar 2023 12:40:29 -0700
Subject: [PATCH] Fix `select` on empty lists (#8651)

This PR fixes `select` when given an empty list; it used to return
`null` when given an empty list. I also cleaned up other `select` tests
while I was in the area.

### Before:

```
> [] | select a | to nuon
null
```

### After:

```
> [] | select a | to nuon
[]
```

It looks like the previous behaviour was accidentally introduced by
[this PR](https://github.com/nushell/nushell/pull/7639).
---
 crates/nu-command/src/filters/select.rs    | 15 ++---
 crates/nu-command/tests/commands/select.rs | 77 ++++++++--------------
 2 files changed, 31 insertions(+), 61 deletions(-)

diff --git a/crates/nu-command/src/filters/select.rs b/crates/nu-command/src/filters/select.rs
index 6084771f1f..d40d33709c 100644
--- a/crates/nu-command/src/filters/select.rs
+++ b/crates/nu-command/src/filters/select.rs
@@ -167,7 +167,6 @@ fn select(
         ) => {
             let mut output = vec![];
             let mut columns_with_value = Vec::new();
-            let mut allempty = true;
             for input_val in input_vals {
                 if !columns.is_empty() {
                     let mut cols = vec![];
@@ -176,7 +175,6 @@ fn select(
                         //FIXME: improve implementation to not clone
                         match input_val.clone().follow_cell_path(&path.members, false) {
                             Ok(fetcher) => {
-                                allempty = false;
                                 cols.push(path.into_string().replace('.', "_"));
                                 vals.push(fetcher);
                                 if !columns_with_value.contains(&path) {
@@ -194,14 +192,11 @@ fn select(
                     output.push(input_val)
                 }
             }
-            if allempty {
-                Ok(Value::nothing(call_span).into_pipeline_data())
-            } else {
-                Ok(output
-                    .into_iter()
-                    .into_pipeline_data(engine_state.ctrlc.clone())
-                    .set_metadata(metadata))
-            }
+
+            Ok(output
+                .into_iter()
+                .into_pipeline_data(engine_state.ctrlc.clone())
+                .set_metadata(metadata))
         }
         PipelineData::ListStream(stream, metadata, ..) => {
             let mut values = vec![];
diff --git a/crates/nu-command/tests/commands/select.rs b/crates/nu-command/tests/commands/select.rs
index 5b4fd9b7cf..85954face2 100644
--- a/crates/nu-command/tests/commands/select.rs
+++ b/crates/nu-command/tests/commands/select.rs
@@ -4,7 +4,7 @@ use nu_test_support::{nu, pipeline};
 
 #[test]
 fn regular_columns() {
-    let actual = nu!(cwd: ".", pipeline(
+    let actual = nu!(pipeline(
         r#"
             echo [
                 [first_name, last_name, rusty_at, type];
@@ -67,7 +67,7 @@ fn complex_nested_columns() {
 
 #[test]
 fn fails_if_given_unknown_column_name() {
-    let actual = nu!(cwd: ".", pipeline(
+    let actual = nu!(pipeline(
         r#"
             echo [
                 [first_name, last_name, rusty_at, type];
@@ -86,7 +86,7 @@ fn fails_if_given_unknown_column_name() {
 
 #[test]
 fn column_names_with_spaces() {
-    let actual = nu!(cwd: ".", pipeline(
+    let actual = nu!(pipeline(
         r#"
             echo [
                 ["first name", "last name"];
@@ -105,7 +105,7 @@ fn column_names_with_spaces() {
 
 #[test]
 fn ignores_duplicate_columns_selected() {
-    let actual = nu!(cwd: ".", pipeline(
+    let actual = nu!(pipeline(
         r#"
             echo [
                 ["first name", "last name"];
@@ -162,12 +162,7 @@ fn selects_many_rows() {
 
 #[test]
 fn select_ignores_errors_successfully1() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#"
-        [{a: 1, b: 2} {a: 3, b: 5} {a: 3}] | select b? | length
-        "#
-    ));
+    let actual = nu!("[{a: 1, b: 2} {a: 3, b: 5} {a: 3}] | select b? | length");
 
     assert_eq!(actual.out, "3".to_string());
     assert!(actual.err.is_empty());
@@ -175,12 +170,7 @@ fn select_ignores_errors_successfully1() {
 
 #[test]
 fn select_ignores_errors_successfully2() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#"
-        [{a: 1} {a: 2} {a: 3}] | select b? | to nuon
-            "#
-    ));
+    let actual = nu!("[{a: 1} {a: 2} {a: 3}] | select b? | to nuon");
 
     assert_eq!(actual.out, "[[b]; [null], [null], [null]]".to_string());
     assert!(actual.err.is_empty());
@@ -188,10 +178,7 @@ fn select_ignores_errors_successfully2() {
 
 #[test]
 fn select_ignores_errors_successfully3() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#"sys | select invalid_key? | to nuon"#
-    ));
+    let actual = nu!("sys | select invalid_key? | to nuon");
 
     assert_eq!(actual.out, "{invalid_key: null}".to_string());
     assert!(actual.err.is_empty());
@@ -199,10 +186,8 @@ fn select_ignores_errors_successfully3() {
 
 #[test]
 fn select_ignores_errors_successfully4() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#""key val\na 1\nb 2\n" | lines | split column -c " " | select foo? | to nuon"#
-    ));
+    let actual =
+        nu!(r#""key val\na 1\nb 2\n" | lines | split column -c " " | select foo? | to nuon"#);
 
     assert_eq!(actual.out, r#"[[foo]; [null], [null], [null]]"#.to_string());
     assert!(actual.err.is_empty());
@@ -210,12 +195,7 @@ fn select_ignores_errors_successfully4() {
 
 #[test]
 fn select_failed1() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#"
-        [{a: 1, b: 2} {a: 3, b: 5} {a: 3}] | select b
-            "#
-    ));
+    let actual = nu!("[{a: 1, b: 2} {a: 3, b: 5} {a: 3}] | select b ");
 
     assert!(actual.out.is_empty());
     assert!(actual.err.contains("cannot find column"));
@@ -223,12 +203,7 @@ fn select_failed1() {
 
 #[test]
 fn select_failed2() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#"
-        [{a: 1} {a: 2} {a: 3}] | select b
-            "#
-    ));
+    let actual = nu!("[{a: 1} {a: 2} {a: 3}] | select b");
 
     assert!(actual.out.is_empty());
     assert!(actual.err.contains("cannot find column"));
@@ -236,10 +211,7 @@ fn select_failed2() {
 
 #[test]
 fn select_failed3() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#""key val\na 1\nb 2\n" | lines | split column -c " " | select "100""#
-    ));
+    let actual = nu!(r#""key val\na 1\nb 2\n" | lines | split column -c " " | select "100""#);
 
     assert!(actual.out.is_empty());
     assert!(actual.err.contains("cannot find column"));
@@ -247,25 +219,28 @@ fn select_failed3() {
 
 #[test]
 fn select_failed4() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#"
-        [{a: 1 b: 10}, {a:2, b:11}] | select 0 0
-        "#
-    ));
+    let actual = nu!("[{a: 1 b: 10}, {a:2, b:11}] | select 0 0");
 
     assert!(actual.err.contains("Select can't get the same row twice"));
 }
 
 #[test]
 fn ignore_errors_works() {
-    let actual = nu!(
-        cwd: ".",
-        r#"
+    let actual = nu!(r#"
         let path = "foo";
         [{}] | select -i $path | to nuon
-        "#
-    );
+        "#);
 
     assert_eq!(actual.out, "[[foo]; [null]]");
 }
+
+#[test]
+fn select_on_empty_list_returns_empty_list() {
+    // once with a List
+    let actual = nu!("[] | select foo | to nuon");
+    assert_eq!(actual.out, "[]");
+
+    // and again with a ListStream
+    let actual = nu!("[] | each {|i| $i} | select foo | to nuon");
+    assert_eq!(actual.out, "[]");
+}