mirror of
https://github.com/nushell/nushell
synced 2025-01-12 21:29:07 +00:00
return accurate type errors from blocks/expressions in type unions (#14420)
# User-Facing Changes - `expected <type>` errors are now propagated from `Closure | Block | Expression` instead of falling back to "expected one of..." for the block: Before: ```nushell def foo [bar: bool] {} if true {} else { foo 1 } ────┬──── ╰── expected one of a list of accepted shapes: [Block, Expression] ``` After: ```nushell if true {} else { foo 1 } ┬ ╰── expected bool ```
This commit is contained in:
parent
671640b0a9
commit
7c84634e3f
2 changed files with 67 additions and 70 deletions
|
@ -772,6 +772,51 @@ fn calculate_end_span(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn parse_oneof(
|
||||||
|
working_set: &mut StateWorkingSet,
|
||||||
|
spans: &[Span],
|
||||||
|
spans_idx: &mut usize,
|
||||||
|
possible_shapes: &Vec<SyntaxShape>,
|
||||||
|
multispan: bool,
|
||||||
|
) -> Expression {
|
||||||
|
for shape in possible_shapes {
|
||||||
|
let starting_error_count = working_set.parse_errors.len();
|
||||||
|
let value = match multispan {
|
||||||
|
true => parse_multispan_value(working_set, spans, spans_idx, shape),
|
||||||
|
false => parse_value(working_set, spans[*spans_idx], shape),
|
||||||
|
};
|
||||||
|
|
||||||
|
if starting_error_count == working_set.parse_errors.len() {
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
|
// while trying the possible shapes, ignore Expected type errors
|
||||||
|
// unless they're inside a block, closure, or expression
|
||||||
|
let propagate_error = match working_set.parse_errors.last() {
|
||||||
|
Some(ParseError::Expected(_, error_span))
|
||||||
|
| Some(ParseError::ExpectedWithStringMsg(_, error_span)) => {
|
||||||
|
matches!(
|
||||||
|
shape,
|
||||||
|
SyntaxShape::Block | SyntaxShape::Closure(_) | SyntaxShape::Expression
|
||||||
|
) && *error_span != spans[*spans_idx]
|
||||||
|
}
|
||||||
|
_ => true,
|
||||||
|
};
|
||||||
|
if !propagate_error {
|
||||||
|
working_set.parse_errors.truncate(starting_error_count);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if working_set.parse_errors.is_empty() {
|
||||||
|
working_set.error(ParseError::ExpectedWithStringMsg(
|
||||||
|
format!("one of a list of accepted shapes: {possible_shapes:?}"),
|
||||||
|
spans[*spans_idx],
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
Expression::garbage(working_set, spans[*spans_idx])
|
||||||
|
}
|
||||||
|
|
||||||
pub fn parse_multispan_value(
|
pub fn parse_multispan_value(
|
||||||
working_set: &mut StateWorkingSet,
|
working_set: &mut StateWorkingSet,
|
||||||
spans: &[Span],
|
spans: &[Span],
|
||||||
|
@ -800,54 +845,10 @@ pub fn parse_multispan_value(
|
||||||
|
|
||||||
arg
|
arg
|
||||||
}
|
}
|
||||||
SyntaxShape::OneOf(shapes) => {
|
SyntaxShape::OneOf(possible_shapes) => {
|
||||||
// handle for `if` command.
|
parse_oneof(working_set, spans, spans_idx, possible_shapes, true)
|
||||||
//let block_then_exp = shapes.as_slice() == [SyntaxShape::Block, SyntaxShape::Expression];
|
|
||||||
for shape in shapes.iter() {
|
|
||||||
let starting_error_count = working_set.parse_errors.len();
|
|
||||||
let s = parse_multispan_value(working_set, spans, spans_idx, shape);
|
|
||||||
|
|
||||||
if starting_error_count == working_set.parse_errors.len() {
|
|
||||||
return s;
|
|
||||||
} else if let Some(
|
|
||||||
ParseError::Expected(..) | ParseError::ExpectedWithStringMsg(..),
|
|
||||||
) = working_set.parse_errors.last()
|
|
||||||
{
|
|
||||||
working_set.parse_errors.truncate(starting_error_count);
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
// `if` is parsing block first and then expression.
|
|
||||||
// when we're writing something like `else if $a`, parsing as a
|
|
||||||
// block will result to error(because it's not a block)
|
|
||||||
//
|
|
||||||
// If parse as a expression also failed, user is more likely concerned
|
|
||||||
// about expression failure rather than "expect block failure"".
|
|
||||||
|
|
||||||
// FIXME FIXME FIXME
|
|
||||||
// if block_then_exp {
|
|
||||||
// match &err {
|
|
||||||
// Some(ParseError::Expected(expected, _)) => {
|
|
||||||
// if expected.starts_with("block") {
|
|
||||||
// err = e
|
|
||||||
// }
|
|
||||||
// }
|
|
||||||
// _ => err = err.or(e),
|
|
||||||
// }
|
|
||||||
// } else {
|
|
||||||
// err = err.or(e)
|
|
||||||
// }
|
|
||||||
}
|
|
||||||
let span = spans[*spans_idx];
|
|
||||||
|
|
||||||
if working_set.parse_errors.is_empty() {
|
|
||||||
working_set.error(ParseError::ExpectedWithStringMsg(
|
|
||||||
format!("one of a list of accepted shapes: {shapes:?}"),
|
|
||||||
span,
|
|
||||||
));
|
|
||||||
}
|
|
||||||
|
|
||||||
Expression::garbage(working_set, span)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
SyntaxShape::Expression => {
|
SyntaxShape::Expression => {
|
||||||
trace!("parsing: expression");
|
trace!("parsing: expression");
|
||||||
|
|
||||||
|
@ -4826,29 +4827,7 @@ pub fn parse_value(
|
||||||
|
|
||||||
SyntaxShape::ExternalArgument => parse_regular_external_arg(working_set, span),
|
SyntaxShape::ExternalArgument => parse_regular_external_arg(working_set, span),
|
||||||
SyntaxShape::OneOf(possible_shapes) => {
|
SyntaxShape::OneOf(possible_shapes) => {
|
||||||
for s in possible_shapes {
|
parse_oneof(working_set, &[span], &mut 0, possible_shapes, false)
|
||||||
let starting_error_count = working_set.parse_errors.len();
|
|
||||||
let value = parse_value(working_set, span, s);
|
|
||||||
|
|
||||||
if starting_error_count == working_set.parse_errors.len() {
|
|
||||||
return value;
|
|
||||||
} else if let Some(
|
|
||||||
ParseError::Expected(..) | ParseError::ExpectedWithStringMsg(..),
|
|
||||||
) = working_set.parse_errors.last()
|
|
||||||
{
|
|
||||||
working_set.parse_errors.truncate(starting_error_count);
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if working_set.parse_errors.is_empty() {
|
|
||||||
working_set.error(ParseError::ExpectedWithStringMsg(
|
|
||||||
format!("one of a list of accepted shapes: {possible_shapes:?}"),
|
|
||||||
span,
|
|
||||||
));
|
|
||||||
}
|
|
||||||
|
|
||||||
Expression::garbage(working_set, span)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
SyntaxShape::Any => {
|
SyntaxShape::Any => {
|
||||||
|
|
|
@ -1,4 +1,5 @@
|
||||||
use crate::repl::tests::{fail_test, run_test, TestResult};
|
use crate::repl::tests::{fail_test, run_test, TestResult};
|
||||||
|
use rstest::rstest;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn chained_operator_typecheck() -> TestResult {
|
fn chained_operator_typecheck() -> TestResult {
|
||||||
|
@ -154,3 +155,20 @@ fn in_variable_expression_wrong_output_type() -> TestResult {
|
||||||
"expected int",
|
"expected int",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[rstest]
|
||||||
|
#[case("if true {} else { foo 1 }")]
|
||||||
|
#[case("if true {} else if (foo 1) == null { }")]
|
||||||
|
#[case("match 1 { 0 => { foo 1 } }")]
|
||||||
|
#[case("try { } catch { foo 1 }")]
|
||||||
|
/// type errors should propagate from `OneOf(Block | Closure | Expression, ..)`
|
||||||
|
fn in_oneof_block_expected_type(#[case] input: &str) -> TestResult {
|
||||||
|
let def = "def foo [bar: bool] {};";
|
||||||
|
|
||||||
|
fail_test(&format!("{def} {input}"), "expected bool")
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn in_oneof_block_expected_block() -> TestResult {
|
||||||
|
fail_test("match 1 { 0 => { try 3 } }", "expected block")
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue