diff --git a/crates/nu-command/src/filters/sort.rs b/crates/nu-command/src/filters/sort.rs index 179235302d..b8efa1c03b 100644 --- a/crates/nu-command/src/filters/sort.rs +++ b/crates/nu-command/src/filters/sort.rs @@ -1,9 +1,5 @@ -use alphanumeric_sort::compare_str; use nu_engine::command_prelude::*; -use nu_utils::IgnoreCaseExt; -use std::cmp::Ordering; - #[derive(Clone)] pub struct Sort; @@ -14,10 +10,13 @@ impl Command for Sort { fn signature(&self) -> nu_protocol::Signature { Signature::build("sort") - .input_output_types(vec![( - Type::List(Box::new(Type::Any)), - Type::List(Box::new(Type::Any)), - ), (Type::record(), Type::record()),]) + .input_output_types(vec![ + ( + Type::List(Box::new(Type::Any)), + Type::List(Box::new(Type::Any)) + ), + (Type::record(), Type::record()) + ]) .switch("reverse", "Sort in reverse order", Some('r')) .switch( "ignore-case", @@ -134,233 +133,52 @@ impl Command for Sort { call: &Call, input: PipelineData, ) -> Result { - let head = call.head; let reverse = call.has_flag(engine_state, stack, "reverse")?; let insensitive = call.has_flag(engine_state, stack, "ignore-case")?; let natural = call.has_flag(engine_state, stack, "natural")?; + let sort_by_value = call.has_flag(engine_state, stack, "values")?; let metadata = input.metadata(); let span = input.span().unwrap_or(call.head); - match input { - // Records have two sorting methods, toggled by presence or absence of -v - PipelineData::Value(Value::Record { val, .. }, ..) => { - let sort_by_value = call.has_flag(engine_state, stack, "values")?; - let record = sort_record( + let value = input.into_value(span)?; + let sorted: Value = match value { + Value::Record { val, .. } => { + // Records have two sorting methods, toggled by presence or absence of -v + let record = crate::sort_record( val.into_owned(), - span, sort_by_value, reverse, insensitive, natural, - ); - Ok(record.into_pipeline_data()) + )?; + Value::record(record, span) } - // Other values are sorted here - PipelineData::Value(v, ..) - if !matches!(v, Value::List { .. } | Value::Range { .. }) => - { - Ok(v.into_pipeline_data()) - } - pipe_data => { - let mut vec: Vec<_> = pipe_data.into_iter().collect(); + Value::List { vals, .. } => { + let mut vec = vals.to_owned(); - sort(&mut vec, head, insensitive, natural)?; + crate::sort(&mut vec, insensitive, natural)?; if reverse { vec.reverse() } - let iter = vec.into_iter(); - Ok(iter.into_pipeline_data_with_metadata( - head, - engine_state.signals().clone(), - metadata, - )) + Value::list(vec, span) } - } - } -} - -fn sort_record( - record: Record, - rec_span: Span, - sort_by_value: bool, - reverse: bool, - insensitive: bool, - natural: bool, -) -> Value { - let mut input_pairs: Vec<(String, Value)> = record.into_iter().collect(); - input_pairs.sort_by(|a, b| { - // Extract the data (if sort_by_value) or the column names for comparison - let left_res = if sort_by_value { - match &a.1 { - Value::String { val, .. } => val.clone(), - val => { - if let Ok(val) = val.coerce_string() { - val - } else { - // Values that can't be turned to strings are disregarded by the sort - // (same as in sort_utils.rs) - return Ordering::Equal; - } - } + Value::Nothing { .. } => { + return Err(ShellError::PipelineEmpty { + dst_span: value.span(), + }) } - } else { - a.0.clone() - }; - let right_res = if sort_by_value { - match &b.1 { - Value::String { val, .. } => val.clone(), - val => { - if let Ok(val) = val.coerce_string() { - val - } else { - // Values that can't be turned to strings are disregarded by the sort - // (same as in sort_utils.rs) - return Ordering::Equal; - } - } + _ => { + return Err(ShellError::PipelineMismatch { + exp_input_type: "record or list".to_string(), + dst_span: call.head, + src_span: value.span(), + }) } - } else { - b.0.clone() }; - - // Fold case if case-insensitive - let left = if insensitive { - left_res.to_folded_case() - } else { - left_res - }; - let right = if insensitive { - right_res.to_folded_case() - } else { - right_res - }; - - if natural { - compare_str(left, right) - } else { - left.cmp(&right) - } - }); - - if reverse { - input_pairs.reverse(); + Ok(sorted.into_pipeline_data_with_metadata(metadata)) } - - Value::record(input_pairs.into_iter().collect(), rec_span) -} - -pub fn sort( - vec: &mut [Value], - span: Span, - insensitive: bool, - natural: bool, -) -> Result<(), ShellError> { - match vec.first() { - Some(Value::Record { val, .. }) => { - let columns: Vec = val.columns().cloned().collect(); - vec.sort_by(|a, b| process(a, b, &columns, span, insensitive, natural)); - } - _ => { - vec.sort_by(|a, b| { - let span_a = a.span(); - let span_b = b.span(); - if insensitive { - let folded_left = match a { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_a), - _ => a.clone(), - }; - - let folded_right = match b { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_b), - _ => b.clone(), - }; - - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) - } - } else if natural { - match (a.coerce_str(), b.coerce_str()) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - a.partial_cmp(b).unwrap_or(Ordering::Equal) - } - }); - } - } - Ok(()) -} - -pub fn process( - left: &Value, - right: &Value, - columns: &[String], - span: Span, - insensitive: bool, - natural: bool, -) -> Ordering { - for column in columns { - let left_value = left.get_data_by_key(column); - - let left_res = match left_value { - Some(left_res) => left_res, - None => Value::nothing(span), - }; - - let right_value = right.get_data_by_key(column); - - let right_res = match right_value { - Some(right_res) => right_res, - None => Value::nothing(span), - }; - - let result = if insensitive { - let span_left = left_res.span(); - let span_right = right_res.span(); - let folded_left = match left_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_left), - _ => left_res, - }; - - let folded_right = match right_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_right), - _ => right_res, - }; - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) - } - } else { - left_res.partial_cmp(&right_res).unwrap_or(Ordering::Equal) - }; - if result != Ordering::Equal { - return result; - } - } - - Ordering::Equal } #[cfg(test)] diff --git a/crates/nu-command/src/filters/sort_by.rs b/crates/nu-command/src/filters/sort_by.rs index e832255a26..43ba5d3d75 100644 --- a/crates/nu-command/src/filters/sort_by.rs +++ b/crates/nu-command/src/filters/sort_by.rs @@ -1,5 +1,7 @@ use nu_engine::command_prelude::*; +use crate::Comparator; + #[derive(Clone)] pub struct SortBy; @@ -18,16 +20,23 @@ impl Command for SortBy { (Type::record(), Type::table()), (Type::table(), Type::table()), ]) - .rest("columns", SyntaxShape::Any, "The column(s) to sort by.") + .rest( + "comparator", + SyntaxShape::OneOf(vec![ + SyntaxShape::CellPath, + SyntaxShape::Closure(Some(vec![SyntaxShape::Any, SyntaxShape::Any])), + ]), + "The cell path(s) or closure(s) to compare elements by.", + ) .switch("reverse", "Sort in reverse order", Some('r')) .switch( "ignore-case", - "Sort string-based columns case-insensitively", + "Sort string-based data case-insensitively", Some('i'), ) .switch( "natural", - "Sort alphanumeric string-based columns naturally (1, 9, 10, 99, 100, ...)", + "Sort alphanumeric string-based data naturally (1, 9, 10, 99, 100, ...)", Some('n'), ) .allow_variants_without_examples(true) @@ -79,21 +88,44 @@ impl Command for SortBy { input: PipelineData, ) -> Result { let head = call.head; - let columns: Vec = call.rest(engine_state, stack, 0)?; + let comparator_vals: Vec = call.rest(engine_state, stack, 0)?; let reverse = call.has_flag(engine_state, stack, "reverse")?; let insensitive = call.has_flag(engine_state, stack, "ignore-case")?; let natural = call.has_flag(engine_state, stack, "natural")?; let metadata = input.metadata(); let mut vec: Vec<_> = input.into_iter_strict(head)?.collect(); - if columns.is_empty() { + if comparator_vals.is_empty() { return Err(ShellError::MissingParameter { - param_name: "columns".into(), + param_name: "comparator".into(), span: head, }); } - crate::sort(&mut vec, columns, head, insensitive, natural)?; + let mut comparators = vec![]; + for val in comparator_vals.into_iter() { + match val { + Value::CellPath { val, .. } => { + comparators.push(Comparator::CellPath(val)); + } + Value::Closure { val, .. } => { + comparators.push(Comparator::Closure( + *val, + engine_state.clone(), + stack.clone(), + )); + } + _ => { + return Err(ShellError::TypeMismatch { + err_message: + "Cannot sort using a value which is not a cell path or closure".into(), + span: val.span(), + }) + } + } + } + + crate::sort_by(&mut vec, comparators, head, insensitive, natural)?; if reverse { vec.reverse() diff --git a/crates/nu-command/src/sort_utils.rs b/crates/nu-command/src/sort_utils.rs index 01bb604eac..4fb861ffde 100644 --- a/crates/nu-command/src/sort_utils.rs +++ b/crates/nu-command/src/sort_utils.rs @@ -1,6 +1,9 @@ -use alphanumeric_sort::compare_str; -use nu_engine::column::nonexistent_column; -use nu_protocol::{ShellError, Span, Value}; +use nu_engine::ClosureEval; +use nu_protocol::{ + ast::CellPath, + engine::{Closure, EngineState, Stack}, + PipelineData, Record, ShellError, Span, Value, +}; use nu_utils::IgnoreCaseExt; use std::cmp::Ordering; @@ -8,344 +11,521 @@ use std::cmp::Ordering; // Eventually it would be nice to find a better home for it; sorting logic is only coupled // to commands for historical reasons. -/// Sort a value. This only makes sense for lists and list-like things, -/// so for everything else we just return the value as-is. -/// CustomValues are converted to their base value and then sorted. -pub fn sort_value( - val: &Value, - sort_columns: Vec, - ascending: bool, - insensitive: bool, - natural: bool, -) -> Result { - let span = val.span(); - match val { - Value::List { vals, .. } => { - let mut vals = vals.clone(); - sort(&mut vals, sort_columns, span, insensitive, natural)?; +pub enum Comparator { + Closure(Closure, EngineState, Stack), + CellPath(CellPath), +} - if !ascending { - vals.reverse(); - } +pub fn sort(vec: &mut [Value], insensitive: bool, natural: bool) -> Result<(), ShellError> { + // to apply insensitive or natural sorting, all values must be strings + let string_sort: bool = vec + .iter() + .all(|value| matches!(value, &Value::String { .. })); - Ok(Value::list(vals, span)) - } - Value::Custom { val, .. } => { - let base_val = val.to_base_value(span)?; - sort_value(&base_val, sort_columns, ascending, insensitive, natural) - } - _ => Ok(val.to_owned()), + // allow the comparator function to indicate error + // by mutating this option captured by the closure, + // since sort_by closure must be infallible + let mut compare_err: Option = None; + + vec.sort_by(|a, b| { + crate::compare_values(a, b, insensitive && string_sort, natural && string_sort) + .unwrap_or_else(|err| { + compare_err.get_or_insert(err); + Ordering::Equal + }) + }); + + if let Some(err) = compare_err { + Err(err) + } else { + Ok(()) } } -/// Sort a value in-place. This is more efficient than sort_value() because it -/// avoids cloning, but it does not work for CustomValues; they are returned as-is. -pub fn sort_value_in_place( - val: &mut Value, - sort_columns: Vec, - ascending: bool, - insensitive: bool, - natural: bool, -) -> Result<(), ShellError> { - let span = val.span(); - if let Value::List { vals, .. } = val { - sort(vals, sort_columns, span, insensitive, natural)?; - if !ascending { - vals.reverse(); - } - } - Ok(()) -} - -pub fn sort( +pub fn sort_by( vec: &mut [Value], - sort_columns: Vec, + comparators: Vec, span: Span, insensitive: bool, natural: bool, ) -> Result<(), ShellError> { - let val_span = vec.first().map(|v| v.span()).unwrap_or(span); - match vec.first() { - Some(Value::Record { val: record, .. }) => { - if sort_columns.is_empty() { - // This uses the same format as the 'requires a column name' error in split_by.rs - return Err(ShellError::GenericError { - error: "expected name".into(), - msg: "requires a column name to sort table data".into(), - span: Some(span), - help: None, - inner: vec![], - }); - } - - if let Some(nonexistent) = nonexistent_column(&sort_columns, record.columns()) { - return Err(ShellError::CantFindColumn { - col_name: nonexistent, - span: Some(span), - src_span: val_span, - }); - } - - // check to make sure each value in each column in the record - // that we asked for is a string. So, first collect all the columns - // that we asked for into vals, then later make sure they're all - // strings. - let mut vals = vec![]; - for item in vec.iter() { - for col in &sort_columns { - let val = item - .get_data_by_key(col) - .unwrap_or_else(|| Value::nothing(Span::unknown())); - vals.push(val); - } - } - - let should_sort_case_insensitively = insensitive - && vals - .iter() - .all(|x| matches!(x.get_type(), nu_protocol::Type::String)); - - let should_sort_case_naturally = natural - && vals - .iter() - .all(|x| matches!(x.get_type(), nu_protocol::Type::String)); - - vec.sort_by(|a, b| { - compare( - a, - b, - &sort_columns, - span, - should_sort_case_insensitively, - should_sort_case_naturally, - ) - }); - } - _ => { - vec.sort_by(|a, b| { - if insensitive { - let span_a = a.span(); - let span_b = b.span(); - let folded_left = match a { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_a), - _ => a.clone(), - }; - - let folded_right = match b { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_b), - _ => b.clone(), - }; - - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) - } - } else if natural { - match (a.coerce_str(), b.coerce_str()) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - a.partial_cmp(b).unwrap_or(Ordering::Equal) - } - }); - } + if comparators.is_empty() { + // This uses the same format as the 'requires a column name' error in split_by.rs + return Err(ShellError::GenericError { + error: "expected name".into(), + msg: "requires a cell path or closure to sort data".into(), + span: Some(span), + help: None, + inner: vec![], + }); + } + + // to apply insensitive or natural sorting, all values must be strings + let string_sort: bool = comparators.iter().all(|cmp| { + let Comparator::CellPath(cell_path) = cmp else { + // closures shouldn't affect whether cell paths are sorted naturally/insensitively + return true; + }; + vec.iter().all(|value| { + let inner = value.clone().follow_cell_path(&cell_path.members, false); + matches!(inner, Ok(Value::String { .. })) + }) + }); + + // allow the comparator function to indicate error + // by mutating this option captured by the closure, + // since sort_by closure must be infallible + let mut compare_err: Option = None; + + vec.sort_by(|a, b| { + compare_by( + a, + b, + &comparators, + span, + insensitive && string_sort, + natural && string_sort, + &mut compare_err, + ) + }); + + if let Some(err) = compare_err { + Err(err) + } else { + Ok(()) } - Ok(()) } -pub fn compare( +pub fn sort_record( + record: Record, + sort_by_value: bool, + reverse: bool, + insensitive: bool, + natural: bool, +) -> Result { + let mut input_pairs: Vec<(String, Value)> = record.into_iter().collect(); + + // allow the comparator function to indicate error + // by mutating this option captured by the closure, + // since sort_by closure must be infallible + let mut compare_err: Option = None; + + input_pairs.sort_by(|a, b| { + if sort_by_value { + compare_values(&a.1, &b.1, insensitive, natural).unwrap_or_else(|err| { + compare_err.get_or_insert(err); + Ordering::Equal + }) + } else { + compare_strings(&a.0, &b.0, insensitive, natural) + } + }); + + if reverse { + input_pairs.reverse() + } + + if let Some(err) = compare_err { + return Err(err); + } + + Ok(input_pairs.into_iter().collect()) +} + +pub fn compare_by( left: &Value, right: &Value, - columns: &[String], + comparators: &[Comparator], span: Span, insensitive: bool, natural: bool, + error: &mut Option, +) -> Ordering { + for cmp in comparators.iter() { + let result = match cmp { + Comparator::CellPath(cell_path) => { + compare_cell_path(left, right, cell_path, insensitive, natural) + } + Comparator::Closure(closure, engine_state, stack) => { + let closure_eval = ClosureEval::new(engine_state, stack, closure.clone()); + compare_closure(left, right, closure_eval, span) + } + }; + match result { + Ok(Ordering::Equal) => {} + Ok(ordering) => return ordering, + Err(err) => { + // don't bother continuing through the remaining comparators as we've hit an error + // don't overwrite if there's an existing error + error.get_or_insert(err); + return Ordering::Equal; + } + } + } + Ordering::Equal +} + +pub fn compare_values( + left: &Value, + right: &Value, + insensitive: bool, + natural: bool, +) -> Result { + if insensitive || natural { + let left_str = left.coerce_string()?; + let right_str = right.coerce_string()?; + Ok(compare_strings(&left_str, &right_str, insensitive, natural)) + } else { + Ok(left.partial_cmp(right).unwrap_or(Ordering::Equal)) + } +} + +pub fn compare_strings( + left: &String, + right: &String, + insensitive: bool, + natural: bool, ) -> Ordering { - for column in columns { - let left_value = left.get_data_by_key(column); + // declare these names now to appease compiler + // not needed in nightly, but needed as of 1.77.2, so can be removed later + let (left_copy, right_copy); - let left_res = match left_value { - Some(left_res) => left_res, - None => Value::nothing(span), - }; + // only allocate new String if necessary for case folding, + // so callers don't need to pass an owned String + let (left_str, right_str) = if insensitive { + left_copy = left.to_folded_case(); + right_copy = right.to_folded_case(); + (&left_copy, &right_copy) + } else { + (left, right) + }; - let right_value = right.get_data_by_key(column); - - let right_res = match right_value { - Some(right_res) => right_res, - None => Value::nothing(span), - }; - - let result = if insensitive { - let span_left = left_res.span(); - let span_right = right_res.span(); - let folded_left = match left_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_left), - _ => left_res, - }; - - let folded_right = match right_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_right), - _ => right_res, - }; - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) - } - } else if natural { - match ( - left_res.coerce_into_string(), - right_res.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - left_res.partial_cmp(&right_res).unwrap_or(Ordering::Equal) - }; - if result != Ordering::Equal { - return result; - } + if natural { + alphanumeric_sort::compare_str(left_str, right_str) + } else { + left_str.partial_cmp(right_str).unwrap_or(Ordering::Equal) } +} - Ordering::Equal +pub fn compare_cell_path( + left: &Value, + right: &Value, + cell_path: &CellPath, + insensitive: bool, + natural: bool, +) -> Result { + let left = left.clone().follow_cell_path(&cell_path.members, false)?; + let right = right.clone().follow_cell_path(&cell_path.members, false)?; + compare_values(&left, &right, insensitive, natural) +} + +pub fn compare_closure( + left: &Value, + right: &Value, + mut closure_eval: ClosureEval, + span: Span, +) -> Result { + closure_eval + .add_arg(left.clone()) + .add_arg(right.clone()) + .run_with_input(PipelineData::Empty) + .and_then(|data| data.into_value(span)) + .map(|val| { + if val.is_true() { + Ordering::Less + } else { + Ordering::Greater + } + }) } #[cfg(test)] mod tests { use super::*; - use nu_protocol::{record, Value}; + use nu_protocol::Value; #[test] - fn test_sort_value() { - let val = Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - ]); + fn test_sort_basic() { + let mut list = vec![ + Value::test_string("foo"), + Value::test_int(2), + Value::test_int(3), + Value::test_string("bar"), + Value::test_int(1), + Value::test_string("baz"), + ]; - let sorted_alphabetically = - sort_value(&val, vec!["fruit".to_string()], true, false, false).unwrap(); + assert!(sort(&mut list, false, false).is_ok()); assert_eq!( - sorted_alphabetically, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) - ); - - let sorted_by_count_desc = - sort_value(&val, vec!["count".to_string()], false, false, false).unwrap(); - assert_eq!( - sorted_by_count_desc, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) + list, + vec![ + Value::test_int(1), + Value::test_int(2), + Value::test_int(3), + Value::test_string("bar"), + Value::test_string("baz"), + Value::test_string("foo") + ] ); } #[test] - fn test_sort_value_in_place() { - let mut val = Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - ]); + fn test_sort_nothing() { + // Nothing values should always be sorted to the end of any list + let mut list = vec![ + Value::test_int(1), + Value::test_nothing(), + Value::test_int(2), + Value::test_string("foo"), + Value::test_nothing(), + Value::test_string("bar"), + ]; - sort_value_in_place(&mut val, vec!["fruit".to_string()], true, false, false).unwrap(); + assert!(sort(&mut list, false, false).is_ok()); assert_eq!( - val, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) + list, + vec![ + Value::test_int(1), + Value::test_int(2), + Value::test_string("bar"), + Value::test_string("foo"), + Value::test_nothing(), + Value::test_nothing() + ] ); - sort_value_in_place(&mut val, vec!["count".to_string()], false, false, false).unwrap(); + // Ensure that nothing values are sorted after *all* types, + // even types which may follow `Nothing` in the PartialOrd order + + // unstable_name_collision + // can be switched to std intersperse when stabilized + let mut values: Vec<_> = + itertools::intersperse(Value::test_values().into_iter(), Value::test_nothing()) + .collect(); + + let nulls = values + .iter() + .filter(|item| item == &&Value::test_nothing()) + .count(); + + assert!(sort(&mut values, false, false).is_ok()); + + // check if the last `nulls` values of the sorted list are indeed null + assert_eq!(&values[..nulls], vec![Value::test_nothing(); nulls]) + } + + #[test] + fn test_sort_natural_basic() { + let mut list = vec![ + Value::test_string("99"), + Value::test_string("9"), + Value::test_string("1"), + Value::test_string("100"), + Value::test_string("10"), + ]; + + assert!(sort(&mut list, false, false).is_ok()); assert_eq!( - val, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) + list, + vec![ + Value::test_string("1"), + Value::test_string("10"), + Value::test_string("100"), + Value::test_string("9"), + Value::test_string("99"), + ] + ); + + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_string("1"), + Value::test_string("9"), + Value::test_string("10"), + Value::test_string("99"), + Value::test_string("100"), + ] + ); + } + + #[test] + fn test_sort_natural_mixed_types() { + let mut list = vec![ + Value::test_string("1"), + Value::test_int(99), + Value::test_int(1), + Value::test_int(9), + Value::test_string("9"), + Value::test_int(100), + Value::test_string("99"), + Value::test_string("100"), + Value::test_int(10), + Value::test_string("10"), + ]; + + assert!(sort(&mut list, false, false).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_int(9), + Value::test_int(10), + Value::test_int(99), + Value::test_int(100), + Value::test_string("1"), + Value::test_string("10"), + Value::test_string("100"), + Value::test_string("9"), + Value::test_string("99") + ] + ); + + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_string("1"), + Value::test_int(9), + Value::test_string("9"), + Value::test_int(10), + Value::test_string("10"), + Value::test_int(99), + Value::test_string("99"), + Value::test_int(100), + Value::test_string("100"), + ] + ); + } + + #[test] + fn test_sort_natural_no_numeric_values() { + // If list contains no numeric values (numeric strings, ints, floats), + // it should be sorted the same with or without natural sorting + let mut normal = vec![ + Value::test_string("golf"), + Value::test_bool(false), + Value::test_string("alfa"), + Value::test_string("echo"), + Value::test_int(7), + Value::test_int(10), + Value::test_bool(true), + Value::test_string("uniform"), + Value::test_int(3), + Value::test_string("tango"), + ]; + let mut natural = normal.clone(); + + assert!(sort(&mut normal, false, false).is_ok()); + assert!(sort(&mut natural, false, true).is_ok()); + assert_eq!(normal, natural); + } + + #[test] + fn test_sort_natural_type_order() { + // This test is to prevent regression to a previous natural sort behavior + // where values of different types would be intermixed. + // Only numeric values (ints, floats, and numeric strings) should be intermixed + // + // This list would previously be incorrectly sorted like this: + // ╭────┬─────────╮ + // │ 0 │ 1 │ + // │ 1 │ golf │ + // │ 2 │ false │ + // │ 3 │ 7 │ + // │ 4 │ 10 │ + // │ 5 │ alfa │ + // │ 6 │ true │ + // │ 7 │ uniform │ + // │ 8 │ true │ + // │ 9 │ 3 │ + // │ 10 │ false │ + // │ 11 │ tango │ + // ╰────┴─────────╯ + + let mut list = vec![ + Value::test_string("golf"), + Value::test_int(1), + Value::test_bool(false), + Value::test_string("alfa"), + Value::test_int(7), + Value::test_int(10), + Value::test_bool(true), + Value::test_string("uniform"), + Value::test_bool(true), + Value::test_int(3), + Value::test_bool(false), + Value::test_string("tango"), + ]; + + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_int(3), + Value::test_int(7), + Value::test_int(10), + Value::test_bool(false), + Value::test_bool(false), + Value::test_bool(true), + Value::test_bool(true), + Value::test_string("alfa"), + Value::test_string("golf"), + Value::test_string("tango"), + Value::test_string("uniform") + ] + ); + + // Only ints, floats, and numeric strings should be intermixed + // While binary primitives and datetimes can be coerced into strings, it doesn't make sense to sort them with numbers + // Binary primitives can hold multiple values, not just one, so shouldn't be compared to single values + // Datetimes don't have a single obvious numeric representation, and if we chose one it would be ambigious to the user + + let year_three = chrono::NaiveDate::from_ymd_opt(3, 1, 1) + .unwrap() + .and_hms_opt(0, 0, 0) + .unwrap() + .and_utc(); + + let mut list = vec![ + Value::test_int(10), + Value::test_float(6.0), + Value::test_int(1), + Value::test_binary([3]), + Value::test_string("2"), + Value::test_date(year_three.into()), + Value::test_int(4), + Value::test_binary([52]), + Value::test_float(9.0), + Value::test_string("5"), + Value::test_date(chrono::DateTime::UNIX_EPOCH.into()), + Value::test_int(7), + Value::test_string("8"), + Value::test_float(3.0), + ]; + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_string("2"), + Value::test_float(3.0), + Value::test_int(4), + Value::test_string("5"), + Value::test_float(6.0), + Value::test_int(7), + Value::test_string("8"), + Value::test_float(9.0), + Value::test_int(10), + // the ordering of date and binary here may change if the PartialOrd order is changed, + // but they should not be intermixed with the above + Value::test_binary([3]), + Value::test_binary([52]), + Value::test_date(year_three.into()), + Value::test_date(chrono::DateTime::UNIX_EPOCH.into()), + ] ); } }