diff --git a/crates/nu-cmd-dataframe/src/dataframe/eager/schema.rs b/crates/nu-cmd-dataframe/src/dataframe/eager/schema.rs index 9dd0c9a858..349a374e2c 100644 --- a/crates/nu-cmd-dataframe/src/dataframe/eager/schema.rs +++ b/crates/nu-cmd-dataframe/src/dataframe/eager/schema.rs @@ -33,7 +33,7 @@ impl Command for SchemaDF { description: "Dataframe schema", example: r#"[[a b]; [1 "foo"] [3 "bar"]] | dfr into-df | dfr schema"#, result: Some(Value::record( - Record::from_raw_cols_vals( + Record::from_raw_cols_vals_unchecked( vec!["a".to_string(), "b".to_string()], vec![ Value::string("i64", Span::test_data()), @@ -98,7 +98,7 @@ fn datatype_list(span: Span) -> Value { ] .iter() .map(|(dtype, note)| { - Value::record(Record::from_raw_cols_vals( + Value::record(Record::from_raw_cols_vals_unchecked( vec!["dtype".to_string(), "note".to_string()], vec![Value::string(*dtype, span), Value::string(*note, span)], ),span) diff --git a/crates/nu-cmd-dataframe/src/dataframe/values/nu_dataframe/conversion.rs b/crates/nu-cmd-dataframe/src/dataframe/values/nu_dataframe/conversion.rs index e7a06c2060..fabb26f3a7 100644 --- a/crates/nu-cmd-dataframe/src/dataframe/values/nu_dataframe/conversion.rs +++ b/crates/nu-cmd-dataframe/src/dataframe/values/nu_dataframe/conversion.rs @@ -1041,7 +1041,7 @@ fn series_to_values( .iter() .map(|field| field.name.to_string()) .collect(); - let record = Record::from_raw_cols_vals(cols, vals?); + let record = Record::from_raw_cols_vals_unchecked(cols, vals?); Ok(Value::record(record, span)) }) .collect(); @@ -1149,7 +1149,7 @@ fn any_value_to_value(any_value: &AnyValue, span: Span) -> Result, span: Span) -> Value { }) .unzip(); - let record = Record::from_raw_cols_vals(cols, vals); + let record = Record::from_raw_cols_vals_unchecked(cols, vals); Value::record(record, Span::unknown()) } @@ -193,7 +193,7 @@ mod test { #[test] fn test_value_to_schema() { let value = Value::Record { - val: Record::from_raw_cols_vals( + val: Record::from_raw_cols_vals_unchecked( vec!["name".into(), "age".into(), "address".into()], vec![ Value::String { @@ -205,7 +205,7 @@ mod test { internal_span: Span::test_data(), }, Value::Record { - val: Record::from_raw_cols_vals( + val: Record::from_raw_cols_vals_unchecked( vec!["street".into(), "city".into()], vec![ Value::String { diff --git a/crates/nu-cmd-extra/src/extra/filters/roll/mod.rs b/crates/nu-cmd-extra/src/extra/filters/roll/mod.rs index f1a46e31db..54779238d0 100644 --- a/crates/nu-cmd-extra/src/extra/filters/roll/mod.rs +++ b/crates/nu-cmd-extra/src/extra/filters/roll/mod.rs @@ -70,7 +70,10 @@ fn horizontal_rotate_value( HorizontalDirection::Left => vals.rotate_left(rotations), } - Ok(Value::record(Record::from_raw_cols_vals(cols, vals), span)) + Ok(Value::record( + Record::from_raw_cols_vals_unchecked(cols, vals), + span, + )) } Value::List { vals, .. } => { let values = vals diff --git a/crates/nu-cmd-extra/src/extra/filters/rotate.rs b/crates/nu-cmd-extra/src/extra/filters/rotate.rs index dd98a2f07f..575d799e97 100644 --- a/crates/nu-cmd-extra/src/extra/filters/rotate.rs +++ b/crates/nu-cmd-extra/src/extra/filters/rotate.rs @@ -161,7 +161,7 @@ pub fn rotate( ) -> Result { let metadata = input.metadata(); let col_given_names: Vec = call.rest(engine_state, stack, 0)?; - let span = input.span(); + let input_span = input.span().unwrap_or(call.head); let mut values = input.into_iter().collect::>(); let mut old_column_names = vec![]; let mut new_values = vec![]; @@ -203,7 +203,7 @@ pub fn rotate( msg: "list input is empty".to_string(), input: "value originates from here".into(), msg_span: call.head, - input_span: span.unwrap_or(call.head), + input_span, }); } @@ -234,15 +234,14 @@ pub fn rotate( } if not_a_record { - return Ok(Value::list( - vec![Value::record( - Record::from_raw_cols_vals(new_column_names, new_values), - call.head, - )], - call.head, - ) - .into_pipeline_data() - .set_metadata(metadata)); + let record = + Record::from_raw_cols_vals(new_column_names, new_values, input_span, call.head)?; + + return Ok( + Value::list(vec![Value::record(record, call.head)], call.head) + .into_pipeline_data() + .set_metadata(metadata), + ); } // holder for the new records @@ -281,10 +280,11 @@ pub fn rotate( } res.to_vec() }; - final_values.push(Value::record( - Record::from_raw_cols_vals(new_column_names.clone(), new_vals), - call.head, - )) + + let record = + Record::from_raw_cols_vals(new_column_names.clone(), new_vals, input_span, call.head)?; + + final_values.push(Value::record(record, call.head)) } Ok(Value::list(final_values, call.head) diff --git a/crates/nu-command/src/charting/histogram.rs b/crates/nu-command/src/charting/histogram.rs index b84ef485bc..b46f1b8a8e 100755 --- a/crates/nu-command/src/charting/histogram.rs +++ b/crates/nu-command/src/charting/histogram.rs @@ -258,7 +258,7 @@ fn histogram_impl( result.push(( count, // attach count first for easily sorting. Value::record( - Record::from_raw_cols_vals( + Record::from_raw_cols_vals_unchecked( result_cols.clone(), vec![ val.into_value(), diff --git a/crates/nu-command/src/database/values/sqlite.rs b/crates/nu-command/src/database/values/sqlite.rs index d9eb2711aa..d3b9b2749e 100644 --- a/crates/nu-command/src/database/values/sqlite.rs +++ b/crates/nu-command/src/database/values/sqlite.rs @@ -499,7 +499,10 @@ pub fn convert_sqlite_row_to_nu_value(row: &Row, span: Span, column_names: Vec Value { diff --git a/crates/nu-command/src/formats/from/delimited.rs b/crates/nu-command/src/formats/from/delimited.rs index 87d22bb22c..26b3c09892 100644 --- a/crates/nu-command/src/formats/from/delimited.rs +++ b/crates/nu-command/src/formats/from/delimited.rs @@ -55,7 +55,7 @@ fn from_delimited_string_to_value( .collect::>(); rows.push(Value::record( - Record::from_raw_cols_vals(headers.clone(), output_row), + Record::from_raw_cols_vals_unchecked(headers.clone(), output_row), span, )); } diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 37bd5f6b53..d0a6df900f 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -417,7 +417,7 @@ fn convert_to_value( .collect::>()?; output.push(Value::record( - Record::from_raw_cols_vals(cols.clone(), vals), + Record::from_raw_cols_vals_unchecked(cols.clone(), vals), span, )); } diff --git a/crates/nu-command/src/strings/detect_columns.rs b/crates/nu-command/src/strings/detect_columns.rs index 55fc244869..f2cecdfedd 100644 --- a/crates/nu-command/src/strings/detect_columns.rs +++ b/crates/nu-command/src/strings/detect_columns.rs @@ -200,7 +200,7 @@ fn detect_columns( if !(l_idx <= r_idx && (r_idx >= 0 || l_idx < (cols.len() as isize))) { return Value::record( - Record::from_raw_cols_vals(cols, vals), + Record::from_raw_cols_vals_unchecked(cols, vals), name_span, ); } @@ -213,7 +213,7 @@ fn detect_columns( } } } else { - return Value::record(Record::from_raw_cols_vals(cols, vals), name_span); + return Value::record(Record::from_raw_cols_vals_unchecked(cols, vals), name_span); }; // Merge Columns @@ -235,7 +235,7 @@ fn detect_columns( vals.push(binding); last_seg.into_iter().for_each(|v| vals.push(v)); - Value::record(Record::from_raw_cols_vals(cols, vals), name_span) + Value::record(Record::from_raw_cols_vals_unchecked(cols, vals), name_span) }) .into_pipeline_data(ctrlc)) } else { diff --git a/crates/nu-command/tests/commands/rotate.rs b/crates/nu-command/tests/commands/rotate.rs index e9531682d6..5394134d14 100644 --- a/crates/nu-command/tests/commands/rotate.rs +++ b/crates/nu-command/tests/commands/rotate.rs @@ -89,3 +89,11 @@ fn clockwise() { assert_eq!(actual.out, expected.out); } + +#[test] +fn different_cols_vals_err() { + let actual = nu!("[[[one], [two, three]]] | first | rotate"); + assert!(actual + .err + .contains("Attempted to create a record from different number of columns and values")) +} diff --git a/crates/nu-engine/src/scope.rs b/crates/nu-engine/src/scope.rs index 25e7cd8a22..548056fe63 100644 --- a/crates/nu-engine/src/scope.rs +++ b/crates/nu-engine/src/scope.rs @@ -198,7 +198,7 @@ impl<'e, 's> ScopeData<'e, 's> { // input sig_records.push(Value::record( - Record::from_raw_cols_vals( + Record::from_raw_cols_vals_unchecked( sig_cols.clone(), vec![ Value::nothing(span), @@ -231,7 +231,7 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), + Record::from_raw_cols_vals_unchecked(sig_cols.clone(), sig_vals), span, )); } @@ -257,7 +257,7 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), + Record::from_raw_cols_vals_unchecked(sig_cols.clone(), sig_vals), span, )); } @@ -279,7 +279,7 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), + Record::from_raw_cols_vals_unchecked(sig_cols.clone(), sig_vals), span, )); } @@ -326,14 +326,14 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), + Record::from_raw_cols_vals_unchecked(sig_cols.clone(), sig_vals), span, )); } // output sig_records.push(Value::record( - Record::from_raw_cols_vals( + Record::from_raw_cols_vals_unchecked( sig_cols, vec![ Value::nothing(span), diff --git a/crates/nu-explore/src/commands/help.rs b/crates/nu-explore/src/commands/help.rs index f687ca27b2..d3f69c6f3c 100644 --- a/crates/nu-explore/src/commands/help.rs +++ b/crates/nu-explore/src/commands/help.rs @@ -165,7 +165,10 @@ fn help_frame_data( let (cols, mut vals) = help_manual_data(manual, aliases); let vals = vals.remove(0); - Value::record(Record::from_raw_cols_vals(cols, vals), NuSpan::unknown()) + Value::record( + Record::from_raw_cols_vals_unchecked(cols, vals), + NuSpan::unknown(), + ) }) .collect(); let commands = Value::list(commands, NuSpan::unknown()); diff --git a/crates/nu-explore/src/views/record/mod.rs b/crates/nu-explore/src/views/record/mod.rs index c2e6bbb27a..514735f761 100644 --- a/crates/nu-explore/src/views/record/mod.rs +++ b/crates/nu-explore/src/views/record/mod.rs @@ -708,7 +708,7 @@ fn build_table_as_list(v: &RecordView) -> Value { .cloned() .map(|vals| { Value::record( - Record::from_raw_cols_vals(headers.clone(), vals), + Record::from_raw_cols_vals_unchecked(headers.clone(), vals), NuSpan::unknown(), ) }) @@ -723,7 +723,10 @@ fn build_table_as_record(v: &RecordView) -> Value { let cols = layer.columns.to_vec(); let vals = layer.records.first().map_or(Vec::new(), |row| row.clone()); - Value::record(Record::from_raw_cols_vals(cols, vals), NuSpan::unknown()) + Value::record( + Record::from_raw_cols_vals_unchecked(cols, vals), + NuSpan::unknown(), + ) } fn report_cursor_position(mode: UIMode, cursor: XYCursor) -> String { diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index f44aab6d74..4245acb809 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -125,7 +125,7 @@ pub trait Eval { } // length equality already ensured in parser output_rows.push(Value::record( - Record::from_raw_cols_vals(output_headers.clone(), row), + Record::from_raw_cols_vals_unchecked(output_headers.clone(), row), expr.span, )); } diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 2fe0b94acf..664582658e 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -607,6 +607,20 @@ pub enum ShellError { first_use: Span, }, + /// Attempted to create a record from different number of columns and values + /// + /// ## Resolution + /// + /// Check the record has the same number of columns as values + #[error("Attempted to create a record from different number of columns and values")] + #[diagnostic(code(nu::shell::record_cols_vals_mismatch))] + RecordColsValsMismatch { + #[label = "problematic value"] + bad_value: Span, + #[label = "attempted to create the record here"] + creation_site: Span, + }, + /// An error happened while performing an external command. /// /// ## Resolution diff --git a/crates/nu-protocol/src/value/record.rs b/crates/nu-protocol/src/value/record.rs index 6e5ee2d3a7..cbdef4e547 100644 --- a/crates/nu-protocol/src/value/record.rs +++ b/crates/nu-protocol/src/value/record.rs @@ -1,6 +1,6 @@ use std::ops::RangeBounds; -use crate::Value; +use crate::{ShellError, Span, Value}; use serde::{Deserialize, Serialize}; @@ -28,14 +28,39 @@ impl Record { // Constructor that checks that `cols` and `vals` are of the same length. // + // WARNING! Panics with assertion failure if cols and vals have different length! + // Should be used only when the same lengths are guaranteed! + // // For perf reasons does not validate the rest of the record assumptions. // - unique keys - pub fn from_raw_cols_vals(cols: Vec, vals: Vec) -> Self { + pub fn from_raw_cols_vals_unchecked(cols: Vec, vals: Vec) -> Self { assert_eq!(cols.len(), vals.len()); Self { cols, vals } } + // Constructor that checks that `cols` and `vals` are of the same length. + // + // Returns None if cols and vals have different length. + // + // For perf reasons does not validate the rest of the record assumptions. + // - unique keys + pub fn from_raw_cols_vals( + cols: Vec, + vals: Vec, + input_span: Span, + creation_site_span: Span, + ) -> Result { + if cols.len() == vals.len() { + Ok(Self { cols, vals }) + } else { + Err(ShellError::RecordColsValsMismatch { + bad_value: input_span, + creation_site: creation_site_span, + }) + } + } + pub fn iter(&self) -> Iter { self.into_iter() } @@ -455,7 +480,7 @@ impl ExactSizeIterator for Drain<'_> { #[macro_export] macro_rules! record { {$($col:expr => $val:expr),+ $(,)?} => { - $crate::Record::from_raw_cols_vals ( + $crate::Record::from_raw_cols_vals_unchecked ( vec![$($col.into(),)+], vec![$($val,)+] ) diff --git a/crates/nu_plugin_example/src/example.rs b/crates/nu_plugin_example/src/example.rs index 45607c8b4b..4e73e2c1c9 100644 --- a/crates/nu_plugin_example/src/example.rs +++ b/crates/nu_plugin_example/src/example.rs @@ -83,7 +83,10 @@ impl Example { .map(|v| Value::int(v * i, call.head)) .collect::>(); - Value::record(Record::from_raw_cols_vals(cols.clone(), vals), call.head) + Value::record( + Record::from_raw_cols_vals_unchecked(cols.clone(), vals), + call.head, + ) }) .collect::>();