Parsing function arguments with a combination of positional and keyword arguments

[jimmycuadra]

I'm trying to parse a grammar with functions that take zero or more positional arguments followed by zero or more keyword arguments. Examples:

fn a()
fn a(b)
fn a(b, c)
fn a(b: c)
fn a(b: c, d: e)
fn a(b, c, d: e, f: g)

I feel like I've figured out much more difficult parsers than this, but for some reason everything I've tried has ended up with the : for the first keyword argument being unexpected by the parser. I'm probably missing something obvious...

Here's a minimal reproducible example. This has two parsers that I've tried, one where the two types of arguments are chained with then in series, and one where a choice combinator tries all three possible combinations explicitly. (The three combinations being: only positional, only keyword, and both.)

What am I overlooking?

#![allow(dead_code)]

use chumsky::extra::Err;
use chumsky::prelude::*;

fn simple_parser<'a>()
-> impl Parser<'a, &'a str, (&'a str, Vec<&'a str>, Vec<(&'a str, &'a str)>), Err<Rich<'a, char>>> {
    just("fn")
        .padded()
        .ignore_then(text::ascii::ident())
        .then_ignore(just('('))
        .then(
            text::ascii::ident()
                .separated_by(just(", "))
                .collect::<Vec<_>>(),
        )
        .then_ignore(just(',').or_not())
        .then(
            text::ascii::ident()
                .then_ignore(just(": "))
                .then(text::ascii::ident())
                .separated_by(just(", "))
                .collect::<Vec<_>>(),
        )
        .then_ignore(just(')'))
        .map(|((name, args), kwargs)| (name, args, kwargs))
}

#[derive(Debug, PartialEq)]
enum Args<'a> {
    Pos(Vec<&'a str>),
    Kw(Vec<(&'a str, &'a str)>),
    Both(Vec<&'a str>, Vec<(&'a str, &'a str)>),
}

fn choice_parser<'a>()
-> impl Parser<'a, &'a str, (&'a str, Args<'a>), Err<Rich<'a, char>>> {
    let pos = text::ascii::ident()
        .separated_by(just(", "))
        .collect::<Vec<_>>();

    let kw = text::ascii::ident()
        .then_ignore(just(": "))
        .then(text::ascii::ident())
        .separated_by(just(", "))
        .collect::<Vec<_>>();

    just("fn")
        .padded()
        .ignore_then(text::ascii::ident())
        .then_ignore(just('('))
        .then(choice((
            pos.clone().map(Args::Pos),
            kw.clone().map(Args::Kw),
            pos.then_ignore(just(", ")).then(kw).map(|(pos, kw)| Args::Both(pos, kw)),
        )))
        .then_ignore(just(')'))
}

#[cfg(test)]
mod simple_parser_tests {
    use super::*;

    #[test]
    fn no_args() {
        assert_eq!(
            simple_parser().parse("fn a()").into_result(),
            Ok(("a", vec![], vec![]))
        );
    }

    #[test]
    fn one_arg() {
        assert_eq!(
            simple_parser().parse("fn a(b)").into_result(),
            Ok(("a", vec!["b"], vec![]))
        );
    }

    #[test]
    fn two_args() {
        assert_eq!(
            simple_parser().parse("fn a(b, c)").into_result(),
            Ok(("a", vec!["b", "c"], vec![]))
        );
    }

    #[test]
    fn one_kwarg() {
        assert_eq!(
            simple_parser().parse("fn a(b: c)").into_result(),
            Ok(("a", vec![], vec![("b", "c")]))
        );
    }

    #[test]
    fn two_kwargs() {
        assert_eq!(
            simple_parser().parse("fn a(b: c, d: e)").into_result(),
            Ok(("a", vec![], vec![("b", "c"), ("d", "e")]))
        );
    }

    #[test]
    fn both_arg_types() {
        assert_eq!(
            simple_parser().parse("fn a(b, c, d: e, f: g)").into_result(),
            Ok(("a", vec!["b", "c"], vec![("d", "e"), ("f", "g")]))
        );
    }
}

#[cfg(test)]
mod choice_parser_tests {
    use super::*;

    #[test]
    fn no_args() {
        assert_eq!(
            choice_parser().parse("fn a()").into_result(),
            Ok(("a", Args::Pos(vec![])))
        );
    }

    #[test]
    fn one_arg() {
        assert_eq!(
            choice_parser().parse("fn a(b)").into_result(),
            Ok(("a", Args::Pos(vec!["b"])))
        );
    }

