To csv fix (#7850)

# Description

Fixes #7800 . 
`to csv` and `to tsv` no longer:
- accept anything but records and tables as input,
- accept lists that are not tables,
- accept tables and records with values that are not primitives (other
lists, tables and records).

# User-Facing Changes

Using `to csv` and `to tsv` on any of inputs mentioned above will result
in `cant_convert` error.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>

crates/nu-command/src/formats/to/csv.rs

@@ -17,7 +17,10 @@ impl Command for ToCsv {
 
     fn signature(&self) -> Signature {
         Signature::build("to csv")
-            .input_output_types(vec![(Type::Any, Type::String)])
+            .input_output_types(vec![
+                (Type::Record(vec![]), Type::String),
+                (Type::Table(vec![]), Type::String),
+            ])
             .named(
                 "separator",
                 SyntaxShape::String,
@@ -44,6 +47,11 @@ impl Command for ToCsv {
                 example: "[[foo bar]; [1 2]] | to csv -s ';' ",
                 result: Some(Value::test_string("foo;bar\n1;2\n")),
             },
+            Example {
+                description: "Outputs an CSV string representing the contents of this record",
+                example: "{a: 1 b: 2} | to csv",
+                result: Some(Value::test_string("a,b\n1,2\n")),
+            },
         ]
     }
 

crates/nu-command/src/formats/to/delimited.rs

@@ -1,7 +1,8 @@
-use csv::WriterBuilder;
+use csv::{Writer, WriterBuilder};
 use indexmap::{indexset, IndexSet};
 use nu_protocol::{Config, IntoPipelineData, PipelineData, ShellError, Span, Value};
 use std::collections::VecDeque;
+use std::error::Error;
 
 fn from_value_to_delimited_string(
     value: &Value,
@@ -11,75 +12,92 @@ fn from_value_to_delimited_string(
 ) -> Result<String, ShellError> {
     match value {
         Value::Record { cols, vals, span } => {
-            let mut wtr = WriterBuilder::new()
+            record_to_delimited(cols, vals, span, separator, config, head)
-                .delimiter(separator as u8)
-                .from_writer(vec![]);
-            let mut fields: VecDeque<String> = VecDeque::new();
-            let mut values: VecDeque<String> = VecDeque::new();
-
-            for (k, v) in cols.iter().zip(vals.iter()) {
-                fields.push_back(k.clone());
-
-                values.push_back(to_string_tagged_value(v, config, head, *span)?);
-            }
-
-            wtr.write_record(fields).expect("can not write.");
-            wtr.write_record(values).expect("can not write.");
-
-            let v = String::from_utf8(wtr.into_inner().map_err(|_| {
-                ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None)
-            })?)
-            .map_err(|_| {
-                ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None)
-            })?;
-            Ok(v)
-        }
-        Value::List { vals, span } => {
-            let mut wtr = WriterBuilder::new()
-                .delimiter(separator as u8)
-                .from_writer(vec![]);
-
-            let merged_descriptors = merge_descriptors(vals);
-
-            if merged_descriptors.is_empty() {
-                wtr.write_record(
-                    vals.iter()
-                        .map(|ele| {
-                            to_string_tagged_value(ele, config, head, *span)
-                                .unwrap_or_else(|_| String::new())
-                        })
-                        .collect::<Vec<_>>(),
-                )
-                .expect("can not write");
-            } else {
-                wtr.write_record(merged_descriptors.iter().map(|item| &item[..]))
-                    .expect("can not write.");
-
-                for l in vals {
-                    let mut row = vec![];
-                    for desc in &merged_descriptors {
-                        row.push(match l.to_owned().get_data_by_key(desc) {
-                            Some(s) => to_string_tagged_value(&s, config, head, *span)?,
-                            None => String::new(),
-                        });
-                    }
-                    wtr.write_record(&row).expect("can not write");
-                }
-            }
-            let v = String::from_utf8(wtr.into_inner().map_err(|_| {
-                ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None)
-            })?)
-            .map_err(|_| {
-                ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None)
-            })?;
-            Ok(v)
         }
+        Value::List { vals, span } => table_to_delimited(vals, span, separator, config, head),
         // Propagate errors by explicitly matching them before the final case.
         Value::Error { error } => Err(error.clone()),
-        other => to_string_tagged_value(value, config, other.expect_span(), head),
+        v => Err(make_unsupported_input_error(v, head, v.expect_span())),
     }
 }
 
