From cb8c753528069e368e214e1ee8919bdfe75528da Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Thu, 13 Aug 2026 10:32:16 +0200 Subject: [PATCH] ast: Allow non-infix `and()`/`or()` set built-in calls The `and`/`or` keywords are infix operators, so when they appear in a term position immediately followed by an opening parens (`(`) there is no ambiguity that we're dealing with a function call and not a logical operation. Being permissive in this position allow users to keep calling the named form (`and()`/`or()`) of the set built-ins (`&`/`|` infixes). Note: calling the named functions, as opposed to using the infix form, is likely very rare, but we can't guarantee this form is never used. Signed-off-by: Johan Fylling --- v1/ast/parser.go | 24 ++ v1/ast/parser_logical_test.go | 380 ++++++++++++++++++ v1/format/format_test.go | 13 +- .../v0/test_logical_builtin_call_overlap.rego | 19 + ...ogical_builtin_call_overlap.rego.formatted | 19 + .../v1/test_logical_builtin_call_overlap.rego | 17 + ...ogical_builtin_call_overlap.rego.formatted | 17 + v1/topdown/topdown_logical_test.go | 82 ++++ 8 files changed, 569 insertions(+), 2 deletions(-) create mode 100644 v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego create mode 100644 v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego.formatted create mode 100644 v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego create mode 100644 v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego.formatted diff --git a/v1/ast/parser.go b/v1/ast/parser.go index ff468f7bba7..75876b01c45 100644 --- a/v1/ast/parser.go +++ b/v1/ast/parser.go @@ -811,6 +811,26 @@ func scanAheadRef(p *Parser) bool { return false } +// scanAheadLogicalCall rewrites an `and`/`or` keyword token to tokens.Ident when +// it's immediately followed by `(`. Only valid where a term is expected: there, +// the operator reading is impossible, so it must be a function (`&`/`|` set built-ins). +// Operator position is decided before any term is parsed, which is what keeps `x and (b)` a keyword. +func scanAheadLogicalCall(p *Parser) { + if p.s.tok != tokens.LogicalAnd && p.s.tok != tokens.LogicalOr { + return + } + + s := p.save() + p.scanWS() + tok := p.s.tok + p.restore(s) + + if tok == tokens.LParen { + // This is a call to a function named `and`/`or` + p.s.tok = tokens.Ident + } +} + func (p *Parser) parseRules() []*Rule { var rule Rule @@ -2348,6 +2368,10 @@ func (p *Parser) parseTerm() *Term { var term *Term var unaryMinusLoc *Location + + // Check if an `and`/`or` token is actually a function call (`&`/`|` set built-ins). + scanAheadLogicalCall(p) + switch p.s.tok { case tokens.Null: term = NullTerm().SetLocation(p.s.Loc()) diff --git a/v1/ast/parser_logical_test.go b/v1/ast/parser_logical_test.go index 2fc332e6e9a..443a9fc285a 100644 --- a/v1/ast/parser_logical_test.go +++ b/v1/ast/parser_logical_test.go @@ -15,6 +15,13 @@ func logicalParserOpts(extraFuture ...string) ParserOptions { } } +func logicalParserOptsForVersion(v RegoVersion, extraFuture ...string) ParserOptions { + opts := logicalParserOpts(extraFuture...) + opts.RegoVersion = v + opts.Capabilities = CapabilitiesForThisVersion(CapabilitiesRegoVersion(v), CapabilitiesExperimentalKeywords(true)) + return opts +} + func TestParseLogical_Parsing(t *testing.T) { // `not` enabled so `not {x or y}` (the explicit-body form) works. opts := logicalParserOpts("not") @@ -1844,3 +1851,376 @@ func TestParseLogical_ParenSerialization(t *testing.T) { }) } } + +// TestParseLogical_BuiltinCallForm covers the term-position disambiguation of +// `and`/`or` keywords and built-in calls (`&`/`|` infixes). +func TestParseLogical_BuiltinCallForm(t *testing.T) { + s1, s2 := SetTerm(IntNumberTerm(1)), SetTerm(IntNumberTerm(2)) + a, b, x := VarTerm("a"), VarTerm("b"), VarTerm("x") + + negated := func(e *Expr) *Expr { + e.Negated = true + return e + } + + exprTests := []struct { + note string + input string + exp *Expr + }{ + { + note: "or call, statement start", + input: "or({1}, {2})", + exp: Or.Expr(s1, s2), + }, + { + note: "and call, statement start", + input: "and({1}, {2})", + exp: And.Expr(s1, s2), + }, + { + note: "or call, assigned", + input: "x := or({1}, {2})", + exp: Assign.Expr(x, Or.Call(s1, s2)), + }, + { + note: "and call, assigned", + input: "x := and({1}, {2})", + exp: Assign.Expr(x, And.Call(s1, s2)), + }, + { + note: "or call, unified", + input: "x = or({1}, {2})", + exp: Equality.Expr(x, Or.Call(s1, s2)), + }, + { + note: "or call, comparison lhs", + input: "or(a, b) == x", + exp: Equal.Expr(Or.Call(a, b), x), + }, + { + note: "and call, comparison rhs", + input: "x == and(a, b)", + exp: Equal.Expr(x, And.Call(a, b)), + }, + { + // Position, not arity, is what disambiguates + note: "or call, non-builtin arity", + input: "or(a)", + exp: NewExpr([]*Term{RefTerm(VarTerm("or")), a}), + }, + { + note: "or call, no arguments", + input: "or()", + exp: NewExpr([]*Term{RefTerm(VarTerm("or"))}), + }, + { + note: "and call, nested in call arguments", + input: "f(and(a, b))", + exp: NewExpr([]*Term{RefTerm(VarTerm("f")), And.Call(a, b)}), + }, + { + note: "or call, nested in or call", + input: "or(or(a, b), {1})", + exp: Or.Expr(Or.Call(a, b), s1), + }, + { + note: "or call, ref operand", + input: "x[or(a, b)]", + exp: NewExpr(RefTerm(x, Or.Call(a, b))), + }, + { + note: "or call, arithmetic operand", + input: "x := count(or(a, b)) + 1", + exp: Assign.Expr(x, Plus.Call(Count.Call(Or.Call(a, b)), IntNumberTerm(1))), + }, + { + note: "or call, set comprehension head", + input: "{or(a, b) | true}", + exp: NewExpr(SetComprehensionTerm(Or.Call(a, b), NewBody(NewExpr(BooleanTerm(true))))), + }, + { + note: "and call, negated", + input: "not and(a, b)", + exp: negated(And.Expr(a, b)), + }, + { + note: "or call, rhs operand of and keyword", + input: "x and or(a, b)", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(x)), + Rhs: NewBody(Or.Expr(a, b)), + }}, + }, + { + note: "and call, lhs operand of or keyword", + input: "and(a, b) or x", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(And.Expr(a, b)), + Rhs: NewBody(NewExpr(x)), + }}, + }, + { + note: "or call, both operands of or keyword", + input: "or(a, b) or or(b, a)", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(Or.Expr(a, b)), + Rhs: NewBody(Or.Expr(b, a)), + }}, + }, + { + note: "and call, operand of parenthesized group", + input: "x and (and(a, b) or b)", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(x)), + Rhs: NewBody(NewExpr(&LogicalOr{ + Lhs: NewBody(And.Expr(a, b)), + Rhs: NewBody(NewExpr(b)), + })), + }}, + }, + { + note: "or call, operand of explicit body", + input: "x and {or(a, b)}", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(x)), + Rhs: NewBody(Or.Expr(a, b)), + ExplicitRhs: true, + }}, + }, + { + note: "or call, every domain", + input: "every y in or(a, b) { y }", + exp: NewExpr(&Every{ + Value: VarTerm("y"), + Domain: Or.Call(a, b), + Body: NewBody(NewExpr(VarTerm("y"))), + }), + }, + { + note: "or call, with modifier target value", + input: "x with data.y as or(a, b)", + exp: &Expr{ + Terms: x, + With: []*With{{Target: MustParseTerm("data.y"), Value: Or.Call(a, b)}}, + }, + }, + } + + modTests := []struct { + note string + v0 string + v1 string + }{ + { + note: "or call, rule value", + v0: `package test + p = or({1}, {2}) + `, + v1: `package test + p := or({1}, {2}) + `, + }, + { + note: "and call, rule value", + v0: `package test + p = and({1}, {2}) + `, + v1: `package test + p := and({1}, {2}) + `, + }, + { + note: "or call, rule body", + v0: `package test + p { + or({1}, {2}) == {1, 2} + } + `, + v1: `package test + p if or({1}, {2}) == {1, 2} + `, + }, + { + note: "or call, function call site", + v0: `package test + p { + or(1) == 1 + } + `, + v1: `package test + p if or(1) == 1 + `, + }, + { + note: "or call, mixed with or keyword", + v0: `package test + p { + or({1}, {2}) == {1, 2} or false + } + `, + v1: `package test + p if or({1}, {2}) == {1, 2} or false + `, + }, + } + + for _, v := range []RegoVersion{RegoV0, RegoV1} { + t.Run(v.String(), func(t *testing.T) { + // `every` is a future keyword in v0 (and implies `in`); a no-op in v1. + exprOpts := logicalParserOptsForVersion(v, "every") + for _, tc := range exprTests { + t.Run(tc.note, func(t *testing.T) { + assertParseOneExpr(t, tc.note, tc.input, tc.exp, exprOpts) + }) + } + + modOpts := logicalParserOptsForVersion(v) + for _, tc := range modTests { + t.Run(tc.note, func(t *testing.T) { + input := tc.v1 + if v == RegoV0 { + input = tc.v0 + } + if _, err := ParseModuleWithOpts("test.rego", input, modOpts); err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } + }) + } + + t.Run("or call, negated with implicit not body", func(t *testing.T) { + opts := logicalParserOpts("not") + exp := NewExpr(&Not{Body: NewBody(Or.Expr(a, b))}) + assertParseOneExpr(t, "not or call", "not or(a, b)", exp, opts) + }) +} + +// TestParseLogical_BuiltinCallFormBoundaries pins the cases the term-position +// lookahead deliberately leaves alone: in operator position `(` starts a grouped +// operand of the keyword, and bare names in term position keep failing. +func TestParseLogical_BuiltinCallFormBoundaries(t *testing.T) { + a, b, x := VarTerm("a"), VarTerm("b"), VarTerm("x") + + keywordTests := []struct { + note string + input string + exp *Expr + }{ + { + note: "and, group operand, no space", + input: "x and(b)", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(x)), + Rhs: NewBody(NewExpr(b)), + }}, + }, + { + note: "or, group operand, no space", + input: "x or(b)", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(NewExpr(x)), + Rhs: NewBody(NewExpr(b)), + }}, + }, + { + note: "and, group operand, with space", + input: "x and (b)", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(x)), + Rhs: NewBody(NewExpr(b)), + }}, + }, + { + note: "or, group operand after explicit body lhs", + input: "{a} or(b)", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(NewExpr(a)), + Rhs: NewBody(NewExpr(b)), + ExplicitLhs: true, + }}, + }, + { + note: "or, multi-expression group operand", + input: "x or(a or b)", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(NewExpr(x)), + Rhs: NewBody(NewExpr(&LogicalOr{ + Lhs: NewBody(NewExpr(a)), + Rhs: NewBody(NewExpr(b)), + })), + }}, + }, + } + + errTests := []struct { + note string + input string + expected string + }{ + {"bare or, assigned", "x := or", "unexpected or keyword"}, + {"bare and, assigned", "x := and", "unexpected and keyword"}, + {"bare or, call argument", "f(or)", "unexpected or keyword"}, + {"bare and, comparison lhs", "and == x", "unexpected and keyword"}, + {"or call, space before paren", "or (a, b)", "unexpected or keyword"}, + {"and call, space before paren", "and (a, b)", "unexpected and keyword"}, + {"or call, operator position", "x or or y", "unexpected or keyword"}, + } + + for _, v := range []RegoVersion{RegoV0, RegoV1} { + t.Run(v.String(), func(t *testing.T) { + opts := logicalParserOptsForVersion(v) + for _, tc := range keywordTests { + t.Run(tc.note, func(t *testing.T) { + assertParseOneExpr(t, tc.note, tc.input, tc.exp, opts) + }) + } + for _, tc := range errTests { + t.Run(tc.note, func(t *testing.T) { + assertParseErrorContains(t, tc.note, tc.input, tc.expected, opts) + }) + } + }) + } +} + +// TestParseLogical_BuiltinCallFormInactive asserts the call form is unaffected +// when the keywords aren't active: `or(x, y)` parses as a call either way. +func TestParseLogical_BuiltinCallFormInactive(t *testing.T) { + for _, v := range []RegoVersion{RegoV0, RegoV1} { + t.Run(v.String(), func(t *testing.T) { + opts := ParserOptions{RegoVersion: v} + for _, tc := range []struct { + note string + input string + exp *Expr + }{ + { + note: "or call", + input: "x := or({1}, {2})", + exp: Assign.Expr(VarTerm("x"), Or.Call(SetTerm(IntNumberTerm(1)), SetTerm(IntNumberTerm(2)))), + }, + { + note: "and call", + input: "x := and({1}, {2})", + exp: Assign.Expr(VarTerm("x"), And.Call(SetTerm(IntNumberTerm(1)), SetTerm(IntNumberTerm(2)))), + }, + { + note: "bare or", + input: "x := or", + exp: Assign.Expr(VarTerm("x"), VarTerm("or")), + }, + } { + t.Run(tc.note, func(t *testing.T) { + assertParseOneExpr(t, tc.note, tc.input, tc.exp, opts) + }) + } + + // A space before `(` never forms a call, keywords active or not. + t.Run("or call, space before paren", func(t *testing.T) { + assertParseErrorContains(t, "or call, space before paren", "or (a, b)", "non-terminated expression", opts) + }) + }) + } +} diff --git a/v1/format/format_test.go b/v1/format/format_test.go index 2f09de14e9b..0e6cd0f86da 100644 --- a/v1/format/format_test.go +++ b/v1/format/format_test.go @@ -100,6 +100,13 @@ func TestFormatSourceError(t *testing.T) { } } +// TODO: Remove once `and`/`or` are no longer experimental keywords. +func experimentalKeywordCapabilities(v ast.RegoVersion) *ast.Capabilities { + return ast.CapabilitiesForThisVersion( + ast.CapabilitiesRegoVersion(v), + ast.CapabilitiesExperimentalKeywords(true)) +} + func TestFormatV0Source(t *testing.T) { regoFiles, err := filepath.Glob("testfiles/v0/*.rego") if err != nil { @@ -119,7 +126,8 @@ func TestFormatV0Source(t *testing.T) { } popts := ast.ParserOptions{ - RegoVersion: ast.RegoV0, + RegoVersion: ast.RegoV0, + Capabilities: experimentalKeywordCapabilities(ast.RegoV0), } opts := Opts{ RegoVersion: ast.RegoV0, @@ -179,7 +187,8 @@ func TestFormatV1Source(t *testing.T) { } popts := ast.ParserOptions{ - RegoVersion: ast.RegoV1, + RegoVersion: ast.RegoV1, + Capabilities: experimentalKeywordCapabilities(ast.RegoV1), } opts := Opts{ RegoVersion: ast.RegoV1, diff --git a/v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego b/v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego new file mode 100644 index 00000000000..723c6239c83 --- /dev/null +++ b/v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego @@ -0,0 +1,19 @@ +package test + +import future.keywords.and +import future.keywords.or + +p = or({1}, {2}) + +q = and({1, 2}, {2, 3}) + +r { + or({1}, {2}) == {1, 2} +} + +s = x { + x = and(input.a, input.b) + x == or(input.c, input.d) +} + +t = or(count(input.a), 1) diff --git a/v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego.formatted b/v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego.formatted new file mode 100644 index 00000000000..860212ffbb5 --- /dev/null +++ b/v1/format/testfiles/v0/test_logical_builtin_call_overlap.rego.formatted @@ -0,0 +1,19 @@ +package test + +import future.keywords.and +import future.keywords.or + +p = {1} | {2} + +q = {1, 2} & {2, 3} + +r { + {1} | {2} == {1, 2} +} + +s = x { + x = input.a & input.b + x == input.c | input.d +} + +t = count(input.a) | 1 diff --git a/v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego b/v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego new file mode 100644 index 00000000000..eb16a41c136 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego @@ -0,0 +1,17 @@ +package test + +import future.keywords.and +import future.keywords.or + +p := or({1}, {2}) + +q := and({1, 2}, {2, 3}) + +r if or({1}, {2}) == {1, 2} + +s := x if { + x := and(input.a, input.b) + x == or(input.c, input.d) +} + +t := or(count(input.a), 1) diff --git a/v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego.formatted b/v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego.formatted new file mode 100644 index 00000000000..21fb41c75cf --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_builtin_call_overlap.rego.formatted @@ -0,0 +1,17 @@ +package test + +import future.keywords.and +import future.keywords.or + +p := {1} | {2} + +q := {1, 2} & {2, 3} + +r if {1} | {2} == {1, 2} + +s := x if { + x := input.a & input.b + x == input.c | input.d +} + +t := count(input.a) | 1 diff --git a/v1/topdown/topdown_logical_test.go b/v1/topdown/topdown_logical_test.go index 938e04b07d6..d5a8bc9f283 100644 --- a/v1/topdown/topdown_logical_test.go +++ b/v1/topdown/topdown_logical_test.go @@ -421,3 +421,85 @@ func runTouchCase(t *testing.T, label, module string, wantTouch int) { PrettyTrace(os.Stderr, *tr) } } + +// TestTopDownLogicalBuiltinCallForm covers evaluation of calls to the `and`/`or` +// set builtins in modules where the keywords of those names are active. +func TestTopDownLogicalBuiltinCallForm(t *testing.T) { + t.Parallel() + + tests := []struct { + note string + module string + exp string + }{ + { + note: "or call, rule value", + module: `package test + import future.keywords.or + p := or({1}, {2})`, + exp: `{1, 2}`, + }, + { + note: "and call, rule value", + module: `package test + import future.keywords.and + p := and({1, 2}, {2, 3})`, + exp: `{2}`, + }, + { + note: "or call equals infix form", + module: `package test + import future.keywords.or + p if or({1}, {2}) == {1} | {2}`, + exp: `true`, + }, + { + note: "or call as operand of or keyword", + module: `package test + import future.keywords.or + p if { + false or or({1}, {2}) == {1, 2} + }`, + exp: `true`, + }, + { + note: "and call as operand of and keyword", + module: `package test + import future.keywords.and + p if { + and({1, 2}, {2}) == {2} and true + }`, + exp: `true`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + t.Parallel() + + mod, err := ast.ParseModuleWithOpts("test.rego", tc.module, logicalParserOptions()) + if err != nil { + t.Fatalf("unexpected parse error: %v", err) + } + + c := ast.NewCompiler() + c.Compile(map[string]*ast.Module{"test": mod}) + if c.Failed() { + t.Fatal(c.Errors) + } + + res, err := NewQuery(ast.MustParseBody("data.test.p = x")).WithCompiler(c).Run(t.Context()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(res) != 1 { + t.Fatalf("expected 1 result, got %d: %v", len(res), res) + } + + exp := ast.MustParseTerm(tc.exp) + if exp.Value.Compare(res[0]["x"].Value) != 0 { + t.Errorf("expected %v, got %v", exp, res[0]["x"]) + } + }) + } +}