Switch to Chumsky for parsing #14
Comments
I'd like to benchmark both and then decide from there. If we need to support both we can cross that bridge. I like the idea of only supporting one parser if possible so it might be worth a performance audit of the chumsky parser if it's significantly different then LALRPOP. |
|
Yeah that sounds like a good idea. If it isn't too hard to process |
|
Good idea. I think we can use this function to parse those files. I'll need to do some digging to get the full schema of those types, but that shouldn't be difficult to find. |
|
I'll make another PR to remove the DoubleMinus and DoubleNot, and then I'm down to 2 failing tests with the chumsky parser. 🤞🏼 |
|
Somewhat related to testing topic mentioned here I tried to reuse Gerkin files from here to run some conformance tests and it works pretty well using the cucumber-rs crate. I had to make some changes expected output in the feature files, but it more or less works. There are some issues with recognising hex and unsigned integers, so I had to adjust the grammar to be able to parse them correctly: - r"-?[0-9]+" => Atom::Int(<>.parse().unwrap()),
- r"-?0[xX]([0-9a-fA-F]+)" => Atom::Int(i64::from_str_radix(<>, 16).unwrap()),
- r"-?[0-9]+ [uU]" => Atom::UInt(<>.parse().unwrap()),
- r"-?0[xX]([0-9a-fA-F]+) [uU]" => Atom::UInt(u64::from_str_radix(<>, 16).unwrap()),
+ r"[-+]?[0-9]+" => Atom::Int(<>.parse().unwrap()),
+ r"([-+])?0[xX]([0-9a-fA-F]+)" => {
+ // We cannot extract capture groups, see https://github.com/lalrpop/lalrpop/issues/575
+ let re = Regex::new(r"([-+])?0[xX]([0-9a-fA-F]+)").unwrap();
+ let captures = re.captures(<>).unwrap();
+
+ let sign = match captures.get(1) {
+ Some(v) => v.as_str(),
+ _ => "",
+ };
+
+ Atom::Int(
+ i64::from_str_radix(
+ format!("{}{}", sign, captures.get(2).unwrap().as_str()).as_str(), 16
+ ).unwrap()
+ )
+ },
+ r"([0-9]+)[uU]" => {
+ // We cannot extract capture groups, see https://github.com/lalrpop/lalrpop/issues/575
+ let re = Regex::new(r"([0-9]+)[uU]").unwrap();
+ let captures = re.captures(<>).unwrap();
+
+ Atom::UInt(captures.get(1).unwrap().as_str().parse().unwrap())
+ },
+ r"0[xX]([0-9a-fA-F]+)[uU]" => {
+ // We cannot extract capture groups, see https://github.com/lalrpop/lalrpop/issues/575
+ let re = Regex::new(r"0[xX]([0-9a-fA-F]+)[uU]").unwrap();
+ let captures = re.captures(<>).unwrap();
+
+ Atom::UInt(u64::from_str_radix(captures.get(1).unwrap().as_str(), 16).unwrap())
+ },But there are other issues as well related to parsing unicode codes like The code for the test itself is pretty simple, but it does not support all features yet: use cel_interpreter::{Context, Program};
use cucumber::{given, then, when, World};
// `World` is your shared, likely mutable state.
// Cucumber constructs it via `Default::default()` for each scenario.
#[derive(Debug, Default, World)]
pub struct CelWorld {
expression: String,
}
#[when(expr = "CEL expression \"{word}\" is evaluated")]
fn expression_evaluated_with_double_quotes(world: &mut CelWorld, expression: String) {
world.expression = expression;
}
#[when(expr = "CEL expression '{word}\' is evaluated")]
fn expression_evaluated_with_single_quotes(world: &mut CelWorld, expression: String) {
world.expression = expression;
}
#[then(regex = "value is (.*)")]
fn evaluation_result_is(world: &mut CelWorld, expected_result: String) {
let program = Program::compile(&world.expression).unwrap();
let mut context = Context::default();
let result = program.execute(&context);
assert_eq!(expected_result, format!("{:?}", result));
}
// This runs before everything else, so you can setup things here.
fn main() {
// You may choose any executor you like (`tokio`, `async-std`, etc.).
// You may even have an `async` main, it doesn't matter. The point is that
// Cucumber is composable. :)
futures::executor::block_on(CelWorld::run("tests/features"));
}and the output looks like this |
|
@inikolaev this is fantastic! Definitely makes sense to incorporate into the project. I took a look at the proto-based tests but read an announcement from the maintainer about deprecating the protobuf dependency so I stopped pursuing that. This looks great however. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Benefits of using chumsky for parsing:
High level plan:
&&,||,inand ternary operations'1'.double()or10.double()DoubleMinusandDoubleNot. Should be e.g.Unary(Negative, Unary(Negative, member)). Feature/remove double negation from ast #17Do you want to keep both parsers? If so, how should the API work to pick between them? Assume it wouldn't be too tricky to add unsigned ints and un-escaped strings to the current lalrpop version?
The text was updated successfully, but these errors were encountered: