From dd7b7311b33741e1ab46135a7cca71c82dea3852 Mon Sep 17 00:00:00 2001 From: Leon Date: Fri, 23 Dec 2022 16:48:53 +1000 Subject: [PATCH] Standardise the use of ShellError::UnsupportedInput and ShellError::TypeMismatch and add spans to every instance of the former (#7217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description * I was dismayed to discover recently that UnsupportedInput and TypeMismatch are used *extremely* inconsistently across the codebase. UnsupportedInput is sometimes used for input type-checks (as per the name!!), but *also* used for argument type-checks. TypeMismatch is also used for both. I thus devised the following standard: input type-checking *only* uses UnsupportedInput, and argument type-checking *only* uses TypeMismatch. Moreover, to differentiate them, UnsupportedInput now has *two* error arrows (spans), one pointing at the command and the other at the input origin, while TypeMismatch only has the one (because the command should always be nearby) * In order to apply that standard, a very large number of UnsupportedInput uses were changed so that the input's span could be retrieved and delivered to it. * Additionally, I noticed many places where **errors are not propagated correctly**: there are lots of `match` sites which take a Value::Error, then throw it away and replace it with a new Value::Error with less/misleading information (such as reporting the error as an "incorrect type"). I believe that the earliest errors are the most important, and should always be propagated where possible. * Also, to standardise one broad subset of UnsupportedInput error messages, who all used slightly different wordings of "expected ``, got ``", I created OnlySupportsThisInputType as a variant of it. * Finally, a bunch of error sites that had "repeated spans" - i.e. where an error expected two spans, but `call.head` was given for both - were fixed to use different spans. # Example BEFORE ``` 〉20b | str starts-with 'a' Error: nu::shell::unsupported_input (link) × Unsupported input ╭─[entry #31:1:1] 1 │ 20b | str starts-with 'a' · ┬ · ╰── Input's type is filesize. This command only works with strings. ╰──── 〉'a' | math cos Error: nu::shell::unsupported_input (link) × Unsupported input ╭─[entry #33:1:1] 1 │ 'a' | math cos · ─┬─ · ╰── Only numerical values are supported, input type: String ╰──── 〉0x[12] | encode utf8 Error: nu::shell::unsupported_input (link) × Unsupported input ╭─[entry #38:1:1] 1 │ 0x[12] | encode utf8 · ───┬── · ╰── non-string input ╰──── ``` AFTER ``` 〉20b | str starts-with 'a' Error: nu::shell::pipeline_mismatch (link) × Pipeline mismatch. ╭─[entry #1:1:1] 1 │ 20b | str starts-with 'a' · ┬ ───────┬─────── · │ ╰── only string input data is supported · ╰── input type: filesize ╰──── 〉'a' | math cos Error: nu::shell::pipeline_mismatch (link) × Pipeline mismatch. ╭─[entry #2:1:1] 1 │ 'a' | math cos · ─┬─ ────┬─── · │ ╰── only numeric input data is supported · ╰── input type: string ╰──── 〉0x[12] | encode utf8 Error: nu::shell::pipeline_mismatch (link) × Pipeline mismatch. ╭─[entry #3:1:1] 1 │ 0x[12] | encode utf8 · ───┬── ───┬── · │ ╰── only string input data is supported · ╰── input type: binary ╰──── ``` # User-Facing Changes Various error messages suddenly make more sense (i.e. have two arrows instead of one). # Tests + Formatting Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass # After Submitting If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --- crates/nu-command/src/bits/and.rs | 18 +- crates/nu-command/src/bits/not.rs | 23 ++- crates/nu-command/src/bits/or.rs | 18 +- crates/nu-command/src/bits/rotate_left.rs | 21 +- crates/nu-command/src/bits/rotate_right.rs | 21 +- crates/nu-command/src/bits/shift_left.rs | 21 +- crates/nu-command/src/bits/shift_right.rs | 21 +- crates/nu-command/src/bits/xor.rs | 19 +- crates/nu-command/src/bytes/add.rs | 12 +- crates/nu-command/src/bytes/at.rs | 46 +++-- crates/nu-command/src/bytes/build_.rs | 8 +- crates/nu-command/src/bytes/collect.rs | 16 +- crates/nu-command/src/bytes/ends_with.rs | 12 +- crates/nu-command/src/bytes/index_of.rs | 12 +- crates/nu-command/src/bytes/length.rs | 12 +- crates/nu-command/src/bytes/remove.rs | 14 +- crates/nu-command/src/bytes/replace.rs | 14 +- crates/nu-command/src/bytes/reverse.rs | 12 +- crates/nu-command/src/bytes/starts_with.rs | 12 +- .../nu-command/src/charting/hashable_value.rs | 15 +- crates/nu-command/src/charting/histogram.rs | 43 ++-- crates/nu-command/src/conversions/fmt.rs | 11 +- .../nu-command/src/conversions/into/binary.rs | 17 +- .../nu-command/src/conversions/into/bool.rs | 11 +- .../src/conversions/into/datetime.rs | 191 ++++++------------ .../src/conversions/into/decimal.rs | 23 +-- .../src/conversions/into/duration.rs | 17 +- .../src/conversions/into/filesize.rs | 16 +- crates/nu-command/src/conversions/into/int.rs | 33 ++- .../nu-command/src/conversions/into/record.rs | 16 +- .../nu-command/src/conversions/into/string.rs | 8 +- crates/nu-command/src/core_commands/debug.rs | 2 + .../src/database/commands/into_sqlite.rs | 12 +- crates/nu-command/src/dataframe/eager/drop.rs | 2 +- .../values/nu_dataframe/conversion.rs | 8 + crates/nu-command/src/date/format.rs | 11 +- crates/nu-command/src/date/humanize.rs | 4 + crates/nu-command/src/date/to_record.rs | 7 +- crates/nu-command/src/date/to_table.rs | 7 +- crates/nu-command/src/date/to_timezone.rs | 36 ++-- crates/nu-command/src/env/load_env.rs | 2 + crates/nu-command/src/filesystem/save.rs | 49 +++-- crates/nu-command/src/filesystem/touch.rs | 2 +- crates/nu-command/src/filesystem/watch.rs | 11 +- crates/nu-command/src/filters/drop/nth.rs | 6 +- crates/nu-command/src/filters/find.rs | 24 +-- crates/nu-command/src/filters/first.rs | 36 ++-- crates/nu-command/src/filters/flatten.rs | 34 ++-- crates/nu-command/src/filters/group_by.rs | 24 ++- crates/nu-command/src/filters/headers.rs | 2 +- crates/nu-command/src/filters/insert.rs | 13 +- crates/nu-command/src/filters/lines.rs | 31 ++- crates/nu-command/src/filters/rename.rs | 27 ++- crates/nu-command/src/filters/rotate.rs | 14 +- crates/nu-command/src/filters/skip/skip_.rs | 2 +- crates/nu-command/src/filters/take/take_.rs | 38 ++-- .../nu-command/src/formats/from/delimited.rs | 4 +- crates/nu-command/src/formats/from/eml.rs | 2 +- crates/nu-command/src/formats/from/ics.rs | 4 +- crates/nu-command/src/formats/from/ini.rs | 12 +- crates/nu-command/src/formats/from/json.rs | 2 +- crates/nu-command/src/formats/from/nuon.rs | 2 +- crates/nu-command/src/formats/from/ods.rs | 18 +- crates/nu-command/src/formats/from/ssv.rs | 2 +- crates/nu-command/src/formats/from/toml.rs | 2 +- crates/nu-command/src/formats/from/url.rs | 6 +- crates/nu-command/src/formats/from/vcf.rs | 4 +- crates/nu-command/src/formats/from/xlsx.rs | 17 +- crates/nu-command/src/formats/from/xml.rs | 6 +- crates/nu-command/src/formats/from/yaml.rs | 52 +++-- crates/nu-command/src/formats/to/csv.rs | 2 +- crates/nu-command/src/formats/to/delimited.rs | 34 ++-- crates/nu-command/src/formats/to/nuon.rs | 22 +- crates/nu-command/src/formats/to/toml.rs | 16 +- crates/nu-command/src/formats/to/url.rs | 14 +- crates/nu-command/src/generators/cal.rs | 5 +- crates/nu-command/src/hash/generic_digest.rs | 12 +- crates/nu-command/src/input_handler.rs | 19 +- crates/nu-command/src/math/abs.rs | 10 +- crates/nu-command/src/math/arccos.rs | 18 +- crates/nu-command/src/math/arccosh.rs | 18 +- crates/nu-command/src/math/arcsin.rs | 18 +- crates/nu-command/src/math/arcsinh.rs | 16 +- crates/nu-command/src/math/arctan.rs | 16 +- crates/nu-command/src/math/arctanh.rs | 18 +- crates/nu-command/src/math/avg.rs | 4 +- crates/nu-command/src/math/ceil.rs | 16 +- crates/nu-command/src/math/cos.rs | 16 +- crates/nu-command/src/math/cosh.rs | 16 +- crates/nu-command/src/math/eval.rs | 30 ++- crates/nu-command/src/math/floor.rs | 16 +- crates/nu-command/src/math/ln.rs | 18 +- crates/nu-command/src/math/log.rs | 21 +- crates/nu-command/src/math/max.rs | 4 +- crates/nu-command/src/math/median.rs | 33 ++- crates/nu-command/src/math/min.rs | 4 +- crates/nu-command/src/math/mode.rs | 7 +- crates/nu-command/src/math/product.rs | 4 +- crates/nu-command/src/math/reducers.rs | 50 +++-- crates/nu-command/src/math/round.rs | 16 +- crates/nu-command/src/math/sin.rs | 16 +- crates/nu-command/src/math/sinh.rs | 16 +- crates/nu-command/src/math/sqrt.rs | 24 ++- crates/nu-command/src/math/stddev.rs | 24 ++- crates/nu-command/src/math/sum.rs | 4 +- crates/nu-command/src/math/tan.rs | 16 +- crates/nu-command/src/math/tanh.rs | 16 +- crates/nu-command/src/math/utils.rs | 73 ++++--- crates/nu-command/src/math/variance.rs | 36 ++-- crates/nu-command/src/network/fetch.rs | 26 +-- crates/nu-command/src/network/post.rs | 30 +-- crates/nu-command/src/network/url/parse.rs | 13 +- crates/nu-command/src/path/basename.rs | 9 +- crates/nu-command/src/path/dirname.rs | 9 +- crates/nu-command/src/path/exists.rs | 8 +- crates/nu-command/src/path/expand.rs | 7 +- crates/nu-command/src/path/join.rs | 50 +++-- crates/nu-command/src/path/mod.rs | 8 +- crates/nu-command/src/path/parse.rs | 7 +- crates/nu-command/src/path/relative_to.rs | 7 +- crates/nu-command/src/path/split.rs | 6 +- crates/nu-command/src/path/type.rs | 6 +- crates/nu-command/src/platform/ansi/ansi_.rs | 6 +- crates/nu-command/src/strings/char_.rs | 6 +- .../src/strings/encode_decode/base64.rs | 7 +- .../src/strings/encode_decode/decode.rs | 18 +- .../src/strings/encode_decode/encode.rs | 19 +- .../nu-command/src/strings/format/command.rs | 20 +- .../nu-command/src/strings/format/filesize.rs | 12 +- crates/nu-command/src/strings/size.rs | 4 + .../src/strings/str_/case/capitalize.rs | 12 +- .../src/strings/str_/case/downcase.rs | 12 +- .../nu-command/src/strings/str_/case/mod.rs | 12 +- .../src/strings/str_/case/upcase.rs | 15 +- .../nu-command/src/strings/str_/contains.rs | 12 +- .../nu-command/src/strings/str_/distance.rs | 14 +- .../nu-command/src/strings/str_/ends_with.rs | 12 +- .../nu-command/src/strings/str_/index_of.rs | 36 ++-- crates/nu-command/src/strings/str_/length.rs | 12 +- crates/nu-command/src/strings/str_/lpad.rs | 19 +- crates/nu-command/src/strings/str_/replace.rs | 19 +- crates/nu-command/src/strings/str_/reverse.rs | 13 +- crates/nu-command/src/strings/str_/rpad.rs | 19 +- .../src/strings/str_/starts_with.rs | 12 +- .../nu-command/src/strings/str_/substring.rs | 36 ++-- .../nu-command/src/strings/str_/trim/trim_.rs | 11 +- .../nu-command/tests/commands/date/format.rs | 2 +- crates/nu-command/tests/commands/rename.rs | 3 +- crates/nu-command/tests/commands/roll.rs | 11 +- crates/nu-command/tests/commands/take/rows.rs | 2 +- crates/nu-command/tests/commands/url/parse.rs | 2 +- crates/nu-protocol/src/pipeline_data.rs | 12 +- crates/nu-protocol/src/shell_error.rs | 39 +++- crates/nu-protocol/src/value/mod.rs | 39 +++- 154 files changed, 1617 insertions(+), 1025 deletions(-) diff --git a/crates/nu-command/src/bits/and.rs b/crates/nu-command/src/bits/and.rs index f6e5fd2268..47d77e441a 100644 --- a/crates/nu-command/src/bits/and.rs +++ b/crates/nu-command/src/bits/and.rs @@ -43,6 +43,10 @@ impl Command for SubCommand { let head = call.head; let target: i64 = call.req(engine_state, stack, 0)?; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, target, head), engine_state.ctrlc.clone(), @@ -74,13 +78,15 @@ fn operate(value: Value, target: i64, head: Span) -> Value { val: val & target, span, }, + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only integer values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bits/not.rs b/crates/nu-command/src/bits/not.rs index 885c887bc5..33ff6e1ddc 100644 --- a/crates/nu-command/src/bits/not.rs +++ b/crates/nu-command/src/bits/not.rs @@ -56,11 +56,17 @@ impl Command for SubCommand { if let Some(val) = number_bytes { return Err(ShellError::UnsupportedInput( "Only 1, 2, 4, 8, or 'auto' bytes are supported as word sizes".to_string(), + "value originates from here".to_string(), + head, val.span, )); } } + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, signed, bytes_len), engine_state.ctrlc.clone(), @@ -140,14 +146,17 @@ fn operate(value: Value, head: Span, signed: bool, number_size: NumberBytes) -> Value::Int { val: out_val, span } } } - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() + other => match other { + // Propagate errors inside the value + Value::Error { .. } => other, + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), - other.span().unwrap_or(head), - ), + }, }, } } diff --git a/crates/nu-command/src/bits/or.rs b/crates/nu-command/src/bits/or.rs index 99bcb1f0a8..7c40919a6b 100644 --- a/crates/nu-command/src/bits/or.rs +++ b/crates/nu-command/src/bits/or.rs @@ -43,6 +43,10 @@ impl Command for SubCommand { let head = call.head; let target: i64 = call.req(engine_state, stack, 0)?; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, target, head), engine_state.ctrlc.clone(), @@ -74,13 +78,15 @@ fn operate(value: Value, target: i64, head: Span) -> Value { val: val | target, span, }, + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only integer values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bits/rotate_left.rs b/crates/nu-command/src/bits/rotate_left.rs index d50bd5e83f..d44c60cd20 100644 --- a/crates/nu-command/src/bits/rotate_left.rs +++ b/crates/nu-command/src/bits/rotate_left.rs @@ -60,11 +60,16 @@ impl Command for SubCommand { if let Some(val) = number_bytes { return Err(ShellError::UnsupportedInput( "Only 1, 2, 4, 8, or 'auto' bytes are supported as word sizes".to_string(), + "value originates from here".to_string(), + head, val.span, )); } } - + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, bits, head, signed, bytes_len), engine_state.ctrlc.clone(), @@ -130,13 +135,15 @@ fn operate(value: Value, bits: usize, head: Span, signed: bool, number_size: Num SignedEight => get_rotate_left(val as i64, bits, span), } } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only integer values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bits/rotate_right.rs b/crates/nu-command/src/bits/rotate_right.rs index 858ea63610..b026da148e 100644 --- a/crates/nu-command/src/bits/rotate_right.rs +++ b/crates/nu-command/src/bits/rotate_right.rs @@ -60,11 +60,16 @@ impl Command for SubCommand { if let Some(val) = number_bytes { return Err(ShellError::UnsupportedInput( "Only 1, 2, 4, 8, or 'auto' bytes are supported as word sizes".to_string(), + "value originates from here".to_string(), + head, val.span, )); } } - + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, bits, head, signed, bytes_len), engine_state.ctrlc.clone(), @@ -134,13 +139,15 @@ fn operate(value: Value, bits: usize, head: Span, signed: bool, number_size: Num SignedEight => get_rotate_right(val as i64, bits, span), } } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only integer values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bits/shift_left.rs b/crates/nu-command/src/bits/shift_left.rs index b666af4d07..cb087296c3 100644 --- a/crates/nu-command/src/bits/shift_left.rs +++ b/crates/nu-command/src/bits/shift_left.rs @@ -60,11 +60,16 @@ impl Command for SubCommand { if let Some(val) = number_bytes { return Err(ShellError::UnsupportedInput( "Only 1, 2, 4, 8, or 'auto' bytes are supported as word sizes".to_string(), + "value originates from here".to_string(), + head, val.span, )); } } - + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, bits, head, signed, bytes_len), engine_state.ctrlc.clone(), @@ -156,13 +161,15 @@ fn operate(value: Value, bits: usize, head: Span, signed: bool, number_size: Num SignedEight => get_shift_left(val as i64, bits, span), } } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only integer values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bits/shift_right.rs b/crates/nu-command/src/bits/shift_right.rs index 63e4c19fb8..8a721894bb 100644 --- a/crates/nu-command/src/bits/shift_right.rs +++ b/crates/nu-command/src/bits/shift_right.rs @@ -60,11 +60,16 @@ impl Command for SubCommand { if let Some(val) = number_bytes { return Err(ShellError::UnsupportedInput( "Only 1, 2, 4, 8, or 'auto' bytes are supported as word sizes".to_string(), + "value originates from here".to_string(), + head, val.span, )); } } - + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, bits, head, signed, bytes_len), engine_state.ctrlc.clone(), @@ -146,13 +151,15 @@ fn operate(value: Value, bits: usize, head: Span, signed: bool, number_size: Num SignedEight => get_shift_right(val as i64, bits, span), } } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only integer values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bits/xor.rs b/crates/nu-command/src/bits/xor.rs index 2fa5dabaa1..3ce0d5a3cf 100644 --- a/crates/nu-command/src/bits/xor.rs +++ b/crates/nu-command/src/bits/xor.rs @@ -42,7 +42,10 @@ impl Command for SubCommand { ) -> Result { let head = call.head; let target: i64 = call.req(engine_state, stack, 0)?; - + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, target, head), engine_state.ctrlc.clone(), @@ -74,13 +77,15 @@ fn operate(value: Value, target: i64, head: Span) -> Value { val: val ^ target, span, }, + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only integer values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/add.rs b/crates/nu-command/src/bytes/add.rs index 5713249ebb..a7f321b4fa 100644 --- a/crates/nu-command/src/bytes/add.rs +++ b/crates/nu-command/src/bytes/add.rs @@ -122,13 +122,15 @@ fn add(val: &Value, args: &Arguments, span: Span) -> Value { val, span: val_span, } => add_impl(val, args, *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/at.rs b/crates/nu-command/src/bytes/at.rs index c90672d637..fccea9c0cd 100644 --- a/crates/nu-command/src/bytes/at.rs +++ b/crates/nu-command/src/bytes/at.rs @@ -30,7 +30,9 @@ fn parse_range(range: Value, head: Span) -> Result<(isize, isize, Span), ShellEr Value::List { mut vals, span } => { if vals.len() != 2 { return Err(ShellError::UnsupportedInput( - "More than two indices given".to_string(), + "More than two indices in range".to_string(), + "value originates from here".to_string(), + head, span, )); } else { @@ -38,10 +40,14 @@ fn parse_range(range: Value, head: Span) -> Result<(isize, isize, Span), ShellEr let end = match end { Value::Int { val, .. } => val.to_string(), Value::String { val, .. } => val, + // Explictly propagate errors instead of dropping them. + Value::Error { error } => return Err(error), other => { return Err(ShellError::UnsupportedInput( - "could not perform subbytes. Expecting a string or int".to_string(), - other.span().unwrap_or(head), + "Only string or list ranges are supported".into(), + format!("input type: {:?}", other.get_type()), + head, + other.expect_span(), )) } }; @@ -49,10 +55,14 @@ fn parse_range(range: Value, head: Span) -> Result<(isize, isize, Span), ShellEr let start = match start { Value::Int { val, .. } => val.to_string(), Value::String { val, .. } => val, + // Explictly propagate errors instead of dropping them. + Value::Error { error } => return Err(error), other => { return Err(ShellError::UnsupportedInput( - "could not perform subbytes. Expecting a string or int".to_string(), - other.span().unwrap_or(head), + "Only string or list ranges are supported".into(), + format!("input type: {:?}", other.get_type()), + head, + other.expect_span(), )) } }; @@ -66,15 +76,21 @@ fn parse_range(range: Value, head: Span) -> Result<(isize, isize, Span), ShellEr None => { return Err(ShellError::UnsupportedInput( "could not perform subbytes".to_string(), + "with this range".to_string(), + head, span, )) } } } + // Explictly propagate errors instead of dropping them. + Value::Error { error } => return Err(error), other => { return Err(ShellError::UnsupportedInput( "could not perform subbytes".to_string(), - other.span().unwrap_or(head), + "with this range".to_string(), + head, + other.expect_span(), )) } }; @@ -87,6 +103,8 @@ fn parse_range(range: Value, head: Span) -> Result<(isize, isize, Span), ShellEr Err(_) => { return Err(ShellError::UnsupportedInput( "could not perform subbytes".to_string(), + "with this range".to_string(), + head, span, )) } @@ -100,6 +118,8 @@ fn parse_range(range: Value, head: Span) -> Result<(isize, isize, Span), ShellEr Err(_) => { return Err(ShellError::UnsupportedInput( "could not perform subbytes".to_string(), + "with this range".to_string(), + head, span, )) } @@ -232,13 +252,15 @@ fn at(val: &Value, args: &Arguments, span: Span) -> Value { val, span: val_span, } => at_impl(val, args, *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } @@ -262,7 +284,7 @@ fn at_impl(input: &[u8], arg: &Arguments, span: Span) -> Value { match start.cmp(&end) { Ordering::Equal => Value::Binary { val: vec![], span }, Ordering::Greater => Value::Error { - error: ShellError::UnsupportedInput( + error: ShellError::TypeMismatch( "End must be greater than or equal to Start".to_string(), arg.arg_span, ), diff --git a/crates/nu-command/src/bytes/build_.rs b/crates/nu-command/src/bytes/build_.rs index e4ab0247f7..0ea3155845 100644 --- a/crates/nu-command/src/bytes/build_.rs +++ b/crates/nu-command/src/bytes/build_.rs @@ -52,10 +52,12 @@ impl Command for BytesBuild { let val = eval_expression(engine_state, stack, expr)?; match val { Value::Binary { mut val, .. } => output.append(&mut val), + // Explictly propagate errors instead of dropping them. + Value::Error { error } => return Err(error), other => { - return Err(ShellError::UnsupportedInput( - "only support expression which yields to binary data".to_string(), - other.span().unwrap_or(call.head), + return Err(ShellError::TypeMismatch( + "only binary data arguments are supported".to_string(), + other.expect_span(), )) } } diff --git a/crates/nu-command/src/bytes/collect.rs b/crates/nu-command/src/bytes/collect.rs index 92173fe4c7..495aed08a1 100644 --- a/crates/nu-command/src/bytes/collect.rs +++ b/crates/nu-command/src/bytes/collect.rs @@ -54,14 +54,16 @@ impl Command for BytesCollect { output_binary.append(&mut work_sep) } } + // Explictly propagate errors instead of dropping them. + Value::Error { error } => return Err(error), other => { - return Err(ShellError::UnsupportedInput( - format!( - "The element type is {}, this command only works with bytes.", - other.get_type() - ), - other.span().unwrap_or(call.head), - )) + return Err(ShellError::OnlySupportsThisInputType( + "integer".into(), + other.get_type().to_string(), + call.head, + // This line requires the Value::Error match above. + other.expect_span(), + )); } } } diff --git a/crates/nu-command/src/bytes/ends_with.rs b/crates/nu-command/src/bytes/ends_with.rs index b25d046c13..725c1ceb05 100644 --- a/crates/nu-command/src/bytes/ends_with.rs +++ b/crates/nu-command/src/bytes/ends_with.rs @@ -90,13 +90,15 @@ fn ends_with(val: &Value, args: &Arguments, span: Span) -> Value { val, span: val_span, } => Value::boolean(val.ends_with(&args.pattern), *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "binary".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/index_of.rs b/crates/nu-command/src/bytes/index_of.rs index 4b922cb445..d8609296f0 100644 --- a/crates/nu-command/src/bytes/index_of.rs +++ b/crates/nu-command/src/bytes/index_of.rs @@ -132,13 +132,15 @@ fn index_of(val: &Value, args: &Arguments, span: Span) -> Value { val, span: val_span, } => index_of_impl(val, args, *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "binary".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/length.rs b/crates/nu-command/src/bytes/length.rs index e8630ae613..1bca896f55 100644 --- a/crates/nu-command/src/bytes/length.rs +++ b/crates/nu-command/src/bytes/length.rs @@ -71,13 +71,15 @@ fn length(val: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { val, span: val_span, } => Value::int(val.len() as i64, *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "binary".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/remove.rs b/crates/nu-command/src/bytes/remove.rs index 335fb07778..380187cad9 100644 --- a/crates/nu-command/src/bytes/remove.rs +++ b/crates/nu-command/src/bytes/remove.rs @@ -61,7 +61,7 @@ impl Command for BytesRemove { let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths); let pattern_to_remove = call.req::>>(engine_state, stack, 0)?; if pattern_to_remove.item.is_empty() { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "the pattern to remove cannot be empty".to_string(), pattern_to_remove.span, )); @@ -139,13 +139,15 @@ fn remove(val: &Value, args: &Arguments, span: Span) -> Value { val, span: val_span, } => remove_impl(val, args, *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "binary".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/replace.rs b/crates/nu-command/src/bytes/replace.rs index 447345fe0c..12a72b170c 100644 --- a/crates/nu-command/src/bytes/replace.rs +++ b/crates/nu-command/src/bytes/replace.rs @@ -61,7 +61,7 @@ impl Command for BytesReplace { let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths); let find = call.req::>>(engine_state, stack, 0)?; if find.item.is_empty() { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "the pattern to find cannot be empty".to_string(), find.span, )); @@ -130,13 +130,15 @@ fn replace(val: &Value, args: &Arguments, span: Span) -> Value { val, span: val_span, } => replace_impl(val, args, *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "binary".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/reverse.rs b/crates/nu-command/src/bytes/reverse.rs index b51ef7225c..f905a12fb3 100644 --- a/crates/nu-command/src/bytes/reverse.rs +++ b/crates/nu-command/src/bytes/reverse.rs @@ -81,13 +81,15 @@ fn reverse(val: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { span: *val_span, } } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "binary".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/bytes/starts_with.rs b/crates/nu-command/src/bytes/starts_with.rs index 7478c90157..8360947545 100644 --- a/crates/nu-command/src/bytes/starts_with.rs +++ b/crates/nu-command/src/bytes/starts_with.rs @@ -96,13 +96,15 @@ fn starts_with(val: &Value, args: &Arguments, span: Span) -> Value { val, span: val_span, } => Value::boolean(val.starts_with(&args.pattern), *val_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => val.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with bytes.", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "binary".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/charting/hashable_value.rs b/crates/nu-command/src/charting/hashable_value.rs index e70cd32131..6fc7863fe8 100644 --- a/crates/nu-command/src/charting/hashable_value.rs +++ b/crates/nu-command/src/charting/hashable_value.rs @@ -78,13 +78,14 @@ impl HashableValue { Value::String { val, span } => Ok(HashableValue::String { val, span }), Value::Binary { val, span } => Ok(HashableValue::Binary { val, span }), - _ => { - let input_span = value.span().unwrap_or(span); - Err(ShellError::UnsupportedInput( - format!("input value {value:?} is not hashable"), - input_span, - )) - } + // Explictly propagate errors instead of dropping them. + Value::Error { error } => Err(error), + _ => Err(ShellError::UnsupportedInput( + "input value is not hashable".into(), + format!("input type: {:?}", value.get_type()), + span, + value.expect_span(), + )), } } diff --git a/crates/nu-command/src/charting/histogram.rs b/crates/nu-command/src/charting/histogram.rs index 6ba4bdaa21..08e77b39fa 100644 --- a/crates/nu-command/src/charting/histogram.rs +++ b/crates/nu-command/src/charting/histogram.rs @@ -98,12 +98,11 @@ impl Command for Histogram { let frequency_name_arg = call.opt::>(engine_state, stack, 1)?; let frequency_column_name = match frequency_name_arg { Some(inner) => { - let span = inner.span; if ["value", "count", "quantile", "percentage"].contains(&inner.item.as_str()) { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "frequency-column-name can't be 'value', 'count' or 'percentage'" .to_string(), - span, + inner.span, )); } inner.item @@ -119,7 +118,7 @@ impl Command for Histogram { "normalize" => PercentageCalcMethod::Normalize, "relative" => PercentageCalcMethod::Relative, _ => { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "calc method can only be 'normalize' or 'relative'".to_string(), inner.span, )) @@ -137,6 +136,8 @@ impl Command for Histogram { frequency_column_name, calc_method, span, + // Note that as_list() filters out Value::Error here. + data_as_value.expect_span(), ), Err(e) => Err(e), } @@ -149,6 +150,7 @@ fn run_histogram( freq_column: String, calc_method: PercentageCalcMethod, head_span: Span, + list_span: Span, ) -> Result { let mut inputs = vec![]; // convert from inputs to hashable values. @@ -157,14 +159,24 @@ fn run_histogram( // some invalid input scenario needs to handle: // Expect input is a list of hashable value, if one value is not hashable, throw out error. for v in values { - let current_span = v.span().unwrap_or(head_span); - inputs.push(HashableValue::from_value(v, head_span).map_err(|_| { - ShellError::UnsupportedInput( - "--column-name is not provided, can only support a list of simple value." - .to_string(), - current_span, - ) - })?); + match v { + // Propagate existing errors. + Value::Error { error } => return Err(error), + _ => { + let t = v.get_type(); + let span = v.expect_span(); + inputs.push(HashableValue::from_value(v, head_span).map_err(|_| { + ShellError::UnsupportedInput( + "Since --column-name was not provided, only lists of hashable values are supported.".to_string(), + format!( + "input type: {:?}", t + ), + head_span, + span, + ) + })?) + } + } } } Some(ref col) => { @@ -186,14 +198,17 @@ fn run_histogram( } } } + // Propagate existing errors. + Value::Error { error } => return Err(error), _ => continue, } } if inputs.is_empty() { - return Err(ShellError::UnsupportedInput( - format!("expect input is table, and inputs doesn't contain any value which has {col_name} column"), + return Err(ShellError::CantFindColumn( + col_name.clone(), head_span, + list_span, )); } } diff --git a/crates/nu-command/src/conversions/fmt.rs b/crates/nu-command/src/conversions/fmt.rs index 7ffd211005..c16ad08270 100644 --- a/crates/nu-command/src/conversions/fmt.rs +++ b/crates/nu-command/src/conversions/fmt.rs @@ -84,10 +84,15 @@ fn action(input: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { match input { Value::Int { val, .. } => fmt_it(*val, span), Value::Filesize { val, .. } => fmt_it(*val, span), - _ => Value::Error { - error: ShellError::UnsupportedInput( - format!("unsupported input type: {:?}", input.get_type()), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "integer or filesize".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/conversions/into/binary.rs b/crates/nu-command/src/conversions/into/binary.rs index 188d30e57a..fab732b7cb 100644 --- a/crates/nu-command/src/conversions/into/binary.rs +++ b/crates/nu-command/src/conversions/into/binary.rs @@ -177,13 +177,24 @@ pub fn action(input: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { val: int_to_endian(i64::from(*val)), span, }, + Value::Duration { val, .. } => Value::Binary { + val: int_to_endian(*val), + span, + }, Value::Date { val, .. } => Value::Binary { val: val.format("%c").to_string().as_bytes().to_vec(), span, }, - - _ => Value::Error { - error: ShellError::UnsupportedInput("'into binary' for unsupported type".into(), span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "integer, float, filesize, string, date, duration, binary or bool".into(), + other.get_type().to_string(), + span, + // This line requires the Value::Error match above. + other.expect_span(), + ), }, } } diff --git a/crates/nu-command/src/conversions/into/bool.rs b/crates/nu-command/src/conversions/into/bool.rs index abd82f9016..4811d50076 100644 --- a/crates/nu-command/src/conversions/into/bool.rs +++ b/crates/nu-command/src/conversions/into/bool.rs @@ -163,10 +163,15 @@ fn action(input: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { Ok(val) => Value::Bool { val, span }, Err(error) => Value::Error { error }, }, - _ => Value::Error { - error: ShellError::UnsupportedInput( - "'into bool' does not support this input".into(), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "bool, integer, float or string".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/conversions/into/datetime.rs b/crates/nu-command/src/conversions/into/datetime.rs index 3c0289a159..7f8f044369 100644 --- a/crates/nu-command/src/conversions/into/datetime.rs +++ b/crates/nu-command/src/conversions/into/datetime.rs @@ -147,41 +147,19 @@ impl Command for SubCommand { } fn examples(&self) -> Vec { - let example_result_1 = |secs: i64, nsecs: u32| { - let dt = match Utc.timestamp_opt(secs, nsecs) { - LocalResult::Single(dt) => Some(dt), - _ => None, - }; - match dt { - Some(dt) => Some(Value::Date { - val: dt.into(), - span: Span::test_data(), - }), - None => Some(Value::Error { - error: ShellError::UnsupportedInput( - "The given datetime representation is unsupported.".to_string(), - Span::test_data(), - ), - }), - } + let example_result_1 = |secs: i64, nsecs: u32| match Utc.timestamp_opt(secs, nsecs) { + LocalResult::Single(dt) => Some(Value::Date { + val: dt.into(), + span: Span::test_data(), + }), + _ => panic!("datetime: help example is invalid"), }; - let example_result_2 = |millis: i64| { - let dt = match Utc.timestamp_millis_opt(millis) { - LocalResult::Single(dt) => Some(dt), - _ => None, - }; - match dt { - Some(dt) => Some(Value::Date { - val: dt.into(), - span: Span::test_data(), - }), - None => Some(Value::Error { - error: ShellError::UnsupportedInput( - "The given datetime representation is unsupported.".to_string(), - Span::test_data(), - ), - }), - } + let example_result_2 = |millis: i64| match Utc.timestamp_millis_opt(millis) { + LocalResult::Single(dt) => Some(Value::Date { + val: dt.into(), + span: Span::test_data(), + }), + _ => panic!("datetime: help example is invalid"), }; vec![ Example { @@ -213,7 +191,7 @@ impl Command for SubCommand { }, Example { description: - "Convert timestamps like the sqlite history t", + "Convert a millisecond-precise timestamp", example: "1656165681720 | into datetime", result: example_result_2(1656165681720) }, @@ -231,11 +209,16 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { let timestamp = match input { Value::Int { val, .. } => Ok(*val), Value::String { val, .. } => val.parse::(), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => return input.clone(), other => { return Value::Error { - error: ShellError::UnsupportedInput( - format!("Expected string or int, got {} instead", other.get_type()), + error: ShellError::OnlySupportsThisInputType( + "string and integer".into(), + other.get_type().to_string(), head, + // This line requires the Value::Error match above. + other.expect_span(), ), }; } @@ -248,113 +231,68 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { if ts.abs() > TIMESTAMP_BOUND { return Value::Error { error: ShellError::UnsupportedInput( - "Given timestamp is out of range, it should between -8e+12 and 8e+12" - .to_string(), + "timestamp is out of range; it should between -8e+12 and 8e+12".to_string(), + format!("timestamp is {:?}", ts), head, + // Again, can safely unwrap this from here on + input.expect_span(), ), }; } + macro_rules! match_datetime { + ($expr:expr) => { + match $expr { + LocalResult::Single(dt) => Value::Date { + val: dt.into(), + span: head, + }, + _ => { + return Value::Error { + error: ShellError::UnsupportedInput( + "The given local datetime representation is invalid.".into(), + format!("timestamp is {:?}", ts), + head, + head, + ), + }; + } + } + }; + } + return match timezone { // default to UTC None => { // be able to convert chrono::Utc::now() - let dt = match ts.to_string().len() { - x if x > 13 => Utc.timestamp_nanos(ts).into(), - x if x > 10 => match Utc.timestamp_millis_opt(ts) { - LocalResult::Single(dt) => dt.into(), - _ => { - return Value::Error { - // This error message is from chrono - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid." - .to_string(), - head, - ), - }; - } + match ts.to_string().len() { + x if x > 13 => Value::Date { + val: Utc.timestamp_nanos(ts).into(), + span: head, }, - _ => match Utc.timestamp_opt(ts, 0) { - LocalResult::Single(dt) => dt.into(), - _ => { - return Value::Error { - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid." - .to_string(), - head, - ), - } - } - }, - }; - - Value::Date { - val: dt, - span: head, + x if x > 10 => match_datetime!(Utc.timestamp_millis_opt(ts)), + _ => match_datetime!(Utc.timestamp_opt(ts, 0)), } } Some(Spanned { item, span }) => match item { - Zone::Utc => match Utc.timestamp_opt(ts, 0) { - LocalResult::Single(val) => Value::Date { - val: val.into(), - span: head, - }, - _ => Value::Error { - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid.".to_string(), - *span, - ), - }, - }, - Zone::Local => match Local.timestamp_opt(ts, 0) { - LocalResult::Single(val) => Value::Date { - val: val.into(), - span: head, - }, - _ => Value::Error { - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid.".to_string(), - *span, - ), - }, - }, + Zone::Utc => match_datetime!(Utc.timestamp_opt(ts, 0)), + Zone::Local => match_datetime!(Local.timestamp_opt(ts, 0)), Zone::East(i) => match FixedOffset::east_opt((*i as i32) * HOUR) { - Some(eastoffset) => match eastoffset.timestamp_opt(ts, 0) { - LocalResult::Single(val) => Value::Date { val, span: head }, - _ => Value::Error { - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid.".to_string(), - *span, - ), - }, - }, + Some(eastoffset) => match_datetime!(eastoffset.timestamp_opt(ts, 0)), None => Value::Error { - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid.".to_string(), - *span, - ), + error: ShellError::DatetimeParseError(*span), }, }, Zone::West(i) => match FixedOffset::west_opt((*i as i32) * HOUR) { - Some(westoffset) => match westoffset.timestamp_opt(ts, 0) { - LocalResult::Single(val) => Value::Date { val, span: head }, - _ => Value::Error { - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid.".to_string(), - *span, - ), - }, - }, + Some(westoffset) => match_datetime!(westoffset.timestamp_opt(ts, 0)), None => Value::Error { - error: ShellError::UnsupportedInput( - "The given local datetime representation is invalid.".to_string(), - *span, - ), + error: ShellError::DatetimeParseError(*span), }, }, Zone::Error => Value::Error { - error: ShellError::UnsupportedInput( - "Cannot convert given timezone or offset to timestamp".to_string(), + // This is an argument error, not an input error + error: ShellError::TypeMismatch( + "Invalid timezone or offset".to_string(), *span, ), }, @@ -391,10 +329,15 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { }, } } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), other => Value::Error { - error: ShellError::UnsupportedInput( - format!("Expected string, got {} instead", other.get_type()), + error: ShellError::OnlySupportsThisInputType( + "string".into(), + other.get_type().to_string(), head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/conversions/into/decimal.rs b/crates/nu-command/src/conversions/into/decimal.rs index ee5c13e1fc..1df8c76e1f 100644 --- a/crates/nu-command/src/conversions/into/decimal.rs +++ b/crates/nu-command/src/conversions/into/decimal.rs @@ -105,18 +105,17 @@ fn action(input: &Value, _args: &CellPathOnlyArgs, head: Span) -> Value { }, span: *span, }, - other => { - let span = other.span(); - match span { - Ok(s) => { - let got = format!("Expected a string, got {} instead", other.get_type()); - Value::Error { - error: ShellError::UnsupportedInput(got, s), - } - } - Err(e) => Value::Error { error: e }, - } - } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string, integer or bool".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), + ), + }, } } diff --git a/crates/nu-command/src/conversions/into/duration.rs b/crates/nu-command/src/conversions/into/duration.rs index bd042b52fb..30ce744b38 100644 --- a/crates/nu-command/src/conversions/into/duration.rs +++ b/crates/nu-command/src/conversions/into/duration.rs @@ -468,9 +468,11 @@ fn action( } } else { Value::Error { - error: ShellError::UnsupportedInput( - "'into duration' does not support this string input".into(), + error: ShellError::CantConvert( + "string".into(), + "duration".into(), span, + None, ), } } @@ -481,10 +483,15 @@ fn action( } } } - _ => Value::Error { - error: ShellError::UnsupportedInput( - "'into duration' does not support this input".into(), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string or duration".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/conversions/into/filesize.rs b/crates/nu-command/src/conversions/into/filesize.rs index 70f5f4f22b..deb48d32b1 100644 --- a/crates/nu-command/src/conversions/into/filesize.rs +++ b/crates/nu-command/src/conversions/into/filesize.rs @@ -116,20 +116,18 @@ pub fn action(input: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { val: 0, span: value_span, }, - _ => Value::Error { - error: ShellError::UnsupportedInput( - "'into filesize' for unsupported type".into(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string and integer".into(), + other.get_type().to_string(), + span, value_span, ), }, } } else { - Value::Error { - error: ShellError::UnsupportedInput( - "'into filesize' for unsupported type".into(), - span, - ), - } + // Propagate existing errors + input.clone() } } fn int_from_string(a_string: &str, span: Span) -> Result { diff --git a/crates/nu-command/src/conversions/into/int.rs b/crates/nu-command/src/conversions/into/int.rs index 52559354a1..85159e8550 100644 --- a/crates/nu-command/src/conversions/into/int.rs +++ b/crates/nu-command/src/conversions/into/int.rs @@ -70,7 +70,7 @@ impl Command for SubCommand { let radix: u32 = match radix { Some(Value::Int { val, span }) => { if !(2..=36).contains(&val) { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "Radix must lie in the range [2, 36]".to_string(), span, )); @@ -187,9 +187,11 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value { Ok(v) => v, _ => { return Value::Error { - error: ShellError::UnsupportedInput( - "Could not convert float to integer".to_string(), + error: ShellError::CantConvert( + "float".to_string(), + "integer".to_string(), span, + None, ), } } @@ -219,6 +221,7 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value { val: val.timestamp(), span, }, + Value::Duration { val, .. } => Value::Int { val: *val, span }, Value::Binary { val, span } => { use byteorder::{BigEndian, ByteOrder, LittleEndian}; @@ -240,10 +243,15 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value { Value::int(BigEndian::read_i64(&val), *span) } } - _ => Value::Error { - error: ShellError::UnsupportedInput( - format!("'into int' for unsupported type '{}'", input.get_type()), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "integer, float, filesize, date, string, binary, duration or bool".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } @@ -281,13 +289,18 @@ fn convert_int(input: &Value, head: Span, radix: u32) -> Value { } val.to_string() } - _ => { + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => return input.clone(), + other => { return Value::Error { - error: ShellError::UnsupportedInput( - "only strings or integers are supported".to_string(), + error: ShellError::OnlySupportsThisInputType( + "string and integer".into(), + other.get_type().to_string(), head, + // This line requires the Value::Error match above. + other.expect_span(), ), - } + }; } }; match i64::from_str_radix(i.trim(), radix) { diff --git a/crates/nu-command/src/conversions/into/record.rs b/crates/nu-command/src/conversions/into/record.rs index a04c2d5e89..97e7bbd138 100644 --- a/crates/nu-command/src/conversions/into/record.rs +++ b/crates/nu-command/src/conversions/into/record.rs @@ -183,12 +183,16 @@ fn into_record( Value::Record { cols, vals, span } } Value::Record { cols, vals, span } => Value::Record { cols, vals, span }, - other => { - return Err(ShellError::UnsupportedInput( - "'into record' does not support this input".into(), - other.span().unwrap_or(call.head), - )) - } + Value::Error { .. } => input, + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + other.get_type().to_string(), + call.head, + // This line requires the Value::Error match above. + other.expect_span(), + ), + }, }; Ok(res.into_pipeline_data()) } diff --git a/crates/nu-command/src/conversions/into/string.rs b/crates/nu-command/src/conversions/into/string.rs index b27318b4cb..977af8d3df 100644 --- a/crates/nu-command/src/conversions/into/string.rs +++ b/crates/nu-command/src/conversions/into/string.rs @@ -154,7 +154,7 @@ fn string_helper( let decimals_value: Option = call.get_flag(engine_state, stack, "decimals")?; if let Some(decimal_val) = decimals_value { if decimals && decimal_val.is_negative() { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "Cannot accept negative integers for decimals arguments".to_string(), head, )); @@ -251,9 +251,11 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value { vals: _, span: _, } => Value::Error { - error: ShellError::UnsupportedInput( - "Cannot convert Record into string".to_string(), + error: ShellError::CantConvert( + "record".into(), + "string".into(), span, + Some("try using the `to nuon` command".into()), ), }, Value::Binary { .. } => Value::Error { diff --git a/crates/nu-command/src/core_commands/debug.rs b/crates/nu-command/src/core_commands/debug.rs index c07cc4af28..bd4f07b97f 100644 --- a/crates/nu-command/src/core_commands/debug.rs +++ b/crates/nu-command/src/core_commands/debug.rs @@ -39,6 +39,8 @@ impl Command for Debug { let config = engine_state.get_config().clone(); let raw = call.has_flag("raw"); + // Should PipelineData::Empty result in an error here? + input.map( move |x| { if raw { diff --git a/crates/nu-command/src/database/commands/into_sqlite.rs b/crates/nu-command/src/database/commands/into_sqlite.rs index 3c7dd1d99b..b9595e02f0 100644 --- a/crates/nu-command/src/database/commands/into_sqlite.rs +++ b/crates/nu-command/src/database/commands/into_sqlite.rs @@ -218,12 +218,14 @@ fn action( // and we're done Ok(Value::Nothing { span: *span }) } - _ => Err(ShellError::UnsupportedInput( - format!( - "Expected a list but instead received a {}", - input.get_type() - ), + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => Err(error.clone()), + other => Err(ShellError::OnlySupportsThisInputType( + "list".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), )), } } diff --git a/crates/nu-command/src/dataframe/eager/drop.rs b/crates/nu-command/src/dataframe/eager/drop.rs index 748fba1f8d..f53ec78610 100644 --- a/crates/nu-command/src/dataframe/eager/drop.rs +++ b/crates/nu-command/src/dataframe/eager/drop.rs @@ -70,7 +70,7 @@ fn command( .ok_or_else(|| { ShellError::GenericError( "Empty names list".into(), - "No column names where found".into(), + "No column names were found".into(), Some(col_span), None, Vec::new(), diff --git a/crates/nu-command/src/dataframe/values/nu_dataframe/conversion.rs b/crates/nu-command/src/dataframe/values/nu_dataframe/conversion.rs index 138dafd5bf..b9fd0b741b 100644 --- a/crates/nu-command/src/dataframe/values/nu_dataframe/conversion.rs +++ b/crates/nu-command/src/dataframe/values/nu_dataframe/conversion.rs @@ -467,7 +467,9 @@ pub fn create_column( error: ShellError::UnsupportedInput( "The given local datetime representation is invalid." .to_string(), + format!("timestamp is {:?}", a), span, + Span::unknown(), ), } } @@ -480,7 +482,9 @@ pub fn create_column( error: ShellError::UnsupportedInput( "The given local datetime representation is invalid." .to_string(), + format!("timestamp is {:?}", a), span, + Span::unknown(), ), } } @@ -528,7 +532,9 @@ pub fn create_column( error: ShellError::UnsupportedInput( "The given local datetime representation is invalid." .to_string(), + format!("timestamp is {:?}", a), span, + Span::unknown(), ), } } @@ -541,7 +547,9 @@ pub fn create_column( error: ShellError::UnsupportedInput( "The given local datetime representation is invalid." .to_string(), + format!("timestamp is {:?}", a), span, + Span::unknown(), ), } } diff --git a/crates/nu-command/src/date/format.rs b/crates/nu-command/src/date/format.rs index f88412f0e0..6d7c59a42f 100644 --- a/crates/nu-command/src/date/format.rs +++ b/crates/nu-command/src/date/format.rs @@ -61,13 +61,10 @@ impl Command for SubCommand { let format = call.opt::>(engine_state, stack, 0)?; - if input.is_nothing() { - return Err(ShellError::UnsupportedInput( - "Input was nothing. You must pipe an input to this command.".into(), - head, - )); + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); } - input.map( move |value| match &format { Some(format) => format_helper(value, format.item.as_str(), format.span, head), @@ -135,7 +132,7 @@ where span, }, Err(_) => Value::Error { - error: ShellError::UnsupportedInput("invalid format".to_string(), span), + error: ShellError::TypeMismatch("invalid format".to_string(), span), }, } } diff --git a/crates/nu-command/src/date/humanize.rs b/crates/nu-command/src/date/humanize.rs index 91b0cab7a8..6731dbbe6e 100644 --- a/crates/nu-command/src/date/humanize.rs +++ b/crates/nu-command/src/date/humanize.rs @@ -47,6 +47,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map(move |value| helper(value, head), engine_state.ctrlc.clone()) } diff --git a/crates/nu-command/src/date/to_record.rs b/crates/nu-command/src/date/to_record.rs index 4032675c57..57ab32b3e8 100644 --- a/crates/nu-command/src/date/to_record.rs +++ b/crates/nu-command/src/date/to_record.rs @@ -4,7 +4,8 @@ use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::Type; use nu_protocol::{ - Category, Example, PipelineData, ShellError::DatetimeParseError, Signature, Span, Value, + Category, Example, PipelineData, ShellError::DatetimeParseError, ShellError::PipelineEmpty, + Signature, Span, Value, }; #[derive(Clone)] @@ -41,6 +42,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(PipelineEmpty(head)); + } input.map(move |value| helper(value, head), engine_state.ctrlc.clone()) } diff --git a/crates/nu-command/src/date/to_table.rs b/crates/nu-command/src/date/to_table.rs index 911925222c..68f856bae3 100644 --- a/crates/nu-command/src/date/to_table.rs +++ b/crates/nu-command/src/date/to_table.rs @@ -4,7 +4,8 @@ use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::Type; use nu_protocol::{ - Category, Example, PipelineData, ShellError::DatetimeParseError, Signature, Span, Value, + Category, Example, PipelineData, ShellError::DatetimeParseError, ShellError::PipelineEmpty, + Signature, Span, Value, }; #[derive(Clone)] @@ -41,6 +42,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(PipelineEmpty(head)); + } input.map(move |value| helper(value, head), engine_state.ctrlc.clone()) } diff --git a/crates/nu-command/src/date/to_timezone.rs b/crates/nu-command/src/date/to_timezone.rs index a32f6b3803..68af189c9e 100644 --- a/crates/nu-command/src/date/to_timezone.rs +++ b/crates/nu-command/src/date/to_timezone.rs @@ -56,7 +56,10 @@ impl Command for SubCommand { let head = call.head; let timezone: Spanned = call.req(engine_state, stack, 0)?; - //Ok(PipelineData::new()) + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| helper(value, head, &timezone), engine_state.ctrlc.clone(), @@ -64,26 +67,15 @@ impl Command for SubCommand { } fn examples(&self) -> Vec { - let example_result_1 = || { - let dt = match FixedOffset::east_opt(5 * 3600) { - Some(dt) => match dt.with_ymd_and_hms(2020, 10, 10, 13, 00, 00) { - LocalResult::Single(dt) => Some(dt), - _ => None, - }, - _ => None, - }; - match dt { - Some(dt) => Some(Value::Date { - val: dt, - span: Span::test_data(), - }), - None => Some(Value::Error { - error: ShellError::UnsupportedInput( - "The given datetime representation is unsupported.".to_string(), - Span::test_data(), - ), - }), - } + let example_result_1 = || match FixedOffset::east_opt(5 * 3600) + .expect("to timezone: help example is invalid") + .with_ymd_and_hms(2020, 10, 10, 13, 00, 00) + { + LocalResult::Single(dt) => Some(Value::Date { + val: dt, + span: Span::test_data(), + }), + _ => panic!("to timezone: help example is invalid"), }; vec![ @@ -145,7 +137,7 @@ fn _to_timezone(dt: DateTime, timezone: &Spanned, span: Spa match datetime_in_timezone(&dt, timezone.item.as_str()) { Ok(dt) => Value::Date { val: dt, span }, Err(_) => Value::Error { - error: ShellError::UnsupportedInput(String::from("invalid time zone"), timezone.span), + error: ShellError::TypeMismatch(String::from("invalid time zone"), timezone.span), }, } } diff --git a/crates/nu-command/src/env/load_env.rs b/crates/nu-command/src/env/load_env.rs index cab96972fb..9f08396bbb 100644 --- a/crates/nu-command/src/env/load_env.rs +++ b/crates/nu-command/src/env/load_env.rs @@ -80,7 +80,9 @@ impl Command for LoadEnv { } _ => Err(ShellError::UnsupportedInput( "'load-env' expects a single record".into(), + "value originated from here".into(), span, + input.span().unwrap_or(span), )), }, } diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index c87a22966c..505ccaa1d7 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -2,8 +2,8 @@ use nu_engine::CallExt; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, PipelineData, RawStream, ShellError, Signature, Spanned, SyntaxShape, Type, - Value, + Category, Example, PipelineData, RawStream, ShellError, Signature, Span, Spanned, SyntaxShape, + Type, Value, }; use std::fs::File; use std::io::{BufWriter, Write}; @@ -188,9 +188,14 @@ impl Command for Save { Ok(PipelineData::empty()) } - v => Err(ShellError::UnsupportedInput( - format!("{:?} not supported", v.get_type()), + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => Err(error), + other => Err(ShellError::OnlySupportsThisInputType( + "string, binary or list".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), )), } } else { @@ -203,16 +208,16 @@ impl Command for Save { } => { // delegate a thread to redirect stderr to result. let handler = stderr.map(|stderr_stream| match stderr_file { - Some(stderr_file) => { - std::thread::spawn(move || stream_to_file(stderr_stream, stderr_file)) - } + Some(stderr_file) => std::thread::spawn(move || { + stream_to_file(stderr_stream, stderr_file, span) + }), None => std::thread::spawn(move || { let _ = stderr_stream.into_bytes(); Ok(PipelineData::empty()) }), }); - let res = stream_to_file(stream, file); + let res = stream_to_file(stream, file, span); if let Some(h) = handler { match h.join() { Err(err) => { @@ -264,9 +269,14 @@ impl Command for Save { Ok(PipelineData::empty()) } - v => Err(ShellError::UnsupportedInput( - format!("{:?} not supported", v.get_type()), + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => Err(error), + other => Err(ShellError::OnlySupportsThisInputType( + "string, binary or list".into(), + other.get_type().to_string(), span, + // This line requires the Value::Error match above. + other.expect_span(), )), }, } @@ -304,7 +314,11 @@ impl Command for Save { } } -fn stream_to_file(mut stream: RawStream, file: File) -> Result { +fn stream_to_file( + mut stream: RawStream, + file: File, + span: Span, +) -> Result { let mut writer = BufWriter::new(file); stream @@ -313,10 +327,15 @@ fn stream_to_file(mut stream: RawStream, file: File) -> Result match v { Value::String { val, .. } => val.into_bytes(), Value::Binary { val, .. } => val, - _ => { - return Err(ShellError::UnsupportedInput( - format!("{:?} not supported", v.get_type()), - v.span()?, + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => return Err(error), + other => { + return Err(ShellError::OnlySupportsThisInputType( + "string or binary".into(), + other.get_type().to_string(), + span, + // This line requires the Value::Error match above. + other.expect_span(), )); } }, diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index a107892290..0aca3a35f4 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -94,7 +94,7 @@ impl Command for Touch { Some(reference) => { let reference_path = Path::new(&reference.item); if !reference_path.exists() { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "path provided is invalid".to_string(), reference.span, )); diff --git a/crates/nu-command/src/filesystem/watch.rs b/crates/nu-command/src/filesystem/watch.rs index a30ebc4361..b1cbe97b1a 100644 --- a/crates/nu-command/src/filesystem/watch.rs +++ b/crates/nu-command/src/filesystem/watch.rs @@ -98,8 +98,8 @@ impl Command for Watch { Some(val) => match u64::try_from(val.item) { Ok(val) => Duration::from_millis(val), Err(_) => { - return Err(ShellError::UnsupportedInput( - "Input out of range".to_string(), + return Err(ShellError::TypeMismatch( + "Debounce duration is invalid".to_string(), val.span, )) } @@ -117,7 +117,12 @@ impl Command for Watch { match nu_glob::Pattern::new(&absolute_path.to_string_lossy()) { Ok(pattern) => Some(pattern), - Err(_) => return Err(ShellError::UnsupportedInput("".to_string(), glob.span)), + Err(_) => { + return Err(ShellError::TypeMismatch( + "Glob pattern is invalid".to_string(), + glob.span, + )) + } } } None => None, diff --git a/crates/nu-command/src/filters/drop/nth.rs b/crates/nu-command/src/filters/drop/nth.rs index 657eda93bd..919a44cc08 100644 --- a/crates/nu-command/src/filters/drop/nth.rs +++ b/crates/nu-command/src/filters/drop/nth.rs @@ -124,15 +124,15 @@ impl Command for DropNth { // check for negative range inputs, e.g., (2..-5) if from.is_negative() || to.is_negative() { let span: Spanned = call.req(engine_state, stack, 0)?; - return Err(ShellError::UnsupportedInput( - "Drop nth accepts only positive integers".to_string(), + return Err(ShellError::TypeMismatch( + "drop nth accepts only positive integers".to_string(), span.span, )); } // check if the upper bound is smaller than the lower bound, e.g., do not accept 4..2 if to < from { let span: Spanned = call.req(engine_state, stack, 0)?; - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "The upper bound needs to be equal or larger to the lower bound" .to_string(), span.span, diff --git a/crates/nu-command/src/filters/find.rs b/crates/nu-command/src/filters/find.rs index 48566c60b2..ae49a4891e 100644 --- a/crates/nu-command/src/filters/find.rs +++ b/crates/nu-command/src/filters/find.rs @@ -185,7 +185,7 @@ fn find_with_regex( let regex = flags.to_string() + regex.as_str(); let re = Regex::new(regex.as_str()) - .map_err(|e| ShellError::UnsupportedInput(format!("incorrect regex: {}", e), span))?; + .map_err(|e| ShellError::TypeMismatch(format!("invalid regex: {}", e), span))?; input.filter( move |value| match value { @@ -478,22 +478,20 @@ fn find_with_rest_and_highlight( } } } - _ => { + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => return Err(error), + other => { return Err(ShellError::UnsupportedInput( - format!( - "Unsupport value type '{}' from raw stream", - value.get_type() - ), + "unsupported type from raw stream".into(), + format!("input: {:?}", other.get_type()), span, - )) + // This line requires the Value::Error match above. + other.expect_span(), + )); } }, - _ => { - return Err(ShellError::UnsupportedInput( - "Unsupport type from raw stream".to_string(), - span, - )) - } + // Propagate any errors that were in the stream + Err(e) => return Err(e), }; } Ok(output.into_pipeline_data(ctrlc)) diff --git a/crates/nu-command/src/filters/first.rs b/crates/nu-command/src/filters/first.rs index 34587abfc1..cd52973a32 100644 --- a/crates/nu-command/src/filters/first.rs +++ b/crates/nu-command/src/filters/first.rs @@ -105,19 +105,6 @@ fn first_helper( let ctrlc = engine_state.ctrlc.clone(); let metadata = input.metadata(); - let input_span = input.span(); - let input_not_supported_error = || -> ShellError { - // can't always get a span for input, so try our best and fall back on the span for the `first` call if needed - if let Some(span) = input_span { - ShellError::UnsupportedInput("first does not support this input type".into(), span) - } else { - ShellError::UnsupportedInput( - "first was given an unsupported input type".into(), - call.span(), - ) - } - }; - match input { PipelineData::Value(val, _) => match val { Value::List { vals, .. } => { @@ -153,7 +140,15 @@ fn first_helper( .set_metadata(metadata)) } } - _ => Err(input_not_supported_error()), + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => Err(error), + other => Err(ShellError::OnlySupportsThisInputType( + "list, binary or range".into(), + other.get_type().to_string(), + head, + // This line requires the Value::Error match above. + other.expect_span(), + )), }, PipelineData::ListStream(mut ls, metadata) => { if return_single_element { @@ -169,7 +164,18 @@ fn first_helper( .set_metadata(metadata)) } } - _ => Err(input_not_supported_error()), + PipelineData::ExternalStream { span, .. } => Err(ShellError::OnlySupportsThisInputType( + "list, binary or range".into(), + "raw data".into(), + head, + span, + )), + PipelineData::Empty => Err(ShellError::OnlySupportsThisInputType( + "list, binary or range".into(), + "null".into(), + call.head, + call.head, // TODO: make PipelineData::Empty spanned, so that the span can be used here. + )), } } #[cfg(test)] diff --git a/crates/nu-command/src/filters/flatten.rs b/crates/nu-command/src/filters/flatten.rs index a4d8d9e84d..bc2e1e3927 100644 --- a/crates/nu-command/src/filters/flatten.rs +++ b/crates/nu-command/src/filters/flatten.rs @@ -168,13 +168,18 @@ fn flat_value(columns: &[CellPath], item: &Value, _name_tag: Span, all: bool) -> vals, span: _, } => (cols, vals), - x => { + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => return vec![item.clone()], + other => { return vec![Value::Error { - error: ShellError::UnsupportedInput( - format!("This should be a record, but instead got {}", x.get_type()), - tag, + error: ShellError::OnlySupportsThisInputType( + "record".into(), + other.get_type().to_string(), + _name_tag, + // This line requires the Value::Error match above. + other.expect_span(), ), - }] + }]; } }; @@ -221,13 +226,15 @@ fn flat_value(columns: &[CellPath], item: &Value, _name_tag: Span, all: bool) -> out.insert(column.to_string(), value.clone()); } } - Value::List { vals, span: _ } + Value::List { vals, span } if all && vals.iter().all(|f| f.as_record().is_ok()) => { if need_flatten && inner_table.is_some() { return vec![Value::Error{ error: ShellError::UnsupportedInput( - "can only flatten one inner list at the same time. tried flattening more than one column with inner lists... but is flattened already".to_string(), - s + "can only flatten one inner list at a time. tried flattening more than one column with inner lists... but is flattened already".to_string(), + "value originates from here".into(), + s, + *span )} ]; } @@ -259,14 +266,13 @@ fn flat_value(columns: &[CellPath], item: &Value, _name_tag: Span, all: bool) -> out.insert(column.to_string(), value.clone()); } } - Value::List { - vals: values, - span: _, - } => { + Value::List { vals: values, span } => { if need_flatten && inner_table.is_some() { return vec![Value::Error{ error: ShellError::UnsupportedInput( - "can only flatten one inner list at the same time. tried flattening more than one column with inner lists... but is flattened already".to_string(), - s + "can only flatten one inner list at a time. tried flattening more than one column with inner lists... but is flattened already".to_string(), + "value originates from here".into(), + s, + *span )} ]; } diff --git a/crates/nu-command/src/filters/group_by.rs b/crates/nu-command/src/filters/group_by.rs index 39b71ad7ab..61cb03ea8d 100644 --- a/crates/nu-command/src/filters/group_by.rs +++ b/crates/nu-command/src/filters/group_by.rs @@ -242,17 +242,19 @@ pub fn group( match grouper { Grouper::ByColumn(Some(column_name)) => { - let block = - Box::new( - move |_, row: &Value| match row.get_data_by_key(&column_name.item) { - Some(group_key) => Ok(group_key.as_string()?), - None => Err(ShellError::CantFindColumn( - column_name.item.to_string(), - column_name.span, - row.span().unwrap_or(column_name.span), - )), - }, - ); + let block = Box::new(move |_, row: &Value| { + if let Value::Error { error } = row { + return Err(error.clone()); + }; + match row.get_data_by_key(&column_name.item) { + Some(group_key) => Ok(group_key.as_string()?), + None => Err(ShellError::CantFindColumn( + column_name.item.to_string(), + column_name.span, + row.expect_span(), + )), + } + }); data_group(values, &Some(block), name) } diff --git a/crates/nu-command/src/filters/headers.rs b/crates/nu-command/src/filters/headers.rs index 8ac795bfb5..e1e8bef819 100644 --- a/crates/nu-command/src/filters/headers.rs +++ b/crates/nu-command/src/filters/headers.rs @@ -139,7 +139,7 @@ fn extract_headers(value: &Value, config: &Config) -> Result, ShellE for v in vals { if !is_valid_header(v) { return Err(ShellError::TypeMismatch( - "compatible type: Null, String, Bool, Float, Int".to_string(), + "needs compatible type: Null, String, Bool, Float, Int".to_string(), v.span()?, )); } diff --git a/crates/nu-command/src/filters/insert.rs b/crates/nu-command/src/filters/insert.rs index 985e6614bb..609e5bd4a5 100644 --- a/crates/nu-command/src/filters/insert.rs +++ b/crates/nu-command/src/filters/insert.rs @@ -161,9 +161,12 @@ fn insert( match output { Ok(pd) => { - if let Err(e) = - input.insert_data_at_cell_path(&cell_path.members, pd.into_value(span)) - { + let span = pd.span().unwrap_or(span); + if let Err(e) = input.insert_data_at_cell_path( + &cell_path.members, + pd.into_value(span), + span, + ) { return Value::Error { error: e }; } @@ -197,7 +200,9 @@ fn insert( move |mut input| { let replacement = replacement.clone(); - if let Err(e) = input.insert_data_at_cell_path(&cell_path.members, replacement) { + if let Err(e) = + input.insert_data_at_cell_path(&cell_path.members, replacement, span) + { return Value::Error { error: e }; } diff --git a/crates/nu-command/src/filters/lines.rs b/crates/nu-command/src/filters/lines.rs index 0cef42ba9d..6a944e3792 100644 --- a/crates/nu-command/src/filters/lines.rs +++ b/crates/nu-command/src/filters/lines.rs @@ -109,10 +109,18 @@ impl Command for Lines { Ok(iter.into_pipeline_data(engine_state.ctrlc.clone())) } - PipelineData::Value(val, ..) => Err(ShellError::UnsupportedInput( - format!("Not supported input: {}", val.as_string()?), - head, - )), + PipelineData::Value(val, ..) => { + match val { + // Propagate existing errors + Value::Error { error } => Err(error), + _ => Err(ShellError::OnlySupportsThisInputType( + "string or raw data".into(), + val.get_type().to_string(), + head, + val.expect_span(), + )), + } + } PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::empty()), PipelineData::ExternalStream { stdout: Some(stream), @@ -189,6 +197,7 @@ impl Iterator for RawStreamLinesAdapter { match result { Ok(v) => { match v { + // TODO: Value::Binary support required? Value::String { val, span } => { self.span = span; @@ -223,12 +232,16 @@ impl Iterator for RawStreamLinesAdapter { // save completed lines self.queue.append(&mut lines); } - // TODO: Value::Binary support required? - _ => { - return Some(Err(ShellError::UnsupportedInput( - "Unsupport type from raw stream".to_string(), + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => return Some(Err(error)), + other => { + return Some(Err(ShellError::OnlySupportsThisInputType( + "string".into(), + other.get_type().to_string(), self.span, - ))) + // This line requires the Value::Error match above. + other.expect_span(), + ))); } } } diff --git a/crates/nu-command/src/filters/rename.rs b/crates/nu-command/src/filters/rename.rs index 4c90f6ccea..4962b5fb61 100644 --- a/crates/nu-command/src/filters/rename.rs +++ b/crates/nu-command/src/filters/rename.rs @@ -114,7 +114,7 @@ fn rename( if let Some(ref cols) = specified_column { if cols.len() != 2 { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "The list must contain only two values: the column's name and its replacement value" .to_string(), list_span, @@ -140,8 +140,15 @@ fn rename( if !cols.contains(&c[0]) { return Value::Error { error: ShellError::UnsupportedInput( - "The specified column does not exist".to_string(), - specified_col_span.unwrap_or(span), + format!( + "The column '{}' does not exist in the input", + &c[0] + ), + "value originated from here".into(), + // Arrow 1 points at the specified column name, + specified_col_span.unwrap_or(head_span), + // Arrow 2 points at the input value. + span, ), }; } @@ -165,11 +172,15 @@ fn rename( Value::Record { cols, vals, span } } - x => Value::Error { - error: ShellError::UnsupportedInput( - "can't rename: input is not table, so no column names available for rename" - .to_string(), - x.span().unwrap_or(head_span), + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => item.clone(), + other => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "record".into(), + other.get_type().to_string(), + head_span, + // This line requires the Value::Error match above. + other.expect_span(), ), }, }, diff --git a/crates/nu-command/src/filters/rotate.rs b/crates/nu-command/src/filters/rotate.rs index 08fda1ff33..375a89a72e 100644 --- a/crates/nu-command/src/filters/rotate.rs +++ b/crates/nu-command/src/filters/rotate.rs @@ -208,6 +208,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 mut values = input.into_iter().collect::>(); let mut old_column_names = vec![]; let mut new_values = vec![]; @@ -222,17 +223,13 @@ pub fn rotate( if !values.is_empty() { for val in values.into_iter() { match val { - Value::Record { - cols, - vals, - span: _, - } => { + Value::Record { cols, vals, .. } => { old_column_names = cols; for v in vals { new_values.push(v) } } - Value::List { vals, span: _ } => { + Value::List { vals, .. } => { not_a_record = true; for v in vals { new_values.push(v); @@ -250,8 +247,11 @@ pub fn rotate( } } else { return Err(ShellError::UnsupportedInput( - "Rotate command requires a Nu value as input".to_string(), + "list input is empty".to_string(), + "value originates from here".into(), call.head, + // TODO: Maybe make all Pipelines have spans, so that this doesn't need to be unwrapped. + span.unwrap_or(call.head), )); } diff --git a/crates/nu-command/src/filters/skip/skip_.rs b/crates/nu-command/src/filters/skip/skip_.rs index 1586f285e7..deafd3afe3 100644 --- a/crates/nu-command/src/filters/skip/skip_.rs +++ b/crates/nu-command/src/filters/skip/skip_.rs @@ -79,7 +79,7 @@ impl Command for Skip { let n: usize = match n { Some(Value::Int { val, span }) => val.try_into().map_err(|err| { - ShellError::UnsupportedInput( + ShellError::TypeMismatch( format!("Could not convert {} to unsigned integer: {}", val, err), span, ) diff --git a/crates/nu-command/src/filters/take/take_.rs b/crates/nu-command/src/filters/take/take_.rs index 256cba84a0..5fd4130116 100644 --- a/crates/nu-command/src/filters/take/take_.rs +++ b/crates/nu-command/src/filters/take/take_.rs @@ -53,19 +53,6 @@ impl Command for Take { let ctrlc = engine_state.ctrlc.clone(); let metadata = input.metadata(); - let input_span = input.span(); - let input_not_supported_error = || -> ShellError { - // can't always get a span for input, so try our best and fall back on the span for the `take` call if needed - if let Some(span) = input_span { - ShellError::UnsupportedInput("take does not support this input type".into(), span) - } else { - ShellError::UnsupportedInput( - "take was given an unsupported input type".into(), - call.span(), - ) - } - }; - match input { PipelineData::Value(val, _) => match val { Value::List { vals, .. } => Ok(vals @@ -85,13 +72,34 @@ impl Command for Take { .take(rows_desired) .into_pipeline_data(ctrlc) .set_metadata(metadata)), - _ => Err(input_not_supported_error()), + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => Err(error), + other => Err(ShellError::OnlySupportsThisInputType( + "list, binary or range".into(), + other.get_type().to_string(), + call.head, + // This line requires the Value::Error match above. + other.expect_span(), + )), }, PipelineData::ListStream(ls, metadata) => Ok(ls .take(rows_desired) .into_pipeline_data(ctrlc) .set_metadata(metadata)), - _ => Err(input_not_supported_error()), + PipelineData::ExternalStream { span, .. } => { + Err(ShellError::OnlySupportsThisInputType( + "list, binary or range".into(), + "raw data".into(), + call.head, + span, + )) + } + PipelineData::Empty => Err(ShellError::OnlySupportsThisInputType( + "list, binary or range".into(), + "null".into(), + call.head, + call.head, // TODO: make PipelineData::Empty spanned, so that the span can be used here. + )), } } diff --git a/crates/nu-command/src/formats/from/delimited.rs b/crates/nu-command/src/formats/from/delimited.rs index 3cacd8fc57..e047eff0a3 100644 --- a/crates/nu-command/src/formats/from/delimited.rs +++ b/crates/nu-command/src/formats/from/delimited.rs @@ -64,7 +64,7 @@ pub fn from_delimited_data( input: PipelineData, name: Span, ) -> Result { - let (concat_string, metadata) = input.collect_string_strict(name)?; + let (concat_string, _span, metadata) = input.collect_string_strict(name)?; Ok( from_delimited_string_to_value(concat_string, noheaders, no_infer, sep, trim, name) @@ -80,7 +80,7 @@ pub fn trim_from_str(trim: Option) -> Result { "headers" => Ok(Trim::Headers), "fields" => Ok(Trim::Fields), "none" => Ok(Trim::None), - _ => Err(ShellError::UnsupportedInput( + _ => Err(ShellError::TypeMismatch( "the only possible values for trim are 'all', 'headers', 'fields' and 'none'" .into(), span, diff --git a/crates/nu-command/src/formats/from/eml.rs b/crates/nu-command/src/formats/from/eml.rs index d5170925c7..0759f65482 100644 --- a/crates/nu-command/src/formats/from/eml.rs +++ b/crates/nu-command/src/formats/from/eml.rs @@ -178,7 +178,7 @@ fn from_eml( preview_body: Option>, head: Span, ) -> Result { - let (value, metadata) = input.collect_string_strict(head)?; + let (value, _span, metadata, ..) = input.collect_string_strict(head)?; let body_preview = preview_body .map(|b| b.item as usize) diff --git a/crates/nu-command/src/formats/from/ics.rs b/crates/nu-command/src/formats/from/ics.rs index 6d364d6b6f..b170c34287 100644 --- a/crates/nu-command/src/formats/from/ics.rs +++ b/crates/nu-command/src/formats/from/ics.rs @@ -94,7 +94,7 @@ END:VCALENDAR' | from ics", } fn from_ics(input: PipelineData, head: Span) -> Result { - let (input_string, metadata) = input.collect_string_strict(head)?; + let (input_string, span, metadata) = input.collect_string_strict(head)?; let input_string = input_string .lines() @@ -114,7 +114,9 @@ fn from_ics(input: PipelineData, head: Span) -> Result Err(e) => output.push(Value::Error { error: ShellError::UnsupportedInput( format!("input cannot be parsed as .ics ({})", e), + "value originates from here".into(), head, + span, ), }), } diff --git a/crates/nu-command/src/formats/from/ini.rs b/crates/nu-command/src/formats/from/ini.rs index 87f776067a..b0f369a667 100644 --- a/crates/nu-command/src/formats/from/ini.rs +++ b/crates/nu-command/src/formats/from/ini.rs @@ -56,7 +56,11 @@ b=2' | from ini", } } -pub fn from_ini_string_to_value(s: String, span: Span) -> Result { +pub fn from_ini_string_to_value( + s: String, + span: Span, + val_span: Span, +) -> Result { let v: Result>, serde_ini::de::Error> = serde_ini::from_str(&s); match v { @@ -77,15 +81,17 @@ pub fn from_ini_string_to_value(s: String, span: Span) -> Result Err(ShellError::UnsupportedInput( format!("Could not load ini: {}", err), + "value originates from here".into(), span, + val_span, )), } } fn from_ini(input: PipelineData, head: Span) -> Result { - let (concat_string, metadata) = input.collect_string_strict(head)?; + let (concat_string, span, metadata) = input.collect_string_strict(head)?; - match from_ini_string_to_value(concat_string, head) { + match from_ini_string_to_value(concat_string, head, span) { Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)), Err(other) => Err(other), } diff --git a/crates/nu-command/src/formats/from/json.rs b/crates/nu-command/src/formats/from/json.rs index 7f15be1151..15756ed9bc 100644 --- a/crates/nu-command/src/formats/from/json.rs +++ b/crates/nu-command/src/formats/from/json.rs @@ -64,7 +64,7 @@ impl Command for FromJson { input: PipelineData, ) -> Result { let span = call.head; - let (string_input, metadata) = input.collect_string_strict(span)?; + let (string_input, span, metadata) = input.collect_string_strict(span)?; if string_input.is_empty() { return Ok(PipelineData::new_with_metadata(metadata, span)); diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 68b2c016a9..039a0a8a9b 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -62,7 +62,7 @@ impl Command for FromNuon { input: PipelineData, ) -> Result { let head = call.head; - let (string_input, metadata) = input.collect_string_strict(head)?; + let (string_input, _span, metadata) = input.collect_string_strict(head)?; let engine_state = engine_state.clone(); diff --git a/crates/nu-command/src/formats/from/ods.rs b/crates/nu-command/src/formats/from/ods.rs index f06cf7e59c..28aa7466e3 100644 --- a/crates/nu-command/src/formats/from/ods.rs +++ b/crates/nu-command/src/formats/from/ods.rs @@ -93,10 +93,13 @@ fn collect_binary(input: PipelineData, span: Span) -> Result, ShellError Some(Value::Binary { val: b, .. }) => { bytes.extend_from_slice(&b); } + Some(Value::Error { error }) => return Err(error), Some(x) => { return Err(ShellError::UnsupportedInput( "Expected binary from pipeline".to_string(), - x.span().unwrap_or(span), + "value originates from here".into(), + span, + x.expect_span(), )) } None => break, @@ -111,10 +114,17 @@ fn from_ods( head: Span, sel_sheets: Vec, ) -> Result { + let span = input.span(); let bytes = collect_binary(input, head)?; let buf: Cursor> = Cursor::new(bytes); - let mut ods = Ods::<_>::new(buf) - .map_err(|_| ShellError::UnsupportedInput("Could not load ods file".to_string(), head))?; + let mut ods = Ods::<_>::new(buf).map_err(|_| { + ShellError::UnsupportedInput( + "Could not load ODS file".to_string(), + "value originates from here".into(), + head, + span.unwrap_or(head), + ) + })?; let mut dict = IndexMap::new(); @@ -170,7 +180,9 @@ fn from_ods( } else { return Err(ShellError::UnsupportedInput( "Could not load sheet".to_string(), + "value originates from here".into(), head, + span.unwrap_or(head), )); } } diff --git a/crates/nu-command/src/formats/from/ssv.rs b/crates/nu-command/src/formats/from/ssv.rs index 06812259fb..df93fd65f3 100644 --- a/crates/nu-command/src/formats/from/ssv.rs +++ b/crates/nu-command/src/formats/from/ssv.rs @@ -275,7 +275,7 @@ fn from_ssv( let minimum_spaces: Option> = call.get_flag(engine_state, stack, "minimum-spaces")?; - let (concat_string, metadata) = input.collect_string_strict(name)?; + let (concat_string, _span, metadata) = input.collect_string_strict(name)?; let split_at = match minimum_spaces { Some(number) => number.item, None => DEFAULT_MINIMUM_SPACES, diff --git a/crates/nu-command/src/formats/from/toml.rs b/crates/nu-command/src/formats/from/toml.rs index c86afee183..5ce3cc7c69 100644 --- a/crates/nu-command/src/formats/from/toml.rs +++ b/crates/nu-command/src/formats/from/toml.rs @@ -63,7 +63,7 @@ b = [1, 2]' | from toml", input: PipelineData, ) -> Result { let span = call.head; - let (mut string_input, metadata) = input.collect_string_strict(span)?; + let (mut string_input, span, metadata) = input.collect_string_strict(span)?; string_input.push('\n'); Ok(convert_string_to_value(string_input, span)?.into_pipeline_data_with_metadata(metadata)) } diff --git a/crates/nu-command/src/formats/from/url.rs b/crates/nu-command/src/formats/from/url.rs index 3bb1ea5d64..e20294a9d6 100644 --- a/crates/nu-command/src/formats/from/url.rs +++ b/crates/nu-command/src/formats/from/url.rs @@ -55,7 +55,7 @@ impl Command for FromUrl { } fn from_url(input: PipelineData, head: Span) -> Result { - let (concat_string, metadata) = input.collect_string_strict(head)?; + let (concat_string, span, metadata) = input.collect_string_strict(head)?; let result = serde_urlencoded::from_str::>(&concat_string); @@ -78,8 +78,10 @@ fn from_url(input: PipelineData, head: Span) -> Result )) } _ => Err(ShellError::UnsupportedInput( - "String not compatible with url-encoding".to_string(), + "String not compatible with URL encoding".to_string(), + "value originates from here".into(), head, + span, )), } } diff --git a/crates/nu-command/src/formats/from/vcf.rs b/crates/nu-command/src/formats/from/vcf.rs index 4e90e9d5e1..fd441fc541 100644 --- a/crates/nu-command/src/formats/from/vcf.rs +++ b/crates/nu-command/src/formats/from/vcf.rs @@ -107,7 +107,7 @@ END:VCARD' | from vcf", } fn from_vcf(input: PipelineData, head: Span) -> Result { - let (input_string, metadata) = input.collect_string_strict(head)?; + let (input_string, span, metadata) = input.collect_string_strict(head)?; let input_string = input_string .lines() @@ -124,7 +124,9 @@ fn from_vcf(input: PipelineData, head: Span) -> Result Err(e) => Value::Error { error: ShellError::UnsupportedInput( format!("input cannot be parsed as .vcf ({})", e), + "value originates from here".into(), head, + span, ), }, }); diff --git a/crates/nu-command/src/formats/from/xlsx.rs b/crates/nu-command/src/formats/from/xlsx.rs index a91ab3240e..41bd355b89 100644 --- a/crates/nu-command/src/formats/from/xlsx.rs +++ b/crates/nu-command/src/formats/from/xlsx.rs @@ -96,7 +96,9 @@ fn collect_binary(input: PipelineData, span: Span) -> Result, ShellError Some(x) => { return Err(ShellError::UnsupportedInput( "Expected binary from pipeline".to_string(), - x.span().unwrap_or(span), + "value originates from here".into(), + span, + x.expect_span(), )) } None => break, @@ -111,10 +113,17 @@ fn from_xlsx( head: Span, sel_sheets: Vec, ) -> Result { + let span = input.span(); let bytes = collect_binary(input, head)?; let buf: Cursor> = Cursor::new(bytes); - let mut xlsx = Xlsx::<_>::new(buf) - .map_err(|_| ShellError::UnsupportedInput("Could not load xlsx file".to_string(), head))?; + let mut xlsx = Xlsx::<_>::new(buf).map_err(|_| { + ShellError::UnsupportedInput( + "Could not load XLSX file".to_string(), + "value originates from here".into(), + head, + span.unwrap_or(head), + ) + })?; let mut dict = IndexMap::new(); @@ -170,7 +179,9 @@ fn from_xlsx( } else { return Err(ShellError::UnsupportedInput( "Could not load sheet".to_string(), + "value originates from here".into(), head, + span.unwrap_or(head), )); } } diff --git a/crates/nu-command/src/formats/from/xml.rs b/crates/nu-command/src/formats/from/xml.rs index 75cf110125..6ed356b203 100644 --- a/crates/nu-command/src/formats/from/xml.rs +++ b/crates/nu-command/src/formats/from/xml.rs @@ -178,13 +178,15 @@ pub fn from_xml_string_to_value(s: String, span: Span) -> Result Result { - let (concat_string, metadata) = input.collect_string_strict(head)?; + let (concat_string, span, metadata) = input.collect_string_strict(head)?; match from_xml_string_to_value(concat_string, head) { Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)), _ => Err(ShellError::UnsupportedInput( - "Could not parse string as xml".to_string(), + "Could not parse string as XML".to_string(), + "value originates from here".into(), head, + span, )), } } diff --git a/crates/nu-command/src/formats/from/yaml.rs b/crates/nu-command/src/formats/from/yaml.rs index 64fa4f8e4e..e0624e54cb 100644 --- a/crates/nu-command/src/formats/from/yaml.rs +++ b/crates/nu-command/src/formats/from/yaml.rs @@ -76,9 +76,17 @@ impl Command for FromYml { } } -fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value, span: Span) -> Result { - let err_not_compatible_number = - ShellError::UnsupportedInput("Expected a compatible number".to_string(), span); +fn convert_yaml_value_to_nu_value( + v: &serde_yaml::Value, + span: Span, + val_span: Span, +) -> Result { + let err_not_compatible_number = ShellError::UnsupportedInput( + "Expected a nu-compatible number in YAML input".to_string(), + "value originates from here".into(), + span, + val_span, + ); Ok(match v { serde_yaml::Value::Bool(b) => Value::Bool { val: *b, span }, serde_yaml::Value::Number(n) if n.is_i64() => Value::Int { @@ -96,7 +104,7 @@ fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value, span: Span) -> Result { let result: Result, ShellError> = a .iter() - .map(|x| convert_yaml_value_to_nu_value(x, span)) + .map(|x| convert_yaml_value_to_nu_value(x, span, val_span)) .collect(); Value::List { vals: result?, @@ -113,13 +121,16 @@ fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value, span: Span) -> Result { - collected - .item - .insert(k.clone(), convert_yaml_value_to_nu_value(v, span)?); + collected.item.insert( + k.clone(), + convert_yaml_value_to_nu_value(v, span, val_span)?, + ); } // Hard-code fix for cases where "v" is a string without quotations with double curly braces // e.g. k = value @@ -152,18 +163,27 @@ fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value, span: Span) -> Result Value::nothing(span), - x => unimplemented!("Unsupported yaml case: {:?}", x), + x => unimplemented!("Unsupported YAML case: {:?}", x), }) } -pub fn from_yaml_string_to_value(s: String, span: Span) -> Result { +pub fn from_yaml_string_to_value( + s: String, + span: Span, + val_span: Span, +) -> Result { let mut documents = vec![]; for document in serde_yaml::Deserializer::from_str(&s) { let v: serde_yaml::Value = serde_yaml::Value::deserialize(document).map_err(|x| { - ShellError::UnsupportedInput(format!("Could not load yaml: {}", x), span) + ShellError::UnsupportedInput( + format!("Could not load YAML: {}", x), + "value originates from here".into(), + span, + val_span, + ) })?; - documents.push(convert_yaml_value_to_nu_value(&v, span)?); + documents.push(convert_yaml_value_to_nu_value(&v, span, val_span)?); } match documents.len() { @@ -213,9 +233,9 @@ pub fn get_examples() -> Vec { } fn from_yaml(input: PipelineData, head: Span) -> Result { - let (concat_string, metadata) = input.collect_string_strict(head)?; + let (concat_string, span, metadata) = input.collect_string_strict(head)?; - match from_yaml_string_to_value(concat_string, head) { + match from_yaml_string_to_value(concat_string, head, span) { Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)), Err(other) => Err(other), } @@ -255,7 +275,11 @@ mod test { ]; let config = Config::default(); for tc in tt { - let actual = from_yaml_string_to_value(tc.input.to_owned(), Span::test_data()); + let actual = from_yaml_string_to_value( + tc.input.to_owned(), + Span::test_data(), + Span::test_data(), + ); if actual.is_err() { assert!( tc.expected.is_err(), diff --git a/crates/nu-command/src/formats/to/csv.rs b/crates/nu-command/src/formats/to/csv.rs index 97498420ab..171778131f 100644 --- a/crates/nu-command/src/formats/to/csv.rs +++ b/crates/nu-command/src/formats/to/csv.rs @@ -80,7 +80,7 @@ fn to_csv( } else { let vec_s: Vec = s.chars().collect(); if vec_s.len() != 1 { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "Expected a single separator char from --separator".to_string(), span, )); diff --git a/crates/nu-command/src/formats/to/delimited.rs b/crates/nu-command/src/formats/to/delimited.rs index f749c55579..117925e4e5 100644 --- a/crates/nu-command/src/formats/to/delimited.rs +++ b/crates/nu-command/src/formats/to/delimited.rs @@ -20,17 +20,17 @@ fn from_value_to_delimited_string( for (k, v) in cols.iter().zip(vals.iter()) { fields.push_back(k.clone()); - values.push_back(to_string_tagged_value(v, config, *span)?); + values.push_back(to_string_tagged_value(v, config, head, *span)?); } wtr.write_record(fields).expect("can not write."); wtr.write_record(values).expect("can not write."); let v = String::from_utf8(wtr.into_inner().map_err(|_| { - ShellError::UnsupportedInput("Could not convert record".to_string(), *span) + ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None) })?) .map_err(|_| { - ShellError::UnsupportedInput("Could not convert record".to_string(), *span) + ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None) })?; Ok(v) } @@ -45,7 +45,7 @@ fn from_value_to_delimited_string( wtr.write_record( vals.iter() .map(|ele| { - to_string_tagged_value(ele, config, *span) + to_string_tagged_value(ele, config, head, *span) .unwrap_or_else(|_| String::new()) }) .collect::>(), @@ -59,7 +59,7 @@ fn from_value_to_delimited_string( let mut row = vec![]; for desc in &merged_descriptors { row.push(match l.to_owned().get_data_by_key(desc) { - Some(s) => to_string_tagged_value(&s, config, *span)?, + Some(s) => to_string_tagged_value(&s, config, head, *span)?, None => String::new(), }); } @@ -67,18 +67,25 @@ fn from_value_to_delimited_string( } } let v = String::from_utf8(wtr.into_inner().map_err(|_| { - ShellError::UnsupportedInput("Could not convert record".to_string(), *span) + ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None) })?) .map_err(|_| { - ShellError::UnsupportedInput("Could not convert record".to_string(), *span) + ShellError::CantConvert("record".to_string(), "string".to_string(), *span, None) })?; Ok(v) } - _ => to_string_tagged_value(value, config, head), + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => Err(error.clone()), + other => to_string_tagged_value(value, config, other.expect_span(), head), } } -fn to_string_tagged_value(v: &Value, config: &Config, span: Span) -> Result { +fn to_string_tagged_value( + v: &Value, + config: &Config, + span: Span, + head: Span, +) -> Result { match &v { Value::String { .. } | Value::Bool { .. } @@ -86,7 +93,6 @@ fn to_string_tagged_value(v: &Value, config: &Config, span: Span) -> Result Result Ok(v.clone().into_abbreviated_string(config)), Value::Date { val, .. } => Ok(val.to_string()), Value::Nothing { .. } => Ok(String::new()), + // Propagate existing errors + Value::Error { error } => Err(error.clone()), _ => Err(ShellError::UnsupportedInput( - "Unexpected value".to_string(), - v.span().unwrap_or(span), + "Unexpected type".to_string(), + format!("input type: {:?}", v.get_type()), + head, + span, )), } } diff --git a/crates/nu-command/src/formats/to/nuon.rs b/crates/nu-command/src/formats/to/nuon.rs index db7294482c..ed061ad36a 100644 --- a/crates/nu-command/src/formats/to/nuon.rs +++ b/crates/nu-command/src/formats/to/nuon.rs @@ -58,7 +58,9 @@ fn value_to_string(v: &Value, span: Span) -> Result { if write!(s, "{:02X}", byte).is_err() { return Err(ShellError::UnsupportedInput( "could not convert binary to string".into(), + "value originates from here".into(), span, + v.expect_span(), )); } } @@ -66,11 +68,15 @@ fn value_to_string(v: &Value, span: Span) -> Result { } Value::Block { .. } => Err(ShellError::UnsupportedInput( "blocks are currently not nuon-compatible".into(), + "value originates from here".into(), span, + v.expect_span(), )), Value::Closure { .. } => Err(ShellError::UnsupportedInput( - "closure not supported".into(), + "closures are currently not nuon-compatible".into(), + "value originates from here".into(), span, + v.expect_span(), )), Value::Bool { val, .. } => { if *val { @@ -81,19 +87,21 @@ fn value_to_string(v: &Value, span: Span) -> Result { } Value::CellPath { .. } => Err(ShellError::UnsupportedInput( "cellpaths are currently not nuon-compatible".to_string(), + "value originates from here".into(), span, + v.expect_span(), )), Value::CustomValue { .. } => Err(ShellError::UnsupportedInput( - "customs are currently not nuon-compatible".to_string(), + "custom values are currently not nuon-compatible".to_string(), + "value originates from here".into(), span, + v.expect_span(), )), Value::Date { val, .. } => Ok(val.to_rfc3339()), - // FIXME: make duratiobs use the shortest lossless representation. + // FIXME: make durations use the shortest lossless representation. Value::Duration { val, .. } => Ok(format!("{}ns", *val)), - Value::Error { .. } => Err(ShellError::UnsupportedInput( - "errors are currently not nuon-compatible".to_string(), - span, - )), + // Propagate existing errors + Value::Error { error } => Err(error.clone()), // FIXME: make filesizes use the shortest lossless representation. Value::Filesize { val, .. } => Ok(format!("{}b", *val)), Value::Float { val, .. } => { diff --git a/crates/nu-command/src/formats/to/toml.rs b/crates/nu-command/src/formats/to/toml.rs index 0ba25709e8..2e643c5a54 100644 --- a/crates/nu-command/src/formats/to/toml.rs +++ b/crates/nu-command/src/formats/to/toml.rs @@ -130,7 +130,9 @@ fn value_to_toml_value( Value::List { ref vals, span } => match &vals[..] { [Value::Record { .. }, _end @ ..] => helper(engine_state, v), _ => Err(ShellError::UnsupportedInput( - "Expected a table with TOML-compatible structure from pipeline".to_string(), + "Expected a table with TOML-compatible structure".to_string(), + "value originates from here".into(), + head, *span, )), }, @@ -138,14 +140,20 @@ fn value_to_toml_value( // Attempt to de-serialize the String toml::de::from_str(val).map_err(|_| { ShellError::UnsupportedInput( - format!("{:?} unable to de-serialize string to TOML", val), + "unable to de-serialize string to TOML".into(), + format!("input: '{:?}'", val), + head, *span, ) }) } + // Propagate existing errors + Value::Error { error } => Err(error.clone()), _ => Err(ShellError::UnsupportedInput( - format!("{:?} is not a valid top-level TOML", v.get_type()), - v.span().unwrap_or(head), + format!("{:?} is not valid top-level TOML", v.get_type()), + "value originates from here".into(), + head, + v.expect_span(), )), } } diff --git a/crates/nu-command/src/formats/to/url.rs b/crates/nu-command/src/formats/to/url.rs index 046ec6ae41..5da929bf86 100644 --- a/crates/nu-command/src/formats/to/url.rs +++ b/crates/nu-command/src/formats/to/url.rs @@ -47,7 +47,9 @@ fn to_url(input: PipelineData, head: Span) -> Result { .into_iter() .map(move |value| match value { Value::Record { - ref cols, ref vals, .. + ref cols, + ref vals, + span, } => { let mut row_vec = vec![]; for (k, v) in cols.iter().zip(vals.iter()) { @@ -57,8 +59,10 @@ fn to_url(input: PipelineData, head: Span) -> Result { } _ => { return Err(ShellError::UnsupportedInput( - "Expected table with string values".to_string(), + "Expected a record with string values".to_string(), + "value originates from here".into(), head, + span, )); } } @@ -74,9 +78,13 @@ fn to_url(input: PipelineData, head: Span) -> Result { )), } } + // Propagate existing errors + Value::Error { error } => Err(error), other => Err(ShellError::UnsupportedInput( "Expected a table from pipeline".to_string(), - other.span().unwrap_or(head), + "value originates from here".into(), + head, + other.expect_span(), )), }) .collect(); diff --git a/crates/nu-command/src/generators/cal.rs b/crates/nu-command/src/generators/cal.rs index 1e0a9312aa..792fa4e806 100644 --- a/crates/nu-command/src/generators/cal.rs +++ b/crates/nu-command/src/generators/cal.rs @@ -92,6 +92,7 @@ pub fn cal( engine_state: &EngineState, stack: &mut Stack, call: &Call, + // TODO: Error if a value is piped in _input: PipelineData, ) -> Result { let mut calendar_vec_deque = VecDeque::new(); @@ -141,7 +142,7 @@ pub fn cal( } fn get_invalid_year_shell_error(head: Span) -> ShellError { - ShellError::UnsupportedInput("The year is invalid".to_string(), head) + ShellError::TypeMismatch("The year is invalid".to_string(), head) } struct MonthHelper { @@ -274,7 +275,7 @@ fn add_month_to_table( if days_of_the_week.contains(&s.as_str()) { week_start_day = s.to_string(); } else { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "The specified week start day is invalid".to_string(), day.span, )); diff --git a/crates/nu-command/src/hash/generic_digest.rs b/crates/nu-command/src/hash/generic_digest.rs index 212ee33fac..d7bad3a6f0 100644 --- a/crates/nu-command/src/hash/generic_digest.rs +++ b/crates/nu-command/src/hash/generic_digest.rs @@ -103,6 +103,8 @@ where let (bytes, span) = match input { Value::String { val, span } => (val.as_bytes(), *span), Value::Binary { val, span } => (val.as_slice(), *span), + // Propagate existing errors + Value::Error { .. } => return input.clone(), other => { let span = match input.span() { Ok(span) => span, @@ -110,13 +112,11 @@ where }; return Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Type `{}` is not supported for {} hashing input", - other.get_type(), - D::name() - ), + error: ShellError::OnlySupportsThisInputType( + "string or binary".into(), + other.get_type().to_string(), span, + other.expect_span(), ), }; } diff --git a/crates/nu-command/src/input_handler.rs b/crates/nu-command/src/input_handler.rs index 342c00bf60..256f44c6eb 100644 --- a/crates/nu-command/src/input_handler.rs +++ b/crates/nu-command/src/input_handler.rs @@ -49,7 +49,16 @@ where C: Fn(&Value, &A, Span) -> Value + Send + Sync + 'static + Clone + Copy, { match arg.take_cell_paths() { - None => input.map(move |v| cmd(&v, &arg, v.span().unwrap_or(span)), ctrlc), + None => input.map( + move |v| { + match v { + // Propagate errors inside the input + Value::Error { .. } => v, + _ => cmd(&v, &arg, span), + } + }, + ctrlc, + ), Some(column_paths) => { let arg = Arc::new(arg); input.map( @@ -58,7 +67,13 @@ where let opt = arg.clone(); let r = v.update_cell_path( &path.members, - Box::new(move |old| cmd(old, &opt, old.span().unwrap_or(span))), + Box::new(move |old| { + match old { + // Propagate errors inside the input + Value::Error { .. } => old.clone(), + _ => cmd(old, &opt, span), + } + }), ); if let Err(error) = r { return Value::Error { error }; diff --git a/crates/nu-command/src/math/abs.rs b/crates/nu-command/src/math/abs.rs index 2882475272..caad5cca4a 100644 --- a/crates/nu-command/src/math/abs.rs +++ b/crates/nu-command/src/math/abs.rs @@ -66,13 +66,13 @@ fn abs_helper(val: Value, head: Span) -> Value { val: val.abs(), span, }, + Value::Error { .. } => val, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/arccos.rs b/crates/nu-command/src/math/arccos.rs index f9eb2c89d3..c158c199c6 100644 --- a/crates/nu-command/src/math/arccos.rs +++ b/crates/nu-command/src/math/arccos.rs @@ -35,6 +35,10 @@ impl Command for SubCommand { ) -> Result { let head = call.head; let use_degrees = call.has_flag("degrees"); + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, use_degrees), engine_state.ctrlc.clone(), @@ -75,18 +79,20 @@ fn operate(value: Value, head: Span, use_degrees: bool) -> Value { Value::Error { error: ShellError::UnsupportedInput( "'arccos' undefined for values outside the closed interval [-1, 1].".into(), + "value originates from here".into(), + head, span, ), } } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/arccosh.rs b/crates/nu-command/src/math/arccosh.rs index 17ade19563..9bfb2b2d1d 100644 --- a/crates/nu-command/src/math/arccosh.rs +++ b/crates/nu-command/src/math/arccosh.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -65,18 +69,20 @@ fn operate(value: Value, head: Span) -> Value { Value::Error { error: ShellError::UnsupportedInput( "'arccosh' undefined for values below 1.".into(), + "value originates from here".into(), + head, span, ), } } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/arcsin.rs b/crates/nu-command/src/math/arcsin.rs index 8d373bf38d..ed1c002289 100644 --- a/crates/nu-command/src/math/arcsin.rs +++ b/crates/nu-command/src/math/arcsin.rs @@ -35,6 +35,10 @@ impl Command for SubCommand { ) -> Result { let head = call.head; let use_degrees = call.has_flag("degrees"); + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, use_degrees), engine_state.ctrlc.clone(), @@ -76,18 +80,20 @@ fn operate(value: Value, head: Span, use_degrees: bool) -> Value { Value::Error { error: ShellError::UnsupportedInput( "'arcsin' undefined for values outside the closed interval [-1, 1].".into(), + "value originates from here".into(), + head, span, ), } } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/arcsinh.rs b/crates/nu-command/src/math/arcsinh.rs index edde4cad4b..743cee882e 100644 --- a/crates/nu-command/src/math/arcsinh.rs +++ b/crates/nu-command/src/math/arcsinh.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -61,13 +65,13 @@ fn operate(value: Value, head: Span) -> Value { Value::Float { val, span } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/arctan.rs b/crates/nu-command/src/math/arctan.rs index c4fcaf1b0c..92aabc008f 100644 --- a/crates/nu-command/src/math/arctan.rs +++ b/crates/nu-command/src/math/arctan.rs @@ -35,6 +35,10 @@ impl Command for SubCommand { ) -> Result { let head = call.head; let use_degrees = call.has_flag("degrees"); + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, use_degrees), engine_state.ctrlc.clone(), @@ -72,13 +76,13 @@ fn operate(value: Value, head: Span, use_degrees: bool) -> Value { Value::Float { val, span } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/arctanh.rs b/crates/nu-command/src/math/arctanh.rs index 8ee966941d..d234ef5bd2 100644 --- a/crates/nu-command/src/math/arctanh.rs +++ b/crates/nu-command/src/math/arctanh.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -65,18 +69,20 @@ fn operate(value: Value, head: Span) -> Value { Value::Error { error: ShellError::UnsupportedInput( "'arctanh' undefined for values outside the open interval (-1, 1).".into(), + "value originates from here".into(), + head, span, ), } } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/avg.rs b/crates/nu-command/src/math/avg.rs index b3c282c6c7..cc7d3f66cb 100644 --- a/crates/nu-command/src/math/avg.rs +++ b/crates/nu-command/src/math/avg.rs @@ -45,9 +45,9 @@ impl Command for SubCommand { } } -pub fn average(values: &[Value], head: &Span) -> Result { +pub fn average(values: &[Value], span: Span, head: &Span) -> Result { let sum = reducer_for(Reduce::Summation); - let total = &sum(Value::int(0, *head), values.to_vec(), *head)?; + let total = &sum(Value::int(0, *head), values.to_vec(), span, *head)?; match total { Value::Filesize { val, span } => Ok(Value::Filesize { val: val / values.len() as i64, diff --git a/crates/nu-command/src/math/ceil.rs b/crates/nu-command/src/math/ceil.rs index df13901ff0..37b920cf4d 100644 --- a/crates/nu-command/src/math/ceil.rs +++ b/crates/nu-command/src/math/ceil.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -58,13 +62,13 @@ fn operate(value: Value, head: Span) -> Value { val: val.ceil(), span, }, + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/cos.rs b/crates/nu-command/src/math/cos.rs index d986165f79..c64482a04e 100644 --- a/crates/nu-command/src/math/cos.rs +++ b/crates/nu-command/src/math/cos.rs @@ -35,6 +35,10 @@ impl Command for SubCommand { ) -> Result { let head = call.head; let use_degrees = call.has_flag("degrees"); + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, use_degrees), engine_state.ctrlc.clone(), @@ -82,13 +86,13 @@ fn operate(value: Value, head: Span, use_degrees: bool) -> Value { span, } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/cosh.rs b/crates/nu-command/src/math/cosh.rs index 4e0f28c0ba..e8a65471b1 100644 --- a/crates/nu-command/src/math/cosh.rs +++ b/crates/nu-command/src/math/cosh.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -63,13 +67,13 @@ fn operate(value: Value, head: Span) -> Value { span, } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/eval.rs b/crates/nu-command/src/math/eval.rs index f8401598dd..3be0bea9ec 100644 --- a/crates/nu-command/src/math/eval.rs +++ b/crates/nu-command/src/math/eval.rs @@ -63,6 +63,8 @@ pub fn eval( Ok(value) => Ok(PipelineData::Value(value, None)), Err(err) => Err(ShellError::UnsupportedInput( format!("Math evaluation error: {}", err), + "value originates from here".into(), + head, expr.span, )), } @@ -71,25 +73,19 @@ pub fn eval( return Ok(input); } input.map( - move |val| { - if let Ok(string) = val.as_string() { - match parse(&string, &val.span().unwrap_or(head)) { - Ok(value) => value, - Err(err) => Value::Error { - error: ShellError::UnsupportedInput( - format!("Math evaluation error: {}", err), - val.span().unwrap_or(head), - ), - }, - } - } else { - Value::Error { + move |val| match val.as_string() { + Ok(string) => match parse(&string, &val.span().unwrap_or(head)) { + Ok(value) => value, + Err(err) => Value::Error { error: ShellError::UnsupportedInput( - "Expected a string from pipeline".to_string(), - val.span().unwrap_or(head), + format!("Math evaluation error: {}", err), + "value originates from here".into(), + head, + val.expect_span(), ), - } - } + }, + }, + Err(error) => Value::Error { error }, }, engine_state.ctrlc.clone(), ) diff --git a/crates/nu-command/src/math/floor.rs b/crates/nu-command/src/math/floor.rs index c718bb482f..0354d8b7c2 100644 --- a/crates/nu-command/src/math/floor.rs +++ b/crates/nu-command/src/math/floor.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -58,13 +62,13 @@ fn operate(value: Value, head: Span) -> Value { val: val.floor(), span, }, + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/ln.rs b/crates/nu-command/src/math/ln.rs index 95c33adccb..de8d8cc11f 100644 --- a/crates/nu-command/src/math/ln.rs +++ b/crates/nu-command/src/math/ln.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -65,18 +69,20 @@ fn operate(value: Value, head: Span) -> Value { Value::Error { error: ShellError::UnsupportedInput( "'ln' undefined for values outside the open interval (0, Inf).".into(), + "value originates from here".into(), + head, span, ), } } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/log.rs b/crates/nu-command/src/math/log.rs index 8733ca14de..03db7a3f6e 100644 --- a/crates/nu-command/src/math/log.rs +++ b/crates/nu-command/src/math/log.rs @@ -46,10 +46,15 @@ impl Command for SubCommand { if base.item <= 0.0f64 { return Err(ShellError::UnsupportedInput( "Base has to be greater 0".into(), + "value originates from here".into(), + head, base.span, )); } - + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } let base = base.item; input.map( move |value| operate(value, head, base), @@ -94,6 +99,8 @@ fn operate(value: Value, head: Span, base: f64) -> Value { error: ShellError::UnsupportedInput( "'math log' undefined for values outside the open interval (0, Inf)." .into(), + "value originates from here".into(), + head, span, ), }; @@ -109,13 +116,13 @@ fn operate(value: Value, head: Span, base: f64) -> Value { Value::Float { val, span } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/max.rs b/crates/nu-command/src/math/max.rs index be71eb6de3..290e13be4a 100644 --- a/crates/nu-command/src/math/max.rs +++ b/crates/nu-command/src/math/max.rs @@ -59,9 +59,9 @@ impl Command for SubCommand { } } -pub fn maximum(values: &[Value], head: &Span) -> Result { +pub fn maximum(values: &[Value], span: Span, head: &Span) -> Result { let max_func = reducer_for(Reduce::Maximum); - max_func(Value::nothing(*head), values.to_vec(), *head) + max_func(Value::nothing(*head), values.to_vec(), span, *head) } #[cfg(test)] diff --git a/crates/nu-command/src/math/median.rs b/crates/nu-command/src/math/median.rs index 1f37b35f54..03c3eda5b0 100644 --- a/crates/nu-command/src/math/median.rs +++ b/crates/nu-command/src/math/median.rs @@ -66,7 +66,7 @@ enum Pick { Median, } -pub fn median(values: &[Value], head: &Span) -> Result { +pub fn median(values: &[Value], span: Span, head: &Span) -> Result { let take = if values.len() % 2 == 0 { Pick::MedianAverage } else { @@ -103,9 +103,14 @@ pub fn median(values: &[Value], head: &Span) -> Result { match take { Pick::Median => { let idx = (values.len() as f64 / 2.0).floor() as usize; - let out = sorted - .get(idx) - .ok_or_else(|| ShellError::UnsupportedInput("Empty input".to_string(), *head))?; + let out = sorted.get(idx).ok_or_else(|| { + ShellError::UnsupportedInput( + "Empty input".to_string(), + "value originates from here".into(), + *head, + span, + ) + })?; Ok(out.clone()) } Pick::MedianAverage => { @@ -114,15 +119,29 @@ pub fn median(values: &[Value], head: &Span) -> Result { let left = sorted .get(idx_start) - .ok_or_else(|| ShellError::UnsupportedInput("Empty input".to_string(), *head))? + .ok_or_else(|| { + ShellError::UnsupportedInput( + "Empty input".to_string(), + "value originates from here".into(), + *head, + span, + ) + })? .clone(); let right = sorted .get(idx_end) - .ok_or_else(|| ShellError::UnsupportedInput("Empty input".to_string(), *head))? + .ok_or_else(|| { + ShellError::UnsupportedInput( + "Empty input".to_string(), + "value originates from here".into(), + *head, + span, + ) + })? .clone(); - average(&[left, right], head) + average(&[left, right], span, head) } } } diff --git a/crates/nu-command/src/math/min.rs b/crates/nu-command/src/math/min.rs index d2f62a42e3..2d0403d844 100644 --- a/crates/nu-command/src/math/min.rs +++ b/crates/nu-command/src/math/min.rs @@ -59,9 +59,9 @@ impl Command for SubCommand { } } -pub fn minimum(values: &[Value], head: &Span) -> Result { +pub fn minimum(values: &[Value], span: Span, head: &Span) -> Result { let min_func = reducer_for(Reduce::Minimum); - min_func(Value::nothing(*head), values.to_vec(), *head) + min_func(Value::nothing(*head), values.to_vec(), span, *head) } #[cfg(test)] diff --git a/crates/nu-command/src/math/mode.rs b/crates/nu-command/src/math/mode.rs index 7670836a5c..2fa0ad662e 100644 --- a/crates/nu-command/src/math/mode.rs +++ b/crates/nu-command/src/math/mode.rs @@ -97,7 +97,7 @@ impl Command for SubCommand { } } -pub fn mode(values: &[Value], head: &Span) -> Result { +pub fn mode(values: &[Value], _span: Span, head: &Span) -> Result { if let Some(Err(values)) = values .windows(2) .map(|elem| { @@ -132,9 +132,12 @@ pub fn mode(values: &[Value], head: &Span) -> Result { Value::Filesize { val, .. } => { Ok(HashableType::new(val.to_ne_bytes(), NumberTypes::Filesize)) } + Value::Error { error } => Err(error.clone()), other => Err(ShellError::UnsupportedInput( "Unable to give a result with this input".to_string(), - other.span()?, + "value originates from here".into(), + *head, + other.expect_span(), )), }) .collect::, ShellError>>()?; diff --git a/crates/nu-command/src/math/product.rs b/crates/nu-command/src/math/product.rs index 17071c2c1f..b21c7be283 100644 --- a/crates/nu-command/src/math/product.rs +++ b/crates/nu-command/src/math/product.rs @@ -46,9 +46,9 @@ impl Command for SubCommand { } /// Calculate product of given values -pub fn product(values: &[Value], head: &Span) -> Result { +pub fn product(values: &[Value], span: Span, head: &Span) -> Result { let product_func = reducer_for(Reduce::Product); - product_func(Value::nothing(*head), values.to_vec(), *head) + product_func(Value::nothing(*head), values.to_vec(), span, *head) } #[cfg(test)] diff --git a/crates/nu-command/src/math/reducers.rs b/crates/nu-command/src/math/reducers.rs index 42ca84706b..4a285ac43e 100644 --- a/crates/nu-command/src/math/reducers.rs +++ b/crates/nu-command/src/math/reducers.rs @@ -9,21 +9,28 @@ pub enum Reduce { } pub type ReducerFunction = - Box, Span) -> Result + Send + Sync + 'static>; + Box, Span, Span) -> Result + Send + Sync + 'static>; pub fn reducer_for(command: Reduce) -> ReducerFunction { match command { - Reduce::Summation => Box::new(|_, values, head| sum(values, head)), - Reduce::Product => Box::new(|_, values, head| product(values, head)), - Reduce::Minimum => Box::new(|_, values, head| min(values, head)), - Reduce::Maximum => Box::new(|_, values, head| max(values, head)), + Reduce::Summation => Box::new(|_, values, span, head| sum(values, span, head)), + Reduce::Product => Box::new(|_, values, span, head| product(values, span, head)), + Reduce::Minimum => Box::new(|_, values, span, head| min(values, span, head)), + Reduce::Maximum => Box::new(|_, values, span, head| max(values, span, head)), } } -pub fn max(data: Vec, head: Span) -> Result { +pub fn max(data: Vec, span: Span, head: Span) -> Result { let mut biggest = data .first() - .ok_or_else(|| ShellError::UnsupportedInput("Empty input".to_string(), head))? + .ok_or_else(|| { + ShellError::UnsupportedInput( + "Empty input".to_string(), + "value originates from here".into(), + head, + span, + ) + })? .clone(); for value in &data { @@ -44,10 +51,17 @@ pub fn max(data: Vec, head: Span) -> Result { Ok(biggest) } -pub fn min(data: Vec, head: Span) -> Result { +pub fn min(data: Vec, span: Span, head: Span) -> Result { let mut smallest = data .first() - .ok_or_else(|| ShellError::UnsupportedInput("Empty input".to_string(), head))? + .ok_or_else(|| { + ShellError::UnsupportedInput( + "Empty input".to_string(), + "value originates from here".into(), + head, + span, + ) + })? .clone(); for value in &data { @@ -68,7 +82,7 @@ pub fn min(data: Vec, head: Span) -> Result { Ok(smallest) } -pub fn sum(data: Vec, head: Span) -> Result { +pub fn sum(data: Vec, span: Span, head: Span) -> Result { let initial_value = data.get(0); let mut acc = match initial_value { @@ -83,7 +97,9 @@ pub fn sum(data: Vec, head: Span) -> Result { Some(Value::Int { span, .. }) | Some(Value::Float { span, .. }) => Ok(Value::int(0, *span)), None => Err(ShellError::UnsupportedInput( "Empty input".to_string(), + "value originates from here".into(), head, + span, )), _ => Ok(Value::nothing(head)), }?; @@ -96,10 +112,13 @@ pub fn sum(data: Vec, head: Span) -> Result { | Value::Duration { .. } => { acc = acc.add(head, value, head)?; } + Value::Error { error } => return Err(error.clone()), other => { return Err(ShellError::UnsupportedInput( "Attempted to compute the sum of a value that cannot be summed".to_string(), - other.span().unwrap_or(head), + "value originates from here".into(), + head, + other.expect_span(), )); } } @@ -107,14 +126,16 @@ pub fn sum(data: Vec, head: Span) -> Result { Ok(acc) } -pub fn product(data: Vec, head: Span) -> Result { +pub fn product(data: Vec, span: Span, head: Span) -> Result { let initial_value = data.get(0); let mut acc = match initial_value { Some(Value::Int { span, .. }) | Some(Value::Float { span, .. }) => Ok(Value::int(1, *span)), None => Err(ShellError::UnsupportedInput( "Empty input".to_string(), + "value originates from here".into(), head, + span, )), _ => Ok(Value::nothing(head)), }?; @@ -124,11 +145,14 @@ pub fn product(data: Vec, head: Span) -> Result { Value::Int { .. } | Value::Float { .. } => { acc = acc.mul(head, value, head)?; } + Value::Error { error } => return Err(error.clone()), other => { return Err(ShellError::UnsupportedInput( "Attempted to compute the product of a value that cannot be multiplied" .to_string(), - other.span().unwrap_or(head), + "value originates from here".into(), + head, + other.expect_span(), )); } } diff --git a/crates/nu-command/src/math/round.rs b/crates/nu-command/src/math/round.rs index 562fff5af5..14495c2b61 100644 --- a/crates/nu-command/src/math/round.rs +++ b/crates/nu-command/src/math/round.rs @@ -43,6 +43,10 @@ impl Command for SubCommand { ) -> Result { let precision_param: Option = call.get_flag(engine_state, stack, "precision")?; let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, precision_param), engine_state.ctrlc.clone(), @@ -89,13 +93,13 @@ fn operate(value: Value, head: Span, precision: Option) -> Value { }, }, Value::Int { .. } => value, + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/sin.rs b/crates/nu-command/src/math/sin.rs index 1947a690d1..dcf91897fe 100644 --- a/crates/nu-command/src/math/sin.rs +++ b/crates/nu-command/src/math/sin.rs @@ -35,6 +35,10 @@ impl Command for SubCommand { ) -> Result { let head = call.head; let use_degrees = call.has_flag("degrees"); + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, use_degrees), engine_state.ctrlc.clone(), @@ -82,13 +86,13 @@ fn operate(value: Value, head: Span, use_degrees: bool) -> Value { span, } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/sinh.rs b/crates/nu-command/src/math/sinh.rs index 225abaf747..c92a7d6cee 100644 --- a/crates/nu-command/src/math/sinh.rs +++ b/crates/nu-command/src/math/sinh.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -63,13 +67,13 @@ fn operate(value: Value, head: Span) -> Value { span, } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/sqrt.rs b/crates/nu-command/src/math/sqrt.rs index d04754fc16..6ed450afa2 100644 --- a/crates/nu-command/src/math/sqrt.rs +++ b/crates/nu-command/src/math/sqrt.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -56,33 +60,35 @@ fn operate(value: Value, head: Span) -> Value { Value::Int { val, span } => { let squared = (val as f64).sqrt(); if squared.is_nan() { - return error_negative_sqrt(span); + return error_negative_sqrt(head, span); } Value::Float { val: squared, span } } Value::Float { val, span } => { let squared = val.sqrt(); if squared.is_nan() { - return error_negative_sqrt(span); + return error_negative_sqrt(head, span); } Value::Float { val: squared, span } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } } -fn error_negative_sqrt(span: Span) -> Value { +fn error_negative_sqrt(head: Span, span: Span) -> Value { Value::Error { error: ShellError::UnsupportedInput( String::from("Can't square root a negative number"), + "value originates from here".into(), + head, span, ), } diff --git a/crates/nu-command/src/math/stddev.rs b/crates/nu-command/src/math/stddev.rs index c37ad11778..27a643f790 100644 --- a/crates/nu-command/src/math/stddev.rs +++ b/crates/nu-command/src/math/stddev.rs @@ -65,17 +65,21 @@ impl Command for SubCommand { } } -pub fn compute_stddev(sample: bool) -> impl Fn(&[Value], &Span) -> Result { - move |values: &[Value], span: &Span| { - let variance = variance(sample)(values, span); +pub fn compute_stddev(sample: bool) -> impl Fn(&[Value], Span, &Span) -> Result { + move |values: &[Value], span: Span, head: &Span| { + let variance = variance(sample)(values, span, head); match variance { - Ok(Value::Float { val, span }) => Ok(Value::Float { val: val.sqrt(), span }), - Ok(Value::Int { val, span }) => Ok(Value::Float { val: (val as f64).sqrt(), span }), - Err(ShellError::UnsupportedInput(_, err_span)) => Err(ShellError::UnsupportedInput( - "Attempted to compute the standard deviation with an item that cannot be used for that.".to_string(), - err_span, - )), - other => other + Ok(Value::Float { val, span }) => Ok(Value::Float { + val: val.sqrt(), + span, + }), + Ok(Value::Int { val, span }) => Ok(Value::Float { + val: (val as f64).sqrt(), + span, + }), + // variance() produces its own usable error, which can simply be propagated. + Err(e) => Err(e), + other => other, } } } diff --git a/crates/nu-command/src/math/sum.rs b/crates/nu-command/src/math/sum.rs index 44138ab3ac..13fedc15b0 100644 --- a/crates/nu-command/src/math/sum.rs +++ b/crates/nu-command/src/math/sum.rs @@ -52,9 +52,9 @@ impl Command for SubCommand { } } -pub fn summation(values: &[Value], head: &Span) -> Result { +pub fn summation(values: &[Value], span: Span, head: &Span) -> Result { let sum_func = reducer_for(Reduce::Summation); - sum_func(Value::nothing(*head), values.to_vec(), *head) + sum_func(Value::nothing(*head), values.to_vec(), span, *head) } #[cfg(test)] diff --git a/crates/nu-command/src/math/tan.rs b/crates/nu-command/src/math/tan.rs index c1b400b541..762036caca 100644 --- a/crates/nu-command/src/math/tan.rs +++ b/crates/nu-command/src/math/tan.rs @@ -35,6 +35,10 @@ impl Command for SubCommand { ) -> Result { let head = call.head; let use_degrees = call.has_flag("degrees"); + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head, use_degrees), engine_state.ctrlc.clone(), @@ -80,13 +84,13 @@ fn operate(value: Value, head: Span, use_degrees: bool) -> Value { span, } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/tanh.rs b/crates/nu-command/src/math/tanh.rs index ab12f6659a..4493a671ff 100644 --- a/crates/nu-command/src/math/tanh.rs +++ b/crates/nu-command/src/math/tanh.rs @@ -33,6 +33,10 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| operate(value, head), engine_state.ctrlc.clone(), @@ -62,13 +66,13 @@ fn operate(value: Value, head: Span) -> Value { span, } } + Value::Error { .. } => value, other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Only numerical values are supported, input type: {:?}", - other.get_type() - ), - other.span().unwrap_or(head), + error: ShellError::OnlySupportsThisInputType( + "numeric".into(), + other.get_type().to_string(), + head, + other.expect_span(), ), }, } diff --git a/crates/nu-command/src/math/utils.rs b/crates/nu-command/src/math/utils.rs index 04f73cb741..a7a5d975da 100644 --- a/crates/nu-command/src/math/utils.rs +++ b/crates/nu-command/src/math/utils.rs @@ -5,7 +5,7 @@ use nu_protocol::{IntoPipelineData, PipelineData, ShellError, Span, Spanned, Val pub fn run_with_function( call: &Call, input: PipelineData, - mf: impl Fn(&[Value], &Span) -> Result, + mf: impl Fn(&[Value], Span, &Span) -> Result, ) -> Result { let name = call.head; let res = calculate(input, name, mf); @@ -17,36 +17,43 @@ pub fn run_with_function( fn helper_for_tables( values: &[Value], + val_span: Span, name: Span, - mf: impl Fn(&[Value], &Span) -> Result, + mf: impl Fn(&[Value], Span, &Span) -> Result, ) -> Result { // If we are not dealing with Primitives, then perhaps we are dealing with a table // Create a key for each column name let mut column_values = IndexMap::new(); for val in values { - if let Value::Record { cols, vals, .. } = val { - for (key, value) in cols.iter().zip(vals.iter()) { - column_values - .entry(key.clone()) - .and_modify(|v: &mut Vec| v.push(value.clone())) - .or_insert_with(|| vec![value.clone()]); + match val { + Value::Record { cols, vals, .. } => { + for (key, value) in cols.iter().zip(vals.iter()) { + column_values + .entry(key.clone()) + .and_modify(|v: &mut Vec| v.push(value.clone())) + .or_insert_with(|| vec![value.clone()]); + } + } + Value::Error { error } => return Err(error.clone()), + _ => { + //Turns out we are not dealing with a table + return mf(values, val.expect_span(), &name); } - } else { - //Turns out we are not dealing with a table - return mf(values, &name); } } // The mathematical function operates over the columns of the table let mut column_totals = IndexMap::new(); for (col_name, col_vals) in column_values { - if let Ok(out) = mf(&col_vals, &name) { + if let Ok(out) = mf(&col_vals, val_span, &name) { column_totals.insert(col_name, out); } } if column_totals.keys().len() == 0 { return Err(ShellError::UnsupportedInput( "Unable to give a result with this input".to_string(), + "value originates from here".into(), name, + val_span, )); } @@ -59,17 +66,28 @@ fn helper_for_tables( pub fn calculate( values: PipelineData, name: Span, - mf: impl Fn(&[Value], &Span) -> Result, + mf: impl Fn(&[Value], Span, &Span) -> Result, ) -> Result { + // TODO implement spans for ListStream, thus negating the need for unwrap_or(). + let span = values.span().unwrap_or(name); match values { - PipelineData::ListStream(s, ..) => helper_for_tables(&s.collect::>(), name, mf), - PipelineData::Value(Value::List { ref vals, .. }, ..) => match &vals[..] { - [Value::Record { .. }, _end @ ..] => helper_for_tables(vals, name, mf), - _ => mf(vals, &name), + PipelineData::ListStream(s, ..) => { + helper_for_tables(&s.collect::>(), span, name, mf) + } + PipelineData::Value(Value::List { ref vals, span }, ..) => match &vals[..] { + [Value::Record { .. }, _end @ ..] => helper_for_tables( + vals, + values.span().expect("PipelineData::Value had no span"), + name, + mf, + ), + _ => mf(vals, span, &name), }, PipelineData::Value(Value::Record { vals, cols, span }, ..) => { - let new_vals: Result, ShellError> = - vals.into_iter().map(|val| mf(&[val], &name)).collect(); + let new_vals: Result, ShellError> = vals + .into_iter() + .map(|val| mf(&[val], span, &name)) + .collect(); match new_vals { Ok(vec) => Ok(Value::Record { cols, @@ -79,18 +97,23 @@ pub fn calculate( Err(err) => Err(err), } } - PipelineData::Value(Value::Range { val, .. }, ..) => { + PipelineData::Value(Value::Range { val, span, .. }, ..) => { let new_vals: Result, ShellError> = val .into_range_iter(None)? - .map(|val| mf(&[val], &name)) + .map(|val| mf(&[val], span, &name)) .collect(); - mf(&new_vals?, &name) + mf(&new_vals?, span, &name) } - PipelineData::Value(val, ..) => mf(&[val], &name), - _ => Err(ShellError::UnsupportedInput( - "Input data is not supported by this command.".to_string(), + PipelineData::Value(val, ..) => mf(&[val], span, &name), + PipelineData::Empty { .. } => Err(ShellError::PipelineEmpty(name)), + val => Err(ShellError::UnsupportedInput( + "Only integers, floats, lists, records or ranges are supported".into(), + "value originates from here".into(), name, + // This requires both the ListStream and Empty match arms to be above it. + val.span() + .expect("non-Empty non-ListStream PipelineData had no span"), )), } } diff --git a/crates/nu-command/src/math/variance.rs b/crates/nu-command/src/math/variance.rs index d9eef95ca6..799f702143 100644 --- a/crates/nu-command/src/math/variance.rs +++ b/crates/nu-command/src/math/variance.rs @@ -63,14 +63,15 @@ fn sum_of_squares(values: &[Value], span: &Span) -> Result { let mut sum_x2 = Value::int(0, *span); for value in values { let v = match &value { - Value::Int { .. } - | Value::Float { .. } => { - Ok(value) - }, + Value::Int { .. } | Value::Float { .. } => Ok(value), + Value::Error { error } => Err(error.clone()), _ => Err(ShellError::UnsupportedInput( - "Attempted to compute the sum of squared values of a value that cannot be summed or squared.".to_string(), - value.span().unwrap_or(*span), - )) + "Attempted to compute the sum of squares of a non-integer, non-float value" + .to_string(), + "value originates from here".into(), + *span, + value.expect_span(), + )), }?; let v_squared = &v.mul(*span, v, *span)?; sum_x2 = sum_x2.add(*span, v_squared, *span)?; @@ -85,24 +86,19 @@ fn sum_of_squares(values: &[Value], span: &Span) -> Result { Ok(ss) } -pub fn compute_variance(sample: bool) -> impl Fn(&[Value], &Span) -> Result { - move |values: &[Value], span: &Span| { +pub fn compute_variance( + sample: bool, +) -> impl Fn(&[Value], Span, &Span) -> Result { + move |values: &[Value], span: Span, head: &Span| { let n = if sample { values.len() - 1 } else { values.len() }; - let sum_of_squares = sum_of_squares(values, span); - let ss = match sum_of_squares { - Err(ShellError::UnsupportedInput(_, err_span)) => Err(ShellError::UnsupportedInput( - "Attempted to compute the variance with an item that cannot be used for that." - .to_string(), - err_span, - )), - other => other, - }?; - let n = Value::int(n as i64, *span); - ss.div(*span, &n, *span) + // sum_of_squares() needs the span of the original value, not the call head. + let ss = sum_of_squares(values, &span)?; + let n = Value::int(n as i64, *head); + ss.div(*head, &n, *head) } } diff --git a/crates/nu-command/src/network/fetch.rs b/crates/nu-command/src/network/fetch.rs index a8e524a176..a9fb04a73f 100644 --- a/crates/nu-command/src/network/fetch.rs +++ b/crates/nu-command/src/network/fetch.rs @@ -114,7 +114,7 @@ impl Command for SubCommand { } struct Arguments { - url: Option, + url: Value, raw: bool, user: Option, password: Option, @@ -129,14 +129,14 @@ fn run_fetch( _input: PipelineData, ) -> Result { let args = Arguments { - url: Some(call.req(engine_state, stack, 0)?), + url: call.req(engine_state, stack, 0)?, raw: call.has_flag("raw"), user: call.get_flag(engine_state, stack, "user")?, password: call.get_flag(engine_state, stack, "password")?, timeout: call.get_flag(engine_state, stack, "timeout")?, headers: call.get_flag(engine_state, stack, "headers")?, }; - helper(engine_state, stack, call, args) + helper(engine_state, stack, args) } // Helper function that actually goes to retrieve the resource from the url given @@ -144,25 +144,18 @@ fn run_fetch( fn helper( engine_state: &EngineState, stack: &mut Stack, - call: &Call, args: Arguments, ) -> std::result::Result { - let url_value = if let Some(val) = args.url { - val - } else { - return Err(ShellError::UnsupportedInput( - "Expecting a url as a string but got nothing".to_string(), - call.head, - )); - }; + // There is no need to error-check this, as the URL is already guaranteed by basic nu command argument type checks. + let url_value = args.url; let span = url_value.span()?; let requested_url = url_value.as_string()?; let url = match url::Url::parse(&requested_url) { Ok(u) => u, Err(_e) => { - return Err(ShellError::UnsupportedInput( - "Incomplete or incorrect url. Expected a full url, e.g., https://www.example.com" + return Err(ShellError::TypeMismatch( + "Incomplete or incorrect URL. Expected a full URL, e.g., https://www.example.com" .to_string(), span, )); @@ -185,9 +178,10 @@ fn helper( if let Some(timeout) = timeout { let val = timeout.as_i64()?; if val.is_negative() || val < 1 { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( "Timeout value must be an integer and larger than 0".to_string(), - timeout.span().unwrap_or_else(|_| Span::new(0, 0)), + // timeout is already guaranteed to not be an error + timeout.expect_span(), )); } diff --git a/crates/nu-command/src/network/post.rs b/crates/nu-command/src/network/post.rs index 393d7081a2..441e342211 100644 --- a/crates/nu-command/src/network/post.rs +++ b/crates/nu-command/src/network/post.rs @@ -120,8 +120,8 @@ impl Command for SubCommand { } struct Arguments { - path: Option, - body: Option, + path: Value, + body: Value, headers: Option, raw: bool, insecure: Option, @@ -145,8 +145,8 @@ fn run_post( _input: PipelineData, ) -> Result { let args = Arguments { - path: Some(call.req(engine_state, stack, 0)?), - body: Some(call.req(engine_state, stack, 1)?), + path: call.req(engine_state, stack, 0)?, + body: call.req(engine_state, stack, 1)?, headers: call.get_flag(engine_state, stack, "headers")?, raw: call.has_flag("raw"), user: call.get_flag(engine_state, stack, "user")?, @@ -165,30 +165,18 @@ fn helper( call: &Call, args: Arguments, ) -> std::result::Result { - let url_value = if let Some(val) = args.path { - val - } else { - return Err(ShellError::UnsupportedInput( - "Expecting a URL as a string but got nothing".to_string(), - call.head, - )); - }; - let body = if let Some(body) = args.body { - body - } else { - return Err(ShellError::UnsupportedInput( - "Expecting a body parameter but got nothing".to_string(), - call.head, - )); - }; + let url_value = args.path; + let body = args.body; let span = url_value.span()?; let requested_url = url_value.as_string()?; let url = match url::Url::parse(&requested_url) { Ok(u) => u, Err(_e) => { return Err(ShellError::UnsupportedInput( - "Incomplete or incorrect url. Expected a full url, e.g., https://www.example.com" + "Incomplete or incorrect URL. Expected a full URL, e.g., https://www.example.com" .to_string(), + format!("value: '{:?}'", requested_url), + call.head, span, )); } diff --git a/crates/nu-command/src/network/url/parse.rs b/crates/nu-command/src/network/url/parse.rs index 726a293e76..d1820dba24 100644 --- a/crates/nu-command/src/network/url/parse.rs +++ b/crates/nu-command/src/network/url/parse.rs @@ -43,7 +43,7 @@ impl Command for SubCommand { call: &Call, input: PipelineData, ) -> Result { - parse(input.into_value(call.head), engine_state) + parse(input.into_value(call.head), call.head, engine_state) } fn examples(&self) -> Vec { @@ -91,12 +91,13 @@ fn get_url_string(value: &Value, engine_state: &EngineState) -> String { value.into_string("", engine_state.get_config()) } -fn parse(value: Value, engine_state: &EngineState) -> Result { +fn parse(value: Value, head: Span, engine_state: &EngineState) -> Result { let url_string = get_url_string(&value, engine_state); let result_url = Url::parse(url_string.as_str()); - let head = value.span()?; + // This is the span of the original string, not the call head. + let span = value.span()?; match result_url { Ok(url) => { @@ -176,14 +177,18 @@ fn parse(value: Value, engine_state: &EngineState) -> Result Err(ShellError::UnsupportedInput( "String not compatible with url-encoding".to_string(), + "value originates from here".into(), head, + span, )), } } Err(_e) => Err(ShellError::UnsupportedInput( - "Incomplete or incorrect url. Expected a full url, e.g., https://www.example.com" + "Incomplete or incorrect URL. Expected a full URL, e.g., https://www.example.com" .to_string(), + "value originates from here".into(), head, + span, )), } } diff --git a/crates/nu-command/src/path/basename.rs b/crates/nu-command/src/path/basename.rs index 222319de1a..ce75b6240e 100644 --- a/crates/nu-command/src/path/basename.rs +++ b/crates/nu-command/src/path/basename.rs @@ -1,7 +1,10 @@ use std::path::Path; use nu_engine::CallExt; -use nu_protocol::{engine::Command, Example, Signature, Span, Spanned, SyntaxShape, Type, Value}; +use nu_protocol::{ + engine::Command, Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, + Type, Value, +}; use super::PathSubcommandArguments; @@ -62,6 +65,10 @@ impl Command for SubCommand { replace: call.get_flag(engine_state, stack, "replace")?, }; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&get_basename, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/path/dirname.rs b/crates/nu-command/src/path/dirname.rs index 48a762178b..8858f12d68 100644 --- a/crates/nu-command/src/path/dirname.rs +++ b/crates/nu-command/src/path/dirname.rs @@ -1,7 +1,10 @@ use std::path::Path; use nu_engine::CallExt; -use nu_protocol::{engine::Command, Example, Signature, Span, Spanned, SyntaxShape, Type, Value}; +use nu_protocol::{ + engine::Command, Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, + Type, Value, +}; use super::PathSubcommandArguments; @@ -66,6 +69,10 @@ impl Command for SubCommand { num_levels: call.get_flag(engine_state, stack, "num-levels")?, }; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&get_dirname, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/path/exists.rs b/crates/nu-command/src/path/exists.rs index cf16412878..5a0dae9f45 100644 --- a/crates/nu-command/src/path/exists.rs +++ b/crates/nu-command/src/path/exists.rs @@ -2,7 +2,9 @@ use std::path::{Path, PathBuf}; use nu_engine::{current_dir, CallExt}; use nu_path::expand_path_with; -use nu_protocol::{engine::Command, Example, Signature, Span, SyntaxShape, Type, Value}; +use nu_protocol::{ + engine::Command, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, +}; use super::PathSubcommandArguments; @@ -57,6 +59,10 @@ If you need to distinguish dirs and files, please use `path type`."# columns: call.get_flag(engine_state, stack, "columns")?, pwd: current_dir(engine_state, stack)?, }; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&exists, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/path/expand.rs b/crates/nu-command/src/path/expand.rs index 1423c40aa8..0c2b81cd82 100644 --- a/crates/nu-command/src/path/expand.rs +++ b/crates/nu-command/src/path/expand.rs @@ -4,7 +4,7 @@ use nu_engine::env::current_dir_str; use nu_engine::CallExt; use nu_path::{canonicalize_with, expand_path_with}; use nu_protocol::{ - engine::Command, Example, ShellError, Signature, Span, SyntaxShape, Type, Value, + engine::Command, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, }; use super::PathSubcommandArguments; @@ -65,7 +65,10 @@ impl Command for SubCommand { cwd: current_dir_str(engine_state, stack)?, not_follow_symlink: call.has_flag("no-symlink"), }; - + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&expand, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/path/join.rs b/crates/nu-command/src/path/join.rs index 94d8179b93..ab78c8c3f4 100644 --- a/crates/nu-command/src/path/join.rs +++ b/crates/nu-command/src/path/join.rs @@ -78,9 +78,14 @@ the output of 'path parse' and 'path split' subcommands."# handle_value(input.into_value(head), &args, head), metadata, )), + PipelineData::Empty { .. } => Err(ShellError::PipelineEmpty(head)), _ => Err(ShellError::UnsupportedInput( - "Input data is not supported by this command.".to_string(), + "Input value cannot be joined".to_string(), + "value originates from here".into(), head, + input + .span() + .expect("non-Empty non-ListStream PipelineData had no span"), )), } } @@ -156,35 +161,35 @@ the output of 'path parse' and 'path split' subcommands."# fn handle_value(v: Value, args: &Arguments, head: Span) -> Value { match v { - Value::String { ref val, span } => join_single(Path::new(val), span, args), - Value::Record { cols, vals, span } => join_record(&cols, &vals, span, args), - Value::List { vals, span } => join_list(&vals, span, head, args), + Value::String { ref val, .. } => join_single(Path::new(val), head, args), + Value::Record { cols, vals, span } => join_record(&cols, &vals, head, span, args), + Value::List { vals, span } => join_list(&vals, head, span, args), _ => super::handle_invalid_values(v, head), } } -fn join_single(path: &Path, span: Span, args: &Arguments) -> Value { +fn join_single(path: &Path, head: Span, args: &Arguments) -> Value { let mut result = path.to_path_buf(); for path_to_append in &args.append { result.push(&path_to_append.item) } - Value::string(result.to_string_lossy(), span) + Value::string(result.to_string_lossy(), head) } -fn join_list(parts: &[Value], span: Span, head: Span, args: &Arguments) -> Value { +fn join_list(parts: &[Value], head: Span, span: Span, args: &Arguments) -> Value { let path: Result = parts.iter().map(Value::as_string).collect(); match path { - Ok(ref path) => join_single(path, span, args), + Ok(ref path) => join_single(path, head, args), Err(_) => { let records: Result, ShellError> = parts.iter().map(Value::as_record).collect(); match records { Ok(vals) => { let vals = vals .iter() - .map(|(k, v)| join_record(k, v, span, args)) + .map(|(k, v)| join_record(k, v, head, span, args)) .collect(); Value::List { vals, span } @@ -197,7 +202,7 @@ fn join_list(parts: &[Value], span: Span, head: Span, args: &Arguments) -> Value } } -fn join_record(cols: &[String], vals: &[Value], span: Span, args: &Arguments) -> Value { +fn join_record(cols: &[String], vals: &[Value], head: Span, span: Span, args: &Arguments) -> Value { if args.columns.is_some() { super::operate( &join_single, @@ -210,22 +215,31 @@ fn join_record(cols: &[String], vals: &[Value], span: Span, args: &Arguments) -> span, ) } else { - match merge_record(cols, vals, span) { - Ok(p) => join_single(p.as_path(), span, args), + match merge_record(cols, vals, head, span) { + Ok(p) => join_single(p.as_path(), head, args), Err(error) => Value::Error { error }, } } } -fn merge_record(cols: &[String], vals: &[Value], span: Span) -> Result { +fn merge_record( + cols: &[String], + vals: &[Value], + head: Span, + span: Span, +) -> Result { for key in cols { if !super::ALLOWED_COLUMNS.contains(&key.as_str()) { let allowed_cols = super::ALLOWED_COLUMNS.join(", "); - let msg = format!( - "Column '{}' is not valid for a structured path. Allowed columns are: {}", - key, allowed_cols - ); - return Err(ShellError::UnsupportedInput(msg, span)); + return Err(ShellError::UnsupportedInput( + format!( + "Column '{}' is not valid for a structured path. Allowed columns on this platform are: {}", + key, allowed_cols + ), + "value originates from here".into(), + head, + span + )); } } diff --git a/crates/nu-command/src/path/mod.rs b/crates/nu-command/src/path/mod.rs index a27543d0bd..7e068fa200 100644 --- a/crates/nu-command/src/path/mod.rs +++ b/crates/nu-command/src/path/mod.rs @@ -50,7 +50,9 @@ where return Value::Error { error: ShellError::UnsupportedInput( String::from("when the input is a table, you must specify the columns"), + "value originates from here".into(), name, + span, ), }; } @@ -91,9 +93,11 @@ fn err_from_value(rest: &Value, name: Span) -> ShellError { match rest.span() { Ok(span) => { if rest.is_nothing() { - ShellError::UnsupportedInput( - "Input type is nothing, expected: string, row or list".into(), + ShellError::OnlySupportsThisInputType( + "string, record or list".into(), + "nothing".into(), name, + span, ) } else { ShellError::PipelineMismatch("string, row or list".into(), name, span) diff --git a/crates/nu-command/src/path/parse.rs b/crates/nu-command/src/path/parse.rs index a2217c71ea..c36193d85e 100644 --- a/crates/nu-command/src/path/parse.rs +++ b/crates/nu-command/src/path/parse.rs @@ -3,7 +3,8 @@ use std::path::Path; use indexmap::IndexMap; use nu_engine::CallExt; use nu_protocol::{ - engine::Command, Example, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value, + engine::Command, Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, + Type, Value, }; use super::PathSubcommandArguments; @@ -66,6 +67,10 @@ On Windows, an extra 'prefix' column is added."# extension: call.get_flag(engine_state, stack, "extension")?, }; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&parse, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/path/relative_to.rs b/crates/nu-command/src/path/relative_to.rs index 716be73582..236a2f7ff2 100644 --- a/crates/nu-command/src/path/relative_to.rs +++ b/crates/nu-command/src/path/relative_to.rs @@ -3,7 +3,8 @@ use std::path::Path; use nu_engine::CallExt; use nu_path::expand_to_real_path; use nu_protocol::{ - engine::Command, Example, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value, + engine::Command, Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, + Type, Value, }; use super::PathSubcommandArguments; @@ -66,6 +67,10 @@ path."# columns: call.get_flag(engine_state, stack, "columns")?, }; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&relative_to, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/path/split.rs b/crates/nu-command/src/path/split.rs index 95bbee2c01..677b529594 100644 --- a/crates/nu-command/src/path/split.rs +++ b/crates/nu-command/src/path/split.rs @@ -2,7 +2,7 @@ use std::path::{Component, Path}; use nu_engine::CallExt; use nu_protocol::{ - engine::Command, Example, ShellError, Signature, Span, SyntaxShape, Type, Value, + engine::Command, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, }; use super::PathSubcommandArguments; @@ -52,6 +52,10 @@ impl Command for SubCommand { columns: call.get_flag(engine_state, stack, "columns")?, }; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&split, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/path/type.rs b/crates/nu-command/src/path/type.rs index 959d814bbf..f270d065bf 100644 --- a/crates/nu-command/src/path/type.rs +++ b/crates/nu-command/src/path/type.rs @@ -2,7 +2,7 @@ use std::path::Path; use nu_engine::CallExt; use nu_protocol::{ - engine::Command, Example, ShellError, Signature, Span, SyntaxShape, Type, Value, + engine::Command, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, }; use super::PathSubcommandArguments; @@ -57,6 +57,10 @@ If nothing is found, an empty string will be returned."# columns: call.get_flag(engine_state, stack, "columns")?, }; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(head)); + } input.map( move |value| super::operate(&r#type, &args, value, head), engine_state.ctrlc.clone(), diff --git a/crates/nu-command/src/platform/ansi/ansi_.rs b/crates/nu-command/src/platform/ansi/ansi_.rs index f69f0028ea..dddea11398 100644 --- a/crates/nu-command/src/platform/ansi/ansi_.rs +++ b/crates/nu-command/src/platform/ansi/ansi_.rs @@ -656,8 +656,8 @@ Format: # if (escape || osc) && (param_is_valid_string) { let code_vec: Vec = code_string.chars().collect(); if code_vec[0] == '\\' { - return Err(ShellError::UnsupportedInput( - String::from("no need for escape characters"), + return Err(ShellError::TypeMismatch( + "no need for escape characters".into(), call.get_flag_expr("escape") .expect("Unexpected missing argument") .span, @@ -695,7 +695,7 @@ Format: # match str_to_ansi(&code_string) { Some(c) => c, None => { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( String::from("Unknown ansi code"), call.positional_nth(0) .expect("Unexpected missing argument") diff --git a/crates/nu-command/src/strings/char_.rs b/crates/nu-command/src/strings/char_.rs index 89561b3d8a..df4b8ed111 100644 --- a/crates/nu-command/src/strings/char_.rs +++ b/crates/nu-command/src/strings/char_.rs @@ -288,7 +288,7 @@ impl Command for Char { if let Some(output) = special_character { Ok(Value::string(output, call_span).into_pipeline_data()) } else { - Err(ShellError::UnsupportedInput( + Err(ShellError::TypeMismatch( "error finding named character".into(), call.positional_nth(0) .expect("Unexpected missing argument") @@ -305,7 +305,7 @@ fn integer_to_unicode_char(value: i64, t: &Span) -> Result { if let Some(ch) = decoded_char { Ok(ch) } else { - Err(ShellError::UnsupportedInput( + Err(ShellError::TypeMismatch( "not a valid Unicode codepoint".into(), *t, )) @@ -320,7 +320,7 @@ fn string_to_unicode_char(s: &str, t: &Span) -> Result { if let Some(ch) = decoded_char { Ok(ch) } else { - Err(ShellError::UnsupportedInput( + Err(ShellError::TypeMismatch( "error decoding Unicode character".into(), *t, )) diff --git a/crates/nu-command/src/strings/encode_decode/base64.rs b/crates/nu-command/src/strings/encode_decode/base64.rs index e9d5c4f5a5..bd74dddcd9 100644 --- a/crates/nu-command/src/strings/encode_decode/base64.rs +++ b/crates/nu-command/src/strings/encode_decode/base64.rs @@ -98,14 +98,19 @@ fn action( )} }; match input { + // Propagate existing errors. + Value::Error { .. } => input.clone(), Value::Binary { val, .. } => match base64_config.action_type { ActionType::Encode => { Value::string(encode_config(val, base64_config_enum), command_span) } ActionType::Decode => Value::Error { error: ShellError::UnsupportedInput( - "Binary data can only support encoding".to_string(), + "Binary data can only be encoded".to_string(), + "value originates from here".into(), command_span, + // This line requires the Value::Error {} match above. + input.expect_span(), ), }, }, diff --git a/crates/nu-command/src/strings/encode_decode/decode.rs b/crates/nu-command/src/strings/encode_decode/decode.rs index eb78389087..2ce93f1282 100644 --- a/crates/nu-command/src/strings/encode_decode/decode.rs +++ b/crates/nu-command/src/strings/encode_decode/decode.rs @@ -74,12 +74,24 @@ documentation link at https://docs.rs/encoding_rs/0.8.28/encoding_rs/#statics"# let bytes: Vec = stream.into_bytes()?.item; super::encoding::decode(head, encoding, &bytes).map(|val| val.into_pipeline_data()) } - PipelineData::Value(Value::Binary { val: bytes, .. }, ..) => { - super::encoding::decode(head, encoding, &bytes).map(|val| val.into_pipeline_data()) - } + PipelineData::Value(v, ..) => match v { + Value::Binary { val: bytes, .. } => super::encoding::decode(head, encoding, &bytes) + .map(|val| val.into_pipeline_data()), + Value::Error { error } => Err(error), + _ => Err(ShellError::OnlySupportsThisInputType( + "binary".into(), + v.get_type().to_string(), + head, + v.expect_span(), + )), + }, + // This should be more precise, but due to difficulties in getting spans + // from PipelineData::ListData, this is as it is. _ => Err(ShellError::UnsupportedInput( "non-binary input".into(), + "value originates from here".into(), head, + input.span().unwrap_or(head), )), } } diff --git a/crates/nu-command/src/strings/encode_decode/encode.rs b/crates/nu-command/src/strings/encode_decode/encode.rs index b3e9688a56..65d2d69eb9 100644 --- a/crates/nu-command/src/strings/encode_decode/encode.rs +++ b/crates/nu-command/src/strings/encode_decode/encode.rs @@ -74,12 +74,25 @@ documentation link at https://docs.rs/encoding_rs/0.8.28/encoding_rs/#statics"# let s = stream.into_string()?.item; super::encoding::encode(head, encoding, &s).map(|val| val.into_pipeline_data()) } - PipelineData::Value(Value::String { val: s, .. }, ..) => { - super::encoding::encode(head, encoding, &s).map(|val| val.into_pipeline_data()) - } + PipelineData::Value(v, ..) => match v { + Value::String { val: s, .. } => { + super::encoding::encode(head, encoding, &s).map(|val| val.into_pipeline_data()) + } + Value::Error { error } => Err(error), + _ => Err(ShellError::OnlySupportsThisInputType( + "string".into(), + v.get_type().to_string(), + head, + v.expect_span(), + )), + }, + // This should be more precise, but due to difficulties in getting spans + // from PipelineData::ListData, this is as it is. _ => Err(ShellError::UnsupportedInput( "non-string input".into(), + "value originates from here".into(), head, + input.span().unwrap_or(head), )), } } diff --git a/crates/nu-command/src/strings/format/command.rs b/crates/nu-command/src/strings/format/command.rs index 06776ca468..4b51e4e8ed 100644 --- a/crates/nu-command/src/strings/format/command.rs +++ b/crates/nu-command/src/strings/format/command.rs @@ -230,11 +230,13 @@ fn format( } } } - + Value::Error { error } => return Err(error.clone()), _ => { - return Err(ShellError::UnsupportedInput( - "Input data is not supported by this command.".to_string(), + return Err(ShellError::OnlySupportsThisInputType( + "record".to_string(), + val.get_type().to_string(), head_span, + val.expect_span(), )) } } @@ -245,9 +247,15 @@ fn format( None, )) } - _ => Err(ShellError::UnsupportedInput( - "Input data is not supported by this command.".to_string(), + // Unwrapping this ShellError is a bit unfortunate. + // Ideally, its Span would be preserved. + Value::Error { error } => Err(error), + _ => Err(ShellError::OnlySupportsThisInputType( + "record".to_string(), + data_as_value.get_type().to_string(), head_span, + // This line requires the Value::Error match above. + data_as_value.expect_span(), )), } } @@ -291,7 +299,7 @@ fn format_record( } } Some(err) => { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( format!("expression is invalid, detail message: {:?}", err), *span, )) diff --git a/crates/nu-command/src/strings/format/filesize.rs b/crates/nu-command/src/strings/format/filesize.rs index 753296f735..a8ddd2264b 100644 --- a/crates/nu-command/src/strings/format/filesize.rs +++ b/crates/nu-command/src/strings/format/filesize.rs @@ -104,13 +104,13 @@ fn format_value_impl(val: &Value, arg: &Arguments, span: Span) -> Value { val: format_filesize(*val, &arg.format_value, None), span: *span, }, - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is not supported, support type: , current_type: {}", - other.get_type() - ), + Value::Error { .. } => val.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "filesize".into(), + val.get_type().to_string(), span, + val.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/size.rs b/crates/nu-command/src/strings/size.rs index ee83db9acb..89904a6117 100644 --- a/crates/nu-command/src/strings/size.rs +++ b/crates/nu-command/src/strings/size.rs @@ -116,6 +116,10 @@ fn size( input: PipelineData, ) -> Result { let span = call.head; + // This doesn't match explicit nulls + if matches!(input, PipelineData::Empty) { + return Err(ShellError::PipelineEmpty(span)); + } input.map( move |v| { // First, obtain the span. If this fails, propagate the error that results. diff --git a/crates/nu-command/src/strings/str_/case/capitalize.rs b/crates/nu-command/src/strings/str_/case/capitalize.rs index cc37b276f5..9728d1b7f3 100644 --- a/crates/nu-command/src/strings/str_/case/capitalize.rs +++ b/crates/nu-command/src/strings/str_/case/capitalize.rs @@ -108,13 +108,13 @@ fn action(input: &Value, head: Span) -> Value { val: uppercase_helper(val), span: head, }, - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/case/downcase.rs b/crates/nu-command/src/strings/str_/case/downcase.rs index 871b3ec300..4f29539eb1 100644 --- a/crates/nu-command/src/strings/str_/case/downcase.rs +++ b/crates/nu-command/src/strings/str_/case/downcase.rs @@ -123,13 +123,13 @@ fn action(input: &Value, head: Span) -> Value { val: val.to_ascii_lowercase(), span: head, }, - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/case/mod.rs b/crates/nu-command/src/strings/str_/case/mod.rs index 56b45bc760..0044274a2e 100644 --- a/crates/nu-command/src/strings/str_/case/mod.rs +++ b/crates/nu-command/src/strings/str_/case/mod.rs @@ -67,13 +67,13 @@ where val: case_operation(val), span: head, }, - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/case/upcase.rs b/crates/nu-command/src/strings/str_/case/upcase.rs index 14faf289da..61f51b9dc9 100644 --- a/crates/nu-command/src/strings/str_/case/upcase.rs +++ b/crates/nu-command/src/strings/str_/case/upcase.rs @@ -84,12 +84,15 @@ fn action(input: &Value, head: Span) -> Value { val: s.to_uppercase(), span: head, }, - other => { - let got = format!("Expected string but got {}", other.get_type()); - Value::Error { - error: ShellError::UnsupportedInput(got, head), - } - } + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), + head, + input.expect_span(), + ), + }, } } diff --git a/crates/nu-command/src/strings/str_/contains.rs b/crates/nu-command/src/strings/str_/contains.rs index 27a9f666e0..117866f2bf 100644 --- a/crates/nu-command/src/strings/str_/contains.rs +++ b/crates/nu-command/src/strings/str_/contains.rs @@ -190,13 +190,13 @@ fn action( }, head, ), - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/distance.rs b/crates/nu-command/src/strings/str_/distance.rs index abd0149fe0..1b5d4daf7d 100644 --- a/crates/nu-command/src/strings/str_/distance.rs +++ b/crates/nu-command/src/strings/str_/distance.rs @@ -92,18 +92,18 @@ impl Command for SubCommand { fn action(input: &Value, args: &Arguments, head: Span) -> Value { let compare_string = &args.compare_string; - match &input { + match input { Value::String { val, .. } => { let distance = levenshtein_distance(val, compare_string); Value::int(distance as i64, head) } - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/ends_with.rs b/crates/nu-command/src/strings/str_/ends_with.rs index 3c39bf700e..b6054b8586 100644 --- a/crates/nu-command/src/strings/str_/ends_with.rs +++ b/crates/nu-command/src/strings/str_/ends_with.rs @@ -81,13 +81,13 @@ impl Command for SubCommand { fn action(input: &Value, args: &Arguments, head: Span) -> Value { match input { Value::String { val, .. } => Value::boolean(val.ends_with(&args.substring), head), - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/index_of.rs b/crates/nu-command/src/strings/str_/index_of.rs index d8a0cb0339..991b1a5d8b 100644 --- a/crates/nu-command/src/strings/str_/index_of.rs +++ b/crates/nu-command/src/strings/str_/index_of.rs @@ -150,13 +150,13 @@ fn action( Value::int(-1, head) } } - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } @@ -185,8 +185,8 @@ fn process_range( } Value::List { vals, .. } => { if vals.len() > 2 { - Err(ShellError::UnsupportedInput( - String::from("there shouldn't be more than two indexes. too many indexes"), + Err(ShellError::TypeMismatch( + String::from("there shouldn't be more than two indexes"), head, )) } else { @@ -201,12 +201,12 @@ fn process_range( Ok((start_index, end_index)) } } - other => Err(ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { error } => Err(error.clone()), + _ => Err(ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), )), }?; @@ -214,18 +214,16 @@ fn process_range( let end_index = r.1.parse::().unwrap_or(input_len as i32); if start_index < 0 || start_index > end_index { - return Err(ShellError::UnsupportedInput( - String::from( - "start index can't be negative or greater than end index. Invalid start index", - ), + return Err(ShellError::TypeMismatch( + String::from("start index can't be negative or greater than end index"), head, )); } if end_index < 0 || end_index < start_index || end_index > input_len as i32 { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( String::from( - "end index can't be negative, smaller than start index or greater than input length. Invalid end index"), + "end index can't be negative, smaller than start index or greater than input length"), head, )); } diff --git a/crates/nu-command/src/strings/str_/length.rs b/crates/nu-command/src/strings/str_/length.rs index b575d639ee..4dca859509 100644 --- a/crates/nu-command/src/strings/str_/length.rs +++ b/crates/nu-command/src/strings/str_/length.rs @@ -67,13 +67,13 @@ impl Command for SubCommand { fn action(input: &Value, _arg: &CellPathOnlyArgs, head: Span) -> Value { match input { Value::String { val, .. } => Value::int(val.len() as i64, head), - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/lpad.rs b/crates/nu-command/src/strings/str_/lpad.rs index fa3064b031..540801e76f 100644 --- a/crates/nu-command/src/strings/str_/lpad.rs +++ b/crates/nu-command/src/strings/str_/lpad.rs @@ -69,7 +69,7 @@ impl Command for SubCommand { }; if args.length.expect("this exists") < 0 { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( String::from("The length of the string cannot be negative"), call.head, )); @@ -137,19 +137,16 @@ fn action( } } None => Value::Error { - error: ShellError::UnsupportedInput( - String::from("Length argument is missing"), - head, - ), + error: ShellError::TypeMismatch(String::from("Length argument is missing"), head), }, }, - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/replace.rs b/crates/nu-command/src/strings/str_/replace.rs index 12877beec2..5676eeb8c5 100644 --- a/crates/nu-command/src/strings/str_/replace.rs +++ b/crates/nu-command/src/strings/str_/replace.rs @@ -209,18 +209,23 @@ fn action( } } Err(e) => Value::Error { - error: ShellError::UnsupportedInput(format!("{e}"), find.span), + error: ShellError::UnsupportedInput( + format!("{e}"), + "value originates from here".into(), + head, + find.span, + ), }, } } } - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/reverse.rs b/crates/nu-command/src/strings/str_/reverse.rs index 90438addd1..bd67a495a7 100644 --- a/crates/nu-command/src/strings/str_/reverse.rs +++ b/crates/nu-command/src/strings/str_/reverse.rs @@ -75,14 +75,13 @@ fn action(input: &Value, _arg: &CellPathOnlyArgs, head: Span) -> Value { val: val.chars().rev().collect::(), span: head, }, - - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/rpad.rs b/crates/nu-command/src/strings/str_/rpad.rs index 0a019ad00c..97dc53dd72 100644 --- a/crates/nu-command/src/strings/str_/rpad.rs +++ b/crates/nu-command/src/strings/str_/rpad.rs @@ -69,7 +69,7 @@ impl Command for SubCommand { }; if args.length.expect("this exists") < 0 { - return Err(ShellError::UnsupportedInput( + return Err(ShellError::TypeMismatch( String::from("The length of the string cannot be negative"), call.head, )); @@ -129,19 +129,16 @@ fn action( } } None => Value::Error { - error: ShellError::UnsupportedInput( - String::from("Length argument is missing"), - head, - ), + error: ShellError::TypeMismatch(String::from("Length argument is missing"), head), }, }, - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/starts_with.rs b/crates/nu-command/src/strings/str_/starts_with.rs index 56061508f3..4299bb5b0e 100644 --- a/crates/nu-command/src/strings/str_/starts_with.rs +++ b/crates/nu-command/src/strings/str_/starts_with.rs @@ -92,13 +92,13 @@ fn action(input: &Value, Arguments { substring, .. }: &Arguments, head: Span) -> let starts_with = s.starts_with(substring); Value::boolean(starts_with, head) } - other => Value::Error { - error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + Value::Error { .. } => input.clone(), + _ => Value::Error { + error: ShellError::OnlySupportsThisInputType( + "string".into(), + input.get_type().to_string(), head, + input.expect_span(), ), }, } diff --git a/crates/nu-command/src/strings/str_/substring.rs b/crates/nu-command/src/strings/str_/substring.rs index 73bfc59a78..793e36dcf9 100644 --- a/crates/nu-command/src/strings/str_/substring.rs +++ b/crates/nu-command/src/strings/str_/substring.rs @@ -136,7 +136,7 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { match start.cmp(&end) { Ordering::Equal => Value::string("", head), Ordering::Greater => Value::Error { - error: ShellError::UnsupportedInput( + error: ShellError::TypeMismatch( "End must be greater than or equal to Start".to_string(), head, ), @@ -165,13 +165,15 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { Value::string("", head) } } + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), other => Value::Error { error: ShellError::UnsupportedInput( - format!( - "Input's type is {}. This command only works with strings.", - other.get_type() - ), + "Only string values are supported".into(), + format!("input type: {:?}", other.get_type()), head, + // This line requires the Value::Error match above. + other.expect_span(), ), }, } @@ -186,7 +188,7 @@ fn process_arguments(range: &Value, head: Span) -> Result<(isize, isize), ShellE } Value::List { vals, .. } => { if vals.len() > 2 { - Err(ShellError::UnsupportedInput( + Err(ShellError::TypeMismatch( "More than two indices given".to_string(), head, )) @@ -197,7 +199,7 @@ fn process_arguments(range: &Value, head: Span) -> Result<(isize, isize), ShellE match v { Value::Int { val, .. } => Ok(val.to_string()), Value::String { val, .. } => Ok(val.to_string()), - _ => Err(ShellError::UnsupportedInput( + _ => Err(ShellError::TypeMismatch( "could not perform substring. Expecting a string or int" .to_string(), head, @@ -210,19 +212,13 @@ fn process_arguments(range: &Value, head: Span) -> Result<(isize, isize), ShellE let start = idx .get(0) .ok_or_else(|| { - ShellError::UnsupportedInput( - "could not perform substring".to_string(), - head, - ) + ShellError::TypeMismatch("could not perform substring".to_string(), head) })? .to_string(); let end = idx .get(1) .ok_or_else(|| { - ShellError::UnsupportedInput( - "could not perform substring".to_string(), - head, - ) + ShellError::TypeMismatch("could not perform substring".to_string(), head) })? .to_string(); Ok(SubstringText(start, end)) @@ -234,19 +230,19 @@ fn process_arguments(range: &Value, head: Span) -> Result<(isize, isize), ShellE let start = idx .first() .ok_or_else(|| { - ShellError::UnsupportedInput("could not perform substring".to_string(), head) + ShellError::TypeMismatch("could not perform substring".to_string(), head) })? .to_string(); let end = idx .get(1) .ok_or_else(|| { - ShellError::UnsupportedInput("could not perform substring".to_string(), head) + ShellError::TypeMismatch("could not perform substring".to_string(), head) })? .to_string(); Ok(SubstringText(start, end)) } - _ => Err(ShellError::UnsupportedInput( + _ => Err(ShellError::TypeMismatch( "could not perform substring".to_string(), head, )), @@ -254,14 +250,14 @@ fn process_arguments(range: &Value, head: Span) -> Result<(isize, isize), ShellE let start = match &search { SubstringText(start, _) if start.is_empty() || start == "_" => 0, SubstringText(start, _) => start.trim().parse().map_err(|_| { - ShellError::UnsupportedInput("could not perform substring".to_string(), head) + ShellError::TypeMismatch("could not perform substring".to_string(), head) })?, }; let end = match &search { SubstringText(_, end) if end.is_empty() || end == "_" => isize::max_value(), SubstringText(_, end) => end.trim().parse().map_err(|_| { - ShellError::UnsupportedInput("could not perform substring".to_string(), head) + ShellError::TypeMismatch("could not perform substring".to_string(), head) })?, }; diff --git a/crates/nu-command/src/strings/str_/trim/trim_.rs b/crates/nu-command/src/strings/str_/trim/trim_.rs index 6705bb6f42..8c8e7ee40f 100644 --- a/crates/nu-command/src/strings/str_/trim/trim_.rs +++ b/crates/nu-command/src/strings/str_/trim/trim_.rs @@ -181,6 +181,8 @@ fn action(input: &Value, arg: &Arguments, head: Span) -> Value { val: trim(s, char_, closure_flags), span: head, }, + // Propagate errors by explicitly matching them before the final case. + Value::Error { .. } => input.clone(), other => match mode { ActionMode::Global => match other { Value::Record { cols, vals, span } => { @@ -203,9 +205,14 @@ fn action(input: &Value, arg: &Arguments, head: Span) -> Value { _ => input.clone(), }, ActionMode::Local => { - let got = format!("Input must be a string. Found {}", other.get_type()); Value::Error { - error: ShellError::UnsupportedInput(got, head), + error: ShellError::UnsupportedInput( + "Only string values are supported".into(), + format!("input type: {:?}", other.get_type()), + head, + // This line requires the Value::Error match above. + other.expect_span(), + ), } } }, diff --git a/crates/nu-command/tests/commands/date/format.rs b/crates/nu-command/tests/commands/date/format.rs index a50a95bff8..672fd1db18 100644 --- a/crates/nu-command/tests/commands/date/format.rs +++ b/crates/nu-command/tests/commands/date/format.rs @@ -23,7 +23,7 @@ fn fails_without_input() { ) ); - assert!(actual.err.contains("Unsupported input")); + assert!(actual.err.contains("Pipeline empty")); } #[test] diff --git a/crates/nu-command/tests/commands/rename.rs b/crates/nu-command/tests/commands/rename.rs index eeec77602b..efbf8af3ee 100644 --- a/crates/nu-command/tests/commands/rename.rs +++ b/crates/nu-command/tests/commands/rename.rs @@ -83,7 +83,6 @@ fn errors_if_no_columns_present() { "# )); - assert!(actual.err.contains("no column names available")); - assert!(actual.err.contains("can't rename")); + assert!(actual.err.contains("only record input data is supported")); }) } diff --git a/crates/nu-command/tests/commands/roll.rs b/crates/nu-command/tests/commands/roll.rs index ccbd640e31..2964283276 100644 --- a/crates/nu-command/tests/commands/roll.rs +++ b/crates/nu-command/tests/commands/roll.rs @@ -88,12 +88,12 @@ mod columns { assert_eq!(actual.out, "origin-stars-commit_author"); } - struct ThirtieTwo<'a>(usize, &'a str); + struct ThirtyTwo<'a>(usize, &'a str); #[test] fn can_roll_the_cells_only_keeping_the_header_names() { let four_bitstring = bitstring_to_nu_row_pipeline("00000100"); - let expected_value = ThirtieTwo(32, "bit1-bit2-bit3-bit4-bit5-bit6-bit7-bit8"); + let expected_value = ThirtyTwo(32, "bit1-bit2-bit3-bit4-bit5-bit6-bit7-bit8"); let actual = nu!( cwd: ".", @@ -106,7 +106,7 @@ mod columns { #[test] fn four_in_bitstring_left_shifted_with_three_bits_should_be_32_in_decimal() { let four_bitstring = "00000100"; - let expected_value = ThirtieTwo(32, "00100000"); + let expected_value = ThirtyTwo(32, "00100000"); assert_eq!( shift_three_bits_to_the_left_to_bitstring(four_bitstring), @@ -141,7 +141,10 @@ mod columns { | math sum "#, ); - + println!( + "{} | roll left --by 3 | {}", + bitstring_as_nu_row_pipeline, nu_row_literal_bitstring_to_decimal_value_pipeline + ); nu!( cwd: ".", format!("{} | roll left --by 3 | {}", bitstring_as_nu_row_pipeline, nu_row_literal_bitstring_to_decimal_value_pipeline) diff --git a/crates/nu-command/tests/commands/take/rows.rs b/crates/nu-command/tests/commands/take/rows.rs index f496acd70c..e5819bfe60 100644 --- a/crates/nu-command/tests/commands/take/rows.rs +++ b/crates/nu-command/tests/commands/take/rows.rs @@ -51,7 +51,7 @@ fn fails_on_string() { "# )); - assert!(actual.err.contains("unsupported_input")); + assert!(actual.err.contains("pipeline_mismatch")); } #[test] diff --git a/crates/nu-command/tests/commands/url/parse.rs b/crates/nu-command/tests/commands/url/parse.rs index 2f95abf23a..9564765fc5 100644 --- a/crates/nu-command/tests/commands/url/parse.rs +++ b/crates/nu-command/tests/commands/url/parse.rs @@ -154,6 +154,6 @@ fn url_parse_error_empty_url() { )); assert!(actual.err.contains( - "Incomplete or incorrect url. Expected a full url, e.g., https://www.example.com" + "Incomplete or incorrect URL. Expected a full URL, e.g., https://www.example.com" )); } diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 625456f0a9..949a484da6 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -243,10 +243,10 @@ impl PipelineData { pub fn collect_string_strict( self, span: Span, - ) -> Result<(String, Option), ShellError> { + ) -> Result<(String, Span, Option), ShellError> { match self { - PipelineData::Empty => Ok((String::new(), None)), - PipelineData::Value(Value::String { val, .. }, metadata) => Ok((val, metadata)), + PipelineData::Empty => Ok((String::new(), span, None)), + PipelineData::Value(Value::String { val, span }, metadata) => Ok((val, span, metadata)), PipelineData::Value(val, _) => { Err(ShellError::TypeMismatch("string".into(), val.span()?)) } @@ -254,13 +254,15 @@ impl PipelineData { PipelineData::ExternalStream { stdout: None, metadata, + span, .. - } => Ok((String::new(), metadata)), + } => Ok((String::new(), span, metadata)), PipelineData::ExternalStream { stdout: Some(stdout), metadata, + span, .. - } => Ok((stdout.into_string()?.item, metadata)), + } => Ok((stdout.into_string()?.item, span, metadata)), } } diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 43aa0bf801..a0ca4ef07c 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -51,21 +51,39 @@ pub enum ShellError { #[label("value originates from here")] Span, ), - /// A command received an argument of the wrong type. + #[error("Pipeline mismatch.")] + #[diagnostic(code(nu::shell::pipeline_mismatch), url(docsrs))] + OnlySupportsThisInputType( + String, + String, + #[label("only {0} input data is supported")] Span, + #[label("input type: {1}")] Span, + ), + + /// No input value was piped into the command. /// /// ## Resolution /// - /// Convert the argument type before passing it in, or change the command to accept the type. - #[error("Type mismatch")] - #[diagnostic(code(nu::shell::type_mismatch), url(docsrs))] - TypeMismatch(String, #[label = "needs {0}"] Span), + /// Only use this command to process values from a previous expression. + #[error("Pipeline empty.")] + #[diagnostic(code(nu::shell::pipeline_mismatch), url(docsrs))] + PipelineEmpty(#[label("no input value was piped in")] Span), /// A command received an argument of the wrong type. /// /// ## Resolution /// /// Convert the argument type before passing it in, or change the command to accept the type. - #[error("Type mismatch")] + #[error("Type mismatch.")] + #[diagnostic(code(nu::shell::type_mismatch), url(docsrs))] + TypeMismatch(String, #[label = "{0}"] Span), + + /// A command received an argument of the wrong type. + /// + /// ## Resolution + /// + /// Convert the argument type before passing it in, or change the command to accept the type. + #[error("Type mismatch.")] #[diagnostic(code(nu::shell::type_mismatch), url(docsrs))] TypeMismatchGenericMessage { err_message: String, @@ -469,7 +487,12 @@ Either make sure {0} is a string, or add a 'to_string' entry for it in ENV_CONVE /// This error is fairly generic. Refer to the specific error message for further details. #[error("Unsupported input")] #[diagnostic(code(nu::shell::unsupported_input), url(docsrs))] - UnsupportedInput(String, #[label("{0}")] Span), + UnsupportedInput( + String, + String, + #[label("{0}")] Span, // call head (the name of the command itself) + #[label("input type: {1}")] Span, + ), /// Failed to parse an input into a datetime value. /// @@ -785,7 +808,7 @@ Either make sure {0} is a string, or add a 'to_string' entry for it in ENV_CONVE #[diagnostic(code(nu::shell::missing_config_value), url(docsrs))] MissingConfigValue(String, #[label = "missing {0}"] Span), - /// Negative value passed when positive ons is required. + /// Negative value passed when positive one is required. /// /// ## Resolution /// diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 24cc506340..b744aafd1c 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -352,6 +352,13 @@ impl Value { } } + /// Special variant of the above designed to be called only in + /// situations where the value not being a Value::Error has been guaranteed + /// by match arms. + pub fn expect_span(&self) -> Span { + self.span().expect("non-Error Value had no span") + } + /// Update the value with a new span pub fn with_span(mut self, new_span: Span) -> Value { match &mut self { @@ -1180,6 +1187,7 @@ impl Value { &mut self, cell_path: &[PathMember], new_val: Value, + head_span: Span, ) -> Result<(), ShellError> { match cell_path.first() { Some(path_member) => match path_member { @@ -1207,6 +1215,7 @@ impl Value { return col.1.insert_data_at_cell_path( &cell_path[1..], new_val, + head_span, ); } } @@ -1215,9 +1224,13 @@ impl Value { cols.push(col_name.clone()); vals.push(new_val.clone()); } + // SIGH... + Value::Error { error } => return Err(error.clone()), _ => { return Err(ShellError::UnsupportedInput( - "table or record".into(), + "expected table or record".into(), + format!("input type: {:?}", val.get_type()), + head_span, *span, )) } @@ -1238,9 +1251,11 @@ impl Value { *v_span, )); } else { - return col - .1 - .insert_data_at_cell_path(&cell_path[1..], new_val); + return col.1.insert_data_at_cell_path( + &cell_path[1..], + new_val, + head_span, + ); } } } @@ -1248,9 +1263,11 @@ impl Value { cols.push(col_name.clone()); vals.push(new_val); } - _ => { + other => { return Err(ShellError::UnsupportedInput( "table or record".into(), + format!("input type: {:?}", other.get_type()), + head_span, *span, )) } @@ -1258,7 +1275,7 @@ impl Value { 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)? + v.insert_data_at_cell_path(&cell_path[1..], new_val, head_span)? } else if vals.len() == *row_num && cell_path.len() == 1 { // If the insert is at 1 + the end of the list, it's OK. // Otherwise, it's prohibited. @@ -2626,8 +2643,14 @@ impl Value { // We are leaving some performance on the table by compiling the regex every time. // Small regexes compile in microseconds, and the simplicity of this approach currently // outweighs the performance costs. Revisit this if it ever becomes a bottleneck. - let regex = Regex::new(rhs) - .map_err(|e| ShellError::UnsupportedInput(format!("{e}"), *rhs_span))?; + let regex = Regex::new(rhs).map_err(|e| { + ShellError::UnsupportedInput( + format!("{e}"), + "value originated from here".into(), + span, + *rhs_span, + ) + })?; let is_match = regex.is_match(lhs); Ok(Value::Bool { val: if invert {