Support OPA v1 'if' rules as single rules - #23
Merged
Conversation
Owner
|
Hello @lovesegfault ! Thank you for your PR. Can you please rebase it with the latest master? I fixed the yarn.lock, why it was failing - it was resolving for some reason towards the npm of Apple |
lovesegfault
force-pushed
the
fix-v1-if-rules
branch
from
July 4, 2026 23:47
8c17a74 to
cdc39c4
Compare
Contributor
Author
|
done! :) |
A v1 rule like `allow := true if { input.admin }` previously parsed
as two rules: a value head plus a phantom rule whose head variable
was `if`. Since OPA 1.0 rejects bodied rules without `if`, virtually
every current-syntax policy produced these split trees.
Grammar changes:
- A new rule alternative covers the v1 comp+if form (value bound in
the head, `if`, then a braced query or single literal). Restricting
the first body keeps a following v0 rule that happens to be named
`if` from being fused in. Dynamic precedence prefers the one-rule
parse over the phantom split.
- rule_head drops its prec.right wrapper, which statically resolved
`var :=` toward the head and prevented GLR from exploring the
constant-rule parse; the head forms instead share an explicit
dynamic precedence so brackets, args, and `if` stay in the head
rather than demoting to body literals.
- rule_head_comp accepts full expressions as values
(`f(x) := x + 1 if ...`), not just plain terms.
- The rule_body value branch accepts `=` alongside `:=`, fixing plain
unification constants (`a = 1`) and terminal `else = 2` clauses.
- else clauses take an optional `if` (`else := 2 if { ... }`).
Corpus: `conditionals` and `ordered (else)` expectations previously
codified the split-parse trees and now assert single rules; `union`
(FallenAngel97#8) and `reference heads` (FallenAngel97#7) remain broken per their own issues
and re-pin the current recovery trees; new v1_rules.txt and
v0_heads.txt cases cover the v1 forms and guard the v0 head shapes.
Known remaining gaps: bodiless partial-set rules
(`deny contains "msg"` with no body) still swallow the following
statement, as before this change; a v0 function literally named `if`
directly after a constant rule can fuse into it (`if` is a reserved
word in v1).
Fixes FallenAngel97#20
lovesegfault
force-pushed
the
fix-v1-if-rules
branch
from
July 5, 2026 22:47
cdc39c4 to
02d4deb
Compare
This was referenced Jul 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #20.
Problem
A v1 rule like
parsed as two rules: a value head (
allowwith a:= truebody) plus a phantom rule whose head variable isif. Since OPA 1.0 rejects bodied rules withoutif, virtually every current-syntax policy produced these split trees. The same split affectedelsechains, andelse := 2 if { ... }clauses could swallow following rules.Approach
The head grammar was missing the plain-var value form (
var [":=" term] ["if"]from the OPA spec) — the bracket/args head forms already carried[rule-head-comp] [if], but a plainallow := true ifhad nowhere to go.rule_head_v1, aliased torule_headso consumers see the usual node), then a first body restricted to a braced query or single literal (rule_body_v1, aliased torule_body). Restricting the first body means a following v0 rule that happens to be namedif(a := 1thenif := 2) cannot be fused in — its:= 2is not a valid v1 body, so that GLR path dies and the two-rule parse wins. Dynamic precedence prefers the one-rule parse over the phantom split.rule_headdropsprec.right: it statically resolvedvar • :=toward the head path, which would break plain constants (x := 1) once the comp+if form exists. The head forms instead share oneprec.dynamic(1)so brackets, args, andifstay in the head rather than demoting to body literals (pinned by the newv0_heads.txtcorpus cases at end-of-file, where the tie-break used to be arbitrary).rule_head_compaccepts full expressions (f(x) := x + 1 if x > 0,roles := input.roles | {"anonymous"} if { ... }), not just plain terms.=alongside:=, fixing plain unification constants (a = 1,a = 1 { true }) and terminalelse = 2clauses.elseclauses take an optionalif(else := 2 if { ... },else if input.b).Corpus
conditionalsandordered (else)previously codified the split-parse trees; they now assert single rules.union(Parsing error - union #8) andreference heads(Parsing error - reference heads #7) remain broken per their own issues; their expectations re-pin the current recovery trees (thereference headsrecovery actually improves — one rule with the ERROR confined to the dotted segments instead of a phantomifsplit).v1_rules.txt(9 cases: braced/unbraced bodies, else-if chains, default+conditional, infix values, unification forms, constants staying separate) andv0_heads.txt(5 cases guarding v0 head shapes and the rule-named-ifnon-fusion).53/53 corpus tests pass.
src/regenerated withtree-sitter generate.Known remaining gaps
deny contains "msg"with no body) still swallow the following statement — unchanged from before; fixing it trips over the grammar'srepeat1body requirement and deserves its own change.ifdirectly following a constant rule can fuse into it.ifis a reserved word in v1, so this only affects legacy policies defining a function namedif.Note: this branch is independent of #22; whichever merges second just needs
tree-sitter generatere-run to resolve the generated-file overlap.