From 21b84a6d658e917b437b3ba9bd52fe81160217a5 Mon Sep 17 00:00:00 2001 From: Reilly Wood <26268125+rgwood@users.noreply.github.com> Date: Wed, 15 Mar 2023 20:50:58 -0700 Subject: [PATCH] Optional members in cell paths: Attempt 2 (#8379) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a follow up from https://github.com/nushell/nushell/pull/7540. Please provide feedback if you have the time! ## Summary This PR lets you use `?` to indicate that a member in a cell path is optional and Nushell should return `null` if that member cannot be accessed. Unlike the previous PR, `?` is now a _postfix_ modifier for cell path members. A cell path of `.foo?.bar` means that `foo` is optional and `bar` is not. `?` does _not_ suppress all errors; it is intended to help in situations where data has "holes", i.e. the data types are correct but something is missing. Type mismatches (like trying to do a string path access on a date) will still fail. ### Record Examples ```bash { foo: 123 }.foo # returns 123 { foo: 123 }.bar # errors { foo: 123 }.bar? # returns null { foo: 123 } | get bar # errors { foo: 123 } | get bar? # returns null { foo: 123 }.bar.baz # errors { foo: 123 }.bar?.baz # errors because `baz` is not present on the result from `bar?` { foo: 123 }.bar.baz? # errors { foo: 123 }.bar?.baz? # returns null ``` ### List Examples ``` 〉[{foo: 1} {foo: 2} {}].foo Error: nu::shell::column_not_found × Cannot find column ╭─[entry #30:1:1] 1 │ [{foo: 1} {foo: 2} {}].foo · ─┬ ─┬─ · │ ╰── cannot find column 'foo' · ╰── value originates here ╰──── 〉[{foo: 1} {foo: 2} {}].foo? ╭───┬───╮ │ 0 │ 1 │ │ 1 │ 2 │ │ 2 │ │ ╰───┴───╯ 〉[{foo: 1} {foo: 2} {}].foo?.2 | describe nothing 〉[a b c].4? | describe nothing 〉[{foo: 1} {foo: 2} {}] | where foo? == 1 ╭───┬─────╮ │ # │ foo │ ├───┼─────┤ │ 0 │ 1 │ ╰───┴─────╯ ``` # Breaking changes 1. Column names with `?` in them now need to be quoted. 2. The `-i`/`--ignore-errors` flag has been removed from `get` and `select` 1. After this PR, most `get` error handling can be done with `?` and/or `try`/`catch`. 4. Cell path accesses like this no longer work without a `?`: ```bash 〉[{a:1 b:2} {a:3}].b.0 2 ``` We had some clever code that was able to recognize that since we only want row `0`, it's OK if other rows are missing column `b`. I removed that because it's tricky to maintain, and now that query needs to be written like: ```bash 〉[{a:1 b:2} {a:3}].b?.0 2 ``` I think the regression is acceptable for now. I plan to do more work in the future to enable streaming of cell path accesses, and when that happens I'll be able to make `.b.0` work again. --- crates/nu-cli/src/repl.rs | 101 +++++++------ .../nu-command/src/charting/hashable_value.rs | 6 +- crates/nu-command/src/debug/inspect_table.rs | 3 +- crates/nu-command/src/filters/drop/column.rs | 14 +- crates/nu-command/src/filters/empty.rs | 2 +- crates/nu-command/src/filters/flatten.rs | 2 +- crates/nu-command/src/filters/get.rs | 12 +- crates/nu-command/src/filters/select.rs | 30 +--- crates/nu-command/src/filters/update.rs | 2 +- crates/nu-command/src/filters/upsert.rs | 2 +- .../nu-command/src/strings/format/command.rs | 6 +- crates/nu-command/src/viewers/griddle.rs | 2 +- crates/nu-command/src/viewers/table.rs | 11 +- crates/nu-command/tests/commands/cal.rs | 2 +- crates/nu-command/tests/commands/reject.rs | 2 +- crates/nu-command/tests/commands/select.rs | 38 +---- crates/nu-command/tests/commands/where_.rs | 2 +- crates/nu-engine/src/env.rs | 2 + crates/nu-engine/src/eval.rs | 3 +- crates/nu-explore/src/nu_common/table.rs | 16 +- crates/nu-explore/src/nu_common/value.rs | 3 +- crates/nu-parser/src/eval.rs | 2 +- crates/nu-parser/src/parse_keywords.rs | 2 + crates/nu-parser/src/parser.rs | 126 ++++++++++------ crates/nu-parser/tests/test_parser.rs | 115 ++++++++++++++- crates/nu-protocol/src/ast/cell_path.rs | 51 ++++++- crates/nu-protocol/src/pipeline_data.rs | 5 +- crates/nu-protocol/src/value/from_value.rs | 2 + crates/nu-protocol/src/value/mod.rs | 138 +++++++++--------- crates/nu_plugin_inc/src/inc.rs | 2 +- src/tests/test_cell_path.rs | 50 +++++++ src/tests/test_table_operations.rs | 33 ++++- 32 files changed, 510 insertions(+), 277 deletions(-) diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index bac7f889f8..ef907c9357 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -879,12 +879,14 @@ pub fn eval_hook( let condition_path = PathMember::String { val: "condition".to_string(), span: value_span, + optional: false, }; let mut output = PipelineData::empty(); let code_path = PathMember::String { val: "code".to_string(), span: value_span, + optional: false, }; match value { @@ -894,63 +896,60 @@ pub fn eval_hook( } } Value::Record { .. } => { - let do_run_hook = if let Ok(condition) = - value - .clone() - .follow_cell_path(&[condition_path], false, false) - { - match condition { - Value::Block { - val: block_id, - span: block_span, - .. - } - | Value::Closure { - val: block_id, - span: block_span, - .. - } => { - match run_hook_block( - engine_state, - stack, - block_id, - None, - arguments.clone(), - block_span, - ) { - Ok(pipeline_data) => { - if let PipelineData::Value(Value::Bool { val, .. }, ..) = - pipeline_data - { - val - } else { - return Err(ShellError::UnsupportedConfigValue( - "boolean output".to_string(), - "other PipelineData variant".to_string(), - block_span, - )); + let do_run_hook = + if let Ok(condition) = value.clone().follow_cell_path(&[condition_path], false) { + match condition { + Value::Block { + val: block_id, + span: block_span, + .. + } + | Value::Closure { + val: block_id, + span: block_span, + .. + } => { + match run_hook_block( + engine_state, + stack, + block_id, + None, + arguments.clone(), + block_span, + ) { + Ok(pipeline_data) => { + if let PipelineData::Value(Value::Bool { val, .. }, ..) = + pipeline_data + { + val + } else { + return Err(ShellError::UnsupportedConfigValue( + "boolean output".to_string(), + "other PipelineData variant".to_string(), + block_span, + )); + } + } + Err(err) => { + return Err(err); } } - Err(err) => { - return Err(err); - } + } + other => { + return Err(ShellError::UnsupportedConfigValue( + "block".to_string(), + format!("{}", other.get_type()), + other.span()?, + )); } } - other => { - return Err(ShellError::UnsupportedConfigValue( - "block".to_string(), - format!("{}", other.get_type()), - other.span()?, - )); - } - } - } else { - // always run the hook - true - }; + } else { + // always run the hook + true + }; if do_run_hook { - match value.clone().follow_cell_path(&[code_path], false, false)? { + match value.clone().follow_cell_path(&[code_path], false)? { Value::String { val, span: source_span, diff --git a/crates/nu-command/src/charting/hashable_value.rs b/crates/nu-command/src/charting/hashable_value.rs index 680a298b88..61ec9d82ce 100644 --- a/crates/nu-command/src/charting/hashable_value.rs +++ b/crates/nu-command/src/charting/hashable_value.rs @@ -240,7 +240,11 @@ mod test { }, Value::CellPath { val: CellPath { - members: vec![PathMember::Int { val: 0, span }], + members: vec![PathMember::Int { + val: 0, + span, + optional: false, + }], }, span, }, diff --git a/crates/nu-command/src/debug/inspect_table.rs b/crates/nu-command/src/debug/inspect_table.rs index 8a51e18233..29004eab2b 100644 --- a/crates/nu-command/src/debug/inspect_table.rs +++ b/crates/nu-command/src/debug/inspect_table.rs @@ -208,10 +208,11 @@ mod util { let path = PathMember::String { val: header.to_owned(), span: Span::unknown(), + optional: false, }; item.clone() - .follow_cell_path(&[path], false, false) + .follow_cell_path(&[path], false) .unwrap_or_else(|_| item.clone()) } item => item.clone(), diff --git a/crates/nu-command/src/filters/drop/column.rs b/crates/nu-command/src/filters/drop/column.rs index a3d5ec7acf..0ad10f140a 100644 --- a/crates/nu-command/src/filters/drop/column.rs +++ b/crates/nu-command/src/filters/drop/column.rs @@ -109,10 +109,7 @@ fn dropcol( let mut vals = vec![]; for path in &keep_columns { - let fetcher = - input_val - .clone() - .follow_cell_path(&path.members, false, false)?; + let fetcher = input_val.clone().follow_cell_path(&path.members, false)?; cols.push(path.into_string()); vals.push(fetcher); } @@ -136,10 +133,7 @@ fn dropcol( let mut vals = vec![]; for path in &keep_columns { - let fetcher = - input_val - .clone() - .follow_cell_path(&path.members, false, false)?; + let fetcher = input_val.clone().follow_cell_path(&path.members, false)?; cols.push(path.into_string()); vals.push(fetcher); } @@ -155,9 +149,7 @@ fn dropcol( let mut vals = vec![]; for cell_path in &keep_columns { - let result = v - .clone() - .follow_cell_path(&cell_path.members, false, false)?; + let result = v.clone().follow_cell_path(&cell_path.members, false)?; cols.push(cell_path.into_string()); vals.push(result); diff --git a/crates/nu-command/src/filters/empty.rs b/crates/nu-command/src/filters/empty.rs index ca02f40408..0b6af5d97c 100644 --- a/crates/nu-command/src/filters/empty.rs +++ b/crates/nu-command/src/filters/empty.rs @@ -74,7 +74,7 @@ fn empty( for val in input { for column in &columns { let val = val.clone(); - match val.follow_cell_path(&column.members, false, false) { + match val.follow_cell_path(&column.members, false) { Ok(Value::Nothing { .. }) => {} Ok(_) => return Ok(Value::boolean(false, head).into_pipeline_data()), Err(err) => return Err(err), diff --git a/crates/nu-command/src/filters/flatten.rs b/crates/nu-command/src/filters/flatten.rs index 85bb95f1db..c7f7fe881e 100644 --- a/crates/nu-command/src/filters/flatten.rs +++ b/crates/nu-command/src/filters/flatten.rs @@ -279,7 +279,7 @@ fn flat_value(columns: &[CellPath], item: &Value, _name_tag: Span, all: bool) -> if !columns.is_empty() { let cell_path = column_requested.and_then(|x| match x.members.first() { - Some(PathMember::String { val, span: _ }) => Some(val), + Some(PathMember::String { val, span: _, .. }) => Some(val), _ => None, }); diff --git a/crates/nu-command/src/filters/get.rs b/crates/nu-command/src/filters/get.rs index 0ab32c8e39..50f9d5e140 100644 --- a/crates/nu-command/src/filters/get.rs +++ b/crates/nu-command/src/filters/get.rs @@ -41,11 +41,6 @@ If multiple cell paths are given, this will produce a list of values."# "the cell path to the data", ) .rest("rest", SyntaxShape::CellPath, "additional cell paths") - .switch( - "ignore-errors", - "when there are empty cells, instead of erroring out, replace them with nothing", - Some('i'), - ) .switch( "sensitive", "get path in a case sensitive manner", @@ -65,13 +60,12 @@ If multiple cell paths are given, this will produce a list of values."# let cell_path: CellPath = call.req(engine_state, stack, 0)?; let rest: Vec = call.rest(engine_state, stack, 1)?; let sensitive = call.has_flag("sensitive"); - let ignore_errors = call.has_flag("ignore-errors"); let ctrlc = engine_state.ctrlc.clone(); let metadata = input.metadata(); if rest.is_empty() { input - .follow_cell_path(&cell_path.members, call.head, !sensitive, ignore_errors) + .follow_cell_path(&cell_path.members, call.head, !sensitive) .map(|x| x.into_pipeline_data()) } else { let mut output = vec![]; @@ -81,9 +75,7 @@ If multiple cell paths are given, this will produce a list of values."# let input = input.into_value(span); for path in paths { - let val = input - .clone() - .follow_cell_path(&path.members, !sensitive, false); + let val = input.clone().follow_cell_path(&path.members, !sensitive); output.push(val?); } diff --git a/crates/nu-command/src/filters/select.rs b/crates/nu-command/src/filters/select.rs index b1a627487b..8e91cb9ae3 100644 --- a/crates/nu-command/src/filters/select.rs +++ b/crates/nu-command/src/filters/select.rs @@ -22,11 +22,6 @@ impl Command for Select { (Type::Record(vec![]), Type::Record(vec![])), (Type::Table(vec![]), Type::Table(vec![])), ]) - .switch( - "ignore-errors", - "when an error occurs, instead of erroring out, suppress the error message", - Some('i'), - ) .rest( "rest", SyntaxShape::CellPath, @@ -58,9 +53,8 @@ produce a table, a list will produce a list, and a record will produce a record. ) -> Result { let columns: Vec = call.rest(engine_state, stack, 0)?; let span = call.head; - let ignore_errors = call.has_flag("ignore-errors"); - select(engine_state, span, columns, input, ignore_errors) + select(engine_state, span, columns, input) } fn examples(&self) -> Vec { @@ -97,7 +91,6 @@ fn select( call_span: Span, columns: Vec, input: PipelineData, - ignore_errors: bool, ) -> Result { let mut unique_rows: HashSet = HashSet::new(); @@ -106,11 +99,8 @@ fn select( for column in columns { let CellPath { ref members } = column; match members.get(0) { - Some(PathMember::Int { val, span }) => { + Some(PathMember::Int { val, span, .. }) => { if members.len() > 1 { - if ignore_errors { - return Ok(Value::nothing(call_span).into_pipeline_data()); - } return Err(ShellError::GenericError( "Select only allows row numbers for rows".into(), "extra after row number".into(), @@ -172,11 +162,7 @@ fn select( let mut vals = vec![]; for path in &columns { //FIXME: improve implementation to not clone - match input_val.clone().follow_cell_path( - &path.members, - false, - ignore_errors, - ) { + match input_val.clone().follow_cell_path(&path.members, false) { Ok(fetcher) => { allempty = false; cols.push(path.into_string().replace('.', "_")); @@ -214,10 +200,7 @@ fn select( let mut vals = vec![]; for path in &columns { //FIXME: improve implementation to not clone - match x - .clone() - .follow_cell_path(&path.members, false, ignore_errors) - { + match x.clone().follow_cell_path(&path.members, false) { Ok(value) => { cols.push(path.into_string().replace('.', "_")); vals.push(value); @@ -246,10 +229,7 @@ fn select( for cell_path in columns { // FIXME: remove clone - match v - .clone() - .follow_cell_path(&cell_path.members, false, ignore_errors) - { + match v.clone().follow_cell_path(&cell_path.members, false) { Ok(result) => { cols.push(cell_path.into_string().replace('.', "_")); vals.push(result); diff --git a/crates/nu-command/src/filters/update.rs b/crates/nu-command/src/filters/update.rs index a7259af035..0796cde54e 100644 --- a/crates/nu-command/src/filters/update.rs +++ b/crates/nu-command/src/filters/update.rs @@ -143,7 +143,7 @@ fn update( ctrlc, ) } else { - if let Some(PathMember::Int { val, span }) = cell_path.members.get(0) { + if let Some(PathMember::Int { val, span, .. }) = cell_path.members.get(0) { let mut input = input.into_iter(); let mut pre_elems = vec![]; diff --git a/crates/nu-command/src/filters/upsert.rs b/crates/nu-command/src/filters/upsert.rs index ddc916e399..1c0fcaac5d 100644 --- a/crates/nu-command/src/filters/upsert.rs +++ b/crates/nu-command/src/filters/upsert.rs @@ -165,7 +165,7 @@ fn upsert( ctrlc, ) } else { - if let Some(PathMember::Int { val, span }) = cell_path.members.get(0) { + if let Some(PathMember::Int { val, span, .. }) = cell_path.members.get(0) { let mut input = input.into_iter(); let mut pre_elems = vec![]; diff --git a/crates/nu-command/src/strings/format/command.rs b/crates/nu-command/src/strings/format/command.rs index 8f3bf3e50e..e3363e159c 100644 --- a/crates/nu-command/src/strings/format/command.rs +++ b/crates/nu-command/src/strings/format/command.rs @@ -279,12 +279,10 @@ fn format_record( .map(|path| PathMember::String { val: path.to_string(), span: *span, + optional: false, }) .collect(); - match data_as_value - .clone() - .follow_cell_path(&path_members, false, false) - { + match data_as_value.clone().follow_cell_path(&path_members, false) { Ok(value_at_column) => { output.push_str(value_at_column.into_string(", ", config).as_str()) } diff --git a/crates/nu-command/src/viewers/griddle.rs b/crates/nu-command/src/viewers/griddle.rs index 9c50c22c74..b26e5ef8f6 100644 --- a/crates/nu-command/src/viewers/griddle.rs +++ b/crates/nu-command/src/viewers/griddle.rs @@ -277,9 +277,9 @@ fn convert_to_list( &[PathMember::String { val: header.into(), span: head, + optional: false, }], false, - false, ), _ => Ok(item.clone()), }; diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 870550c939..e87bd28fd7 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -934,8 +934,9 @@ fn convert_to_table( let path = PathMember::String { val: text.clone(), span: head, + optional: false, }; - let val = item.clone().follow_cell_path(&[path], false, false); + let val = item.clone().follow_cell_path(&[path], false); match val { Ok(val) => DeferredStyleComputation::Value { value: val }, @@ -1321,8 +1322,12 @@ fn create_table2_entry( match item { Value::Record { .. } => { let val = header.to_owned(); - let path = PathMember::String { val, span: head }; - let val = item.clone().follow_cell_path(&[path], false, false); + let path = PathMember::String { + val, + span: head, + optional: false, + }; + let val = item.clone().follow_cell_path(&[path], false); match val { Ok(val) => convert_to_table2_entry( diff --git a/crates/nu-command/tests/commands/cal.rs b/crates/nu-command/tests/commands/cal.rs index 6fe361861a..70261fd4c7 100644 --- a/crates/nu-command/tests/commands/cal.rs +++ b/crates/nu-command/tests/commands/cal.rs @@ -71,7 +71,7 @@ fn cal_sees_pipeline_year() { let actual = nu!( cwd: ".", pipeline( r#" - cal --full-year 1020 | get -i monday | first 4 | to json -r + cal --full-year 1020 | get monday | first 4 | to json -r "# )); diff --git a/crates/nu-command/tests/commands/reject.rs b/crates/nu-command/tests/commands/reject.rs index fa9add2afa..06440b0f79 100644 --- a/crates/nu-command/tests/commands/reject.rs +++ b/crates/nu-command/tests/commands/reject.rs @@ -23,7 +23,7 @@ fn regular_columns() { #[test] fn skip_cell_rejection() { let actual = nu!(cwd: ".", pipeline( - r#"[ {a: 1, b: 2,c:txt}, { a:val } ] | reject a | get c.0"#)); + r#"[ {a: 1, b: 2,c:txt}, { a:val } ] | reject a | get c?.0"#)); assert_eq!(actual.out, "txt"); } diff --git a/crates/nu-command/tests/commands/select.rs b/crates/nu-command/tests/commands/select.rs index 9f7d3a99ba..f72a131439 100644 --- a/crates/nu-command/tests/commands/select.rs +++ b/crates/nu-command/tests/commands/select.rs @@ -165,7 +165,7 @@ fn select_ignores_errors_successfully1() { let actual = nu!( cwd: ".", pipeline( r#" - [{a: 1, b: 2} {a: 3, b: 5} {a: 3}] | select -i b | length + [{a: 1, b: 2} {a: 3, b: 5} {a: 3}] | select b? | length "# )); @@ -178,7 +178,7 @@ fn select_ignores_errors_successfully2() { let actual = nu!( cwd: ".", pipeline( r#" - [{a: 1} {a: 2} {a: 3}] | select -i b | to nuon + [{a: 1} {a: 2} {a: 3}] | select b? | to nuon "# )); @@ -190,7 +190,7 @@ fn select_ignores_errors_successfully2() { fn select_ignores_errors_successfully3() { let actual = nu!( cwd: ".", pipeline( - r#"sys | select -i invalid_key | to nuon"# + r#"sys | select invalid_key? | to nuon"# )); assert_eq!(actual.out, "{invalid_key: null}".to_string()); @@ -201,38 +201,10 @@ fn select_ignores_errors_successfully3() { fn select_ignores_errors_successfully4() { let actual = nu!( cwd: ".", pipeline( - r#"[a b c] | select -i invalid_key | to nuon"# + r#""key val\na 1\nb 2\n" | lines | split column -c " " | select foo? | to nuon"# )); - assert_eq!( - actual.out, - "[[invalid_key]; [null], [null], [null]]".to_string() - ); - assert!(actual.err.is_empty()); -} - -#[test] -fn select_ignores_errors_successfully5() { - let actual = nu!( - cwd: ".", pipeline( - r#"[a b c] | select -i 0.0"# - )); - - assert!(actual.out.is_empty()); - assert!(actual.err.is_empty()); -} - -#[test] -fn select_ignores_errors_successfully6() { - let actual = nu!( - cwd: ".", pipeline( - r#""key val\na 1\nb 2\n" | lines | split column -c " " | select -i "100" | to nuon"# - )); - - assert_eq!( - actual.out, - r#"[["100"]; [null], [null], [null]]"#.to_string() - ); + assert_eq!(actual.out, r#"[[foo]; [null], [null], [null]]"#.to_string()); assert!(actual.err.is_empty()); } diff --git a/crates/nu-command/tests/commands/where_.rs b/crates/nu-command/tests/commands/where_.rs index d9e0feca50..68fec34461 100644 --- a/crates/nu-command/tests/commands/where_.rs +++ b/crates/nu-command/tests/commands/where_.rs @@ -15,7 +15,7 @@ fn filters_by_unit_size_comparison() { fn filters_with_nothing_comparison() { let actual = nu!( cwd: "tests/fixtures/formats", - r#"'[{"foo": 3}, {"foo": null}, {"foo": 4}]' | from json | get -i foo | compact | where $it > 1 | math sum"# + r#"'[{"foo": 3}, {"foo": null}, {"foo": 4}]' | from json | get foo | compact | where $it > 1 | math sum"# ); assert_eq!(actual.out, "7"); diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index ed91fe6ed7..3a04366397 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -319,10 +319,12 @@ fn get_converted_value( PathMember::String { val: name.to_string(), span: env_span, + optional: false, }, PathMember::String { val: direction.to_string(), span: env_span, + optional: false, }, ]; diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 064b69732d..f87b19e4e2 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -300,7 +300,7 @@ pub fn eval_expression( Expr::FullCellPath(cell_path) => { let value = eval_expression(engine_state, stack, &cell_path.head)?; - value.follow_cell_path(&cell_path.tail, false, false) + value.follow_cell_path(&cell_path.tail, false) } Expr::ImportPattern(_) => Ok(Value::Nothing { span: expr.span }), Expr::Overlay(_) => { @@ -484,7 +484,6 @@ pub fn eval_expression( let vardata = lhs.follow_cell_path( &[cell_path.tail[0].clone()], false, - false, )?; match &cell_path.tail[0] { PathMember::String { val, .. } => { diff --git a/crates/nu-explore/src/nu_common/table.rs b/crates/nu-explore/src/nu_common/table.rs index 696e620264..a033528053 100644 --- a/crates/nu-explore/src/nu_common/table.rs +++ b/crates/nu-explore/src/nu_common/table.rs @@ -599,8 +599,12 @@ fn create_table2_entry_basic( match item { Value::Record { .. } => { let val = header.to_owned(); - let path = PathMember::String { val, span: head }; - let val = item.clone().follow_cell_path(&[path], false, false); + let path = PathMember::String { + val, + span: head, + optional: false, + }; + let val = item.clone().follow_cell_path(&[path], false); match val { Ok(val) => value_to_styled_string(&val, config, style_computer), @@ -627,8 +631,12 @@ fn create_table2_entry( match item { Value::Record { .. } => { let val = header.to_owned(); - let path = PathMember::String { val, span: head }; - let val = item.clone().follow_cell_path(&[path], false, false); + let path = PathMember::String { + val, + span: head, + optional: false, + }; + let val = item.clone().follow_cell_path(&[path], false); match val { Ok(val) => convert_to_table2_entry( diff --git a/crates/nu-explore/src/nu_common/value.rs b/crates/nu-explore/src/nu_common/value.rs index be09e7d3e5..a20f96e27c 100644 --- a/crates/nu-explore/src/nu_common/value.rs +++ b/crates/nu-explore/src/nu_common/value.rs @@ -172,10 +172,11 @@ fn record_lookup_value(item: &Value, header: &str) -> Value { let path = PathMember::String { val: header.to_owned(), span: NuSpan::unknown(), + optional: false, }; item.clone() - .follow_cell_path(&[path], false, false) + .follow_cell_path(&[path], false) .unwrap_or_else(|_| item.clone()) } item => item.clone(), diff --git a/crates/nu-parser/src/eval.rs b/crates/nu-parser/src/eval.rs index e46699e8a1..e3cc6ec9e3 100644 --- a/crates/nu-parser/src/eval.rs +++ b/crates/nu-parser/src/eval.rs @@ -31,7 +31,7 @@ pub fn eval_constant( Expr::FullCellPath(cell_path) => { let value = eval_constant(working_set, &cell_path.head)?; - match value.follow_cell_path(&cell_path.tail, false, false) { + match value.follow_cell_path(&cell_path.tail, false) { Ok(val) => Ok(val), // TODO: Better error conversion Err(shell_error) => Err(ParseError::LabeledError( diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 62b0e3d428..ddf00bd2fe 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -941,10 +941,12 @@ pub fn parse_old_alias( PathMember::String { val: "scope".to_string(), span: Span::new(0, 0), + optional: false, }, PathMember::String { val: "aliases".to_string(), span: Span::new(0, 0), + optional: false, }, ]; let expr = Expression { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 233ec8fb15..b62faf8a0b 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -2015,54 +2015,100 @@ pub fn parse_variable_expr( pub fn parse_cell_path( working_set: &mut StateWorkingSet, tokens: impl Iterator, - mut expect_dot: bool, + expect_dot: bool, expand_aliases_denylist: &[usize], - span: Span, ) -> (Vec, Option) { + enum TokenType { + Dot, // . + QuestionOrDot, // ? or . + PathMember, // an int or string, like `1` or `foo` + } + + // Parsing a cell path is essentially a state machine, and this is the state + let mut expected_token = if expect_dot { + TokenType::Dot + } else { + TokenType::PathMember + }; + let mut error = None; let mut tail = vec![]; for path_element in tokens { let bytes = working_set.get_span_contents(path_element.span); - if expect_dot { - expect_dot = false; - if bytes.len() != 1 || bytes[0] != b'.' { - error = error.or_else(|| Some(ParseError::Expected('.'.into(), path_element.span))); + match expected_token { + TokenType::Dot => { + if bytes.len() != 1 || bytes[0] != b'.' { + return ( + tail, + Some(ParseError::Expected('.'.into(), path_element.span)), + ); + } + expected_token = TokenType::PathMember; } - } else { - expect_dot = true; - - match parse_int(bytes, path_element.span) { - ( - Expression { - expr: Expr::Int(val), - span, - .. - }, - None, - ) => tail.push(PathMember::Int { - val: val as usize, - span, - }), - _ => { - let (result, err) = - parse_string(working_set, path_element.span, expand_aliases_denylist); - error = error.or(err); - match result { + TokenType::QuestionOrDot => { + if bytes.len() == 1 && bytes[0] == b'.' { + expected_token = TokenType::PathMember; + } else if bytes.len() == 1 && bytes[0] == b'?' { + if let Some(last) = tail.last_mut() { + match last { + PathMember::String { + ref mut optional, .. + } => *optional = true, + PathMember::Int { + ref mut optional, .. + } => *optional = true, + } + } + expected_token = TokenType::Dot; + } else { + return ( + tail, + Some(ParseError::Expected(". or ?".into(), path_element.span)), + ); + } + } + TokenType::PathMember => { + match parse_int(bytes, path_element.span) { + ( Expression { - expr: Expr::String(string), + expr: Expr::Int(val), span, .. - } => { - tail.push(PathMember::String { val: string, span }); - } - _ => { - error = - error.or_else(|| Some(ParseError::Expected("string".into(), span))); + }, + None, + ) => tail.push(PathMember::Int { + val: val as usize, + span, + optional: false, + }), + _ => { + let (result, err) = + parse_string(working_set, path_element.span, expand_aliases_denylist); + error = error.or(err); + match result { + Expression { + expr: Expr::String(string), + span, + .. + } => { + tail.push(PathMember::String { + val: string, + span, + optional: false, + }); + } + _ => { + return ( + tail, + Some(ParseError::Expected("string".into(), path_element.span)), + ); + } } } } + expected_token = TokenType::QuestionOrDot; } } } @@ -2081,7 +2127,7 @@ pub fn parse_full_cell_path( let source = working_set.get_span_contents(span); let mut error = None; - let (tokens, err) = lex(source, span.start, &[b'\n', b'\r'], &[b'.'], true); + let (tokens, err) = lex(source, span.start, &[b'\n', b'\r'], &[b'.', b'?'], true); error = error.or(err); let mut tokens = tokens.into_iter().peekable(); @@ -2202,13 +2248,7 @@ pub fn parse_full_cell_path( ); }; - let (tail, err) = parse_cell_path( - working_set, - tokens, - expect_dot, - expand_aliases_denylist, - span, - ); + let (tail, err) = parse_cell_path(working_set, tokens, expect_dot, expand_aliases_denylist); error = error.or(err); ( @@ -4597,13 +4637,13 @@ pub fn parse_value( let source = working_set.get_span_contents(span); let mut error = None; - let (tokens, err) = lex(source, span.start, &[b'\n', b'\r'], &[b'.'], true); + let (tokens, err) = lex(source, span.start, &[b'\n', b'\r'], &[b'.', b'?'], true); error = error.or(err); let tokens = tokens.into_iter().peekable(); let (cell_path, err) = - parse_cell_path(working_set, tokens, false, expand_aliases_denylist, span); + parse_cell_path(working_set, tokens, false, expand_aliases_denylist); error = error.or(err); ( diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index f4c153479b..1f03bf78c9 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -1,6 +1,7 @@ use nu_parser::ParseError; use nu_parser::*; -use nu_protocol::ast::Call; +use nu_protocol::ast::{Call, PathMember}; +use nu_protocol::Span; use nu_protocol::{ ast::{Expr, Expression, PipelineElement}, engine::{Command, EngineState, Stack, StateWorkingSet}, @@ -321,6 +322,118 @@ pub fn parse_int_with_underscores() { )) } +#[test] +pub fn parse_cell_path() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_variable( + "foo".to_string().into_bytes(), + Span::test_data(), + nu_protocol::Type::Record(vec![]), + false, + ); + + let (block, err) = parse(&mut working_set, None, b"$foo.bar.baz", true, &[]); + + assert!(err.is_none()); + assert_eq!(block.len(), 1); + let expressions = &block[0]; + assert_eq!(expressions.len(), 1); + + // hoo boy this pattern matching is a pain + if let PipelineElement::Expression(_, expr) = &expressions[0] { + if let Expr::FullCellPath(b) = &expr.expr { + assert!(matches!( + b.head, + Expression { + expr: Expr::Var(_), + .. + } + )); + if let [a, b] = &b.tail[..] { + if let PathMember::String { val, optional, .. } = a { + assert_eq!(val, "bar"); + assert_eq!(optional, &false); + } else { + panic!("wrong type") + } + + if let PathMember::String { val, optional, .. } = b { + assert_eq!(val, "baz"); + assert_eq!(optional, &false); + } else { + panic!("wrong type") + } + } else { + panic!("cell path tail is unexpected") + } + } else { + panic!("Not a cell path"); + } + } else { + panic!("Not an expression") + } +} + +#[test] +pub fn parse_cell_path_optional() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_variable( + "foo".to_string().into_bytes(), + Span::test_data(), + nu_protocol::Type::Record(vec![]), + false, + ); + + let (block, err) = parse(&mut working_set, None, b"$foo.bar?.baz", true, &[]); + + if let Some(err) = err { + dbg!(err); + panic!(); + } + + assert_eq!(block.len(), 1); + let expressions = &block[0]; + assert_eq!(expressions.len(), 1); + + // hoo boy this pattern matching is a pain + if let PipelineElement::Expression(_, expr) = &expressions[0] { + if let Expr::FullCellPath(b) = &expr.expr { + assert!(matches!( + b.head, + Expression { + expr: Expr::Var(_), + .. + } + )); + if let [a, b] = &b.tail[..] { + if let PathMember::String { val, optional, .. } = a { + assert_eq!(val, "bar"); + assert_eq!(optional, &true); + } else { + panic!("wrong type") + } + + if let PathMember::String { val, optional, .. } = b { + assert_eq!(val, "baz"); + assert_eq!(optional, &false); + } else { + panic!("wrong type") + } + } else { + panic!("cell path tail is unexpected") + } + } else { + panic!("Not a cell path"); + } + } else { + panic!("Not an expression") + } +} + #[test] pub fn parse_binary_with_hex_format() { let engine_state = EngineState::new(); diff --git a/crates/nu-protocol/src/ast/cell_path.rs b/crates/nu-protocol/src/ast/cell_path.rs index 07d5a14235..8892207ad3 100644 --- a/crates/nu-protocol/src/ast/cell_path.rs +++ b/crates/nu-protocol/src/ast/cell_path.rs @@ -5,15 +5,45 @@ use std::fmt::Write; #[derive(Debug, Clone, PartialOrd, Serialize, Deserialize)] pub enum PathMember { - String { val: String, span: Span }, - Int { val: usize, span: Span }, + String { + val: String, + span: Span, + optional: bool, + }, + Int { + val: usize, + span: Span, + optional: bool, + }, } impl PartialEq for PathMember { fn eq(&self, other: &Self) -> bool { match (self, other) { - (Self::String { val: l_val, .. }, Self::String { val: r_val, .. }) => l_val == r_val, - (Self::Int { val: l_val, .. }, Self::Int { val: r_val, .. }) => l_val == r_val, + ( + Self::String { + val: l_val, + optional: l_opt, + .. + }, + Self::String { + val: r_val, + optional: r_opt, + .. + }, + ) => l_val == r_val && l_opt == r_opt, + ( + Self::Int { + val: l_val, + optional: l_opt, + .. + }, + Self::Int { + val: r_val, + optional: r_opt, + .. + }, + ) => l_val == r_val && l_opt == r_opt, _ => false, } } @@ -42,6 +72,19 @@ impl CellPath { output } + + pub fn make_optional(&mut self) { + for member in &mut self.members { + match member { + PathMember::String { + ref mut optional, .. + } => *optional = true, + PathMember::Int { + ref mut optional, .. + } => *optional = true, + } + } + } } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 0864445ad8..40026e25be 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -322,7 +322,6 @@ impl PipelineData { cell_path: &[PathMember], head: Span, insensitive: bool, - ignore_errors: bool, ) -> Result { match self { // FIXME: there are probably better ways of doing this @@ -330,8 +329,8 @@ impl PipelineData { vals: stream.collect(), span: head, } - .follow_cell_path(cell_path, insensitive, ignore_errors), - PipelineData::Value(v, ..) => v.follow_cell_path(cell_path, insensitive, ignore_errors), + .follow_cell_path(cell_path, insensitive), + PipelineData::Value(v, ..) => v.follow_cell_path(cell_path, insensitive), _ => Err(ShellError::IOError("can't follow stream paths".into())), } } diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index 33510b666b..89ac2926b4 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -302,6 +302,7 @@ impl FromValue for CellPath { members: vec![PathMember::String { val: val.clone(), span, + optional: false, }], }), Value::Int { val, span } => { @@ -312,6 +313,7 @@ impl FromValue for CellPath { members: vec![PathMember::Int { val: *val as usize, span: *span, + optional: false, }], }) } diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index ebc44a2a7e..60bd1ec259 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -681,9 +681,8 @@ impl Value { self, cell_path: &[PathMember], insensitive: bool, - nullify_errors: bool, ) -> Result { - self.follow_cell_path_helper(cell_path, insensitive, nullify_errors, true) + self.follow_cell_path_helper(cell_path, insensitive, true) } pub fn follow_cell_path_not_from_user_input( @@ -691,24 +690,20 @@ impl Value { cell_path: &[PathMember], insensitive: bool, ) -> Result { - self.follow_cell_path_helper(cell_path, insensitive, false, false) + self.follow_cell_path_helper(cell_path, insensitive, false) } fn follow_cell_path_helper( self, cell_path: &[PathMember], insensitive: bool, - nullify_errors: bool, // Turn all errors into Value::Nothing from_user_input: bool, ) -> Result { let mut current = self; + // TODO remove this macro_rules! err_or_null { ($e:expr, $span:expr) => { - return if nullify_errors { - Ok(Value::nothing($span)) - } else { - Err($e) - } + return Err($e) }; } for member in cell_path { @@ -718,12 +713,15 @@ impl Value { PathMember::Int { val: count, span: origin_span, + optional, } => { // Treat a numeric path member as `select ` match &mut current { Value::List { vals: val, .. } => { if let Some(item) = val.get(*count) { current = item.clone(); + } else if *optional { + current = Value::nothing(*origin_span); } else if val.is_empty() { err_or_null!( ShellError::AccessEmptyContent { span: *origin_span }, @@ -742,6 +740,8 @@ impl Value { Value::Binary { val, .. } => { if let Some(item) = val.get(*count) { current = Value::int(*item as i64, *origin_span); + } else if *optional { + current = Value::nothing(*origin_span); } else if val.is_empty() { err_or_null!( ShellError::AccessEmptyContent { span: *origin_span }, @@ -760,6 +760,8 @@ impl Value { Value::Range { val, .. } => { if let Some(item) = val.clone().into_range_iter(None)?.nth(*count) { current = item.clone(); + } else if *optional { + current = Value::nothing(*origin_span); } else { err_or_null!( ShellError::AccessBeyondEndOfStream { span: *origin_span }, @@ -768,7 +770,19 @@ impl Value { } } Value::CustomValue { val, .. } => { - current = val.follow_path_int(*count, *origin_span)?; + current = match val.follow_path_int(*count, *origin_span) { + Ok(val) => val, + Err(err) => { + if *optional { + Value::nothing(*origin_span) + } else { + return Err(err); + } + } + }; + } + Value::Nothing { .. } if *optional => { + current = Value::nothing(*origin_span); } // Records (and tables) are the only built-in which support column names, // so only use this message for them. @@ -792,6 +806,7 @@ impl Value { PathMember::String { val: column_name, span: origin_span, + optional, } => match &mut current { Value::Record { cols, vals, span } => { let cols = cols.clone(); @@ -806,6 +821,8 @@ impl Value { } }) { current = found.1.clone(); + } else if *optional { + current = Value::nothing(*origin_span); } else { if from_user_input { if let Some(suggestion) = did_you_mean(&cols, column_name) { @@ -830,6 +847,8 @@ impl Value { if columns.contains(&column_name.as_str()) { current = val.get_column_value(column_name)?; + } else if *optional { + current = Value::nothing(*origin_span); } else { if from_user_input { if let Some(suggestion) = did_you_mean(&columns, column_name) { @@ -852,10 +871,9 @@ impl Value { // String access of Lists always means Table access. // Create a List which contains each matching value for contained // records in the source list. - // If nullify_errors is true, table holes are converted to null. Value::List { vals, span } => { + // TODO: this should stream instead of collecting let mut output = vec![]; - let mut found_at_least_1_value = false; for val in vals { // only look in records; this avoids unintentionally recursing into deeply nested tables if matches!(val, Value::Record { .. }) { @@ -863,69 +881,40 @@ impl Value { &[PathMember::String { val: column_name.clone(), span: *origin_span, + optional: *optional, }], insensitive, - nullify_errors, ) { - found_at_least_1_value = true; output.push(result); } else { - // Consider [{a:1 b:2} {a:3}]: - // [{a:1 b:2} {a:3}].b should error. - // [{a:1 b:2} {a:3}].b.1 should error. - // [{a:1 b:2} {a:3}].b.0 should NOT error because the path can find a proper value (2) - // but if this returns an error immediately, it will. - // - // Solution: push a Value::Error into this result list instead of returning it. - // This also means that `[{a:1 b:2} {a:2}].b | reject 1` also doesn't error. - // Anything that needs to use every value inside the list should propagate - // the error outward, though. - output.push(if nullify_errors { - Value::nothing(*origin_span) - } else { - Value::Error { - error: Box::new(ShellError::CantFindColumn { - col_name: column_name.to_string(), - span: *origin_span, - src_span: val.span().unwrap_or(*span), - }), - } + return Err(ShellError::CantFindColumn { + col_name: column_name.to_string(), + span: *origin_span, + src_span: val.span().unwrap_or(*span), }); } + } else if *optional && matches!(val, Value::Nothing { .. }) { + output.push(Value::nothing(*origin_span)); } else { - // See comment above. - output.push(if nullify_errors { - Value::nothing(*origin_span) - } else { - Value::Error { - error: Box::new(ShellError::CantFindColumn { - col_name: column_name.to_string(), - span: *origin_span, - src_span: val.span().unwrap_or(*span), - }), - } + return Err(ShellError::CantFindColumn { + col_name: column_name.to_string(), + span: *origin_span, + src_span: val.span().unwrap_or(*span), }); } } - if found_at_least_1_value { - current = Value::List { - vals: output, - span: *span, - }; - } else { - err_or_null!( - ShellError::CantFindColumn { - col_name: column_name.to_string(), - span: *origin_span, - src_span: *span - }, - *origin_span - ); - } + + current = Value::List { + vals: output, + span: *span, + }; } Value::CustomValue { val, .. } => { current = val.follow_path_string(column_name.clone(), *origin_span)?; } + Value::Nothing { .. } if *optional => { + current = Value::nothing(*origin_span); + } Value::Error { error } => err_or_null!(*error.to_owned(), *origin_span), x => { err_or_null!( @@ -956,7 +945,7 @@ impl Value { ) -> Result<(), ShellError> { let orig = self.clone(); - let new_val = callback(&orig.follow_cell_path(cell_path, false, false)?); + let new_val = callback(&orig.follow_cell_path(cell_path, false)?); match new_val { Value::Error { error } => Err(*error), @@ -974,6 +963,7 @@ impl Value { PathMember::String { val: col_name, span, + .. } => match self { Value::List { vals, .. } => { for val in vals.iter_mut() { @@ -1055,7 +1045,9 @@ impl Value { }) } }, - PathMember::Int { val: row_num, span } => match self { + PathMember::Int { + val: row_num, span, .. + } => match self { Value::List { vals, .. } => { if let Some(v) = vals.get_mut(*row_num) { v.upsert_data_at_cell_path(&cell_path[1..], new_val)? @@ -1094,7 +1086,7 @@ impl Value { ) -> Result<(), ShellError> { let orig = self.clone(); - let new_val = callback(&orig.follow_cell_path(cell_path, false, false)?); + let new_val = callback(&orig.follow_cell_path(cell_path, false)?); match new_val { Value::Error { error } => Err(*error), @@ -1113,6 +1105,7 @@ impl Value { PathMember::String { val: col_name, span, + .. } => match self { Value::List { vals, .. } => { for val in vals.iter_mut() { @@ -1183,7 +1176,9 @@ impl Value { }) } }, - PathMember::Int { val: row_num, span } => match self { + PathMember::Int { + val: row_num, span, .. + } => match self { Value::List { vals, .. } => { if let Some(v) = vals.get_mut(*row_num) { v.update_data_at_cell_path(&cell_path[1..], new_val)? @@ -1221,6 +1216,7 @@ impl Value { PathMember::String { val: col_name, span, + .. } => match self { Value::List { vals, .. } => { for val in vals.iter_mut() { @@ -1289,7 +1285,9 @@ impl Value { src_span: v.span()?, }), }, - PathMember::Int { val: row_num, span } => match self { + PathMember::Int { + val: row_num, span, .. + } => match self { Value::List { vals, .. } => { if vals.get_mut(*row_num).is_some() { vals.remove(*row_num); @@ -1316,6 +1314,7 @@ impl Value { PathMember::String { val: col_name, span, + .. } => match self { Value::List { vals, .. } => { for val in vals.iter_mut() { @@ -1380,7 +1379,9 @@ impl Value { src_span: v.span()?, }), }, - PathMember::Int { val: row_num, span } => match self { + PathMember::Int { + val: row_num, span, .. + } => match self { Value::List { vals, .. } => { if let Some(v) = vals.get_mut(*row_num) { v.remove_data_at_cell_path(&cell_path[1..]) @@ -1414,6 +1415,7 @@ impl Value { PathMember::String { val: col_name, span, + .. } => match self { Value::List { vals, .. } => { for val in vals.iter_mut() { @@ -1492,7 +1494,9 @@ impl Value { )) } }, - PathMember::Int { val: row_num, span } => match self { + PathMember::Int { + val: row_num, span, .. + } => match self { Value::List { vals, .. } => { if let Some(v) = vals.get_mut(*row_num) { v.insert_data_at_cell_path(&cell_path[1..], new_val, head_span)? diff --git a/crates/nu_plugin_inc/src/inc.rs b/crates/nu_plugin_inc/src/inc.rs index 1955782b1c..d6cd56c028 100644 --- a/crates/nu_plugin_inc/src/inc.rs +++ b/crates/nu_plugin_inc/src/inc.rs @@ -97,7 +97,7 @@ impl Inc { pub fn inc(&self, head: Span, value: &Value) -> Result { if let Some(cell_path) = &self.cell_path { let working_value = value.clone(); - let cell_value = working_value.follow_cell_path(&cell_path.members, false, false)?; + let cell_value = working_value.follow_cell_path(&cell_path.members, false)?; let cell_value = self.inc_value(head, &cell_value)?; diff --git a/src/tests/test_cell_path.rs b/src/tests/test_cell_path.rs index 49636506c6..04a08a37a7 100644 --- a/src/tests/test_cell_path.rs +++ b/src/tests/test_cell_path.rs @@ -17,6 +17,21 @@ fn record_single_field_success() -> TestResult { run_test("{foo: 'bar'}.foo == 'bar'", "true") } +#[test] +fn record_single_field_optional_success() -> TestResult { + run_test("{foo: 'bar'}.foo? == 'bar'", "true") +} + +#[test] +fn get_works_with_cell_path_success() -> TestResult { + run_test("{foo: 'bar'} | get foo?", "bar") +} + +#[test] +fn get_works_with_cell_path_missing_data() -> TestResult { + run_test("{foo: 'bar'} | get foobar? | to nuon", "null") +} + #[test] fn record_single_field_failure() -> TestResult { fail_test("{foo: 'bar'}.foobar", "") @@ -27,6 +42,21 @@ fn record_int_failure() -> TestResult { fail_test("{foo: 'bar'}.3", "") } +#[test] +fn record_single_field_optional() -> TestResult { + run_test("{foo: 'bar'}.foobar? | to nuon", "null") +} + +#[test] +fn record_single_field_optional_does_not_short_circuit() -> TestResult { + fail_test("{foo: 'bar'}.foobar?.baz", "nothing") +} + +#[test] +fn record_multiple_optional_fields() -> TestResult { + run_test("{foo: 'bar'}.foobar?.baz? | to nuon", "null") +} + #[test] fn nested_record_field_success() -> TestResult { run_test("{foo: {bar: 'baz'} }.foo.bar == 'baz'", "true") @@ -37,6 +67,11 @@ fn nested_record_field_failure() -> TestResult { fail_test("{foo: {bar: 'baz'} }.foo.asdf", "") } +#[test] +fn nested_record_field_optional() -> TestResult { + run_test("{foo: {bar: 'baz'} }.foo.asdf? | to nuon", "null") +} + #[test] fn record_with_nested_list_success() -> TestResult { run_test("{foo: [{bar: 'baz'}]}.foo.0.bar == 'baz'", "true") @@ -72,12 +107,27 @@ fn jagged_list_access_fails() -> TestResult { fail_test("[{}, {foo: 'bar'}].foo", "cannot find column") } +#[test] +fn jagged_list_optional_access_succeeds() -> TestResult { + run_test("[{foo: 'bar'}, {}].foo?.0", "bar")?; + run_test("[{foo: 'bar'}, {}].foo?.1 | to nuon", "null")?; + + run_test("[{}, {foo: 'bar'}].foo?.0 | to nuon", "null")?; + run_test("[{}, {foo: 'bar'}].foo?.1", "bar") +} + // test that accessing a nonexistent row fails #[test] fn list_row_access_failure() -> TestResult { fail_test("[{foo: 'bar'}, {foo: 'baz'}].2", "") } +#[test] +fn list_row_optional_access_succeeds() -> TestResult { + run_test("[{foo: 'bar'}, {foo: 'baz'}].2? | to nuon", "null")?; + run_test("[{foo: 'bar'}, {foo: 'baz'}].3? | to nuon", "null") +} + // regression test for an old bug #[test] fn do_not_delve_too_deep_in_nested_lists() -> TestResult { diff --git a/src/tests/test_table_operations.rs b/src/tests/test_table_operations.rs index a7a2e9caa5..ae4e36585f 100644 --- a/src/tests/test_table_operations.rs +++ b/src/tests/test_table_operations.rs @@ -179,6 +179,33 @@ fn missing_column_errors() -> TestResult { ) } +#[test] +fn missing_optional_column_fills_in_nothing() -> TestResult { + // The empty value will be replaced with $nothing because of the ? + run_test( + r#"[ { name: ABC, size: 20 }, { name: HIJ } ].size?.1 == $nothing"#, + "true", + ) +} + +#[test] +fn missing_required_row_fails() -> TestResult { + // .3 will fail if there is no 3rd row + fail_test( + r#"[ { name: ABC, size: 20 }, { name: HIJ } ].3"#, + "", // we just care if it errors + ) +} + +#[test] +fn missing_optional_row_fills_in_nothing() -> TestResult { + // ?.3 will return $nothing if there is no 3rd row + run_test( + r#"[ { name: ABC, size: 20 }, { name: HIJ } ].3? == $nothing"#, + "true", + ) +} + #[test] fn string_cell_path() -> TestResult { run_test( @@ -257,9 +284,9 @@ fn length_defaulted_columns() -> TestResult { #[test] fn nullify_errors() -> TestResult { - run_test("([{a:1} {a:2} {a:3}] | get -i foo | length) == 3", "true")?; + run_test("([{a:1} {a:2} {a:3}] | get foo? | length) == 3", "true")?; run_test( - "([{a:1} {a:2} {a:3}] | get -i foo | to nuon) == '[null, null, null]'", + "([{a:1} {a:2} {a:3}] | get foo? | to nuon) == '[null, null, null]'", "true", ) } @@ -267,7 +294,7 @@ fn nullify_errors() -> TestResult { #[test] fn nullify_holes() -> TestResult { run_test( - "([{a:1} {b:2} {a:3}] | get -i a | to nuon) == '[1, null, 3]'", + "([{a:1} {b:2} {a:3}] | get a? | to nuon) == '[1, null, 3]'", "true", ) }