From d153f8a425a5aa8fb028d6ba1581cc3762eae86a Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Thu, 13 Aug 2026 11:35:20 -0500 Subject: [PATCH 1/2] ast: Report source var names in print() undeclared errors Local var rewriting runs before print calls are rewritten, so a diagnostic about a print operand named the generated var (`var __local0__ is undeclared`) rather than the one the author wrote. Map generated names back through RewrittenVars, as the template-string rewriter already does, and skip the redundant second walk of nested bodies that reported these errors twice. Fixes: #5624 Signed-off-by: Sebastian Spaink --- v1/ast/compile.go | 48 ++++++++++++++++-------- v1/ast/compile_test.go | 83 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 16 deletions(-) diff --git a/v1/ast/compile.go b/v1/ast/compile.go index a2009e28c7..b20a456d9a 100644 --- a/v1/ast/compile.go +++ b/v1/ast/compile.go @@ -2682,11 +2682,12 @@ func (c *Compiler) rewritePrintCalls() { } bodyVis := func(b Body) bool { - modrec, errs := rewritePrintCalls(c.localvargen, c.GetArity, vis.vars, b) + modrec, errs := rewritePrintCalls(c.localvargen, c.GetArity, vis.vars, c.RewrittenVars, b) if modrec { modified = true } - if !c.err(errs...) { + if len(errs) > 0 { + c.err(errs...) return true } return false @@ -2732,15 +2733,15 @@ func checkVoidCalls(env *TypeEnv, x any) Errors { // The expression would be rewritten to: // // print({__local0__ | __local0__ = "the value of x is:"}, {__local1__ | __local1__ = input.x}) -func rewritePrintCalls(gen *localVarGenerator, getArity func(Ref) int, globals VarSet, body Body) (bool, Errors) { +func rewritePrintCalls(gen *localVarGenerator, getArity func(Ref) int, globals VarSet, rewritten map[Var]Var, body Body) (bool, Errors) { var errs Errors var modified bool - // Visit comprehension bodies recursively to ensure print statements inside - // those bodies only close over variables that are safe. + // Visit nested bodies recursively to ensure print statements inside those + // bodies only close over variables that are safe. for i := range body { - if ContainsClosures(body[i]) { + if containsNestedBody(body[i]) { safe := outputVarsForBody(body[:i], getArity, globals, nil) safe.Update(globals) WalkClosures(body[i], func(x any) bool { @@ -2748,28 +2749,28 @@ func rewritePrintCalls(gen *localVarGenerator, getArity func(Ref) int, globals V var errsrec Errors switch x := x.(type) { case *SetComprehension: - modrec, errsrec = rewritePrintCalls(gen, getArity, safe, x.Body) + modrec, errsrec = rewritePrintCalls(gen, getArity, safe, rewritten, x.Body) case *ArrayComprehension: - modrec, errsrec = rewritePrintCalls(gen, getArity, safe, x.Body) + modrec, errsrec = rewritePrintCalls(gen, getArity, safe, rewritten, x.Body) case *ObjectComprehension: - modrec, errsrec = rewritePrintCalls(gen, getArity, safe, x.Body) + modrec, errsrec = rewritePrintCalls(gen, getArity, safe, rewritten, x.Body) case *Every: safe.Update(x.KeyValueVars()) - modrec, errsrec = rewritePrintCalls(gen, getArity, safe, x.Body) + modrec, errsrec = rewritePrintCalls(gen, getArity, safe, rewritten, x.Body) case *Not: - modrec, errsrec = rewritePrintCalls(gen, getArity, safe, x.Body) + modrec, errsrec = rewritePrintCalls(gen, getArity, safe, rewritten, x.Body) case *LogicalAnd: var modR bool var errsR Errors - modrec, errsrec = rewritePrintCalls(gen, getArity, safe, x.Lhs) - modR, errsR = rewritePrintCalls(gen, getArity, safe, x.Rhs) + modrec, errsrec = rewritePrintCalls(gen, getArity, safe, rewritten, x.Lhs) + modR, errsR = rewritePrintCalls(gen, getArity, safe, rewritten, x.Rhs) modrec = modrec || modR errsrec = append(errsrec, errsR...) case *LogicalOr: var modR bool var errsR Errors - modrec, errsrec = rewritePrintCalls(gen, getArity, safe, x.Lhs) - modR, errsR = rewritePrintCalls(gen, getArity, safe, x.Rhs) + modrec, errsrec = rewritePrintCalls(gen, getArity, safe, rewritten, x.Lhs) + modR, errsR = rewritePrintCalls(gen, getArity, safe, rewritten, x.Rhs) modrec = modrec || modR errsrec = append(errsrec, errsR...) } @@ -2821,6 +2822,9 @@ func rewritePrintCalls(gen *localVarGenerator, getArity func(Ref) int, globals V if vars.DiffCount(safe) > 0 { unsafe := vars.Diff(safe) for _, v := range unsafe.Sorted() { + if w, ok := rewritten[v]; ok { + v = w + } errs = append(errs, NewError(CompileErr, args[j].Loc(), "var %v is undeclared", v)) } } @@ -2847,6 +2851,18 @@ func rewritePrintCalls(gen *localVarGenerator, getArity func(Ref) int, globals V return modified, nil } +// containsNestedBody returns true if x contains any node that carries a nested +// body which rewritePrintCalls needs to descend into. This is a superset of +// ContainsClosures, which ignores not/and/or expressions. +func containsNestedBody(x any) bool { + found := false + WalkClosures(x, func(any) bool { + found = true + return found + }) + return found +} + func erasePrintCalls(node any) bool { var modified bool NewGenericVisitor(func(x any) bool { @@ -3867,7 +3883,7 @@ func (qc *queryCompiler) rewritePrintCalls(_ *QueryContext, body Body) (Body, er return cpy, nil } gen := newLocalVarGenerator("q", body) - if _, errs := rewritePrintCalls(gen, qc.compiler.GetArity, ReservedVars, body); len(errs) > 0 { + if _, errs := rewritePrintCalls(gen, qc.compiler.GetArity, ReservedVars, qc.RewrittenVars(), body); len(errs) > 0 { return nil, errs } return body, nil diff --git a/v1/ast/compile_test.go b/v1/ast/compile_test.go index b50bdfa7ae..33041c60f6 100644 --- a/v1/ast/compile_test.go +++ b/v1/ast/compile_test.go @@ -7821,6 +7821,7 @@ func TestCompilerRewritePrintCallsErrors(t *testing.T) { note string module string exp error + exps []string // when set, asserts the full set of error messages errCode string }{ { @@ -7845,6 +7846,7 @@ func TestCompilerRewritePrintCallsErrors(t *testing.T) { p if { {1 | print(x)} = {1 | print(7)} } `, exp: errors.New("var x is undeclared"), + exps: []string{"var x is undeclared"}, errCode: CompileErr, }, { @@ -7855,6 +7857,40 @@ func TestCompilerRewritePrintCallsErrors(t *testing.T) { exp: errors.New("print(42) used as value"), errCode: TypeErr, }, + { + note: "some declaration inside comprehension body", + module: `package test + + p if { + xs := [1 | some x; print(x)] + xs == xs + }`, + exp: errors.New("var x is undeclared"), + exps: []string{"var x is undeclared"}, + errCode: CompileErr, + }, + { + note: "declared var shadowed inside comprehension", + module: `package test + + p if { + x := 1 + ys := [x | some x; print(x)] + ys == ys + }`, + exp: errors.New("var x is undeclared"), + exps: []string{"var x is undeclared"}, + errCode: CompileErr, + }, + { + note: "unsafe var alongside function argument", + module: `package test + + f(x) if { print(x, y) }`, + exp: errors.New("var y is undeclared"), + exps: []string{"var y is undeclared"}, + errCode: CompileErr, + }, } for _, tc := range cases { @@ -7869,6 +7905,15 @@ func TestCompilerRewritePrintCallsErrors(t *testing.T) { if c.Errors[0].Code != tc.errCode || c.Errors[0].Message != tc.exp.Error() { t.Fatal("unexpected error:", c.Errors) } + if tc.exps != nil { + got := make([]string, len(c.Errors)) + for i, err := range c.Errors { + got[i] = err.Message + } + if !slices.Equal(got, tc.exps) { + t.Fatalf("expected errors %v but got %v", tc.exps, got) + } + } }) } } @@ -7880,6 +7925,38 @@ func TestCompilterRewritePrintCallsNestedComprehensionLocalsSafe(t *testing.T) { note string module string }{ + { + note: "comprehension in every domain", + module: `package test + p if { every z in [c | c := 1; print(c)] { z == z } }`, + }, + { + note: "comprehension in every body", + module: `package test + p if { every z in [1] { y := [1 | print(z)]; y == y } }`, + }, + { + note: "comprehension in rule head", + module: `package test + p := {1 | print("h")}`, + }, + { + note: "comprehension in with value", + module: `package test + q := 1 + p if { q with input as [1 | print(input)] }`, + }, + { + note: "print after not expression", + module: `package test + q := 1 + r if { not q; print(q) }`, + }, + { + note: "outer and every-key vars visible in nested comprehension", + module: `package test + p if { a := 1; every z in [1] { b := [1 | print(a, z)]; b == b } }`, + }, { note: "print variable from nested comprehension without error", module: `package test @@ -7898,6 +7975,12 @@ func TestCompilterRewritePrintCallsNestedComprehensionLocalsSafe(t *testing.T) { c := NewCompiler().WithEnablePrintStatements(true) c.Compile(map[string]*Module{"test.rego": module(tc.module)}) assertNotFailed(t, c) + + // Every print call must have been rewritten into internal.print, + // including those in nested bodies. + if str := c.Modules["test.rego"].String(); strings.Contains(str, " print(") { + t.Fatalf("expected all print calls to be rewritten, got:\n\n%v", str) + } }) } } From d13ee9149940109c970f18e2f9cc0cbcf04f7947 Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Thu, 13 Aug 2026 11:35:20 -0500 Subject: [PATCH 2/2] ast: Add compile cases for print() undeclared var diagnostics Pin the positions, codes and messages in the compilecases corpus so that other Rego implementations can consume them. Reaching these diagnostics needs print calls left intact, so add an optional print_statements case option alongside strict and experimental_keywords. Signed-off-by: Sebastian Spaink --- v1/ast/compile_cases_test.go | 2 +- v1/test/compilecases/cases.go | 1 + .../v1/print/test-print-undeclared-var.yaml | 65 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 v1/test/compilecases/testdata/v1/print/test-print-undeclared-var.yaml diff --git a/v1/ast/compile_cases_test.go b/v1/ast/compile_cases_test.go index c362b700aa..cabf65d19d 100644 --- a/v1/ast/compile_cases_test.go +++ b/v1/ast/compile_cases_test.go @@ -52,7 +52,7 @@ func runCompileCase(t *testing.T, tc compilecases.TestCase) { modules[name] = parsed } - c := NewCompiler().WithStrict(tc.Strict) + c := NewCompiler().WithStrict(tc.Strict).WithEnablePrintStatements(tc.PrintStatements) c.Compile(modules) if !c.Failed() { diff --git a/v1/test/compilecases/cases.go b/v1/test/compilecases/cases.go index e6bdda5a30..71d3d604d1 100644 --- a/v1/test/compilecases/cases.go +++ b/v1/test/compilecases/cases.go @@ -39,6 +39,7 @@ type TestCase struct { RegoVersion string `json:"rego_version,omitempty" yaml:"rego_version,omitempty"` // rego version to parse the modules as: v0, v1 (default), or v0-compat-v1 Strict bool `json:"strict,omitempty" yaml:"strict,omitempty"` // enable the compiler's strict mode ExperimentalKeywords bool `json:"experimental_keywords,omitempty" yaml:"experimental_keywords,omitempty"` // opt-in to experimental future keywords + PrintStatements bool `json:"print_statements,omitempty" yaml:"print_statements,omitempty"` // keep print() calls instead of erasing them, as required to reach diagnostics about their operands WantErrors []Error `json:"want_errors" yaml:"want_errors"` // diagnostics the compilation must produce Exhaustive bool `json:"exhaustive,omitempty" yaml:"exhaustive,omitempty"` // require want_errors to be the complete set, not a subset } diff --git a/v1/test/compilecases/testdata/v1/print/test-print-undeclared-var.yaml b/v1/test/compilecases/testdata/v1/print/test-print-undeclared-var.yaml new file mode 100644 index 0000000000..9f8a84b0cd --- /dev/null +++ b/v1/test/compilecases/testdata/v1/print/test-print-undeclared-var.yaml @@ -0,0 +1,65 @@ +--- +# print() operands are captured in comprehensions so that an undefined operand +# does not stop the print from being reached. That rewrite runs after local vars +# have been renamed to generated ones, so a diagnostic about an operand has to +# map the name back to the one the author wrote. See issue #5624. +# +# These cases need print_statements: with print erased, there is no operand left +# to diagnose and the modules compile. +cases: + - note: print/undeclared-var-declared-by-some + print_statements: true + modules: + - | + package test + + p if { + some x + print(x) + } + want_errors: + - code: rego_compile_error + row: 5 + col: 8 + message: var x is undeclared + exhaustive: true + - note: print/undeclared-var-declared-by-some-reports-each-operand + print_statements: true + modules: + - | + package test + + p if { + some x, y + print(x, y) + } + want_errors: + - code: rego_compile_error + row: 5 + col: 8 + message: var x is undeclared + - code: rego_compile_error + row: 5 + col: 11 + message: var y is undeclared + exhaustive: true + - note: print/undeclared-var-in-every-body-does-not-implicate-key-var + # `z` is bound by the every, so only `q` is undeclared. Reporting is per + # print operand, and the body is walked once: a var that is safe inside the + # nested body must not be reported, and `q` must not be reported twice. + print_statements: true + modules: + - | + package test + + p if { + every z in [1] { + print(q, z) + } + } + want_errors: + - code: rego_compile_error + row: 5 + col: 9 + message: var q is undeclared + exhaustive: true