Skip to content

Commit

Permalink
ast: Fix print call rewriting in else rules
Browse files Browse the repository at this point in the history
The compiler was accidentally checking/rewriting print calls in the
body of else rules before the implicit args of the else rule were
processed. As a result, the compiler was generating false-positive
errors for refs to undeclared args. The root of the problem was the
usage of WalkBodies on the top-level rule (which implicitly walks the
bodies of else rules under the top-level rule). With this change, the
compiler will call WalkBodies on the head and body of each rule rather
than the entire rule itself (which includes the else chain).

Fixes #4489

Signed-off-by: Torin Sandall <[email protected]>
  • Loading branch information
tsandall committed Mar 25, 2022
1 parent f4324f0 commit a0044b1
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
6 changes: 4 additions & 2 deletions ast/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -1496,12 +1496,14 @@ func (c *Compiler) rewritePrintCalls() {
WalkRules(mod, func(r *Rule) bool {
safe := r.Head.Args.Vars()
safe.Update(ReservedVars)
WalkBodies(r, func(b Body) bool {
vis := func(b Body) bool {
for _, err := range rewritePrintCalls(c.localvargen, c.GetArity, safe, b) {
c.err(err)
}
return false
})
}
WalkBodies(r.Head, vis)
WalkBodies(r.Body, vis)
return false
})
}
Expand Down
43 changes: 41 additions & 2 deletions ast/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4113,6 +4113,45 @@ func TestCompilerRewritePrintCalls(t *testing.T) {
}
}

func TestRewritePrintCallsWithElseImplicitArgs(t *testing.T) {

module := `package test
f(x, y) {
x = y
}
else = false {
print(x, y)
}`

c := NewCompiler().WithEnablePrintStatements(true)
opts := ParserOptions{AllFutureKeywords: true, unreleasedKeywords: true}
c.Compile(map[string]*Module{
"test.rego": MustParseModuleWithOpts(module, opts),
})

if c.Failed() {
t.Fatal(c.Errors)
}

exp := MustParseModuleWithOpts(`package test
f(__local0__, __local1__) = true { __local0__ = __local1__ }
else = false { __local6__ = {__local4__ | __local4__ = __local2__}; __local7__ = {__local5__ | __local5__ = __local3__}; internal.print([__local6__, __local7__]) }
`, opts)

// NOTE(tsandall): we have to patch the implicit args on the else rule
// because of how the parser copies the arg names across from the first
// rule.
exp.Rules[0].Else.Head.Args[0] = VarTerm("__local2__")
exp.Rules[0].Else.Head.Args[1] = VarTerm("__local3__")

if !exp.Equal(c.Modules["test.rego"]) {
t.Fatalf("Expected:\n\n%v\n\nGot:\n\n%v", exp, c.Modules["test.rego"])
}
}

func TestCompilerMockFunction(t *testing.T) {
c := NewCompiler()
c.Modules["test"] = MustParseModule(`
Expand Down Expand Up @@ -4299,7 +4338,7 @@ func TestCompilerCheckUnusedAssignedVar(t *testing.T) {
{
note: "rule with nested closure",
module: `package test
p {
p {
x := 1
a := 1
{ y | y := [ z | z:=[1,2,3][a]; z > 1 ][_] }
Expand All @@ -4312,7 +4351,7 @@ func TestCompilerCheckUnusedAssignedVar(t *testing.T) {
{
note: "rule with nested closure and unused inner var",
module: `package test
p {
p {
x := 1
{ y | y := [ z | z:=[1,2,3][x]; z > 1; a := 2 ][_] }
}
Expand Down

0 comments on commit a0044b1

Please sign in to comment.