From 37b14851b812598e12f4691f0c4ec58b23408787 Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Fri, 26 Jun 2026 14:07:16 +0200 Subject: [PATCH] topdown: `and`/`or` expression evaluation (#8793) Contains simplified PE: expressions are plugged and saved, but not optimized. PE optimization to follow in #8680 Signed-off-by: Johan Fylling --- build/generate-extended-cases/exceptions.yaml | 1 + .../generate-extended-cases/extended_cases.go | 34 +- internal/wasm/sdk/test/e2e/exceptions.yaml | 1 + internal/wasm/sdk/test/e2e/external_test.go | 28 + v1/test/cases/cases.go | 31 +- .../v1/logic_operators/and_basic.yaml | 95 +++ .../v1/logic_operators/and_explicit_body.yaml | 103 +++ .../v1/logic_operators/and_short_circuit.yaml | 52 ++ .../v1/logic_operators/comprehension.yaml | 95 +++ .../testdata/v1/logic_operators/every.yaml | 103 +++ .../v1/logic_operators/iteration.yaml | 109 +++ .../testdata/v1/logic_operators/negation.yaml | 134 +++ .../testdata/v1/logic_operators/nesting.yaml | 96 +++ .../testdata/v1/logic_operators/or_basic.yaml | 110 +++ .../v1/logic_operators/or_explicit_body.yaml | 98 +++ .../v1/logic_operators/or_short_circuit.yaml | 54 ++ .../v1/logic_operators/or_single_result.yaml | 46 ++ .../v1/logic_operators/precedence.yaml | 85 ++ .../v1/logic_operators/with_modifier.yaml | 62 ++ v1/topdown/copypropagation/copypropagation.go | 24 +- v1/topdown/eval.go | 313 +++++-- v1/topdown/exported_test.go | 4 + v1/topdown/topdown_logical_test.go | 423 ++++++++++ v1/topdown/topdown_partial_test.go | 776 +++++++++++++++++- v1/topdown/trace_test.go | 283 +++++++ 25 files changed, 3082 insertions(+), 78 deletions(-) create mode 100644 v1/test/cases/testdata/v1/logic_operators/and_basic.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/and_explicit_body.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/and_short_circuit.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/comprehension.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/every.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/iteration.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/negation.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/nesting.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/or_basic.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/or_explicit_body.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/or_short_circuit.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/or_single_result.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/precedence.yaml create mode 100644 v1/test/cases/testdata/v1/logic_operators/with_modifier.yaml create mode 100644 v1/topdown/topdown_logical_test.go diff --git a/build/generate-extended-cases/exceptions.yaml b/build/generate-extended-cases/exceptions.yaml index 8c3413b8c6..3249fb1f26 100644 --- a/build/generate-extended-cases/exceptions.yaml +++ b/build/generate-extended-cases/exceptions.yaml @@ -1 +1,2 @@ # Exception Format is : +"logic_op/**": "No planner support" \ No newline at end of file diff --git a/build/generate-extended-cases/extended_cases.go b/build/generate-extended-cases/extended_cases.go index b2e4cdf243..3eae38750d 100644 --- a/build/generate-extended-cases/extended_cases.go +++ b/build/generate-extended-cases/extended_cases.go @@ -9,8 +9,10 @@ import ( "io/fs" "os" "slices" + "strings" "time" + "github.com/gobwas/glob" "sigs.k8s.io/yaml" "github.com/open-policy-agent/opa/v1/ast" @@ -25,10 +27,14 @@ import ( var exceptionsFile = flag.String("exceptions", "./exceptions.yaml", "set file to load a list of test names to exclude") -var exceptions map[string]string +var ( + exceptions map[string]string + exceptionGlobs []glob.Glob +) func setup() { exceptions = map[string]string{} + exceptionGlobs = nil bs, err := os.ReadFile(*exceptionsFile) if err != nil { @@ -40,11 +46,33 @@ func setup() { fmt.Println("Unable to parse exceptions file: " + err.Error()) os.Exit(1) } + + for pattern := range exceptions { + if !strings.Contains(pattern, "*") { + continue + } + + g, err := glob.Compile(pattern, '/') + if err != nil { + fmt.Printf("Invalid glob pattern %q in exceptions file: %v\n", pattern, err) + os.Exit(1) + } + exceptionGlobs = append(exceptionGlobs, g) + } } func shouldSkip(tc cases.TestCase) bool { - _, ok := exceptions[tc.Note] - return ok + if _, ok := exceptions[tc.Note]; ok { + return true + } + + for _, g := range exceptionGlobs { + if g.Match(tc.Note) { + return true + } + } + + return false } type ExtendedTestCase struct { diff --git a/internal/wasm/sdk/test/e2e/exceptions.yaml b/internal/wasm/sdk/test/e2e/exceptions.yaml index d8eb5d5283..fb6708d6b1 100644 --- a/internal/wasm/sdk/test/e2e/exceptions.yaml +++ b/internal/wasm/sdk/test/e2e/exceptions.yaml @@ -1,3 +1,4 @@ # Exception Format is : "data/toplevel integer": "https://github.com/open-policy-agent/opa/issues/3711" "data/nested integer": "https://github.com/open-policy-agent/opa/issues/3711" +"logic_op/**": "No planner support" diff --git a/internal/wasm/sdk/test/e2e/external_test.go b/internal/wasm/sdk/test/e2e/external_test.go index 995b1cde97..9dee6f408e 100644 --- a/internal/wasm/sdk/test/e2e/external_test.go +++ b/internal/wasm/sdk/test/e2e/external_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "github.com/gobwas/glob" "github.com/open-policy-agent/opa/internal/wasm/sdk/opa" "github.com/open-policy-agent/opa/v1/ast" "github.com/open-policy-agent/opa/v1/rego" @@ -34,6 +35,13 @@ var exceptionsFile = flag.String("exceptions", "./exceptions.yaml", "set file to var exceptions map[string]string +type exceptionPattern struct { + g glob.Glob + r string +} + +var exceptionGlobs []exceptionPattern + func TestMain(m *testing.M) { exceptions = map[string]string{} @@ -48,6 +56,19 @@ func TestMain(m *testing.M) { os.Exit(1) } + for pattern, reason := range exceptions { + if !strings.Contains(pattern, "*") { + continue + } + + g, err := glob.Compile(pattern, '/') + if err != nil { + fmt.Printf("Invalid glob pattern %q in exceptions file: %v\n", pattern, err) + os.Exit(1) + } + exceptionGlobs = append(exceptionGlobs, exceptionPattern{g: g, r: reason}) + } + addTestSleepBuiltin() os.Exit(m.Run()) @@ -123,6 +144,13 @@ func shouldSkip(t *testing.T, tc cases.TestCase) bool { return true } + for _, p := range exceptionGlobs { + if p.g.Match(tc.Note) { + t.Log("Skipping test case: " + p.r) + return true + } + } + return false } diff --git a/v1/test/cases/cases.go b/v1/test/cases/cases.go index c634680fc5..599fd74687 100644 --- a/v1/test/cases/cases.go +++ b/v1/test/cases/cases.go @@ -35,21 +35,22 @@ func (s Set) Sorted() Set { // TestCase represents a single test case. type TestCase struct { - Filename string `json:"-" yaml:"-"` // name of file that case was loaded from - Note string `json:"note" yaml:"note"` // globally unique identifier for this test case - Query string `json:"query" yaml:"query"` // policy query to execute - Modules []string `json:"modules,omitempty" yaml:"modules,omitempty"` // policies to test against - Data *map[string]any `json:"data,omitempty" yaml:"data,omitempty"` // data to test against - Input *any `json:"input,omitempty" yaml:"input,omitempty"` // parsed input data to use - InputTerm *string `json:"input_term,omitempty" yaml:"input_term,omitempty"` // raw input data (serialized as a string, overrides input) - WantDefined *bool `json:"want_defined,omitempty" yaml:"want_defined,omitempty"` // expect query result to be defined (or not) - WantResult *[]map[string]any `json:"want_result,omitempty" yaml:"want_result,omitempty"` // expect query result (overrides defined) - WantErrorCode *string `json:"want_error_code,omitempty" yaml:"want_error_code,omitempty"` // expect query error code (overrides result) - WantError *string `json:"want_error,omitempty" yaml:"want_error,omitempty"` // expect query error message (overrides error code) - SortBindings bool `json:"sort_bindings,omitempty" yaml:"sort_bindings,omitempty"` // indicates that binding values should be treated as sets - IgnoreGeneratedVars bool `json:"ignore_generated_vars,omitempty" yaml:"ignore_generated_vars,omitempty"` // indicates that generated bindings in the result set should be ignored - StrictError bool `json:"strict_error,omitempty" yaml:"strict_error,omitempty"` // indicates that the error depends on strict builtin error mode - Env map[string]string `json:"env,omitempty" yaml:"env,omitempty"` // environment variables to be set during the test + Filename string `json:"-" yaml:"-"` // name of file that case was loaded from + Note string `json:"note" yaml:"note"` // globally unique identifier for this test case + Query string `json:"query" yaml:"query"` // policy query to execute + Modules []string `json:"modules,omitempty" yaml:"modules,omitempty"` // policies to test against + Data *map[string]any `json:"data,omitempty" yaml:"data,omitempty"` // data to test against + Input *any `json:"input,omitempty" yaml:"input,omitempty"` // parsed input data to use + InputTerm *string `json:"input_term,omitempty" yaml:"input_term,omitempty"` // raw input data (serialized as a string, overrides input) + WantDefined *bool `json:"want_defined,omitempty" yaml:"want_defined,omitempty"` // expect query result to be defined (or not) + WantResult *[]map[string]any `json:"want_result,omitempty" yaml:"want_result,omitempty"` // expect query result (overrides defined) + WantErrorCode *string `json:"want_error_code,omitempty" yaml:"want_error_code,omitempty"` // expect query error code (overrides result) + WantError *string `json:"want_error,omitempty" yaml:"want_error,omitempty"` // expect query error message (overrides error code) + SortBindings bool `json:"sort_bindings,omitempty" yaml:"sort_bindings,omitempty"` // indicates that binding values should be treated as sets + IgnoreGeneratedVars bool `json:"ignore_generated_vars,omitempty" yaml:"ignore_generated_vars,omitempty"` // indicates that generated bindings in the result set should be ignored + StrictError bool `json:"strict_error,omitempty" yaml:"strict_error,omitempty"` // indicates that the error depends on strict builtin error mode + ExperimentalKeywords bool `json:"experimental_keywords,omitempty" yaml:"experimental_keywords,omitempty"` // opt-in to experimental future keywords + Env map[string]string `json:"env,omitempty" yaml:"env,omitempty"` // environment variables to be set during the test } // Load returns a set of built-in test cases. diff --git a/v1/test/cases/testdata/v1/logic_operators/and_basic.yaml b/v1/test/cases/testdata/v1/logic_operators/and_basic.yaml new file mode 100644 index 0000000000..4d9c9c6f3f --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/and_basic.yaml @@ -0,0 +1,95 @@ +--- +cases: + - note: "logic_op/and/basic: both operands true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + true and true + } + want_result: + - x: true + - note: "logic_op/and/basic: lhs false" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + false and true + } + want_result: [] + - note: "logic_op/and/basic: rhs false" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + true and false + } + want_result: [] + - note: "logic_op/and/basic: both false" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + false and false + } + want_result: [] + - note: "logic_op/and/basic: both succeed via input" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + input.x > 0 and input.y > 0 + } + input: + x: 1 + "y": 2 + want_result: + - x: true + - note: "logic_op/and/basic: lhs undefined ref" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + input.missing and input.y > 0 + } + input: + "y": 2 + want_result: [] + - note: "logic_op/and/basic: rhs undefined ref" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + input.x > 0 and input.missing + } + input: + x: 1 + want_result: [] diff --git a/v1/test/cases/testdata/v1/logic_operators/and_explicit_body.yaml b/v1/test/cases/testdata/v1/logic_operators/and_explicit_body.yaml new file mode 100644 index 0000000000..4a029b808f --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/and_explicit_body.yaml @@ -0,0 +1,103 @@ +--- +cases: + - note: "logic_op/and/explicit-body: explicit lhs, implicit rhs" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { a := 1; a > 0 } and input.y == 2 + } + input: + "y": 2 + want_result: + - x: true + - note: "logic_op/and/explicit-body: implicit lhs, explicit rhs" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + input.y == 2 and { a := 1; a > 0 } + } + input: + "y": 2 + want_result: + - x: true + - note: "logic_op/and/explicit-body: both explicit" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { a := 1; a > 0 } and { b := 2; b > 0 } + } + want_result: + - x: true + - note: "logic_op/and/explicit-body: multi-expr operand body" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { a := input.a; a > 0; a < 10 } and true + } + input: + a: 5 + want_result: + - x: true + - note: "logic_op/and/explicit-body: multi-expr operand body fails on second expr" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { a := input.a; a > 0; a < 10 } and true + } + input: + a: 100 + want_result: [] + - note: "logic_op/and/explicit-body: each operand has its own scope" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { a = 1; a > 0 } and { a = -1; a < 0 } + { b := 1; b > 0 } and { b := -1; b < 0 } + } + want_result: + - x: true + - note: "logic_op/and/explicit-body: some-binding inside operand body" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { some n in input.xs; n > 0 } and true + } + input: + xs: [-1, 0, 5] + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/and_short_circuit.yaml b/v1/test/cases/testdata/v1/logic_operators/and_short_circuit.yaml new file mode 100644 index 0000000000..690a751c29 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/and_short_circuit.yaml @@ -0,0 +1,52 @@ +--- +# Short-circuit is observed via strict-builtin-errors: the RHS is wired +# with a builtin that errors at runtime; the test passes only if the RHS is +# *not* evaluated when the LHS already failed. +cases: + - note: "logic_op/and/short-circuit: lhs fails, rhs builtin not evaluated" + experimental_keywords: true + strict_error: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + input.x > 0 and to_number("foo") == 1 + } + input: + x: 0 + want_result: [] + - note: "logic_op/and/short-circuit: lhs succeeds, rhs builtin error surfaces" + experimental_keywords: true + strict_error: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + input.x > 0 and to_number("foo") == 1 + } + input: + x: 1 + want_error_code: eval_builtin_error + want_error: to_number + - note: "logic_op/and/short-circuit: guarded type access" + experimental_keywords: true + strict_error: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + # RHS would error if evaluated + p if { + is_array(input.x) and count(input.x) > 0 + } + input: + x: "not an array" + want_result: [] diff --git a/v1/test/cases/testdata/v1/logic_operators/comprehension.yaml b/v1/test/cases/testdata/v1/logic_operators/comprehension.yaml new file mode 100644 index 0000000000..0a6f56eb56 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/comprehension.yaml @@ -0,0 +1,95 @@ +--- +cases: + - note: "logic_op/comprehension: array, and as filter" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + # One entry per x where both operands succeed. + p := [x | + x := input.xs[_] + x > 0 and x < 10 + ] + input: + xs: [-1, 5, 12, 3] + want_result: + - x: [5, 3] + - note: "logic_op/comprehension: array, or as filter" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p := [x | + x := input.xs[_] + x < 0 or x > 10 + ] + input: + xs: [-1, 5, 12, 3] + want_result: + - x: [-1, 12] + - note: "logic_op/comprehension: set with and (dedup)" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + # Set semantics dedupe even if multiple iterations match. + p := {x | + x := input.xs[_] + x > 0 and x < 10 + } + input: + xs: [1, 1, 2, 2, 3, 3] + sort_bindings: true + want_result: + - x: [1, 2, 3] + - note: "logic_op/comprehension: object with or" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p := {k: v | + v := input[k] + v > 0 or v < -10 + } + input: + a: 5 + b: 0 + c: -20 + d: -1 + want_result: + - x: + a: 5 + c: -20 + - note: "logic_op/comprehension: nested comprehension with and" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p := [r | + xs := input.matrix[_] + r := [n | + n := xs[_] + n > 0 and n < 10 + ] + ] + input: + matrix: + - [1, -2, 5] + - [12, 3, -4] + want_result: + - x: [[1, 5], [3]] diff --git a/v1/test/cases/testdata/v1/logic_operators/every.yaml b/v1/test/cases/testdata/v1/logic_operators/every.yaml new file mode 100644 index 0000000000..82c666f110 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/every.yaml @@ -0,0 +1,103 @@ +--- +cases: + - note: "logic_op/every: and inside every body, all succeed" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + every n in input.xs { + n > 0 and n < 10 + } + } + input: + xs: [1, 2, 3] + want_result: + - x: true + - note: "logic_op/every: and inside every body, one fails" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + every n in input.xs { + n > 0 and n < 10 + } + } + input: + xs: [1, 2, 100] + want_result: [] + - note: "logic_op/every: or inside every body, all match at least one branch" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + every n in input.xs { + n < 0 or n > 0 + } + } + input: + xs: [-3, 1, 5] + want_result: + - x: true + - note: "logic_op/every: or inside every body, one matches neither branch" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + every n in input.xs { + n < 0 or n > 0 + } + } + input: + xs: [-3, 0, 5] + want_result: [] + - note: "logic_op/every: not (or) inside every body" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + import future.keywords.not + + p if { + every n in input.xs { + not { n < 0 or n > 10 } + } + } + input: + xs: [0, 5, 9] + want_result: + - x: true + - note: "logic_op/every: scoped local var inside operand body" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + every n in input.xs { + { m := n; m > 0 } and n < 10 + } + } + input: + xs: [1, 2, 3] + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/iteration.yaml b/v1/test/cases/testdata/v1/logic_operators/iteration.yaml new file mode 100644 index 0000000000..6e245737c8 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/iteration.yaml @@ -0,0 +1,109 @@ +--- +# Operand bodies with internal enumeration. +cases: + - note: "logic_op/iteration: and" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { some n in [1, 2, 3]; n > 0 } and true + } + want_result: + - x: true + - note: "logic_op/iteration: and lhs would yield multiple successes, comprehension emits once" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p := [1 | + { some n in [1, 2, 3]; n > 0 } and true + ] + want_result: + - x: [1] + - note: "logic_op/iteration: and rhs would yield multiple successes, comprehension emits once" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p := [1 | + true and { some n in [1, 2, 3]; n > 0 } + ] + want_result: + - x: [1] + - note: "logic_op/iteration: and lhs+rhs would yield multiple successes, comprehension emits once" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p := [1 | + { some n in [1, 2, 3]; n > 0 } and { some n in [4, 5, 6]; n > 0 } + ] + want_result: + - x: [1] + - note: "logic_op/iteration: or lhs would yield multiple successes, comprehension emits once" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p := [1 | + { some n in [1, 2, 3]; n > 0 } or false + ] + want_result: + - x: [1] + - note: "logic_op/iteration: or rhs would yield multiple successes, comprehension emits once" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p := [1 | + false or { some n in [1, 2, 3]; n > 0 } + ] + want_result: + - x: [1] + - note: "logic_op/iteration: enumeration inside operand body, no fail-and-retry" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + # Only one needs to succeed + p := [1 | + { some n in [-1, 2, 3, 4]; n == 2 } and true + ] + want_result: + - x: [1] + - note: "logic_op/iteration: enumeration inside operand body, all fail" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + # All fail + p := [1 | + { some n in [-1, 2, 3, 4]; n == 42 } and true + ] + want_result: + - x: [] diff --git a/v1/test/cases/testdata/v1/logic_operators/negation.yaml b/v1/test/cases/testdata/v1/logic_operators/negation.yaml new file mode 100644 index 0000000000..49490bccfc --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/negation.yaml @@ -0,0 +1,134 @@ +--- +cases: + - note: "logic_op/negation: not {x and y}, de Morgan equivalent" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.not + + p if { + not { input.x and input.y } + } + input: + x: true + "y": false + want_result: + - x: true + - note: "logic_op/negation: not {x and y}, both true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.not + + p if { + not { input.x and input.y } + } + input: + x: true + "y": true + want_result: [] + - note: "logic_op/negation: not {x or y}, neither true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + import future.keywords.not + + p if { + not { input.x or input.y } + } + input: + x: false + "y": false + want_result: + - x: true + - note: "logic_op/negation: not {x or y}, one true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + import future.keywords.not + + p if { + not { input.x or input.y } + } + input: + x: false + "y": true + want_result: [] + - note: "logic_op/negation: not x and not y, inner negation" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + not input.x and not input.y + } + input: + x: false + "y": false + want_result: + - x: true + - note: "logic_op/negation: not x or not y, inner negation" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + not input.x or not input.y + } + input: + x: false + "y": false + want_result: + - x: true + - note: "logic_op/negation: not {not x or not y}, double negation" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + import future.keywords.not + + p if { + not {not input.x or not input.y} + } + input: + x: true + "y": true + want_result: + - x: true + - note: "logic_op/negation: not {x and not y}, mixed" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.not + + p if { + not {input.x and not input.y} + } + input: + x: true + "y": true + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/nesting.yaml b/v1/test/cases/testdata/v1/logic_operators/nesting.yaml new file mode 100644 index 0000000000..5642ab4b5f --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/nesting.yaml @@ -0,0 +1,96 @@ +--- +cases: + - note: "logic_op/nesting: explicit {a or b} and {c or d}, all true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + p if { + { input.a or input.b } and { input.c or input.d } + } + input: + a: true + b: false + c: false + d: true + want_result: + - x: true + - note: "logic_op/nesting: explicit {a or b} and {c or d}, one branch fails" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + p if { + { input.a or input.b } and { input.c or input.d } + } + input: + a: false + b: false + c: true + d: true + want_result: [] + - note: "logic_op/nesting: a or {b and {c or d}}, three-deep" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + p if { + input.a or { input.b and { input.c or input.d } } + } + input: + a: false + b: true + c: false + d: true + want_result: + - x: true + - note: "logic_op/nesting: a or {b and {c or d}}, top-level lhs short-circuits" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + p if { + input.a or { input.b and { input.c or input.d } } + } + input: + a: true + b: false + c: false + d: false + want_result: + - x: true + - note: "logic_op/nesting: x and {y or {z and w}}, mixed" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + p if { + input.x and { input.y or { input.z and input.w } } + } + input: + x: true + "y": false + z: true + w: true + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/or_basic.yaml b/v1/test/cases/testdata/v1/logic_operators/or_basic.yaml new file mode 100644 index 0000000000..4deeb53aca --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/or_basic.yaml @@ -0,0 +1,110 @@ +--- +cases: + - note: "logic_op/or/basic: lhs true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + true or false + } + want_result: + - x: true + - note: "logic_op/or/basic: rhs true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + false or true + } + want_result: + - x: true + - note: "logic_op/or/basic: both true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + true or true + } + want_result: + - x: true + - note: "logic_op/or/basic: both false" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + false or false + } + want_result: [] + - note: "logic_op/or/basic: lhs succeeds via input" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.role == "admin" or input.role == "user" + } + input: + role: "admin" + want_result: + - x: true + - note: "logic_op/or/basic: rhs succeeds via input" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.role == "admin" or input.role == "user" + } + input: + role: "user" + want_result: + - x: true + - note: "logic_op/or/basic: neither side succeeds via input" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.role == "admin" or input.role == "user" + } + input: + role: "guest" + want_result: [] + - note: "logic_op/or/basic: lhs undefined ref, rhs true" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.missing or true + } + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/or_explicit_body.yaml b/v1/test/cases/testdata/v1/logic_operators/or_explicit_body.yaml new file mode 100644 index 0000000000..918a70bc73 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/or_explicit_body.yaml @@ -0,0 +1,98 @@ +--- +cases: + - note: "logic_op/or/explicit-body: explicit lhs, implicit rhs" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + { a := 0; a > 0 } or input.y == 2 + } + input: + "y": 2 + want_result: + - x: true + - note: "logic_op/or/explicit-body: implicit lhs, explicit rhs" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.y == 2 or { a := 1; a > 0 } + } + input: + "y": 0 + want_result: + - x: true + - note: "logic_op/or/explicit-body: both explicit, lhs succeeds" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + { a := 1; a > 0 } or { b := 0; b > 0 } + } + want_result: + - x: true + - note: "logic_op/or/explicit-body: both explicit, rhs succeeds" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + { a := 0; a > 0 } or { b := 1; b > 0 } + } + want_result: + - x: true + - note: "logic_op/or/explicit-body: both explicit, both fail" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + { a := 0; a > 0 } or { b := 0; b > 0 } + } + want_result: [] + - note: "logic_op/or/explicit-body: multi-expr operand body" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + { a := input.a; a > 0; a < 10 } or true + } + want_result: + - x: true + - note: "logic_op/or/explicit-body: some-binding inside operand body" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + { some n in input.xs; n > 0 } or false + } + input: + xs: [-1, 0, 5] + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/or_short_circuit.yaml b/v1/test/cases/testdata/v1/logic_operators/or_short_circuit.yaml new file mode 100644 index 0000000000..81414814b9 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/or_short_circuit.yaml @@ -0,0 +1,54 @@ +--- +# Short-circuit is observed via strict-builtin-errors: the RHS is wired +# with a builtin that errors at runtime; the test passes only if the RHS is +# *not* evaluated when the LHS already succeeded. +cases: + - note: "logic_op/or/short-circuit: lhs succeeds, rhs builtin not evaluated" + experimental_keywords: true + strict_error: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.x > 0 or to_number("foo") == 1 + } + input: + x: 1 + want_result: + - x: true + - note: "logic_op/or/short-circuit: lhs fails, rhs builtin error surfaces" + experimental_keywords: true + strict_error: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.x > 0 or to_number("foo") == 1 + } + input: + x: 0 + want_error_code: eval_builtin_error + want_error: to_number + - note: "logic_op/or/short-circuit: lhs fails, rhs evaluates and succeeds" + experimental_keywords: true + strict_error: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.x > 0 or input.y > 0 + } + input: + x: 0 + "y": 1 + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/or_single_result.yaml b/v1/test/cases/testdata/v1/logic_operators/or_single_result.yaml new file mode 100644 index 0000000000..8547731a15 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/or_single_result.yaml @@ -0,0 +1,46 @@ +--- +cases: + - note: "logic_op/or/single-result: both operands succeed contributes one element" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + # Comprehension makes cardinality observable: a broken `or` that ran + # both operands and emitted two outer successes would yield [1, 1]. + p := [1 | + true or true + ] + want_result: + - x: [1] + - note: "logic_op/or/single-result: nested or, all branches succeed contributes one element" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p := [1 | + { true or true } or { true or true } + ] + want_result: + - x: [1] + - note: "logic_op/or/single-result: inside iterating comprehension, one entry per outer binding" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p := [i | + i := input.xs[_] + i > 0 or i != 0 # if lhs/rhs both contributed a result, we'd get duplicates in the array + ] + input: + xs: [1, 2, 3] + want_result: + - x: [1, 2, 3] diff --git a/v1/test/cases/testdata/v1/logic_operators/precedence.yaml b/v1/test/cases/testdata/v1/logic_operators/precedence.yaml new file mode 100644 index 0000000000..468a85fb1d --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/precedence.yaml @@ -0,0 +1,85 @@ +--- +cases: + - note: "logic_op/precedence: `not x or y` parses as `(not x) or y`" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + # x=true, y=true: + # `(not x) or y` = false or true = true -> p=true + # `not (x or y)` = not true = false -> undefined + p if { + not input.x or input.y + } + input: + x: true + "y": true + want_result: + - x: true + - note: "logic_op/precedence: `x or y and z` parses as `x or (y and z)`" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + # x=true, y=false, z=false: + # `x or (y and z)` = true or _ = true -> p=true (short-circuit) + # `(x or y) and z` = (true or false) and false = false -> undefined + p if { + input.x or input.y and input.z + } + input: + x: true + "y": false + z: false + want_result: + - x: true + - note: "logic_op/precedence: `x and y or z and w` parses as `(x and y) or (z and w)`" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + # x=false, y=true, z=true, w=true: + # `(x and y) or (z and w)` = false or true = true -> p=true + # `x and (y or z) and w` = false and ... = false -> undefined + p if { + input.x and input.y or input.z and input.w + } + input: + x: false + "y": true + z: true + w: true + want_result: + - x: true + - note: "logic_op/precedence: `not x and not y or z` parses as `((not x) and (not y)) or z`" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + import future.keywords.or + + # x=true, y=true, z=true: + # `((not x) and (not y)) or z` = (false and false) or true = true -> p=true + # `not x and not (y or z)` = false and not true = false -> undefined + p if { + not input.x and not input.y or input.z + } + input: + x: true + "y": true + z: true + want_result: + - x: true diff --git a/v1/test/cases/testdata/v1/logic_operators/with_modifier.yaml b/v1/test/cases/testdata/v1/logic_operators/with_modifier.yaml new file mode 100644 index 0000000000..9514e5bd45 --- /dev/null +++ b/v1/test/cases/testdata/v1/logic_operators/with_modifier.yaml @@ -0,0 +1,62 @@ +--- +cases: + - note: "logic_op/with: outer with on and propagates to both operand bodies" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + input.x > 0 and input.y > 0 with input as {"x": 1, "y": 2} + } + want_result: + - x: true + - note: "logic_op/with: with on lhs only, scoped to lhs" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { input.x < 2 with input.x as 1 } and input.x == 5 + } + input: + x: 5 + want_result: + - x: true + - note: "logic_op/with: with on lhs only, scoped to lhs, but falls through to called scope" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.and + + p if { + { q with input.x as 1 } and input.x == 5 + } + + q if { + input.x < 2 + } + input: + x: 5 + want_result: + - x: true + - note: "logic_op/with: outer with on or, both operands see modified input" + experimental_keywords: true + query: data.test.p = x + modules: + - | + package test + import future.keywords.or + + p if { + input.x > 0 or input.y > 0 with input as {"x": 0, "y": 1} + } + want_result: + - x: true diff --git a/v1/topdown/copypropagation/copypropagation.go b/v1/topdown/copypropagation/copypropagation.go index ae30723dfb..8d70565c8c 100644 --- a/v1/topdown/copypropagation/copypropagation.go +++ b/v1/topdown/copypropagation/copypropagation.go @@ -498,18 +498,20 @@ func makeDisjointSets(livevars ast.VarSet, query ast.Body) (*unionFind, bool) { func isNoop(expr *ast.Expr) bool { - if !expr.IsCall() && !expr.IsEvery() { - term := expr.Terms.(*ast.Term) - if !ast.IsConstant(term.Value) { + switch t := expr.Terms.(type) { + case []*ast.Term: + // A==A can be ignored + if expr.Operator().Equal(ast.Equal.Ref()) { + return expr.Operand(0).Equal(expr.Operand(1)) + } + return false + case *ast.Term: + if !ast.IsConstant(t.Value) { return false } - return !ast.Boolean(false).Equal(term.Value) + return !ast.Boolean(false).Equal(t.Value) + default: + // *ast.Every, *ast.Not, *ast.LogicalAnd, *ast.LogicalOr — none are no-ops. + return false } - - // A==A can be ignored - if expr.Operator().Equal(ast.Equal.Ref()) { - return expr.Operand(0).Equal(expr.Operand(1)) - } - - return false } diff --git a/v1/topdown/eval.go b/v1/topdown/eval.go index b6700549de..7822477e58 100644 --- a/v1/topdown/eval.go +++ b/v1/topdown/eval.go @@ -545,6 +545,30 @@ func (e *eval) evalStep(iter evalIterator) error { return err }) + case *ast.LogicalAnd: + ea := evalLogicalAnd{ + e: e, + and: terms, + } + err = ea.eval(func(e *eval) error { + defined = true + err := iter(e) + e.traceRedo(expr) + return err + }) + + case *ast.LogicalOr: + eo := evalLogicalOr{ + e: e, + or: terms, + } + err = eo.eval(func(e *eval) error { + defined = true + err := iter(e) + e.traceRedo(expr) + return err + }) + default: // guard-rail for adding extra (Expr).Terms types return fmt.Errorf("got %T terms: %[1]v", terms) } @@ -604,6 +628,24 @@ func (e *eval) evalStep(iter evalIterator) error { return iter(e) }) + case *ast.LogicalAnd: + ea := evalLogicalAnd{ + e: e, + and: terms, + } + err = ea.eval(func(e *eval) error { + return iter(e) + }) + + case *ast.LogicalOr: + eo := evalLogicalOr{ + e: e, + or: terms, + } + err = eo.eval(func(e *eval) error { + return iter(e) + }) + default: // guard-rail for adding extra (Expr).Terms types return fmt.Errorf("got %T terms: %[1]v", terms) } @@ -4214,7 +4256,7 @@ func (e *evalEvery) save(iter unifyIterator) error { func (e *evalEvery) plug(expr *ast.Expr) (*ast.Expr, error) { cpy := expr.Copy() every := cpy.Terms.(*ast.Every) - if err := e.plugBody(every.Body); err != nil { + if err := plugBody(e.e, every.Body); err != nil { return nil, err } @@ -4225,45 +4267,6 @@ func (e *evalEvery) plug(expr *ast.Expr) (*ast.Expr, error) { return cpy, nil } -func (e *evalEvery) plugBody(body ast.Body) error { - for i := range body { - switch t := body[i].Terms.(type) { - case *ast.Term: - plugged, err := e.plugTerm(t) - if err != nil { - return err - } - body[i].Terms = plugged - case []*ast.Term: - for j := 1; j < len(t); j++ { // don't plug operator, t[0] - plugged, err := e.plugTerm(t[j]) - if err != nil { - return err - } - t[j] = plugged - } - case *ast.Every: - plugged, err := e.plug(body[i]) - if err != nil { - return err - } - body[i] = plugged - case *ast.Not: - if err := e.plugBody(t.Body); err != nil { - return err - } - } - } - return nil -} - -func (e *evalEvery) plugTerm(t *ast.Term) (*ast.Term, error) { - if ast.IsComprehension(t.Value) { - return e.e.amendComprehension(t, e.e.bindings) - } - return e.e.bindings.PlugNamespaced(t, e.e.caller.bindings), nil -} - type evalNot struct { e *eval not *ast.Not @@ -4364,6 +4367,212 @@ func (e evalNot) evalPartial(iter evalIterator) error { return e.e.evalNotPartial(expr, unNegate, ast.Complement, supportTerms, iter) } +type evalLogicalAnd struct { + e *eval + and *ast.LogicalAnd +} + +func (e evalLogicalAnd) eval(iter evalIterator) error { + if e.e.partial() && (e.e.unknown(e.and.Lhs, e.e.bindings) || e.e.unknown(e.and.Rhs, e.e.bindings)) { + return e.evalPartial(iter) + } + + lhsDefined, err := evalLogicalOperand(e.e, e.and.Lhs) + if err != nil { + return err + } + if !lhsDefined { + // short-circuit: RHS is not evaluated if LHS is undefined + return nil + } + + rhsDefined, err := evalLogicalOperand(e.e, e.and.Rhs) + if err != nil { + return err + } + if !rhsDefined { + return nil + } + + return iter(e.e) +} + +func (e evalLogicalAnd) evalPartial(iter evalIterator) error { + // Plug and save the expression to produce a valid, but non-optimized PE result + expr := e.e.query[e.e.index] + + plugged, err := e.plug(expr) + if err != nil { + return err + } + + return e.e.saveExpr(plugged, e.e.bindings, func() error { + return iter(e.e) + }) +} + +func (e evalLogicalAnd) plug(expr *ast.Expr) (*ast.Expr, error) { + cpy := expr.Copy() + and := cpy.Terms.(*ast.LogicalAnd) + + if err := plugBody(e.e, and.Lhs); err != nil { + return nil, err + } + if err := plugBody(e.e, and.Rhs); err != nil { + return nil, err + } + + cpy.Terms = and + return cpy, nil +} + +type evalLogicalOr struct { + e *eval + or *ast.LogicalOr +} + +func (e evalLogicalOr) eval(iter evalIterator) error { + if e.e.partial() && (e.e.unknown(e.or.Lhs, e.e.bindings) || e.e.unknown(e.or.Rhs, e.e.bindings)) { + return e.evalPartial(iter) + } + + lhsDefined, err := evalLogicalOperand(e.e, e.or.Lhs) + if err != nil { + return err + } + if lhsDefined { + // short-circuit: RHS is not evaluated if LHS is defined + return iter(e.e) + } + + rhsDefined, err := evalLogicalOperand(e.e, e.or.Rhs) + if err != nil { + return err + } + if !rhsDefined { + return nil + } + + return iter(e.e) +} + +func (e evalLogicalOr) evalPartial(iter evalIterator) error { + // Plug and save the expression to produce a valid, but non-optimized PE result + expr := e.e.query[e.e.index] + + plugged, err := e.plug(expr) + if err != nil { + return err + } + + return e.e.saveExpr(plugged, e.e.bindings, func() error { + return iter(e.e) + }) +} + +func (e evalLogicalOr) plug(expr *ast.Expr) (*ast.Expr, error) { + cpy := expr.Copy() + or := cpy.Terms.(*ast.LogicalOr) + + if err := plugBody(e.e, or.Lhs); err != nil { + return nil, err + } + if err := plugBody(e.e, or.Rhs); err != nil { + return nil, err + } + + cpy.Terms = or + return cpy, nil +} + +// evalLogicalOperand runs body as a closed scope that contributes at most one +// success. Returns whether the body succeeded; bindings introduced inside body +// do not propagate to the caller. +func evalLogicalOperand(parent *eval, body ast.Body) (bool, error) { + child := evalPool.Get() + defer evalPool.Put(child) + + parent.closure(body, child) + child.findOne = true + + if parent.traceEnabled { + child.traceEnter(body) + } + + defined := false + err := child.eval(func(*eval) error { + if parent.traceEnabled { + child.traceExit(body) + child.traceRedo(body) + } + defined = true + return nil + }) + + // findOne raises an earlyExitError once the iter callback fires; that's + // our signal to stop, not an error to propagate to the caller. + if err := suppressEarlyExit(err); err != nil { + return false, err + } + + return defined, nil +} + +func plugBody(e *eval, body ast.Body) error { + for i := range body { + switch t := body[i].Terms.(type) { + case *ast.Term: + plugged, err := plugTerm(e, t) + if err != nil { + return err + } + body[i].Terms = plugged + case []*ast.Term: + for j := 1; j < len(t); j++ { // don't plug operator, t[0] + plugged, err := plugTerm(e, t[j]) + if err != nil { + return err + } + t[j] = plugged + } + case *ast.Every: + ev := evalEvery{e: e, every: t, expr: body[i]} + plugged, err := ev.plug(body[i]) + if err != nil { + return err + } + body[i] = plugged + case *ast.Not: + if err := plugBody(e, t.Body); err != nil { + return err + } + case *ast.LogicalAnd: + if err := plugBody(e, t.Lhs); err != nil { + return err + } + if err := plugBody(e, t.Rhs); err != nil { + return err + } + case *ast.LogicalOr: + if err := plugBody(e, t.Lhs); err != nil { + return err + } + if err := plugBody(e, t.Rhs); err != nil { + return err + } + } + } + + return nil +} + +func plugTerm(e *eval, t *ast.Term) (*ast.Term, error) { + if ast.IsComprehension(t.Value) { + return e.amendComprehension(t, e.bindings) + } + return e.bindings.PlugNamespaced(t, e.caller.bindings), nil +} + func (e *eval) comprehensionIndex(term *ast.Term) *ast.ComprehensionIndex { if e.queryCompiler != nil { return e.queryCompiler.ComprehensionIndex(term) @@ -4518,17 +4727,29 @@ func containsNestedRefOrCall(vis *nestedCheckVisitor, expr *ast.Expr) bool { } if n, ok := expr.Terms.(*ast.Not); ok { - for _, nExpr := range n.Body { - if containsNestedRefOrCall(vis, nExpr) { - return true - } - } - return false + return containsNestedRefOrCallInBody(vis, n.Body) + } + + if a, ok := expr.Terms.(*ast.LogicalAnd); ok { + return containsNestedRefOrCallInBody(vis, a.Lhs) || containsNestedRefOrCallInBody(vis, a.Rhs) + } + + if o, ok := expr.Terms.(*ast.LogicalOr); ok { + return containsNestedRefOrCallInBody(vis, o.Lhs) || containsNestedRefOrCallInBody(vis, o.Rhs) } return containsNestedRefOrCallInTerm(vis, expr.Terms.(*ast.Term)) } +func containsNestedRefOrCallInBody(vis *nestedCheckVisitor, body ast.Body) bool { + for _, expr := range body { + if containsNestedRefOrCall(vis, expr) { + return true + } + } + return false +} + func containsNestedRefOrCallInTerm(vis *nestedCheckVisitor, term *ast.Term) bool { switch v := term.Value.(type) { case ast.Ref: diff --git a/v1/topdown/exported_test.go b/v1/topdown/exported_test.go index 6686641d43..47c095eba3 100644 --- a/v1/topdown/exported_test.go +++ b/v1/topdown/exported_test.go @@ -130,6 +130,10 @@ func testRun(t *testing.T, tc cases.TestCase, opts ...testOpt) { o(tos) } + if tc.ExperimentalKeywords && tos.parserOptions.Capabilities == nil { + tos.parserOptions.Capabilities = ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)) + } + for k, v := range tc.Env { t.Setenv(k, v) } diff --git a/v1/topdown/topdown_logical_test.go b/v1/topdown/topdown_logical_test.go new file mode 100644 index 0000000000..938e04b07d --- /dev/null +++ b/v1/topdown/topdown_logical_test.go @@ -0,0 +1,423 @@ +// Copyright 2026 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +package topdown + +import ( + "bytes" + "os" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/open-policy-agent/opa/v1/ast" + "github.com/open-policy-agent/opa/v1/types" +) + +var touchCounters sync.Map // map[string]*atomic.Int64 + +func init() { + ast.RegisterBuiltin(&ast.Builtin{ + Name: "test.touch", + Decl: types.NewFunction( + types.Args(types.S), + types.B, + ), + }) + + RegisterBuiltinFunc("test.touch", func(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) error) error { + label := string(operands[0].Value.(ast.String)) + counter, _ := touchCounters.LoadOrStore(label, &atomic.Int64{}) + counter.(*atomic.Int64).Add(1) + return iter(ast.BooleanTerm(true)) + }) +} + +func touchCount(label string) int { + counter, ok := touchCounters.Load(label) + if !ok { + return 0 + } + return int(counter.(*atomic.Int64).Load()) +} + +// logicalParserOptions opts in to the experimental `and` / `or` keywords. +func logicalParserOptions() ast.ParserOptions { + return ast.ParserOptions{ + Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), + FutureKeywords: []string{"and", "or"}, + } +} + +func TestTopDownLogicalAnd(t *testing.T) { + t.Parallel() + + n := func(ns ...string) []string { return ns } + + tests := []struct { + note string + module string + notes []string + fail bool + }{ + { + note: "both succeed", + module: `package test + p if { + true and true + }`, + }, + { + note: "lhs fails", + module: `package test + p if { + false and true + }`, + fail: true, + }, + { + note: "rhs fails", + module: `package test + p if { + true and false + }`, + fail: true, + }, + { + note: "lhs fails: rhs not evaluated (short-circuit)", + module: `package test + p if { + false and print("rhs") + }`, + notes: n(), + fail: true, + }, + { + note: "lhs succeeds: rhs evaluated", + module: `package test + p if { + print("lhs") and print("rhs") + }`, + notes: n("lhs", "rhs"), + }, + { + note: "explicit body operands", + module: `package test + p if { + {x := 1; x > 0} and {y := 2; y > 0} + }`, + }, + { + note: "explicit body: each operand has its own scope", + module: `package test + p if { + {x := 1; x > 0} and {x := 2; x > 1} + }`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + t.Parallel() + runLogicalCase(t, tc.module, tc.notes, tc.fail) + }) + } +} + +func TestTopDownLogicalOr(t *testing.T) { + t.Parallel() + + n := func(ns ...string) []string { return ns } + + tests := []struct { + note string + module string + notes []string + fail bool + }{ + { + note: "both succeeds", + module: `package test + p if { + true or true + }`, + }, + { + note: "lhs succeeds", + module: `package test + p if { + true or false + }`, + }, + { + note: "lhs fails, rhs succeeds", + module: `package test + p if { + false or true + }`, + }, + { + note: "both fail", + module: `package test + p if { + false or false + }`, + fail: true, + }, + { + note: "lhs succeeds: rhs not evaluated (short-circuit)", + module: `package test + p if { + print("lhs") or print("rhs") + }`, + notes: n("lhs"), + }, + { + note: "lhs fails: rhs evaluated", + module: `package test + p if { + false or print("rhs") + }`, + notes: n("rhs"), + }, + { + note: "explicit body operands", + module: `package test + p if { + {x := 0; x > 0} or {y := 2; y > 0} + }`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + t.Parallel() + runLogicalCase(t, tc.module, tc.notes, tc.fail) + }) + } +} + +// TestTopDownLogicalOrSingleResult locks in the cardinality rule: +// `or` produces exactly one success even when both operands would succeed. +func TestTopDownLogicalOrSingleResult(t *testing.T) { + t.Parallel() + + module := `package test + q contains "x" if { + true or true + }` + + ctx := t.Context() + c := ast.NewCompiler() + mod := ast.MustParseModuleWithOpts(module, logicalParserOptions()) + c.Compile(map[string]*ast.Module{"test": mod}) + if c.Failed() { + t.Fatal(c.Errors) + } + + tr := NewBufferTracer() + query := NewQuery(ast.MustParseBody("data.test.q = x")). + WithCompiler(c). + WithQueryTracer(tr) + + res, err := query.Run(ctx) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(res) != 1 { + t.Fatalf("expected 1 result, got %d: %v", len(res), res) + } + + set, ok := res[0]["x"].Value.(ast.Set) + if !ok { + t.Fatalf("expected set value, got %T: %v", res[0]["x"].Value, res[0]["x"]) + } + if set.Len() != 1 { + t.Errorf("expected set of size 1 (single-result `or`), got %d: %v", set.Len(), set) + } + + if t.Failed() || testing.Verbose() { + PrettyTrace(os.Stderr, *tr) + } +} + +func runLogicalCase(t *testing.T, module string, expectedNotes []string, expectFail bool) { + t.Helper() + + ctx := t.Context() + c := ast.NewCompiler().WithEnablePrintStatements(true) + mod := ast.MustParseModuleWithOpts(module, logicalParserOptions()) + c.Compile(map[string]*ast.Module{"test": mod}) + if c.Failed() { + t.Fatal(c.Errors) + } + if testing.Verbose() { + t.Log(c.Modules) + } + + buf := bytes.Buffer{} + tr := NewBufferTracer() + ph := NewPrintHook(&buf) + query := NewQuery(ast.MustParseBody("data.test.p = x")). + WithCompiler(c). + WithPrintHook(ph). + WithQueryTracer(tr) + + res, err := query.Run(ctx) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !expectFail { + if len(res) == 0 { + t.Errorf("unexpected failure, empty query result set") + } + } else { + if len(res) > 0 { + t.Errorf("unexpected results: %v, expected empty query result set", res) + } + } + + notes := strings.Split(buf.String(), "\n") + notes = notes[:len(notes)-1] // last is empty after trailing "\n" + if len(expectedNotes) != 0 || len(notes) != 0 { + if !slices.Equal(notes, expectedNotes) { + t.Errorf("unexpected prints, expected %q, got %q", expectedNotes, notes) + } + } + + if t.Failed() || testing.Verbose() { + PrettyTrace(os.Stderr, *tr) + } +} + +func TestTopDownLogicalAndShortCircuit(t *testing.T) { + t.Parallel() + + tests := []struct { + note string + label string // unique per case so subcases can run in parallel + module string + wantTouch int + }{ + { + note: "lhs false: rhs builtin not invoked", + label: "and-shortcircuit-lhs-false", + module: `package test + import future.keywords.and + p if { + false and test.touch("and-shortcircuit-lhs-false") + }`, + wantTouch: 0, + }, + { + note: "lhs true: rhs builtin invoked exactly once", + label: "and-shortcircuit-lhs-true", + module: `package test + import future.keywords.and + p if { + true and test.touch("and-shortcircuit-lhs-true") + }`, + wantTouch: 1, + }, + { + note: "lhs undefined ref: rhs builtin not invoked", + label: "and-shortcircuit-lhs-undefined", + module: `package test + import future.keywords.and + p if { + input.missing and test.touch("and-shortcircuit-lhs-undefined") + }`, + wantTouch: 0, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + t.Parallel() + runTouchCase(t, tc.label, tc.module, tc.wantTouch) + }) + } +} + +func TestTopDownLogicalOrShortCircuit(t *testing.T) { + t.Parallel() + + tests := []struct { + note string + label string + module string + wantTouch int + }{ + { + note: "lhs true: rhs builtin not invoked", + label: "or-shortcircuit-lhs-true", + module: `package test + import future.keywords.or + p if { + true or test.touch("or-shortcircuit-lhs-true") + }`, + wantTouch: 0, + }, + { + note: "lhs false: rhs builtin invoked exactly once", + label: "or-shortcircuit-lhs-false", + module: `package test + import future.keywords.or + p if { + false or test.touch("or-shortcircuit-lhs-false") + }`, + wantTouch: 1, + }, + { + note: "lhs undefined ref: rhs builtin invoked exactly once", + label: "or-shortcircuit-lhs-undefined", + module: `package test + import future.keywords.or + p if { + input.missing or test.touch("or-shortcircuit-lhs-undefined") + }`, + wantTouch: 1, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + t.Parallel() + runTouchCase(t, tc.label, tc.module, tc.wantTouch) + }) + } +} + +func runTouchCase(t *testing.T, label, module string, wantTouch int) { + t.Helper() + + ctx := t.Context() + c := ast.NewCompiler() + mod := ast.MustParseModuleWithOpts(module, logicalParserOptions()) + c.Compile(map[string]*ast.Module{"test": mod}) + if c.Failed() { + t.Fatal(c.Errors) + } + + tr := NewBufferTracer() + query := NewQuery(ast.MustParseBody("data.test.p = x")). + WithCompiler(c). + WithQueryTracer(tr) + + if _, err := query.Run(ctx); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := touchCount(label); got != wantTouch { + t.Errorf("test.touch(%q) call count: got %d, want %d", label, got, wantTouch) + } + + if t.Failed() || testing.Verbose() { + PrettyTrace(os.Stderr, *tr) + } +} diff --git a/v1/topdown/topdown_partial_test.go b/v1/topdown/topdown_partial_test.go index ce9defb093..56000d77c9 100644 --- a/v1/topdown/topdown_partial_test.go +++ b/v1/topdown/topdown_partial_test.go @@ -41,6 +41,7 @@ func TestTopDownPartialEval(t *testing.T) { wantSupport []string wantSupportASTs []*ast.Module ignoreOrder bool + experimentalKeywords bool // opt in to experimental and/or keywords }{ { note: "empty", @@ -4293,12 +4294,491 @@ default q := false q if { input.x = 7 }`}, wantQueries: []string{"input.x = 7"}, }, + + // and/or baseline (no unknowns: truth-table) + + { + note: "and: no unknowns, both true", + experimentalKeywords: true, + query: "data.test.p", + modules: []string{`package test + p if { + true and true + }`}, + wantQueries: []string{""}, // unconditionally true + }, + { + note: "and: no unknowns, lhs false", + experimentalKeywords: true, + query: "data.test.p", + modules: []string{`package test + p if { + false and true + }`}, + wantQueries: []string{}, // unconditionally false + }, + { + note: "and: no unknowns, rhs false", + experimentalKeywords: true, + query: "data.test.p", + modules: []string{`package test + p if { + true and false + }`}, + wantQueries: []string{}, // unconditionally false + }, + { + note: "or: no unknowns, lhs true", + experimentalKeywords: true, + query: "data.test.p", + modules: []string{`package test + p if { + true or false + }`}, + wantQueries: []string{""}, // unconditionally true + }, + { + note: "or: no unknowns, rhs true", + experimentalKeywords: true, + query: "data.test.p", + modules: []string{`package test + p if { + false or true + }`}, + wantQueries: []string{""}, // unconditionally true + }, + { + note: "or: no unknowns, both true (single result)", + experimentalKeywords: true, + query: "data.test.p", + modules: []string{`package test + p if { + true or true + }`}, + wantQueries: []string{""}, // unconditionally true + }, + { + note: "or: no unknowns, both false", + experimentalKeywords: true, + query: "data.test.p", + modules: []string{`package test + p if { + false or false + }`}, + wantQueries: []string{}, // unconditionally false + }, + + // and/or baseline (simple unknowns: save the whole expression) + // TODO: PE optimization in #8680 + + { + note: "and: unknown lhs only", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.x > 0 and true + }`}, + wantQueries: []string{`{__local0__1 = input.x; gt(__local0__1, 0)} and true`}, + }, + { + note: "and: unknown rhs only", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + true and input.y > 0 + }`}, + wantQueries: []string{`true and {__local0__1 = input.y; gt(__local0__1, 0)}`}, + }, + { + note: "and: unknowns in both", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.x > 0 and input.y > 0 + }`}, + wantQueries: []string{`{__local0__1 = input.x; gt(__local0__1, 0)} and {__local1__1 = input.y; gt(__local1__1, 0)}`}, + }, + { + note: "or: unknown lhs only", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.x > 0 or false + }`}, + wantQueries: []string{`{__local0__1 = input.x; gt(__local0__1, 0)} or false`}, + }, + { + note: "or: unknown rhs only", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + false or input.y > 0 + }`}, + wantQueries: []string{`false or {__local0__1 = input.y; gt(__local0__1, 0)}`}, + }, + { + note: "or: unknowns in both", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.x > 0 or input.y > 0 + }`}, + wantQueries: []string{`{__local0__1 = input.x; gt(__local0__1, 0)} or {__local1__1 = input.y; gt(__local1__1, 0)}`}, + }, + // Baseline does NOT simplify even when one operand is statically true + { + note: "or: unknown lhs, rhs known-true (no simplification at this layer)", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.x > 0 or true + }`}, + wantQueries: []string{`{__local0__1 = input.x; gt(__local0__1, 0)} or true`}, + }, + { + note: "and: unknown lhs, rhs known-true (no simplification at this layer)", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.x > 0 and true + }`}, + wantQueries: []string{`{__local0__1 = input.x; gt(__local0__1, 0)} and true`}, + }, + + // nested chains, data-ref unknowns, multi-expr explicit bodies + { + note: "and: nested left-leaning, all unknown", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.a > 0 and input.b > 0 and input.c > 0 + }`}, + wantQueries: []string{`{ + __local0__1 = input.a + gt(__local0__1, 0) + } and { + __local1__1 = input.b + gt(__local1__1, 0) + } and { + __local2__1 = input.c + gt(__local2__1, 0) + }`}, + }, + { + note: "or: nested left-leaning, all unknown", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + input.a > 0 or input.b > 0 or input.c > 0 + }`}, + wantQueries: []string{`{ + __local0__1 = input.a + gt(__local0__1, 0) + } or { + __local1__1 = input.b + gt(__local1__1, 0) + } or { + __local2__1 = input.c + gt(__local2__1, 0) + }`}, + }, + { + note: "and: mixed data and input unknowns", + experimentalKeywords: true, + unknowns: []string{"input", "data.foo"}, + query: "data.test.p = true", + modules: []string{`package test + p if { + data.foo.x > 0 and input.y > 0 + }`}, + wantQueries: []string{`{ + __local0__1 = data.foo.x + gt(__local0__1, 0) + } and { + __local1__1 = input.y + gt(__local1__1, 0) + }`}, + }, + { + note: "and: explicit body multi-expr, unknowns inside", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + { input.x > 0; input.x < 10 } and true + }`}, + wantQueries: []string{`{__local0__1 = input.x; gt(__local0__1, 0); __local1__1 = input.x; lt(__local1__1, 10)} and true`}, + }, + + { + note: "and (every): unknown inside", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + { x := input.x; every i in x { i in x; i < input.y } } and true + }`}, + wantQueries: []string{`{ + __local0__1 = input.x + __local3__1 = __local0__1 + every __local1__1, __local2__1 in __local3__1 { + internal.member_2(__local2__1, __local0__1) + __local4__1 = input.y + lt(__local2__1, __local4__1) + } + } and true`}, + }, + { + note: "or (every): unknown inside", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + { x := input.x; every i in x { i in x; i < input.y } } or true + }`}, + wantQueries: []string{`{ + __local0__1 = input.x + __local3__1 = __local0__1 + every __local1__1, __local2__1 in __local3__1 { + internal.member_2(__local2__1, __local0__1) + __local4__1 = input.y + lt(__local2__1, __local4__1) + } + } or true`}, + }, + + // comprehensions inside and/or operand bodies + { + note: "and: rhs set comp, cross-scope vars", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + x := input.x + true and { y := input.y; {z | z := input.z; z < x; z > y} } + }`}, + wantQueries: []string{`true and { + __local1__1 = input.y + {__local2__1 | + __local2__1 = input.z + lt(__local2__1, input.x) + gt(__local2__1, __local1__1) + } + } + __local0__1 = input.x`}, + }, + { + note: "and: lhs set comp, cross-scope vars", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + x := input.x + { y := input.y; {z | z := input.z; z < x; z > y} } and true + }`}, + wantQueries: []string{`{ + __local1__1 = input.y + {__local2__1 | + __local2__1 = input.z + lt(__local2__1, input.x) + gt(__local2__1, __local1__1) + } + } and true + __local0__1 = input.x`}, + }, + { + note: "or: rhs set comp, cross-scope vars", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + x := input.x + false or { y := input.y; {z | z := input.z; z < x; z > y} } + }`}, + wantQueries: []string{`false or { + __local1__1 = input.y + {__local2__1 | + __local2__1 = input.z + lt(__local2__1, input.x) + gt(__local2__1, __local1__1) + } + } + __local0__1 = input.x`}, + }, + { + note: "or: lhs set comp, cross-scope vars", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + x := input.x + { y := input.y; {z | z := input.z; z < x; z > y} } or false + }`}, + wantQueries: []string{`{ + __local1__1 = input.y + {__local2__1 | + __local2__1 = input.z + lt(__local2__1, input.x) + gt(__local2__1, __local1__1) + } + } or false + __local0__1 = input.x`}, + }, + { + note: "and: rhs array comp, cross-scope vars", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + x := input.x + true and { y := input.y; [z | z := input.z; z < x; z > y] } + }`}, + wantQueries: []string{`true and { + __local1__1 = input.y + [__local2__1 | + __local2__1 = input.z + lt(__local2__1, input.x) + gt(__local2__1, __local1__1) + ] + } + __local0__1 = input.x`}, + }, + { + note: "or: lhs object comp, cross-scope vars", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + x := input.x + { y := input.y; {k: v | k := input.k; v := x; v != y} } or false + }`}, + wantQueries: []string{`{ + __local1__1 = input.y + {__local2__1: __local3__1 | + __local2__1 = input.k + __local3__1 = input.x + neq(__local3__1, __local1__1) + } + } or false + __local0__1 = input.x`}, + }, + + // and/or nested inside other outer constructs + { + note: "every: body contains and with unknowns", + experimentalKeywords: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + every x in [1, 2, 3] { x > 0 and x > input } + }`}, + wantQueries: []string{`every __local0__1, __local1__1 in [1, 2, 3] { + gt(__local1__1, 0) and { __local3__1 = input; gt(__local1__1, __local3__1) } + }`}, + }, + { + note: "array comprehension: body contains or with unknowns", + experimentalKeywords: true, + query: "data.test.p = x", + modules: []string{`package test + p := [n | n = input.xs[i]; n > 0 or n < 0]`}, + wantQueries: []string{`x = [n1 | n1 = input.xs[i1]; gt(n1, 0) or lt(n1, 0)]`}, + }, + { + note: "set comprehension: body contains or with unknowns", + experimentalKeywords: true, + query: "data.test.p = x", + modules: []string{`package test + p := {n | n = input.xs[i]; n > 0 or n < 0}`}, + wantQueries: []string{`x = {n1 | n1 = input.xs[i1]; gt(n1, 0) or lt(n1, 0)}`}, + }, + { + note: "object comprehension: body contains and with unknowns", + experimentalKeywords: true, + query: "data.test.p = x", + modules: []string{`package test + p := {k: v | v := input[k]; v > 0 and v < 10}`}, + wantQueries: []string{`x = {k1: __local0__1 | __local0__1 = input[k1]; gt(__local0__1, 0) and lt(__local0__1, 10)}`}, + }, + + // copy-propagation + { + note: "copy propagation: and safety needs extra expr", + experimentalKeywords: true, + query: `data.test.p = true`, + modules: []string{ + `package test + + p if { + x = data.y[c] + x.z = 1 and x.z != 2 + } + `, + }, + unknowns: []string{`data.y`}, + wantQueries: []string{ + `x1 = data.y[c1]; x1.z = 1 and {__local0__1 = x1.z; neq(__local0__1, 2)}`, + }, + }, + { + note: "copy propagation: or safety needs extra expr", + experimentalKeywords: true, + query: `data.test.p = true`, + modules: []string{ + `package test + + p if { + x = data.y[c] + x.z = 1 or x.z = 2 + } + `, + }, + unknowns: []string{`data.y`}, + // Copy-prop inlines x1 into both operand bodies — unlike the `and` + // case above, both operands are plain `=` so there's no rewrite + // barrier — but still re-adds the binding at body level. + wantQueries: []string{ + `data.y[c1].z = 1 or data.y[c1].z = 2; x1 = data.y[c1]`, + }, + }, + { + note: "copy propagation: and safety needs extra expr - no live var overlap", + experimentalKeywords: true, + query: `data.test.p = true`, + modules: []string{ + `package test + + p if { + x = input.y[c] + x.z = 1 and x.z != 2 + } + `, + }, + unknowns: []string{`input.y`}, + wantQueries: []string{ + `x1 = input.y[c1]; x1.z = 1 and {__local0__1 = x1.z; neq(__local0__1, 2)}`, + }, + }, } ctx := t.Context() for _, tc := range tests { popts := ast.ParserOptions{} + if tc.experimentalKeywords { + popts.Capabilities = ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)) + popts.FutureKeywords = []string{"and", "or"} + } params := fixtureParams{ note: tc.note, @@ -4414,6 +4894,7 @@ func TestTopDownPartialEvalNegation(t *testing.T) { wantSupportASTs []*ast.Module ignoreOrder bool notBodyOnly bool + experimentalKeywords bool // opt in to experimental and/or keywords }{ { note: "with+builtin+negation: when replacement has no unknowns (args, defs), save negated expr without replacement", @@ -6126,6 +6607,84 @@ func TestTopDownPartialEvalNegation(t *testing.T) { v1 = input.threshold`, }, }, + + // inner and/or, with unknowns + // TODO: PE optimization in #8680 + + { + note: "not (and): unknown inside", + experimentalKeywords: true, + notBodyOnly: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + not { input.x > 0 and true } + }`}, + wantQueries: []string{`not { + { __local0__1 = input.x; gt(__local0__1, 0) } and true + }`}, + }, + { + note: "not (or): unknowns in both operands", + experimentalKeywords: true, + notBodyOnly: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + not { input.x > 0 or input.y > 0 } + }`}, + wantQueries: []string{`not { + { __local0__1 = input.x; gt(__local0__1, 0) } or { __local1__1 = input.y; gt(__local1__1, 0) } + }`}, + }, + { + note: "not (and): unknown lhs, rhs static false ", + experimentalKeywords: true, + notBodyOnly: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + not { input.x > 0 and false } + }`}, + wantQueries: []string{`not { + {__local0__1 = input.x; gt(__local0__1, 0)} and false + }`}, + }, + { + note: "not (or): unknown rhs, lhs static true", + experimentalKeywords: true, + notBodyOnly: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + not {true or input.y > 0} + }`}, + wantQueries: []string{`not { + true or {__local0__1 = input.y; gt(__local0__1, 0)} + }`}, + }, + { + note: "and (not): unknown inside", + experimentalKeywords: true, + notBodyOnly: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + { x := input.x; not x > 0 } and true + }`}, + wantQueries: []string{`{ __local0__1 = input.x; not gt(__local0__1, 0) } and true`}, + }, + { + note: "or (not): unknown inside", + experimentalKeywords: true, + notBodyOnly: true, + query: "data.test.p = true", + modules: []string{`package test + p if { + { x := input.x; not x > 0 } or true + }`}, + wantQueries: []string{`{ __local0__1 = input.x; not gt(__local0__1, 0) } or true`}, + }, } ctx := t.Context() @@ -6146,6 +6705,12 @@ func TestTopDownPartialEvalNegation(t *testing.T) { continue } + casePopts := popts + if tc.experimentalKeywords { + casePopts.Capabilities = ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)) + casePopts.FutureKeywords = append(append([]string{}, popts.FutureKeywords...), "and", "or") + } + params := fixtureParams{ note: tc.note, query: tc.query, @@ -6153,7 +6718,7 @@ func TestTopDownPartialEvalNegation(t *testing.T) { moduleASTs: tc.moduleASTs, data: tc.data, input: tc.input, - parserOptions: popts, + parserOptions: casePopts, } prepareTest(ctx, t, params, func(ctx context.Context, t *testing.T, f fixture) { @@ -6210,7 +6775,7 @@ func TestTopDownPartialEvalNegation(t *testing.T) { } for i := range tc.wantQueries { - expectedQueries = append(expectedQueries, ast.MustParseBodyWithOpts(tc.wantQueries[i], popts)) + expectedQueries = append(expectedQueries, ast.MustParseBodyWithOpts(tc.wantQueries[i], casePopts)) } queriesA, queriesB := bodySet(partials), bodySet(expectedQueries) @@ -6228,7 +6793,7 @@ func TestTopDownPartialEvalNegation(t *testing.T) { expectedSupport = tc.wantSupportASTs } else { for i := range tc.wantSupport { - expectedSupport = append(expectedSupport, ast.MustParseModuleWithOpts(tc.wantSupport[i], popts)) + expectedSupport = append(expectedSupport, ast.MustParseModuleWithOpts(tc.wantSupport[i], casePopts)) } } supportA, supportB := moduleSet(support), moduleSet(expectedSupport) @@ -6246,6 +6811,211 @@ func TestTopDownPartialEvalNegation(t *testing.T) { } } +// TestTopDownPartialEvalLogicalRoundTrip verifies semantic equivalence between +// the original and/or modules and their residuals. +func TestTopDownPartialEvalLogicalRoundTrip(t *testing.T) { + t.Parallel() + + popts := ast.ParserOptions{ + Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), + FutureKeywords: []string{"and", "or"}, + } + + cases := []struct { + note string + module string + inputs []string + }{ + { + note: "and: both unknowns", + module: `package test + p if { + input.x > 0 and input.y > 0 + }`, + inputs: []string{ + `{"x": 1, "y": 2}`, // both true -> defined + `{"x": -1, "y": 2}`, // lhs false -> undefined + `{"x": 1, "y": -2}`, // rhs false -> undefined + `{"x": -1, "y": -2}`, // both false -> undefined + }, + }, + { + note: "or: both unknowns", + module: `package test + p if { + input.x > 0 or input.y > 0 + }`, + inputs: []string{ + `{"x": 1, "y": 2}`, // both true -> defined (single result) + `{"x": -1, "y": 2}`, // lhs false -> defined via rhs + `{"x": 1, "y": -2}`, // rhs false -> defined via lhs + `{"x": -1, "y": -2}`, // both false -> undefined + }, + }, + { + note: "and: explicit body operands", + module: `package test + p if { + { a := input.x; a > 0} and {b := input.y; b > 0 } + }`, + inputs: []string{ + `{"x": 1, "y": 2}`, + `{"x": -1, "y": 2}`, + `{"x": 1, "y": -2}`, + `{"x": -1, "y": -2}`, + }, + }, + { + note: "or: explicit body operands", + module: `package test + p if { + { a := input.x; a > 0 } or { b := input.y; b > 0 } + }`, + inputs: []string{ + `{"x": 1, "y": 2}`, + `{"x": -1, "y": 2}`, + `{"x": 1, "y": -2}`, + `{"x": -1, "y": -2}`, + }, + }, + { + note: "and: rhs comp, cross-scope vars", + module: `package test + p if { + x := input.x + true and { + y := input.threshold + s := {z | z := input.y[_]; z < x; z > y} + count(s) > 0 + } + }`, + inputs: []string{ + `{"x": 10, "threshold": 2, "y": [3, 5, 7]}`, // s={3,5,7} -> defined + `{"x": 0, "threshold": 5, "y": [3, 5, 7]}`, // none < 0 -> undefined + `{"x": 10, "threshold": 2, "y": [1, 2]}`, // none > 2 -> undefined + }, + }, + { + note: "or: lhs comp, cross-scope vars", + module: `package test + p if { + x := input.x + { + y := input.threshold + s := {z | z := input.y[_]; z < x; z > y} + count(s) > 0 + } or false + }`, + inputs: []string{ + `{"x": 10, "threshold": 2, "y": [3, 5, 7]}`, + `{"x": 0, "threshold": 5, "y": [3, 5, 7]}`, + `{"x": 10, "threshold": 2, "y": [1, 2]}`, + }, + }, + } + + ctx := t.Context() + + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + t.Parallel() + + residualModule := residualModuleFor(t, ctx, tc.module, popts) + + for _, inp := range tc.inputs { + t.Run("input="+inp, func(t *testing.T) { + orig := pIsDefined(t, ctx, tc.module, "test", inp, popts) + residual := pIsDefined(t, ctx, residualModule, "test_residual", inp, popts) + if orig != residual { + t.Errorf("round-trip mismatch for input %s: orig=%v residual=%v\nresidual module:\n%s", + inp, orig, residual, residualModule) + } + }) + } + }) + } +} + +func residualModuleFor(t *testing.T, ctx context.Context, module string, popts ast.ParserOptions) string { + t.Helper() + + compiler, err := ast.CompileModulesWithOpt( + map[string]string{"test.rego": module}, + ast.CompileOpts{ParserOptions: popts}) + if err != nil { + t.Fatalf("compile: %v", err) + } + + queryBody, err := compiler.QueryCompiler().Compile( + ast.MustParseBodyWithOpts("data.test.p = true", popts)) + if err != nil { + t.Fatalf("compile query: %v", err) + } + + store := inmem.New() + txn := storage.NewTransactionOrDie(ctx, store) + defer store.Abort(ctx, txn) + + partials, _, err := NewQuery(queryBody). + WithCompiler(compiler). + WithStore(store). + WithTransaction(txn). + WithUnknowns([]*ast.Term{ast.MustParseTerm("input")}). + PartialRun(ctx) + if err != nil { + t.Fatalf("PartialRun: %v", err) + } + + var sb strings.Builder + sb.WriteString("package test_residual\n\n") + for _, body := range partials { + sb.WriteString("p if {\n") + for _, expr := range body { + sb.WriteString("\t" + expr.String() + "\n") + } + sb.WriteString("}\n") + } + return sb.String() +} + +func pIsDefined(t *testing.T, ctx context.Context, module, pkg, inputJSON string, popts ast.ParserOptions) bool { + t.Helper() + + // An empty residual module (no `p` rule) means PE proved p is unconditionally + // undefined; short-circuit to false without going through the compiler + if pkg == "test_residual" && !strings.Contains(module, "p if") { + return false + } + + compiler, err := ast.CompileModulesWithOpt( + map[string]string{pkg + ".rego": module}, + ast.CompileOpts{ParserOptions: popts}) + if err != nil { + t.Fatalf("compile %s: %v\nmodule:\n%s", pkg, err, module) + } + + queryBody, err := compiler.QueryCompiler().Compile( + ast.MustParseBodyWithOpts("data."+pkg+".p", popts)) + if err != nil { + t.Fatalf("compile query: %v", err) + } + + store := inmem.New() + txn := storage.NewTransactionOrDie(ctx, store) + defer store.Abort(ctx, txn) + + rs, err := NewQuery(queryBody). + WithCompiler(compiler). + WithStore(store). + WithTransaction(txn). + WithInput(ast.MustParseTerm(inputJSON)). + Run(ctx) + if err != nil { + t.Fatalf("query Run: %v", err) + } + return len(rs) > 0 +} + func replaceWildcardsInBodySet(s bodySet) { for i := range s { x, _ := ast.TransformVars(s[i], func(v ast.Var) (ast.Value, error) { diff --git a/v1/topdown/trace_test.go b/v1/topdown/trace_test.go index ecf2e47f94..44844c7219 100644 --- a/v1/topdown/trace_test.go +++ b/v1/topdown/trace_test.go @@ -9,6 +9,7 @@ import ( "fmt" "maps" "reflect" + "slices" "strings" "testing" @@ -1508,3 +1509,285 @@ Redo data.test = _ {_: {"p": true}} compareBuffers(t, expected, buf.String()) } } + +func TestPrettyTraceLogical(t *testing.T) { + t.Parallel() + + module := `package test + + p if { + {true; 2 > 1} and true + false or false + }` + + ctx := t.Context() + compiler, err := ast.CompileModulesWithOpt( + map[string]string{"test.rego": module}, + ast.CompileOpts{ParserOptions: logicalParserOptions()}) + if err != nil { + t.Fatal(err) + } + + tracer := NewBufferTracer() + query := NewQuery(ast.MustParseBody("data.test.p = _")). + WithCompiler(compiler). + WithStore(inmem.New()). + WithQueryTracer(tracer) + + if _, err := query.Run(ctx); err != nil { + t.Fatal(err) + } + + expected := `Enter data.test.p = _ +| Eval data.test.p = _ +| Index data.test.p (matched 1 rule, early exit) +| Enter data.test.p +| | Eval { true; gt(2, 1) } and true +| | Enter true; gt(2, 1) +| | | Eval true +| | | Eval gt(2, 1) +| | | Exit true; gt(2, 1) early +| | Redo true; gt(2, 1) +| | | Redo gt(2, 1) +| | | Redo true +| | Enter true +| | | Eval true +| | | Exit true early +| | Redo true +| | | Redo true +| | Eval false or false +| | Enter false +| | | Eval false +| | | Fail false +| | Enter false +| | | Eval false +| | | Fail false +| | Fail false or false +| | Redo { true; gt(2, 1) } and true +| Fail data.test.p = _ +` + + var buf bytes.Buffer + PrettyTrace(&buf, removeUnifyOps(*tracer)) + compareBuffers(t, expected, buf.String()) +} + +func TestTraceLogicalAnd(t *testing.T) { + t.Parallel() + + tests := []traceLogicalCase{ + { + note: "both succeed: enter+exit on each operand", + query: "data.test.p = x", + module: `package test + import future.keywords.and + p if { + true and true + }`, + exp: []string{ + `Enter true {} (qid=2, pqid=1)`, + `Exit true {} (qid=2, pqid=1)`, + `Enter true {} (qid=3, pqid=1)`, + `Exit true {} (qid=3, pqid=1)`, + }, + }, + { + note: "lhs fails: rhs not entered, parent-level fail emitted", + query: "data.test.p = x", + module: `package test + import future.keywords.and + p if { + false and true + }`, + exp: []string{ + `Enter false {} (qid=2, pqid=1)`, + `Fail false {} (qid=2, pqid=1)`, + // Outer evalStep wrapper Fail at the rule-body qid — no paired Enter at qid=1. + `Fail false and true {} (qid=1, pqid=0)`, + }, + unwanted: []string{ + // RHS must not be entered when LHS already failed. + `Enter true {} (qid=3, pqid=1)`, + `Eval true {} (qid=3, pqid=1)`, + }, + // Wrapper emits Fail; the eval method must not duplicate it. + expFailCountFor: `false and true`, + expFailCount: 1, + }, + { + note: "lhs succeeds, rhs fails: parent-level fail emitted", + query: "data.test.p = x", + module: `package test + import future.keywords.and + p if { + true and false + }`, + exp: []string{ + `Enter true {} (qid=2, pqid=1)`, + `Exit true {} (qid=2, pqid=1)`, + `Enter false {} (qid=3, pqid=1)`, + `Fail false {} (qid=3, pqid=1)`, + `Fail true and false {} (qid=1, pqid=0)`, + }, + expFailCountFor: `true and false`, + expFailCount: 1, + }, + } + + runTraceLogicalCases(t, tests) +} + +func TestTraceLogicalOr(t *testing.T) { + t.Parallel() + + tests := []traceLogicalCase{ + { + note: "lhs succeeds: rhs not entered (short-circuit)", + query: "data.test.p = x", + module: `package test + import future.keywords.or + p if { + true or false + }`, + exp: []string{ + `Enter true {} (qid=2, pqid=1)`, + `Exit true {} (qid=2, pqid=1)`, + }, + unwanted: []string{ + // RHS must not be entered when LHS already succeeded. + `Enter false {} (qid=3, pqid=1)`, + `Eval false {} (qid=3, pqid=1)`, + `Fail false {} (qid=3, pqid=1)`, + }, + }, + { + note: "lhs fails, rhs succeeds: both bodies entered", + query: "data.test.p = x", + module: `package test + import future.keywords.or + p if { + false or true + }`, + exp: []string{ + `Enter false {} (qid=2, pqid=1)`, + `Fail false {} (qid=2, pqid=1)`, + `Enter true {} (qid=3, pqid=1)`, + `Exit true {} (qid=3, pqid=1)`, + }, + }, + { + note: "both fail: parent-level fail emitted exactly once", + query: "data.test.p = x", + module: `package test + import future.keywords.or + p if { + false or false + }`, + exp: []string{ + `Enter false {} (qid=2, pqid=1)`, + `Fail false {} (qid=2, pqid=1)`, + `Enter false {} (qid=3, pqid=1)`, + `Fail false {} (qid=3, pqid=1)`, + `Fail false or false {} (qid=1, pqid=0)`, + }, + expFailCountFor: `false or false`, + expFailCount: 1, + }, + } + + runTraceLogicalCases(t, tests) +} + +type traceLogicalCase struct { + note string + query string + module string + // exp lists events that MUST appear in the captured buffer. Extra + // events are ignored. + exp []string + // unwanted lists events that MUST NOT appear. The load-bearing check + // for short-circuit semantics: a regression that incorrectly evaluates + // the skipped operand would still satisfy `exp` (extra events are + // merely additive), but would trip an `unwanted` line. + unwanted []string + // expFailCountFor / expFailCount, when set, asserts the number of + // captured `Fail ...` events is exactly expFailCount. + // Used to confirm the parent-level Fail isn't duplicated. + expFailCountFor string + expFailCount int +} + +func runTraceLogicalCases(t *testing.T, tests []traceLogicalCase) { + t.Helper() + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + compiler := ast.NewCompiler() + mod := ast.MustParseModuleWithOpts(tc.module, logicalParserOptions()) + compiler.Compile(map[string]*ast.Module{"test.rego": mod}) + if compiler.Failed() { + t.Fatal(compiler.Errors) + } + + queryCompiler := compiler.QueryCompiler() + compiledQuery, err := queryCompiler.Compile(ast.MustParseBody(tc.query)) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + buf := NewBufferTracer() + query := NewQuery(compiledQuery). + WithQueryCompiler(queryCompiler). + WithCompiler(compiler). + WithStore(inmem.New()). + WithQueryTracer(buf) + + if _, err := query.Run(ctx); err != nil { + t.Fatalf("unexpected query error: %s", err) + } + + // Stub Locals=nil per TestTraceEveryEvaluation's convention. + actuals := make([]string, 0, len(*buf)) + for _, ev := range *buf { + ev.Locals = nil + actuals = append(actuals, ev.String()) + } + + for _, want := range tc.exp { + if !slices.Contains(actuals, want) { + t.Errorf("expected event %q to appear; not found", want) + } + } + + for _, unw := range tc.unwanted { + if slices.Contains(actuals, unw) { + t.Errorf("event %q must NOT appear (short-circuit violated)", unw) + } + } + + if tc.expFailCountFor != "" { + prefix := "Fail " + tc.expFailCountFor + " " + count := 0 + for _, s := range actuals { + if strings.HasPrefix(s, prefix) { + count++ + } + } + if count != tc.expFailCount { + t.Errorf("expected %d Fail events for %q, got %d", + tc.expFailCount, tc.expFailCountFor, count) + } + } + + if t.Failed() { + t.Log("captured events:") + for _, s := range actuals { + t.Log(" ", s) + } + } + }) + } +}