opa
opa copied to clipboard
Update Rego EBNF grammar
While working on Rego grammar for tree-sitter (https://github.com/shaded-enmity/tree-sitter-rego/blob/complete-ebnf-grammar/grammar.js => https://github.com/FallenAngel97/tree-sitter-rego/pull/2/files) I encountered a couple of problems that I'd like to bring up for discussion:
- The first symbol for
rule-head-obj
andrule-head-func
in the current grammar is marked as optional, however that makes both rules boil down torule-head-comp
. This patch changes the first term of the two rules into non-optionals. - The first symbol of the
expr-infix
rule is[ term "=" ]
, which causes tree-sitter to error becauseexpr-infix
now conflicts with itself. This patch removes that symbol and adds assignment:=
and unification=
as an infix binary operation rules. - Add support for
in
membership operator via themembership
rule - Add support for parenthesized expressions (necessary for 3. but also to support general stuff like
number := 2 * (3 + 2)
via theexpr-parens
rule
Thanks for taking a look at the EBNF grammar, @shaded-enmity! :smile: The parser in OPA is not generated directly from tools, so it's possible for the grammar definition docs to drift out of line with the language, or have issues that aren't immediately noticed.
I won't be able to review this PR today, but I'll try to take a look at it this week if I can. A good tree-sitter grammar would be useful for tools authors.
Thanks for taking a look at the EBNF grammar, @shaded-enmity! smile The parser in OPA is not generated directly from tools, so it's possible for the grammar definition docs to drift out of line with the language, or have issues that aren't immediately noticed.
I won't be able to review this PR today, but I'll try to take a look at it this week if I can. A good tree-sitter grammar would be useful for tools authors.
No problem, take your time - I've just added two new rules (membership
and expr-parens
) hopefully bringing the grammar definition close to completion (fingers crossed!), but I'll keep adding test cases and breaking things :).
Once I figure out the details how to do it I'll post a link to a tree-sitter playground (something like https://tree-sitter.github.io/tree-sitter/playground) so that you can just type in any Rego code and see how the current version of the parser handles it.
Playground is up and running:
https://shaded-enmity.github.io/tree-sitter-rego/
@shaded-enmity I got to try out the playground demo with a non-trivial example or two, and it looks awesome!
I'll do a more in-depth review of the grammar changes tomorrow morning to make sure nothing major got missed, and then I think this PR will be good-to-merge. :smile:
One thing I just discovered when refining the grammar for floating point numbers is the handling of unary -
:
numeric := -1 # works fine
other := -input.number # error
The second example fails with:
1 error occurred: test_unary.rego:3: rego_parse_error: unexpected assign token: expected rule value term (e.g., other := <VALUE> { ... })
other := -input.number
Is this by design or a bug? :smiley:
Is this by design or a bug? :smiley:
@shaded-enmity That is almost certainly a parser/evaluator bug. :bug:
You can file an issue around that test case if you'd like, or I can file the issue later today. :slightly_smiling_face: I'm hoping it will be straightforward to fix, since this looks like a case where a ref (like input.number
) is just being treated differently than a raw numeric constant.
@philipaconrad Thanks for creating the issue and apologies for trailing off a bit - first I had covid so I had to take a break and now I'm configuring a new laptop and I'm not in the zone yet.
I'll push an update later today containing:
- Changes to
membership
as per @srenatus 's feedback - Unary
-
- Changes to
rule-body
to accommodate for direct literals