    #[test]
    fn two_args() {
        assert_eq!(
            choice_parser().parse("fn a(b, c)").into_result(),
            Ok(("a", Args::Pos(vec!["b", "c"])))
        );
    }

    #[test]
    fn one_kwarg() {
        assert_eq!(
            choice_parser().parse("fn a(b: c)").into_result(),
            Ok(("a", Args::Kw(vec![("b", "c")])))
        );
    }

    #[test]
    fn two_kwargs() {
        assert_eq!(
            choice_parser().parse("fn a(b: c, d: e)").into_result(),
            Ok(("a", Args::Kw(vec![("b", "c"), ("d", "e")])))
        );
    }

    #[test]
    fn both_arg_types() {
        assert_eq!(
            choice_parser().parse("fn a(b, c, d: e, f: g)").into_result(),
            Ok(("a", Args::Both(vec!["b", "c"], vec![("d", "e"), ("f", "g")])))
        );
    }
}

[zesterer]

I'm a bit confused as to why you're bifurcating your argument parser when you want to accept both positional and keyword args in the same set of arguments. Perhaps something like this might suit you better?

// ---- Types

enum Arg<'src> {
    Pos(&'src str),
    Kw(&'src str, &'src str),
}
struct Args<'src>(Vec<Arg<'src>);

// ---- In the parser

let pos = text::ascii::ident().map(Arg::Pos);

let kw = text::ascii::ident()
    .then_ignore(just(": "))
    .then(text::ascii::ident())
    .map(|(lhs, rhs)| Arg::Kw(lhs, rhs));

just("fn")
    .padded()
    .ignore_then(text::ascii::ident())
    .then_ignore(just('('))
    .then(choice((kw, pos))
        .separated_by(", ")
        .collect()
        .map(Args))
    .then_ignore(just(')'))

Just as an additional side note: you might want to start making use of .padded() so that your parser can properly handle whitespace.

[jimmycuadra]

I've been successful using something similar to the second approach for parsing function definitions in my real program, but I still can't get function calls to work. I'm super close, though! Here's my latest minimal reproduction:

#![allow(dead_code)]

use chumsky::extra::Err;
use chumsky::prelude::*;

#[derive(Debug, PartialEq)]
enum Args<'a> {
    Pos(Vec<&'a str>),
    Kw(Vec<(&'a str, &'a str)>),
    Both(Vec<&'a str>, Vec<(&'a str, &'a str)>),
}

fn parser<'a>() -> impl Parser<'a, &'a str, (&'a str, Args<'a>), Err<Rich<'a, char>>> {
    let pos = text::ascii::ident().separated_by(just(',').padded());

    let kw = text::ascii::ident()
        .then_ignore(just(':').padded())
        .then(text::ascii::ident())
        .separated_by(just(',').padded())
        .at_least(1)
        .collect::<Vec<_>>();

    let both = pos
        .clone()
        .at_least(1)
        .collect::<Vec<_>>()
        .then_ignore(just(',').padded()) // Ambiguous delimiter
        .then(kw.clone())
        .map(|(pos, kw)| Args::Both(pos, kw));

    text::ascii::ident().then(
        choice((
            both,
            kw.map(Args::Kw),
            pos.collect::<Vec<_>>().map(Args::Pos),
        ))
        .delimited_by(just('(').padded(), just(')').padded()),
    )
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn no_args() {
        assert_eq!(
            parser().parse("a()").into_result(),
            Ok(("a", Args::Pos(vec![])))
        );
    }

    #[test]
    fn one_pos() {
        assert_eq!(
            parser().parse("a(b)").into_result(),
            Ok(("a", Args::Pos(vec!["b"])))
        );
    }

    #[test]
    fn two_pos() {
        assert_eq!(
            parser().parse("a(b, c)").into_result(),
            Ok(("a", Args::Pos(vec!["b", "c"])))
        );
    }

    #[test]
    fn one_kw() {
        assert_eq!(
            parser().parse("a(b: c)").into_result(),
            Ok(("a", Args::Kw(vec![("b", "c")])))
        );
    }

    #[test]
    fn two_kw() {
        assert_eq!(
            parser().parse("a(b: c, d: e)").into_result(),
            Ok(("a", Args::Kw(vec![("b", "c"), ("d", "e")])))
        );
    }

    #[test]
    fn one_each() {
        assert_eq!(
            parser().parse("a(b, c: d)").into_result(),
            Ok(("a", Args::Both(vec!["b"], vec![("c", "d")])))
        );
    }

    #[test]
    fn two_each() {
        assert_eq!(
            parser().parse("a(b, c, d: e, f: g)").into_result(),
            Ok((
                "a",
                Args::Both(vec!["b", "c"], vec![("d", "e"), ("f", "g")])
            ))
        );
    }
}

That passes every test case except the ones that contain both types of arguments, one_each and two_each. However, if I change the delimiter between the two types of args from a comma to something unambiguous, say ;, then all test cases will pass. That is:

.then_ignore(just(',').padded()) // Ambiguous delimiter

becomes:

.then_ignore(just(';').padded())

and the two relevant tests become:

#[test]
fn one_each() {
    assert_eq!(
        parser().parse("a(b; c: d)").into_result(),
        Ok(("a", Args::Both(vec!["b"], vec![("c", "d")])))
    );
}

#[test]
fn two_each() {
    assert_eq!(
        parser().parse("a(b, c; d: e, f: g)").into_result(),
        Ok((
            "a",
            Args::Both(vec!["b", "c"], vec![("d", "e"), ("f", "g")])
        ))
    );
}

So obviously, the problem is that the parser can't distinguish between a comma that means "more positional args are coming" and one that means "positional args are over, but keyword args are coming". I'm not sure how to disambiguate if I want to use a comma as the delimiter in both cases.

[Zij-IT]

Hi! Not sure I understand what you mean. What would I be using those methods for? The parsers I've tried already reject input that should be valid without further validation rejecting additional cases.

I was suggesting that the parser from zesterer be used, with an additional validation step. By using .validate(..) you allow the parsing to continue, and still catch the semantic error during parsing.

This also wouldn't have the issue that you are describing.

[jimmycuadra]

Ah, I see what you mean. Yes, that approach does work, though I find it much less elegant, since important rules of the grammar are now expressed with imperative logic, rather than with a chain of declarative combinators. Here's a version using try_map:

#![allow(dead_code)]

use chumsky::extra::Err;
use chumsky::prelude::*;

#[derive(Debug, PartialEq)]
struct Call<'a> {
    name: &'a str,
    pos_args: Vec<&'a str>,
    kw_args: Vec<(&'a str, &'a str)>,
}

