Skip to content

Commit

Permalink
ast/compile: don't error on unused declared+generated vars (every) (#…
Browse files Browse the repository at this point in the history
…4421)

When compiling modules containing `every v in vs { ... }`, it will
get rewritten to `every __local0__, v in vs { ... }`.

Sending that module through the compiler _again_, it will complain
about the generated var being declared (via every, it declares its
key and val variables) but unused.

Now, we'll check if the original variable was already a generated
variable, and suppress the error in that case.

Fixes #4420.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
  • Loading branch information
srenatus committed Mar 11, 2022
1 parent 01c22e1 commit 6aa34bd
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
9 changes: 6 additions & 3 deletions ast/compile.go
Expand Up @@ -4050,8 +4050,8 @@ type localDeclaredVars struct {

// rewritten contains a mapping of *all* user-defined variables
// that have been rewritten whereas vars contains the state
// from the current query (not not any nested queries, and all
// vars seen).
// from the current query (not any nested queries, and all vars
// seen).
rewritten map[Var]Var
}

Expand Down Expand Up @@ -4282,7 +4282,10 @@ func checkUnusedDeclaredVars(loc *Location, stack *localDeclaredVars, used VarSe
unused := declared.Diff(bodyvars).Diff(used)

for _, gv := range unused.Sorted() {
errs = append(errs, NewError(CompileErr, loc, "declared var %v unused", dvs.reverse[gv]))
rv := dvs.reverse[gv]
if !rv.IsGenerated() {
errs = append(errs, NewError(CompileErr, loc, "declared var %v unused", rv))
}
}

return errs
Expand Down
24 changes: 23 additions & 1 deletion ast/compile_test.go
Expand Up @@ -3093,11 +3093,33 @@ func TestRewriteDeclaredVars(t *testing.T) {
# import future.keywords.in
# import future.keywords.every
p {
every k, v in [1] { v >= i }
every k, v in [1] { v >= 1 }
}
`,
wantErr: errors.New("declared var k unused"),
},
{
// NOTE(sr): this would happen when compiling modules twice:
// the first run rewrites every to include a generated key var,
// the second one bails because it's not used.
// Seen in the wild when using `opa test -b` on a bundle that
// used `every`, https://github.com/open-policy-agent/opa/issues/4420
note: "rewrite every: unused generated key var",
module: `
package test
p {
every __local0__, v in [1] { v >= 1 }
}
`,
exp: `
package test
p = true {
__local3__ = [1]
every __local1__, __local2__ in __local3__ { __local2__ >= 1 }
}
`,
},
{
note: "rewrite every: unused value var",
module: `
Expand Down

0 comments on commit 6aa34bd

Please sign in to comment.