+fn record_to_delimited(
+    cols: &[String],
+    vals: &[Value],
+    span: &Span,
+    separator: char,
+    config: &Config,
+    head: Span,
+) -> Result<String, ShellError> {
+    let mut wtr = WriterBuilder::new()
+        .delimiter(separator as u8)
+        .from_writer(vec![]);
+    let mut fields: VecDeque<String> = VecDeque::new();
+    let mut values: VecDeque<String> = VecDeque::new();
+
+    for (k, v) in cols.iter().zip(vals.iter()) {
+        fields.push_back(k.clone());
+
+        values.push_back(to_string_tagged_value(v, config, head, *span)?);
+    }
+
+    wtr.write_record(fields).expect("can not write.");
+    wtr.write_record(values).expect("can not write.");
+
+    writer_to_string(wtr).map_err(|_| make_conversion_error("record", span))
+}
+
+fn table_to_delimited(
+    vals: &Vec<Value>,
+    span: &Span,
+    separator: char,
+    config: &Config,
+    head: Span,
+) -> Result<String, ShellError> {
+    if let Some(val) = find_non_record(vals) {
+        return Err(make_unsupported_input_error(val, head, *span));
+    }
+
+    let mut wtr = WriterBuilder::new()
+        .delimiter(separator as u8)
+        .from_writer(vec![]);
+
+    let merged_descriptors = merge_descriptors(vals);
+
+    if merged_descriptors.is_empty() {
+        let vals = vals
+            .iter()
+            .map(|ele| {
+                to_string_tagged_value(ele, config, head, *span).unwrap_or_else(|_| String::new())
+            })
+            .collect::<Vec<_>>();
+        wtr.write_record(vals).expect("can not write");
+    } else {
+        wtr.write_record(merged_descriptors.iter().map(|item| &item[..]))
+            .expect("can not write.");
+
+        for l in vals {
+            let mut row = vec![];
+            for desc in &merged_descriptors {
+                row.push(match l.to_owned().get_data_by_key(desc) {
+                    Some(s) => to_string_tagged_value(&s, config, head, *span)?,
+                    None => String::new(),
+                });
+            }
+            wtr.write_record(&row).expect("can not write");
+        }
+    }
+    writer_to_string(wtr).map_err(|_| make_conversion_error("table", span))
+}
+
+fn writer_to_string(writer: Writer<Vec<u8>>) -> Result<String, Box<dyn Error>> {
+    Ok(String::from_utf8(writer.into_inner()?)?)
+}
+
+fn make_conversion_error(type_from: &str, span: &Span) -> ShellError {
+    ShellError::CantConvert(type_from.to_string(), "string".to_string(), *span, None)
+}
+
 fn to_string_tagged_value(
     v: &Value,
     config: &Config,
@@ -95,22 +113,30 @@ fn to_string_tagged_value(
         | Value::CustomValue { .. }
         | Value::Filesize { .. }
         | Value::CellPath { .. }
-        | Value::List { .. }
-        | Value::Record { .. }
         | Value::Float { .. } => Ok(v.clone().into_abbreviated_string(config)),
         Value::Date { val, .. } => Ok(val.to_string()),
         Value::Nothing { .. } => Ok(String::new()),
         // Propagate existing errors
         Value::Error { error } => Err(error.clone()),
-        _ => Err(ShellError::UnsupportedInput(
+        _ => Err(make_unsupported_input_error(v, head, span)),
-            "Unexpected type".to_string(),
-            format!("input type: {:?}", v.get_type()),
-            head,
-            span,
-        )),
     }
 }
 
+fn make_unsupported_input_error(value: &Value, head: Span, span: Span) -> ShellError {
+    ShellError::UnsupportedInput(
+        "Unexpected type".to_string(),
+        format!("input type: {:?}", value.get_type()),
+        head,
+        span,
+    )
+}
+
+pub fn find_non_record(values: &[Value]) -> Option<&Value> {
+    values
+        .iter()
+        .find(|val| !matches!(val, Value::Record { .. }))
+}
+
 pub fn merge_descriptors(values: &[Value]) -> Vec<String> {
     let mut ret: Vec<String> = vec![];
     let mut seen: IndexSet<String> = indexset! {};

crates/nu-command/src/formats/to/tsv.rs

@@ -15,7 +15,10 @@ impl Command for ToTsv {
 
     fn signature(&self) -> Signature {
         Signature::build("to tsv")
-            .input_output_types(vec![(Type::Any, Type::String)])
+            .input_output_types(vec![
+                (Type::Record(vec![]), Type::String),
+                (Type::Table(vec![]), Type::String),
+            ])
             .switch(
                 "noheaders",
                 "do not output the column names as the first row",
@@ -29,11 +32,18 @@ impl Command for ToTsv {
     }
 
     fn examples(&self) -> Vec<Example> {
-        vec![Example {
+        vec![
-            description: "Outputs an TSV string representing the contents of this table",
+            Example {
-            example: "[[foo bar]; [1 2]] | to tsv",
+                description: "Outputs an TSV string representing the contents of this table",
-            result: Some(Value::test_string("foo\tbar\n1\t2\n")),
+                example: "[[foo bar]; [1 2]] | to tsv",
-        }]
+                result: Some(Value::test_string("foo\tbar\n1\t2\n")),
+            },
+            Example {
+                description: "Outputs an TSV string representing the contents of this record",
+                example: "{a: 1 b: 2} | to tsv",
+                result: Some(Value::test_string("a\tb\n1\t2\n")),
+            },
+        ]
     }
 
     fn run(
@@ -42,7 +52,7 @@ impl Command for ToTsv {
         _stack: &mut Stack,
         call: &Call,
         input: PipelineData,
-    ) -> Result<nu_protocol::PipelineData, ShellError> {
+    ) -> Result<PipelineData, ShellError> {
         let head = call.head;
         let noheaders = call.has_flag("noheaders");
         let config = engine_state.get_config();

crates/nu-command/tests/format_conversions/csv.rs

@@ -207,3 +207,41 @@ fn from_csv_text_skipping_headers_to_table() {
         assert_eq!(actual.out, "3");
     })
 }
+
+#[test]
+fn table_with_record_error() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            [[a b]; [1 2] [3 {a: 1 b: 2}]] 
+            | to csv
+        "#
+    ));
+
+    assert!(actual.err.contains("can't convert"))
+}
+
+#[test]
+fn list_not_table_error() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            [{a: 1 b: 2} {a: 3 b: 4} 1]
+            | to csv
+        "#
+    ));
+
+    assert!(actual.err.contains("can't convert"))
+}
+
+#[test]
+fn string_to_csv_error() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            'qwe' | to csv
+        "#
+    ));
+
+    assert!(actual.err.contains("can't convert"))
+}