#[derive(Debug, PartialEq)]
enum Arg<'a> {
    Pos(&'a str),
    Kw(&'a str, &'a str),
}

fn parser<'a>() -> impl Parser<'a, &'a str, Call<'a>, Err<Rich<'a, char>>> {
    let pos = text::ascii::ident().map(Arg::Pos);

    let kw = text::ascii::ident()
        .then_ignore(just(':').padded())
        .then(text::ascii::ident())
        .map(|(ident, expr)| Arg::Kw(ident, expr));

    text::ascii::ident()
        .then(
            choice((kw, pos))
                .separated_by(just(',').padded())
                .collect::<Vec<_>>()
                .delimited_by(just('(').padded(), just(')').padded()),
        )
        .try_map(|(name, args), span| {
            let mut pos_args = Vec::new();
            let mut kw_args = Vec::new();
            let mut kw_arg_seen = false;

            for arg in args {
                match arg {
                    Arg::Pos(expr) => {
                        if kw_arg_seen {
                            return Err(Rich::custom(
                                span,
                                "Positional arguments cannot follow keyword arguments",
                            ));
                        }

                        pos_args.push(expr);
                    }
                    Arg::Kw(ident, expr) => {
                        kw_arg_seen = true;

                        kw_args.push((ident, expr));
                    }
                }
            }

            Ok(Call {
                name,
                pos_args,
                kw_args,
            })
        })
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn no_args() {
        assert_eq!(
            parser().parse("a()").into_result(),
            Ok(Call {
                name: "a",
                pos_args: vec![],
                kw_args: vec![],
            })
        );
    }

    #[test]
    fn one_pos() {
        assert_eq!(
            parser().parse("a(b)").into_result(),
            Ok(Call {
                name: "a",
                pos_args: vec!["b"],
                kw_args: vec![],
            })
        );
    }

    #[test]
    fn two_pos() {
        assert_eq!(
            parser().parse("a(b, c)").into_result(),
            Ok(Call {
                name: "a",
                pos_args: vec!["b", "c"],
                kw_args: vec![],
            })
        );
    }

    #[test]
    fn one_kw() {
        assert_eq!(
            parser().parse("a(b: c)").into_result(),
            Ok(Call {
                name: "a",
                pos_args: vec![],
                kw_args: vec![("b", "c")],
            })
        );
    }

    #[test]
    fn two_kw() {
        assert_eq!(
            parser().parse("a(b: c, d: e)").into_result(),
            Ok(Call {
                name: "a",
                pos_args: vec![],
                kw_args: vec![("b", "c"), ("d", "e")],
            })
        );
    }

    #[test]
    fn one_each() {
        assert_eq!(
            parser().parse("a(b, c: d)").into_result(),
            Ok(Call {
                name: "a",
                pos_args: vec!["b"],
                kw_args: vec![("c", "d")],
            })
        );
    }

    #[test]
    fn two_each() {
        assert_eq!(
            parser().parse("a(b, c, d: e, f: g)").into_result(),
            Ok(Call {
                name: "a",
                pos_args: vec!["b", "c"],
                kw_args: vec![("d", "e"), ("f", "g")],
            })
        );
    }
}

