holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

improve operation mismatch errors (#8800)

Browse Source 
# Description

This improves the operation mismatch error in a few ways:

* We now detect if the left-hand side of the operation is at fault, and
show a simpler error/error message if it is
* Removed the unhelpful hint
* Updated the error text to make it clear what types are causing the
issue


![image](https://user-images.githubusercontent.com/547158/230666329-537a8cae-6350-4ee7-878e-777e05c4f265.png)


![image](https://user-images.githubusercontent.com/547158/230666353-93529dc2-039a-4774-a84c-a6faac94d8e2.png)


# User-Facing Changes

Error texts and spans will change

# 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
- `cargo run -- crates/nu-utils/standard_library/tests.nu` to run the
tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```

# 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.
This commit is contained in:
JT 2023-04-08 09:32:44 +12:00 committed by GitHub
parent 35e8420780
commit f2b977b9c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 316 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
benches/benchmarks.rs
View File

@ -22,7 +22,7 @@ fn parser_benchmarks(c: &mut Criterion) {
c.bench_function("parse_default_env_file", |b| {
b.iter_batched(
|| nu_protocol::engine::StateWorkingSet::new(&engine_state),
|mut working_set| parse(&mut working_set, None, default_env, false, &[]),
|mut working_set| parse(&mut working_set, None, default_env, false),
BatchSize::SmallInput,
)
});
@ -31,7 +31,7 @@ fn parser_benchmarks(c: &mut Criterion) {
c.bench_function("parse_default_config_file", |b| {
b.iter_batched(
|| nu_protocol::engine::StateWorkingSet::new(&engine_state),
|mut working_set| parse(&mut working_set, None, default_config, false, &[]),
|mut working_set| parse(&mut working_set, None, default_config, false),
BatchSize::SmallInput,
)
});

335
crates/nu-parser/src/type_check.rs
View File

@ -55,17 +55,25 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Any, None),
(_, Type::Any) => (Type::Any, None),
(Type::Int, _) => {
let ty = rhs.ty.clone();
*rhs = Expression::garbage(rhs.span);
(
Type::Int
| Type::Float
| Type::String
| Type::Date
| Type::Duration
| Type::Filesize,
_,
) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"addition".into(),
op.span,
lhs.span,
lhs.ty.clone(),
rhs.span,
ty,
rhs.ty.clone(),
)),
)
}
@ -73,12 +81,11 @@ pub fn math_result_type(
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationLHS(
"addition".into(),
op.span,
lhs.span,
lhs.ty.clone(),
rhs.span,
rhs.ty.clone(),
)),
)
}
@ -102,11 +109,12 @@ pub fn math_result_type(
(Type::String, Type::String) => (Type::String, None),
(Type::Binary, Type::Binary) => (Type::Binary, None),
(Type::Any, _) | (_, Type::Any) => (Type::Any, None),
_ => {
(Type::Table(_) | Type::String | Type::Binary, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"append".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -115,6 +123,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"append".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Math(Math::Minus) => match (&lhs.ty, &rhs.ty) {
(Type::Int, Type::Int) => (Type::Int, None),
@ -131,11 +151,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Any, None),
(_, Type::Any) => (Type::Any, None),
_ => {
(Type::Int | Type::Float | Type::Date | Type::Duration | Type::Filesize, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"subtraction".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -144,6 +165,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"subtraction".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Math(Math::Multiply) => match (&lhs.ty, &rhs.ty) {
(Type::Int, Type::Int) => (Type::Int, None),
@ -168,11 +201,18 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Any, None),
(_, Type::Any) => (Type::Any, None),
_ => {
(Type::Int, _)
| (Type::Float, _)
| (Type::String, _)
| (Type::Date, _)
| (Type::Duration, _)
| (Type::Filesize, _)
| (Type::List(_), _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"multiplication".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -181,6 +221,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"multiplication".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Math(Math::Pow) => match (&lhs.ty, &rhs.ty) {
(Type::Int, Type::Int) => (Type::Int, None),
@ -193,11 +245,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Any, None),
(_, Type::Any) => (Type::Any, None),
_ => {
(Type::Int | Type::Float, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"exponentiation".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -206,6 +259,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"exponentiation".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Math(Math::Divide) | Operator::Math(Math::Modulo) => match (&lhs.ty, &rhs.ty)
{
@ -225,11 +290,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Any, None),
(_, Type::Any) => (Type::Any, None),
_ => {
(Type::Int | Type::Float | Type::Filesize | Type::Duration, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"division".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -238,6 +304,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"division".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Math(Math::FloorDivision) => match (&lhs.ty, &rhs.ty) {
(Type::Int, Type::Int) => (Type::Int, None),
@ -253,11 +331,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Any, None),
(_, Type::Any) => (Type::Any, None),
_ => {
(Type::Int | Type::Float | Type::Filesize | Type::Duration, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"floor division".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -266,6 +345,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"floor division".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Boolean(Boolean::And)
| Operator::Boolean(Boolean::Or)
@ -284,11 +375,12 @@ pub fn math_result_type(
// FIX ME. This is added because there is no type output for custom function
// definitions. As soon as that syntax is added this should be removed
(a, b) if a == b => (Type::Bool, None),
_ => {
(Type::Bool, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"boolean operation".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -297,6 +389,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"boolean operation".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
}
}
Operator::Comparison(Comparison::LessThan) => match (&lhs.ty, &rhs.ty) {
@ -315,11 +419,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Bool, None),
(_, Type::Any) => (Type::Bool, None),
_ => {
(Type::Int | Type::Float | Type::Duration | Type::Filesize, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"less-than comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -328,6 +433,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"less-than comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::LessThanOrEqual) => match (&lhs.ty, &rhs.ty) {
(Type::Int, Type::Int) => (Type::Bool, None),
@ -345,11 +462,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Bool, None),
(_, Type::Any) => (Type::Bool, None),
_ => {
(Type::Int | Type::Float | Type::Duration | Type::Filesize, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"less-than or equal comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -358,6 +476,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"less-than or equal comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::GreaterThan) => match (&lhs.ty, &rhs.ty) {
(Type::Int, Type::Int) => (Type::Bool, None),
@ -375,11 +505,12 @@ pub fn math_result_type(
(Type::Nothing, _) => (Type::Nothing, None),
(_, Type::Nothing) => (Type::Nothing, None),
_ => {
(Type::Int | Type::Float | Type::Duration | Type::Filesize, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"greater-than comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -388,6 +519,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"greater-than comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::GreaterThanOrEqual) => match (&lhs.ty, &rhs.ty) {
(Type::Int, Type::Int) => (Type::Bool, None),
@ -405,11 +548,12 @@ pub fn math_result_type(
(Type::Nothing, _) => (Type::Nothing, None),
(_, Type::Nothing) => (Type::Nothing, None),
_ => {
(Type::Int | Type::Float | Type::Duration | Type::Filesize, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"greater-than or equal comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -418,6 +562,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"greater-than or equal comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::Equal) => match (&lhs.ty, &rhs.ty) {
(Type::Custom(a), Type::Custom(b)) if a == b => (Type::Custom(a.to_string()), None),
@ -439,11 +595,12 @@ pub fn math_result_type(
(Type::Custom(a), Type::Custom(b)) if a == b => (Type::Custom(a.to_string()), None),
(Type::Custom(a), _) => (Type::Custom(a.to_string()), None),
_ => {
(Type::String, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"regex matching".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -452,6 +609,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"regex matching".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::NotRegexMatch) => match (&lhs.ty, &rhs.ty) {
(Type::String, Type::String) => (Type::Bool, None),
@ -461,11 +630,12 @@ pub fn math_result_type(
(Type::Custom(a), Type::Custom(b)) if a == b => (Type::Custom(a.to_string()), None),
(Type::Custom(a), _) => (Type::Custom(a.to_string()), None),
_ => {
(Type::String, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"regex matching".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -474,6 +644,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"regex matching".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::StartsWith) => match (&lhs.ty, &rhs.ty) {
(Type::String, Type::String) => (Type::Bool, None),
@ -483,11 +665,12 @@ pub fn math_result_type(
(Type::Custom(a), Type::Custom(b)) if a == b => (Type::Custom(a.to_string()), None),
(Type::Custom(a), _) => (Type::Custom(a.to_string()), None),
_ => {
(Type::String, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"starts-with comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -496,6 +679,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"starts-with comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::EndsWith) => match (&lhs.ty, &rhs.ty) {
(Type::String, Type::String) => (Type::Bool, None),
@ -505,11 +700,12 @@ pub fn math_result_type(
(Type::Custom(a), Type::Custom(b)) if a == b => (Type::Custom(a.to_string()), None),
(Type::Custom(a), _) => (Type::Custom(a.to_string()), None),
_ => {
(Type::String, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"ends-with comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -518,6 +714,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"ends-with comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::In) => match (&lhs.ty, &rhs.ty) {
(t, Type::List(u)) if type_compatible(t, u) => (Type::Bool, None),
@ -530,11 +738,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Bool, None),
(_, Type::Any) => (Type::Bool, None),
_ => {
(Type::Int | Type::Float | Type::String, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"subset comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -543,6 +752,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"subset comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Comparison(Comparison::NotIn) => match (&lhs.ty, &rhs.ty) {
(t, Type::List(u)) if type_compatible(t, u) => (Type::Bool, None),
@ -555,11 +776,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Bool, None),
(_, Type::Any) => (Type::Bool, None),
_ => {
(Type::Int | Type::Float | Type::String, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"subset comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -568,6 +790,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"subset comparison".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Bits(Bits::ShiftLeft)
| Operator::Bits(Bits::ShiftRight)
@ -578,11 +812,12 @@ pub fn math_result_type(
(Type::Any, _) => (Type::Any, None),
(_, Type::Any) => (Type::Any, None),
_ => {
(Type::Int, _) => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperation(
Some(ParseError::UnsupportedOperationRHS(
"bit operations".into(),
op.span,
lhs.span,
lhs.ty.clone(),
@ -591,6 +826,18 @@ pub fn math_result_type(
)),
)
}
_ => {
*op = Expression::garbage(op.span);
(
Type::Any,
Some(ParseError::UnsupportedOperationLHS(
"bit operations".into(),
op.span,
lhs.span,
lhs.ty.clone(),
)),
)
}
},
Operator::Assignment(_) => match (&lhs.ty, &rhs.ty) {
(x, y) if x == y => (Type::Nothing, None),

28
crates/nu-protocol/src/parse_error.rs
View File

@ -74,16 +74,23 @@ pub enum ParseError {
)]
ShellOutErrRedirect(#[label("use 'out+err>' instead of '2>&1' in Nushell")] Span),
#[error("Types mismatched for operation.")]
#[diagnostic(
code(nu::parser::unsupported_operation),
help("Change {2} or {4} to be the right types and try again.")
)]
UnsupportedOperation(
#[label = "doesn't support these values."] Span,
#[label("{2}")] Span,
#[error("{0} is not supported on values of type {3}")]
#[diagnostic(code(nu::parser::unsupported_operation))]
UnsupportedOperationLHS(
String,
#[label = "doesn't support this value"] Span,
#[label("{3}")] Span,
Type,
#[label("{4}")] Span,
),
#[error("{0} is not supported between {3} and {5}.")]
#[diagnostic(code(nu::parser::unsupported_operation))]
UnsupportedOperationRHS(
String,
#[label = "doesn't support these values"] Span,
#[label("{3}")] Span,
Type,
#[label("{5}")] Span,
Type,
),
@ -437,7 +444,8 @@ impl ParseError {
ParseError::Unbalanced(_, _, s) => *s,
ParseError::Expected(_, s) => *s,
ParseError::Mismatch(_, _, s) => *s,
ParseError::UnsupportedOperation(_, _, _, s, _) => *s,
ParseError::UnsupportedOperationLHS(_, _, s, _) => *s,
ParseError::UnsupportedOperationRHS(_, _, _, _, s, _) => *s,
ParseError::ExpectedKeyword(_, s) => *s,
ParseError::UnexpectedKeyword(_, s) => *s,
ParseError::CantAliasKeyword(_, s) => *s,

2
src/tests/test_ranges.rs
View File

@ -17,7 +17,7 @@ fn int_in_exclusive_range() -> TestResult {
#[test]
fn non_number_in_range() -> TestResult {
fail_test(r#"'a' in 1..3"#, "mismatched for operation")
fail_test(r#"'a' in 1..3"#, "subset comparison is not supported")
}
#[test]

4
src/tests/test_regex.rs
View File

@ -73,10 +73,10 @@ fn invalid_not_regex_fails() -> TestResult {
#[test]
fn regex_on_int_fails() -> TestResult {
fail_test(r#"33 =~ foo"#, "Types mismatched")
fail_test(r#"33 =~ foo"#, "is not supported")
}
#[test]
fn not_regex_on_int_fails() -> TestResult {
fail_test(r#"33 !~ foo"#, "Types mismatched")
fail_test(r#"33 !~ foo"#, "is not supported")
}

2
src/tests/test_strings.rs
View File

@ -20,7 +20,7 @@ fn string_in_string() -> TestResult {
#[test]
fn non_string_in_string() -> TestResult {
fail_test(r#"42 in 'abc'"#, "mismatched for operation")
fail_test(r#"42 in 'abc'"#, "is not supported")
}
#[test]

2
src/tests/test_type_check.rs
View File

@ -12,7 +12,7 @@ fn type_in_list_of_this_type() -> TestResult {
#[test]
fn type_in_list_of_non_this_type() -> TestResult {
fail_test(r#"'hello' in [41 42 43]"#, "mismatched for operation")
fail_test(r#"'hello' in [41 42 43]"#, "is not supported")
}
#[test]