From 6aa34bd7240302039602be9a96fe97e74442f1c7 Mon Sep 17 00:00:00 2001
From: Stephan Renatus <stephan.renatus@gmail.com>
Date: Fri, 11 Mar 2022 17:35:39 +0100
Subject: [PATCH] ast/compile: don't error on unused declared+generated vars
 (every) (#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>
---
 ast/compile.go      |  9 ++++++---
 ast/compile_test.go | 24 +++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/ast/compile.go b/ast/compile.go
index d7238d4b8f..47d60d3a3a 100644
--- a/ast/compile.go
+++ b/ast/compile.go
@@ -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
 }
 
@@ -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
diff --git a/ast/compile_test.go b/ast/compile_test.go
index 2614bba562..c797a143b3 100644
--- a/ast/compile_test.go
+++ b/ast/compile_test.go
@@ -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: `