Skip to content

Commit e00dae1

Browse files
authored
Fix cryptic parser error (#5731)
# Problem Before: "unexpected token |>", highlights the |> After: "This argument needs a label, but it doesn't have one", highlights the argument with no label Closes #5724 # Discussion I am trying a new approach to the parser: instead of parsing the specific correct thing we need, parse a more general form, then later, narrow it down to specifics and return a nice error if it's wrong. For example, instead of parsing labeled arguments, parse labeled OR unlabeled arguments. Then later check that the only unlabeled arg is the first one, and return a nice error if there's any other. This worked nicely for this PR, hopefully the approach will work for other "cryptic error" issues.
1 parent 831c7f7 commit e00dae1

5 files changed

+44
-7
lines changed
-31.1 KB
Loading
-954 Bytes
Loading
-1.25 KB
Loading
-111 Bytes
Loading

rust/kcl-lib/src/parsing/parser.rs

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2809,29 +2809,45 @@ fn fn_call_kw(i: &mut TokenSlice) -> PResult<Node<CallExpressionKw>> {
28092809
let _ = open_paren.parse_next(i)?;
28102810
ignore_whitespace(i);
28112811

2812+
#[allow(clippy::large_enum_variant)]
2813+
pub enum ArgPlace {
2814+
NonCode(Node<NonCodeNode>),
2815+
LabeledArg(LabeledArg),
2816+
UnlabeledArg(Expr),
2817+
}
28122818
let initial_unlabeled_arg = opt((expression, comma, opt(whitespace)).map(|(arg, _, _)| arg)).parse_next(i)?;
28132819
let args: Vec<_> = repeat(
28142820
0..,
28152821
alt((
2816-
terminated(non_code_node.map(NonCodeOr::NonCode), whitespace),
2817-
terminated(labeled_argument, labeled_arg_separator).map(NonCodeOr::Code),
2822+
terminated(non_code_node.map(ArgPlace::NonCode), whitespace),
2823+
terminated(labeled_argument, labeled_arg_separator).map(ArgPlace::LabeledArg),
2824+
expression.map(ArgPlace::UnlabeledArg),
28182825
)),
28192826
)
28202827
.parse_next(i)?;
2821-
let (args, non_code_nodes): (Vec<_>, BTreeMap<usize, _>) = args.into_iter().enumerate().fold(
2828+
let (args, non_code_nodes): (Vec<_>, BTreeMap<usize, _>) = args.into_iter().enumerate().try_fold(
28222829
(Vec::new(), BTreeMap::new()),
28232830
|(mut args, mut non_code_nodes), (i, e)| {
28242831
match e {
2825-
NonCodeOr::NonCode(x) => {
2832+
ArgPlace::NonCode(x) => {
28262833
non_code_nodes.insert(i, vec![x]);
28272834
}
2828-
NonCodeOr::Code(x) => {
2835+
ArgPlace::LabeledArg(x) => {
28292836
args.push(x);
28302837
}
2838+
ArgPlace::UnlabeledArg(arg) => {
2839+
return Err(ErrMode::Cut(
2840+
CompilationError::fatal(
2841+
SourceRange::from(arg),
2842+
"This argument needs a label, but it doesn't have one",
2843+
)
2844+
.into(),
2845+
));
2846+
}
28312847
}
2832-
(args, non_code_nodes)
2848+
Ok((args, non_code_nodes))
28332849
},
2834-
);
2850+
)?;
28352851
if let Some(std_fn) = crate::std::get_stdlib_fn(&fn_name.name) {
28362852
let just_args: Vec<_> = args.iter().collect();
28372853
typecheck_all_kw(std_fn, &just_args)?;
@@ -4641,6 +4657,27 @@ baz = 2
46414657
assert_eq!(actual.operator, UnaryOperator::Not);
46424658
crate::parsing::top_level_parse(some_program_string).unwrap(); // Updated import path
46434659
}
4660+
4661+
#[test]
4662+
fn test_sensible_error_when_missing_equals_in_kwarg() {
4663+
for (i, program) in ["f(x=1,y)", "f(x=1,y,z)", "f(x=1,y,z=1)", "f(x=1, y, z=1)"]
4664+
.into_iter()
4665+
.enumerate()
4666+
{
4667+
let tokens = crate::parsing::token::lex(program, ModuleId::default()).unwrap();
4668+
let err = fn_call_kw.parse(tokens.as_slice()).unwrap_err();
4669+
let cause = err.inner().cause.as_ref().unwrap();
4670+
assert_eq!(
4671+
cause.message, "This argument needs a label, but it doesn't have one",
4672+
"failed test {i}: {program}"
4673+
);
4674+
assert_eq!(
4675+
cause.source_range.start(),
4676+
program.find("y").unwrap(),
4677+
"failed test {i}: {program}"
4678+
);
4679+
}
4680+
}
46444681
}
46454682

46464683
#[cfg(test)]

0 commit comments

Comments
 (0)