I'm still wondering if there is a way to make my previous approach work, at the very least to understand Chumsky better. My understanding is that it follows a "try this, and if it fails, try the next thing" approach, so it makes sense to me that, when parsing a comma-separated sequence of positional args, if it came to something that was not a positional arg, it would understand that the positional args had ended and it should see if that "something" matched the next parser. But what I observe is that it just fails completely at that point rather than trying the next parser.

[Zij-IT]

It can still be done with what you are after, though you lose the benefit of .validate(..) allowing parsing to continue cleanly if an error occurs.

Description

While parsing the args, chumsky has 3 choices:

• both
• kw
• pos

I'm going to try and explain how this happens in chumsky. If something is unclear in any or the steps, do let me know.

With that said, let's try each option:

both

So, it begins to parse both by parsing the list of positional args. However, due to the shared prefix chumsky parses the first identifier of kw as a pos. It attempts again to parse a pos. But because the next token : is neither an ident() nor a , it stops parsing the list of pos. It then attempts to parse a , which fails because the next token is a :. chumsky decides it must not be both, and attempts the next.

kw

kw fails, because obviously the first positional arg lacks a :, so chumsky decides it cannot be a kw.

pos

chumsky begins by parsing a comma separated list of positional args, like in both.

Chumsky then attempts to continue with the starting character being the :. Again, due to the shared prefix it parses the first ident of the first kw param as a positional param. The next token is a :, which is neither an ident() nor a , so it is finished parsing the list. pos has been successfully parsed!

Problem

Your parser continues, having successfully parsed a pos list a and expecting a ) to match the starting (. It finds however the : left over from the half-parsed kw parameter. Your parser now fails with unexpected ':', expected ')'.

Solution

What you are missing is the way to require that each element of pos would not have been a valid kw param.

This can be done through the use of .and_is(..) and .not():

let kw_param = ident()
    .then_ignore(just(':').padded())
    .then(ident());

let pos = ident()
    // this line does the trick
    .and_is(kw_param.not())
    .separated_by(just(','));

let kw = kw_param
    .separated_by(just(','))
    .at_least(1)
    .collect();

/* the rest of your parser */

By using .and_is(kw_param.not()) chumsky will check while parsing pos that it cannot parse a valid kw_param and if it does, it will end the list at the previous element.

[jimmycuadra]

Thanks so much! That was very helpful to understand what Chumsky is doing and why my parser behaved the way it did. I've used the and_is(x.not()) trick before, but I guess the pattern hadn't really sunk in cause I didn't think to use it here. Using this approach, I'm still getting a couple test case failures in my minimal example, but I'm not so worried about that.

The problem I'm running into now is that in my real program, the parser's input type is chumsky::input::BorrowInput, but the parser combinator created by not only implements Parser when the input type is chumsky::input::ValueInput. So when I try to compile it, I get:

help: consider further restricting type parameter `I` with trait `ValueInput`
     |
  12 |         I: BorrowInput<'a, Token = Token<'a>, Span = SimpleSpan> + chumsky::input::ValueInput<'_>,
     |                                                                  ++++++++++++++++++++++++++++++++

But I can't add this constraint, because I doesn't implement ValueInput, and implementing it would change the structure of my entire program.

Is there another way to express the and_is(x.not()) pattern that doesn't require I: ValueInput? Or is my only option to take the approach that uses validate?

[Zij-IT]

Heh, well that's not what I expected. I can't find a reason in the source for this. I've submitted #917 to see if we can get that changed :)

[zesterer]

Apologies about not getting to this earlier: I've recently become a dad, so I've been a bit preoccupied! #917 is now merged and will be in the next release.

[jimmycuadra]

Congratulations, @zesterer! And thanks to you both. :)