diff --git a/cmd/fmt.go b/cmd/fmt.go index 5ca99e9e6e3..a037627f126 100644 --- a/cmd/fmt.go +++ b/cmd/fmt.go @@ -10,6 +10,7 @@ import ( "io" "os" "path/filepath" + "slices" "github.com/sergi/go-diff/diffmatchpatch" "github.com/spf13/cobra" @@ -61,6 +62,49 @@ func (p *fmtCommandParams) regoVersion() ast.RegoVersion { return ast.DefaultRegoVersion } +// parserOptions returns the options for parsing the source to format. +func (p *fmtCommandParams) parserOptions() *ast.ParserOptions { + popts := ast.ParserOptions{ + RegoVersion: ast.DefaultRegoVersion, + Capabilities: p.parserCapabilities(), + } + + switch { + case p.v0Compatible: + popts.RegoVersion = ast.RegoV0 + case p.v1Compatible: + popts.RegoVersion = ast.RegoV1 + case p.regoV1: + // '--rego-v1' formats a v0 module for v1 compatibility, so it is read as v0. + popts.RegoVersion = ast.RegoV0 + } + + return &popts +} + +func (p *fmtCommandParams) parserCapabilities() *ast.Capabilities { + caps := *p.capabilities() + current := ast.CapabilitiesForThisVersion(ast.CapabilitiesRegoVersion(p.regoVersion())) + + // parsing caps must span current version and target + caps.FutureKeywords = union(caps.FutureKeywords, current.FutureKeywords) + caps.Features = union(caps.Features, current.Features) + + return &caps +} + +func union(a, b []string) []string { + out := slices.Clone(a) + for _, s := range b { + if !slices.Contains(out, s) { + out = append(out, s) + } + } + slices.Sort(out) + + return out +} + func opaFmt(args []string, fmtParams *fmtCommandParams) int { if len(args) == 0 { if err := formatStdin(fmtParams, os.Stdin, os.Stdout); err != nil { @@ -118,17 +162,7 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o RegoVersion: params.regoVersion(), DropV0Imports: params.dropV0Imports, Capabilities: params.capabilities(), - } - - if params.regoV1 { - opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV0} - } - - if params.v0Compatible { - // v0 takes precedence over v1 - opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV0} - } else if params.v1Compatible { - opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV1} + ParserOptions: params.parserOptions(), } formatted, err := format.SourceWithOpts(filename, contents, opts) @@ -137,7 +171,10 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o } if params.checkResult { - popts := ast.ParserOptions{RegoVersion: params.regoVersion()} + popts := ast.ParserOptions{ + RegoVersion: params.regoVersion(), + Capabilities: params.parserCapabilities(), + } _, err := ast.ParseModuleWithOpts("formatted", string(formatted), popts) if err != nil { return newError("%s was successfully formatted, but the result is invalid: %v\n\nTo inspect the formatted Rego, you can turn off this check with --check-result=false.", filename, err) @@ -203,19 +240,9 @@ func formatStdin(params *fmtCommandParams, r io.Reader, w io.Writer) error { } opts := format.Opts{ - RegoVersion: params.regoVersion(), - Capabilities: params.capabilities(), - } - - if params.regoV1 { - opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV0} - } - - if params.v0Compatible { - // v0 takes precedence over v1 - opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV0} - } else if params.v1Compatible { - opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV1} + RegoVersion: params.regoVersion(), + Capabilities: params.capabilities(), + ParserOptions: params.parserOptions(), } formatted, err := format.SourceWithOpts("stdin", contents, opts) diff --git a/cmd/fmt_test.go b/cmd/fmt_test.go index b76edefff73..d792201f9a6 100644 --- a/cmd/fmt_test.go +++ b/cmd/fmt_test.go @@ -1346,6 +1346,119 @@ foo["if"]["else"] := true } } +// Experimental future keywords are hidden from the default capabilities, so +// formatting a module that imports one requires --capabilities to list it. +func TestFmtFormatExperimentalKeywords(t *testing.T) { + unformatted := `package test + +import future.keywords.and +import future.keywords.or + +p if { input.a and input.b or input.c } +` + + formatted := `package test + +import future.keywords.and +import future.keywords.or + +p if input.a and input.b or input.c +` + + experimental := func() fmtCommandParams { + params := newFmtCommandParams() + params.capabilitiesFlag.C = ast.CapabilitiesForThisVersion( + ast.CapabilitiesRegoVersion(ast.RegoV1), + ast.CapabilitiesExperimentalKeywords(true)) + return *params + } + + cases := []struct { + note string + params fmtCommandParams + expected string + expectedErr string + }{ + { + note: "default capabilities", + params: *newFmtCommandParams(), + expectedErr: "rego_parse_error: unexpected keyword, must be one of [contains every if in not]", + }, + { + note: "capabilities listing the keywords", + params: experimental(), + expected: formatted, + }, + { + note: "capabilities listing the keywords, --check-result", + params: func() fmtCommandParams { + params := experimental() + params.checkResult = true + return params + }(), + expected: formatted, + }, + } + + for _, tc := range cases { + t.Run("file, "+tc.note, func(t *testing.T) { + var stdout bytes.Buffer + + files := map[string]string{"policy.rego": unformatted} + + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + err = formatFile(&tc.params, &stdout, policyFile, info, err) + + if tc.expectedErr != "" { + if err == nil { + t.Fatalf("Expected error but got: %s", stdout.String()) + } + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Fatalf("Expected error to contain:\n\n%s\n\nGot:\n\n%s", tc.expectedErr, err.Error()) + } + return + } + + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + if actual := stdout.String(); actual != tc.expected { + t.Fatalf("Expected:\n%s\n\nGot:\n%s\n\n", tc.expected, actual) + } + }) + }) + + t.Run("stdin, "+tc.note, func(t *testing.T) { + var stdout bytes.Buffer + + err := formatStdin(&tc.params, bytes.NewReader([]byte(unformatted)), &stdout) + + if tc.expectedErr != "" { + if err == nil { + t.Fatalf("Expected error but got: %s", stdout.String()) + } + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Fatalf("Expected error to contain:\n\n%s\n\nGot:\n\n%s", tc.expectedErr, err.Error()) + } + return + } + + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + if actual := stdout.String(); actual != tc.expected { + t.Fatalf("Expected:\n%s\n\nGot:\n%s\n\n", tc.expected, actual) + } + }) + } +} + func TestFmtFormatFile_KeywordsInRefs(t *testing.T) { cases := []struct { note string diff --git a/v1/ast/parser.go b/v1/ast/parser.go index ff468f7bba7..f82e4ccac23 100644 --- a/v1/ast/parser.go +++ b/v1/ast/parser.go @@ -1309,7 +1309,9 @@ func (p *Parser) parseLiteral() (expr *Expr) { if nb == nil { return nil } - return p.attachWith(nb) + // A not-body is a complete operand, so it may lead an and/or chain: + // `not { x } and y`. + return p.foldLogicalTail(NewBody(nb), false, nb.Location) } switch p.s.tok { @@ -1417,6 +1419,17 @@ func (p *Parser) parseLiteralExpr(negated bool, notLoc *Location) *Expr { expr.SetLoc(startLoc) } + if notLoc == nil && bytes.HasPrefix(expr.Location.Text, []byte("{")) { + // `{}` on its own is an empty body + if isEmptyObjectTerm(expr) { + p.error(expr.Location, "found empty body") + return nil + } + + p.errorBraceLedOperand(expr.Location, expr.Location.Text, p.s.tok.String()) + return nil + } + outer := p.parseLogicalOrChain(NewBody(expr), false, expr.Location) if outer == nil { return nil @@ -1605,7 +1618,9 @@ func (p *Parser) parseNotBody(notLoc *Location) *Expr { failed := p.save() p.restore(s) - if term := p.parseTerm(); term != nil { + // The operand can extend past the braces (`{1, 2} & input.s == set()`), + // and parens group rather than delimit, so it is the whole operand that has to be wrapped. + if term := p.parseTermInfixCall(); term != nil { p.errorOperandBraceNeedsBody(braceLoc, p.s.Text(braceOffset, p.s.lastEnd), term, "not ") return nil } @@ -1662,7 +1677,7 @@ func (p *Parser) parseLogicalOrChain(lhsBody Body, lhsExplicit bool, lhsLoc *Loc for p.s.tok == tokens.LogicalOr { p.scan() - rhsBody, rhsExplicit, rhsLoc := p.parseLogicalOperand() + rhsBody, rhsExplicit, rhsLoc := p.parseLogicalOperand("or") if rhsBody == nil { return nil } @@ -1709,7 +1724,7 @@ func (p *Parser) parseLogicalAndChain(lhsBody Body, lhsExplicit bool, lhsLoc *Lo for p.s.tok == tokens.LogicalAnd { p.scan() - rhsBody, rhsExplicit, _ := p.parseLogicalOperand() + rhsBody, rhsExplicit, _ := p.parseLogicalOperand("and") if rhsBody == nil { return nil } @@ -1751,14 +1766,39 @@ func isNegated(p *Parser) bool { return tok != tokens.Dot && tok != tokens.LBrack } -// parseLogicalOperand parses a single operand of an `and`/`or` expression. -func (p *Parser) parseLogicalOperand() (Body, bool, *Location) { +// parseLogicalOperand parses a single operand of an `and`/`or` expression. op is +// the operator the operand belongs to, or "" when the caller is speculating and +// will restore on failure. +func (p *Parser) parseLogicalOperand(op string) (Body, bool, *Location) { if p.s.tok == tokens.LBrace { braceOffset := p.s.loc.Offset loc := p.s.Loc() + s := p.save() p.scan() + + // `{}` is an empty body, which parseBody reports precisely; only non-empty + // braces are worth re-reading as a value. + empty := p.s.tok == tokens.RBrace + body := p.parseBody(tokens.RBrace) if body == nil { + if empty || op == "" { + return nil, false, nil + } + + // The braces may hold a value rather than a body. + failed := p.save() + p.restore(s) + + // The operand can extend past the braces (`{1, 2} & input.s == set()`), + // and parens group rather than delimit, so it is the whole operand that has to be wrapped. + if term := p.parseTermInfixCall(); term != nil { + p.errorBraceLedOperand(loc, p.s.Text(braceOffset, p.s.lastEnd), op) + return nil, false, nil + } + + p.restore(failed) + return nil, false, nil } p.scan() @@ -1861,10 +1901,50 @@ func isAmbiguousUnionBody(b Body) bool { } // errorOperandBraceNeedsBody reports `{...}` in an operand position holding a value instead of expressions. -func (p *Parser) errorOperandBraceNeedsBody(loc *Location, braces []byte, term *Term, prefix string) { +func (p *Parser) errorOperandBraceNeedsBody(loc *Location, operand []byte, term *Term, prefix string) { p.hint(fmt.Sprintf("write `%s(%s)` to negate the value, or `%s{%s}` for a body holding it", - prefix, braces, prefix, braces)) - p.errorf(loc, "`{...}` in an operand position must contain expression(s), got: %s", ValueName(term.Value)) + prefix, operand, prefix, operand)) + p.errorf(loc, "`{...}` in an operand position must contain expression(s), got: %s", ValueName(braceLedValue(term))) +} + +// braceLedValue returns the value opened by the leading `{` of t. An infix call +// renders its lhs operand first, so `{1, 2} & s` is brace-led by the set; refs are +// left alone, as `{"a": 1}["a"]` is reported as the ref it is. +func braceLedValue(t *Term) Value { + if call, ok := t.Value.(Call); ok && len(call) > 0 { + if bi, ok := BuiltinMap[call[0].String()]; ok && bi.Infix != "" && len(call) == bi.Decl.Arity()+1 { + return braceLedValue(call[1]) + } + } + + return t.Value +} + +// isEmptyObjectTerm reports whether expr is exactly `{}`. In an operand position +// those braces open a body, so an empty one is an empty body - not the empty +// object the term parser read. +func isEmptyObjectTerm(expr *Expr) bool { + if len(expr.With) > 0 { + return false + } + + t, ok := expr.Terms.(*Term) + if !ok { + return false + } + + obj, ok := t.Value.(Object) + + return ok && obj.Len() == 0 +} + +// errorBraceLedOperand reports an `and`/`or` operand whose leading `{` opens a +// value rather than a body. In an operand position the braces are read as an +// explicit body, so the value form has to be parenthesized, on both sides of the +// operator. +func (p *Parser) errorBraceLedOperand(loc *Location, operand []byte, op string) { + p.hint(fmt.Sprintf("wrap the operand to keep the value: `(%s) %s ...`", operand, op)) + p.errorf(loc, "operand of `%s` cannot begin with `{` unless the braces hold a body", op) } // errorParensCannotWrapBody reports `(...)` holding expressions rather than a value. @@ -1951,7 +2031,7 @@ func (p *Parser) parseLogicalGroup(operandContext bool, prefix string) (Body, bo return nil, false, nil, false } - lhsBody, lhsExplicit, lhsLoc := p.parseLogicalOperand() + lhsBody, lhsExplicit, lhsLoc := p.parseLogicalOperand("") if lhsBody == nil { // Parens are not an operand, so a `{...}` that can't be a body is a value: // restore and let the term parser read it, e.g. `not ({})` is an empty object. diff --git a/v1/ast/parser_logical_test.go b/v1/ast/parser_logical_test.go index 2fc332e6e9a..59606bf126e 100644 --- a/v1/ast/parser_logical_test.go +++ b/v1/ast/parser_logical_test.go @@ -2,6 +2,7 @@ package ast import ( "bytes" + "fmt" "strings" "testing" ) @@ -337,6 +338,14 @@ func TestParseLogical_ParseErrors(t *testing.T) { {"or, inside every value", "every x or y in z {x}", "unexpected or keyword"}, {"and, inside every domain", "every x in y and z {x}", "unexpected and keyword"}, {"or, inside every domain", "every x in y or z {x}", "unexpected or keyword"}, + + // `some` and `every` are declarations, not truth-valued operands, so they + // cannot lead a chain. Nothing formats to these shapes: a *SomeDecl or *Every + // operand is only constructible programmatically. + {"some-in as an operand", "some x in xs and y", "unexpected and keyword"}, + {"some decl as an operand", "some x and y", "unexpected and keyword"}, + {"every as an operand", "every x in xs { x } and y", "unexpected and keyword"}, + {"every as an operand, or", "every x in xs { x } or y", "unexpected or keyword"}, } for _, tc := range exprTests { t.Run(tc.note, func(t *testing.T) { @@ -979,6 +988,16 @@ func TestParseLogical_InnerExprHasLocation(t *testing.T) { func TestParseLogical_ParenGrouping(t *testing.T) { opts := logicalParserOpts("not") + // Parens group rather than delimit, so an operand that begins with `(` extends + // past the matching `)`: every spelling below parses to this same operand. + setIntersectionEmpty := Equal.Expr( + And.Call( + SetTerm(NumberTerm("1"), NumberTerm("2")), + RefTerm(VarTerm("input"), StringTerm("s")), + ), + SetTerm(), + ) + tests := []struct { note string input string @@ -1167,6 +1186,62 @@ func TestParseLogical_ParenGrouping(t *testing.T) { ExplicitLhs: true, }}, }, + + // Parens group rather than delimit: wrapping only the leading part of an + // operand yields the same AST as wrapping all of it. This is why + // `z and (1 + 2) > 3` is accepted -- the same shape is plain v1 syntax after + // `not`, see TestParseLogical_ParenNot. + { + note: "rhs operand, leading part wrapped", + input: `z and ({1, 2}) & input.s == set()`, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(setIntersectionEmpty), + }}, + }, + { + note: "rhs operand, whole operand wrapped", + input: `z and ({1, 2} & input.s == set())`, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(setIntersectionEmpty), + }}, + }, + { + note: "lhs operand, leading part wrapped", + input: `({1, 2}) & input.s == set() and z`, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(setIntersectionEmpty), + Rhs: NewBody(NewExpr(VarTerm("z"))), + }}, + }, + { + note: "lhs operand, whole operand wrapped", + input: `({1, 2} & input.s == set()) and z`, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(setIntersectionEmpty), + Rhs: NewBody(NewExpr(VarTerm("z"))), + }}, + }, + { + note: "rhs operand, parenthesized arithmetic", + input: `z and (1 + 2) > 3`, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(GreaterThan.Expr( + Plus.Call(NumberTerm("1"), NumberTerm("2")), + NumberTerm("3"), + )), + }}, + }, + { + note: "rhs operand, parenthesized comparison", + input: `z and (a) == b`, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(Equal.Expr(VarTerm("a"), VarTerm("b"))), + }}, + }, } for _, tc := range tests { @@ -1505,6 +1580,33 @@ func TestParseLogical_ParenNot(t *testing.T) { }, }, }, + + // Parens after `not` group rather than delimit too: where they hold no + // logical group, the operand continues past the matching `)`. These shapes + // also parse in plain v1 with no imports at all -- as `Expr.Negated` rather + // than a not-body -- which is why the and/or equivalents are accepted rather + // than narrowed (see TestParseLogical_ParenGrouping). + { + note: "not, parenthesized arithmetic", + input: "not (1 + 2) > 3", + exp: &Expr{ + Terms: &Not{ + Body: NewBody(GreaterThan.Expr( + Plus.Call(NumberTerm("1"), NumberTerm("2")), + NumberTerm("3"), + )), + }, + }, + }, + { + note: "not, parenthesized comparison", + input: "not (a) == b", + exp: &Expr{ + Terms: &Not{ + Body: NewBody(Equal.Expr(VarTerm("a"), VarTerm("b"))), + }, + }, + }, } for _, tc := range tests { @@ -1512,6 +1614,41 @@ func TestParseLogical_ParenNot(t *testing.T) { assertParseOneExpr(t, tc.note, tc.input, tc.exp, opts) }) } + + // The same shapes with no imports at all: `not` negates the expression in place + // rather than opening a body, and the operand still runs past the `)`. Plain v1 + // syntax, so not narrowable. + noImportTests := []struct { + note string + input string + exp *Expr + }{ + { + note: "no imports, parenthesized arithmetic", + input: "not (1 + 2) > 3", + exp: &Expr{ + Terms: GreaterThan.Expr( + Plus.Call(NumberTerm("1"), NumberTerm("2")), + NumberTerm("3"), + ).Terms, + Negated: true, + }, + }, + { + note: "no imports, parenthesized comparison", + input: "not (a) == b", + exp: &Expr{ + Terms: Equal.Expr(VarTerm("a"), VarTerm("b")).Terms, + Negated: true, + }, + }, + } + + for _, tc := range noImportTests { + t.Run(tc.note, func(t *testing.T) { + assertParseOneExpr(t, tc.note, tc.input, tc.exp, ParserOptions{RegoVersion: RegoV1}) + }) + } } func TestParseLogical_ParenRedundant(t *testing.T) { @@ -1844,3 +1981,472 @@ func TestParseLogical_ParenSerialization(t *testing.T) { }) } } + +// TestParseLogical_BraceLedOperand pins the operand-brace contract for the value +// forms of `{...}`: in an operand position the braces open a body, so an operand +// holding a value must be parenthesized. +func TestParseLogical_BraceLedOperand(t *testing.T) { + opts := logicalParserOpts() + + // Brace forms that hold a value rather than a body. + // `{}` is an empty body and has a different error message. + operands := []struct { + note string + operand string + }{ + {"object", `{"a": 1}`}, + {"set", `{1, 2}`}, + {"object comprehension", `{k: v | v := input[k]}`}, + {"ref into object", `{"a": 1}[_]`}, + {"ref into object, dot", `{"a": 1}.a`}, + {"comparison with set intersection", `{1, 2} & input.s == set()`}, + } + + // Each position the operand can appear in. + positions := []struct { + note string + expr string + expErr string + expParse bool + }{ + { + note: "lhs of and", + expr: "%s and z", + expErr: "operand of `and` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `(%s) and ...`)", + }, + { + note: "lhs of or", + expr: "%s or z", + expErr: "operand of `or` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `(%s) or ...`)", + }, + { + note: "rhs of and", + expr: "z and %s", + expErr: "operand of `and` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `(%s) and ...`)", + }, + { + note: "rhs of or", + expr: "z or %s", + expErr: "operand of `or` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `(%s) or ...`)", + }, + {note: "parenthesized lhs of and", expr: "(%s) and z", expParse: true}, + {note: "parenthesized lhs of or", expr: "(%s) or z", expParse: true}, + {note: "parenthesized rhs of and", expr: "z and (%s)", expParse: true}, + {note: "parenthesized rhs of or", expr: "z or (%s)", expParse: true}, + } + + for _, tc := range operands { + t.Run(tc.note, func(t *testing.T) { + for _, ptc := range positions { + t.Run(ptc.note, func(t *testing.T) { + input := fmt.Sprintf(ptc.expr, tc.operand) + + if ptc.expParse { + if _, err := ParseBodyWithOpts(input, opts); err != nil { + t.Fatalf("unexpected error for %q: %v", input, err) + } + return + } + + assertParseErrorContains(t, ptc.note, input, fmt.Sprintf(ptc.expErr, tc.operand), opts) + }) + } + }) + } +} + +func TestParseLogical_BraceLedOperandScope(t *testing.T) { + opts := logicalParserOpts() + notOpts := logicalParserOpts("not") + + tests := []struct { + note string + input string + opts *ParserOptions + exp *Expr + expErr string + }{ + { + note: "body operand, lhs", + input: "{x} and z", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("x"))), + Rhs: NewBody(NewExpr(VarTerm("z"))), + ExplicitLhs: true, + }}, + }, + { + note: "body operand, rhs", + input: "z and {x}", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(NewExpr(VarTerm("x"))), + ExplicitRhs: true, + }}, + }, + { + note: "multi-expression body operand, lhs", + input: "{x; y} or z", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(NewExpr(VarTerm("x")), NewExpr(VarTerm("y"))), + Rhs: NewBody(NewExpr(VarTerm("z"))), + ExplicitLhs: true, + }}, + }, + { + note: "multi-expression body operand, rhs", + input: "z or {x; y}", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(NewExpr(VarTerm("x")), NewExpr(VarTerm("y"))), + ExplicitRhs: true, + }}, + }, + + { + note: "object body statement", + input: `{"a": 1}`, + exp: NewExpr(ObjectTerm([2]*Term{StringTerm("a"), NumberTerm("1")})), + }, + { + note: "ref into object body statement", + input: `{"a": 1}[_]`, + exp: NewExpr(RefTerm( + ObjectTerm([2]*Term{StringTerm("a"), NumberTerm("1")}), + VarTerm("$0"))), + }, + { + note: "comparison with set body statement", + input: "{x} == input.y", + exp: Equal.Expr(SetTerm(VarTerm("x")), RefTerm(VarTerm("input"), StringTerm("y"))), + }, + { + note: "object as call argument", + input: `f({"a": 1})`, + exp: NewExpr([]*Term{ + RefTerm(VarTerm("f")), + ObjectTerm([2]*Term{StringTerm("a"), NumberTerm("1")}), + }), + }, + + { + note: "chain", + input: `{"a": 1} and z or w`, + expErr: "operand of `and` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `({\"a\": 1}) and ...`)", + }, + { + note: "operand continues past a body-shaped brace, lhs", + input: `{x} == input.y and z`, + expErr: "operand of `and` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `({x} == input.y) and ...`)", + }, + { + note: "operand continues past a body-shaped brace, rhs", + input: `z and {x} == input.y`, + expErr: "unexpected equal token", + }, + + { + note: "negated set term, lhs, not unimported", + input: "not {1} and z", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(&Expr{Terms: SetTerm(NumberTerm("1")), Negated: true}), + Rhs: NewBody(NewExpr(VarTerm("z"))), + }}, + }, + { + note: "negated set term, rhs, not unimported", + input: "z and not {1}", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(&Expr{Terms: SetTerm(NumberTerm("1")), Negated: true}), + }}, + }, + { + note: "not-body, lhs, not imported", + input: "not {1} and z", + opts: ¬Opts, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(NumberTerm("1"))), + ExplicitBody: true, + })), + Rhs: NewBody(NewExpr(VarTerm("z"))), + }}, + }, + { + note: "not-body, rhs, not imported", + input: "z and not {1}", + opts: ¬Opts, + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(NumberTerm("1"))), + ExplicitBody: true, + })), + }}, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + popts := opts + if tc.opts != nil { + popts = *tc.opts + } + + if tc.expErr != "" { + assertParseErrorContains(t, tc.note, tc.input, tc.expErr, popts) + return + } + + assertParseOneExpr(t, tc.note, tc.input, tc.exp, popts) + }) + } +} + +func TestParseLogical_EmptyBraceOperand(t *testing.T) { + opts := logicalParserOpts("not") + + errorTests := []struct { + note string + input string + expErr string + }{ + {note: "lhs of and", input: "{} and z", expErr: "found empty body"}, + {note: "rhs of and", input: "z and {}", expErr: "found empty body"}, + {note: "lhs of or", input: "{} or z", expErr: "found empty body"}, + {note: "rhs of or", input: "z or {}", expErr: "found empty body"}, + {note: "both operands", input: "{} and {}", expErr: "found empty body"}, + {note: "whitespace only", input: "{ } and z", expErr: "found empty body"}, + {note: "after not", input: "not {}", expErr: "found empty body"}, + {note: "after not, as an operand", input: "z and not {}", expErr: "found empty body"}, + + // Braces leading a larger operand hold a value + { + note: "leading a comparison", + input: "{} == x and z", + expErr: "operand of `and` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `({} == x) and ...`)", + }, + { + note: "leading a ref", + input: "{}[_] and z", + expErr: "operand of `and` cannot begin with `{` unless the braces hold a body " + + "(hint: wrap the operand to keep the value: `({}[_]) and ...`)", + }, + } + + for _, tc := range errorTests { + t.Run(tc.note, func(t *testing.T) { + assertParseErrorContains(t, tc.note, tc.input, tc.expErr, opts) + }) + } + + parseTests := []struct { + note string + input string + exp *Expr + }{ + { + note: "parenthesized lhs is the empty object", + input: "({}) and z", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(ObjectTerm())), + Rhs: NewBody(NewExpr(VarTerm("z"))), + }}, + }, + { + note: "parenthesized rhs is the empty object", + input: "z and ({})", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(VarTerm("z"))), + Rhs: NewBody(NewExpr(ObjectTerm())), + }}, + }, + } + + for _, tc := range parseTests { + t.Run(tc.note, func(t *testing.T) { + assertParseOneExpr(t, tc.note, tc.input, tc.exp, opts) + }) + } +} + +func TestParseLogical_BraceLedOperandHintExtent(t *testing.T) { + // The hints name the whole operand, not just its leading braces. + // Parens group rather than delimit -- as they do for `not (1 + 2) > 3` -- + // so the minimal wrap parses too. + + opts := logicalParserOpts("not") + + tests := []struct { + note string + input string + expErr string + }{ + { + note: "and, lhs", + input: `{1, 2} & input.s == set() and z`, + expErr: "wrap the operand to keep the value: " + + "`({1, 2} & input.s == set()) and ...`", + }, + { + note: "and, rhs", + input: `z and {1, 2} & input.s == set()`, + expErr: "wrap the operand to keep the value: " + + "`({1, 2} & input.s == set()) and ...`", + }, + { + note: "or, lhs", + input: `{1, 2} & input.s == set() or z`, + expErr: "wrap the operand to keep the value: " + + "`({1, 2} & input.s == set()) or ...`", + }, + { + note: "or, rhs", + input: `z or {1, 2} + 1 == 2`, + expErr: "wrap the operand to keep the value: " + + "`({1, 2} + 1 == 2) or ...`", + }, + { + note: "not", + input: `not {1, 2} & input.s == set()`, + expErr: "must contain expression(s), got: set " + + "(hint: write `not ({1, 2} & input.s == set())` to negate the value, " + + "or `not {{1, 2} & input.s == set()}` for a body holding it)", + }, + { + note: "not, ref into object", + input: `not {"a": 1}["a"]`, + expErr: "must contain expression(s), got: ref " + + "(hint: write `not ({\"a\": 1}[\"a\"])` to negate the value, " + + "or `not {{\"a\": 1}[\"a\"]}` for a body holding it)", + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + assertParseErrorContains(t, tc.note, tc.input, tc.expErr, opts) + }) + } +} + +// TestParseLogical_NotBodyLeadingOperand covers a not-body leading an and/or chain. +func TestParseLogical_NotBodyLeadingOperand(t *testing.T) { + opts := logicalParserOpts("not") + + tests := []struct { + note string + input string + exp *Expr + }{ + { + note: "single expression body, and", + input: "not {x} and y", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(VarTerm("x"))), + ExplicitBody: true, + })), + Rhs: NewBody(NewExpr(VarTerm("y"))), + }}, + }, + { + note: "single expression body, or", + input: "not {x} or y", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(VarTerm("x"))), + ExplicitBody: true, + })), + Rhs: NewBody(NewExpr(VarTerm("y"))), + }}, + }, + { + note: "multi expression body", + input: "not {x; y} and z", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(VarTerm("x")), NewExpr(VarTerm("y"))), + ExplicitBody: true, + })), + Rhs: NewBody(NewExpr(VarTerm("z"))), + }}, + }, + { + note: "chain", + input: "not {x} and y or z", + exp: &Expr{Terms: &LogicalOr{ + Lhs: NewBody(NewExpr(&LogicalAnd{ + Lhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(VarTerm("x"))), + ExplicitBody: true, + })), + Rhs: NewBody(NewExpr(VarTerm("y"))), + })), + Rhs: NewBody(NewExpr(VarTerm("z"))), + }}, + }, + { + note: "both operands are not-bodies", + input: "not {x} and not {y}", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(VarTerm("x"))), + ExplicitBody: true, + })), + Rhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(VarTerm("y"))), + ExplicitBody: true, + })), + }}, + }, + { + note: "parenthesized, unchanged", + input: "(not {x}) and y", + exp: &Expr{Terms: &LogicalAnd{ + Lhs: NewBody(NewExpr(&Not{ + Body: NewBody(NewExpr(VarTerm("x"))), + ExplicitBody: true, + })), + Rhs: NewBody(NewExpr(VarTerm("y"))), + }}, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + assertParseOneExpr(t, tc.note, tc.input, tc.exp, opts) + }) + } + + t.Run("with modifier binds to the whole expression", func(t *testing.T) { + body, err := ParseBodyWithOpts("not {x} and y with input as 1", opts) + if err != nil { + t.Fatal(err) + } + if len(body) != 1 { + t.Fatalf("expected 1 expression, got %d: %v", len(body), body) + } + if _, ok := body[0].Terms.(*LogicalAnd); !ok { + t.Fatalf("expected *LogicalAnd, got %T", body[0].Terms) + } + if len(body[0].With) != 1 { + t.Fatalf("expected the `with` on the and expression, got %v", body[0]) + } + }) + + t.Run("no operator", func(t *testing.T) { + assertParseOneExpr(t, "no operator", "not {x}", + &Expr{Terms: &Not{ + Body: NewBody(NewExpr(VarTerm("x"))), + ExplicitBody: true, + }}, opts) + }) +} diff --git a/v1/format/format.go b/v1/format/format.go index 76c98869320..80df86fbd45 100644 --- a/v1/format/format.go +++ b/v1/format/format.go @@ -169,7 +169,7 @@ type fmtOpts struct { func (o fmtOpts) keywords() []string { if o.regoV1 { - return ast.KeywordsV1[:] + return append(ast.KeywordsV1[:], o.futureKeywords...) } kws := ast.KeywordsV0[:] return append(kws, o.futureKeywords...) @@ -228,6 +228,16 @@ func AstWithOpts(x any, opts Opts) ([]byte, error) { extraFutureKeywordImports["every"] = struct{}{} case n.IsNot(): extraFutureKeywordImports["not"] = struct{}{} + case n.IsAnd(): + extraFutureKeywordImports["and"] = struct{}{} + case n.IsOr(): + extraFutureKeywordImports["or"] = struct{}{} + } + + if n.Negated && isLogicalExpr(n) { + // A negated logical expression is written parenthesized + // (`not (a or b)`), which requires the `not` keyword. + extraFutureKeywordImports["not"] = struct{}{} } case *ast.Import: @@ -409,10 +419,15 @@ func defaultLocation(x ast.Node) *ast.Location { } type writer struct { - // parenExpr, when set, is an expression that must be wrapped in parens when - // written; consumed by the first writeExpr that sees it. + // parenExpr, when set, is an expression whose terms must be wrapped in parens + // when written; consumed by the first writeExpr that sees it. Any `with` + // clauses stay outside the parens, as `(x | y with p as 1)` doesn't parse. parenExpr *ast.Expr + // parenTerm, when set, is a term that must be wrapped in parens when written; + // consumed by the first writeTermParens that sees it. + parenTerm *ast.Term + buf bytes.Buffer indent string @@ -657,7 +672,9 @@ func (w *writer) writeRule(rule *ast.Rule, isElse bool, comments []*ast.Comment) // this excludes partial sets UNLESS `contains` is used partialSetException := w.fmtOpts.contains || rule.Head.Value != nil - if (w.fmtOpts.regoV1 || w.fmtOpts.ifs) && partialSetException { + usesIf := (w.fmtOpts.regoV1 || w.fmtOpts.ifs) && partialSetException + + if usesIf { w.write(" if") if len(rule.Body) == 1 { // Keep `if ` on one line when the single body term sits on the @@ -694,6 +711,13 @@ func (w *writer) writeRule(rule *ast.Rule, isElse bool, comments []*ast.Comment) w.endLine() } + // A leading set union renders as `x | y`, which the parser reads as a + // comprehension at the brace of a `p if { ... }` body, so it is parenthesized. + // An `else` body has no such ambiguity: its braces always open a body. + if usesIf && !isElse { + w.markUnionLead(rule.Body[0]) + } + w.up() comments, err = w.writeBody(rule.Body, comments) @@ -946,10 +970,9 @@ func (w *writer) writeBody(body ast.Body, comments []*ast.Comment) ([]*ast.Comme } func (w *writer) writeExpr(expr *ast.Expr, comments []*ast.Comment) ([]*ast.Comment, error) { - if w.parenExpr == expr { + parenTerms := w.parenExpr == expr + if parenTerms { w.parenExpr = nil - w.write("(") - defer w.write(")") } var err error @@ -961,8 +984,20 @@ func (w *writer) writeExpr(expr *ast.Expr, comments []*ast.Comment) ([]*ast.Comm w.startLine() } + // `not` binds tighter than `and`/`or`, so a negated logical expression is + // parenthesized. Only reachable through programmatically built ASTs; the + // parser represents `not (a or b)` as an *ast.Not. + negatedLogical := expr.Negated && isLogicalExpr(expr) + if expr.Negated { w.write("not ") + if negatedLogical { + w.write("(") + } + } + + if parenTerms { + w.write("(") } switch t := expr.Terms.(type) { @@ -981,6 +1016,11 @@ func (w *writer) writeExpr(expr *ast.Expr, comments []*ast.Comment) ([]*ast.Comm if err != nil { return nil, err } + case *ast.LogicalAnd, *ast.LogicalOr: + comments, err = w.writeLogical(expr, comments) + if err != nil { + return nil, err + } case []*ast.Term: comments, err = w.writeFunctionCall(expr, comments) if err != nil { @@ -993,6 +1033,14 @@ func (w *writer) writeExpr(expr *ast.Expr, comments []*ast.Comment) ([]*ast.Comm } } + if parenTerms { + w.write(")") + } + + if negatedLogical { + w.write(")") + } + if len(expr.With) == 0 { return comments, nil } @@ -1192,8 +1240,7 @@ func (w *writer) writeNot(not *ast.Not, loc *ast.Location, comments []*ast.Comme } w.write("}") } else { - // A value that renders brace-led would be re-read as an explicit body. - parens := exprRendersBraceLead(not.Body[0]) + parens := notBodyNeedsParens(not.Body[0]) if parens { w.write("(") } @@ -1213,6 +1260,251 @@ func (w *writer) writeNot(not *ast.Not, loc *ast.Location, comments []*ast.Comme return comments, nil } +// notBodyNeedsParens reports whether the sole expression of an implicit `not` +// body must be parenthesized to be read back as that same expression. Mirrors +// notBodyNeedsParens in the ast package. +func notBodyNeedsParens(expr *ast.Expr) bool { + // A `with` on a bare operand of `not` binds to the whole `not` expression. + if len(expr.With) > 0 { + return true + } + + // `not` binds tighter than `and`/`or`. + if isLogicalExpr(expr) { + return true + } + + // A value that renders brace-led would be re-read as an explicit body. + return exprRendersBraceLead(expr) +} + +// logicalOperand is one operand of an `and`/`or` chain. +type logicalOperand struct { + body ast.Body + + // explicit is set for `{...}` operands, which scope their contents and are + // always written braced. + explicit bool + + // parens is set for implicit operands that must be parenthesized to be read + // back as the same expression. + parens bool + + // brace is the location of the operand's opening `{`, for explicit operands. + brace *ast.Location +} + +// logicalStep is one operator application of an `and`/`or` chain. +type logicalStep struct { + op string + rhs logicalOperand + + // lhsEndRow is the row on which everything to the left of the operator ends. + lhsEndRow int +} + +// breaksLine reports whether the rhs operand is written on a line of its own. +// Explicit operands always open their brace on the operator's line, so only an +// implicit operand starting on a later row than the operator breaks. +func (s logicalStep) breaksLine() bool { + return !s.rhs.explicit && s.rhs.body[0].Location.Row > s.lhsEndRow +} + +func (w *writer) writeLogical(expr *ast.Expr, comments []*ast.Comment) ([]*ast.Comment, error) { + lhs, steps := flattenLogical(expr) + + comments, err := w.writeLogicalOperand(lhs, comments) + if err != nil && !errors.As(err, &unexpectedCommentError{}) { + return comments, err + } + + var indented bool + + for _, s := range steps { + w.write(" " + s.op) + + if s.breaksLine() { + if !indented { + w.up() + defer w.down() //nolint:errcheck + indented = true + } + w.endLine() + w.startLine() + } else { + w.write(" ") + } + + comments, err = w.writeLogicalOperand(s.rhs, comments) + if err != nil && !errors.As(err, &unexpectedCommentError{}) { + return comments, err + } + } + + return comments, nil +} + +func (w *writer) writeLogicalOperand(o logicalOperand, comments []*ast.Comment) ([]*ast.Comment, error) { + if !o.explicit { + if o.parens { + w.write("(") + defer w.write(")") + } + + return w.writeExpr(o.body[0], comments) + } + + if len(o.body) == 0 { + w.write("{}") + return comments, nil + } + + // A leading set union renders as `x | y`, which the parser reads as a + // comprehension at the brace, so it is parenthesized. + if isUnionExpr(o.body[0]) { + w.parenExpr = o.body[0] + } + + w.write("{") + comments, err := w.writeComprehensionBody('{', '}', o.body, o.brace, o.brace, comments) + if err != nil { + if !errors.As(err, &unexpectedCommentError{}) { + return comments, err + } + } + + if last := o.body[len(o.body)-1]; last.Location != nil && last.Location.Row == o.brace.Row { + w.write(" ") + } + w.write("}") + + return comments, nil +} + +// flattenLogical returns the leading operand and the operator applications of an +// `and`/`or` chain. Chains are left-associative, so the operands of +// `a and b and c` -- And{And{a, b}, c} -- are collected into a single chain, +// written with one level of continuation indent. A nested node that requires +// parens stays an operand of its own. +func flattenLogical(expr *ast.Expr) (logicalOperand, []logicalStep) { + op, lhs, rhs, explicitLhs, explicitRhs := logicalParts(expr) + + step := logicalStep{ + op: op, + rhs: newLogicalOperand(rhs, explicitRhs, op, true, expr.Location), + } + + if !explicitLhs && len(lhs) == 1 && isLogicalExpr(lhs[0]) && !logicalOperandNeedsParens(lhs[0], op, false) { + step.lhsEndRow = bodyEndRow(lhs) + first, steps := flattenLogical(lhs[0]) + + return first, append(steps, step) + } + + first := newLogicalOperand(lhs, explicitLhs, op, false, expr.Location) + step.lhsEndRow = logicalOperandEndRow(first) + + return first, []logicalStep{step} +} + +func logicalParts(expr *ast.Expr) (op string, lhs, rhs ast.Body, explicitLhs, explicitRhs bool) { + switch t := expr.Terms.(type) { + case *ast.LogicalAnd: + return "and", t.Lhs, t.Rhs, t.ExplicitLhs, t.ExplicitRhs + case *ast.LogicalOr: + return "or", t.Lhs, t.Rhs, t.ExplicitLhs, t.ExplicitRhs + } + + return "", nil, nil, false, false +} + +func newLogicalOperand(b ast.Body, explicit bool, parentOp string, rhs bool, node *ast.Location) logicalOperand { + if explicit || len(b) != 1 { + return logicalOperand{body: b, explicit: true, brace: operandBraceLoc(node, b)} + } + + return logicalOperand{body: b, parens: logicalOperandNeedsParens(b[0], parentOp, rhs)} +} + +// logicalOperandNeedsParens reports whether an implicit operand of parentOp must +// be parenthesized to be read back as that same expression. Mirrors +// logicalOperandNeedsParens in the ast package. +func logicalOperandNeedsParens(expr *ast.Expr, parentOp string, rhs bool) bool { + // A `with` on a bare operand binds to the whole and/or expression. + if len(expr.With) > 0 { + return true + } + + switch expr.Terms.(type) { + case *ast.LogicalOr: + // `or` binds looser than `and`: always parenthesize under `and`; under + // `or`, parenthesize only the rhs to preserve right-nesting. + return parentOp == "and" || rhs + case *ast.LogicalAnd: + // `and` binds tighter: no parens under `or`; under `and`, parenthesize + // only the rhs to preserve right-nesting. + return parentOp == "and" && rhs + } + + // A value that renders brace-led would be re-read as an explicit body. + return exprRendersBraceLead(expr) +} + +func logicalOperandEndRow(o logicalOperand) int { + if o.explicit { + if row := closingLoc(0, 0, '{', '}', o.brace).Row; row > 0 { + return row + } + } + + return bodyEndRow(o.body) +} + +// bodyEndRow returns the row of the last source line occupied by b. +func bodyEndRow(b ast.Body) int { + if len(b) == 0 { + return 0 + } + + loc := b[len(b)-1].Location + if loc == nil { + return 0 + } + + return loc.Row + bytes.Count(bytes.TrimRight(loc.Text, " \t\r\n"), []byte{'\n'}) +} + +// operandBraceLoc returns the location of the `{` opening an explicit operand +// body, derived from the location of the enclosing and/or node. The node +// location is returned as-is if the brace can't be located, e.g. for default +// locations. +func operandBraceLoc(node *ast.Location, b ast.Body) *ast.Location { + if node == nil || len(b) == 0 || b[0].Location == nil { + return node + } + + i := min(b[0].Location.Offset-node.Offset, len(node.Text)) + + for i--; i >= 0; i-- { + if node.Text[i] != '{' { + continue + } + + cpy := *node + cpy.Row = node.Row + bytes.Count(node.Text[:i], []byte{'\n'}) + cpy.Offset = node.Offset + i + cpy.Text = node.Text[i:] + + return &cpy + } + + return node +} + +func isLogicalExpr(expr *ast.Expr) bool { + return expr.IsAnd() || expr.IsOr() +} + func (w *writer) writeFunctionCall(expr *ast.Expr, comments []*ast.Comment) ([]*ast.Comment, error) { terms := expr.Terms.([]*ast.Term) @@ -1393,6 +1685,11 @@ func (w *writer) writeUnformatted(location *ast.Location, currentComments []*ast } func (w *writer) writeTermParens(parens bool, term *ast.Term, comments []*ast.Comment) ([]*ast.Comment, error) { + if w.parenTerm == term { + w.parenTerm = nil + parens = true + } + var err error comments, err = w.insertComments(comments, term.Location) if err != nil { @@ -2067,6 +2364,52 @@ func isUnionExpr(expr *ast.Expr) bool { return ok && len(terms) == 3 && ast.Or.Ref().Equal(terms[0].Value) } +// markUnionLead parenthesizes the set union leading the rendering of expr, if +// there is one: a leading `x | y` reads as comprehension syntax at the brace of +// the body holding expr. The union is either the expression itself, or the +// leading operand of an infix call — one nested deeper is already parenthesized +// by writeCall. +func (w *writer) markUnionLead(expr *ast.Expr) { + if expr.Negated { + return + } + + if isLogicalExpr(expr) { + if lhs, _ := flattenLogical(expr); !lhs.explicit && !lhs.parens { + w.markUnionLead(lhs.body[0]) + } + + return + } + + if isUnionExpr(expr) { + w.parenExpr = expr + return + } + + terms, ok := expr.Terms.([]*ast.Term) + if !ok { + return + } + + // Infix calls render an operand first: the result for the assigned form + // (`z = x | y`), otherwise the lhs (`x | y == z`). + if bi, ok := ast.BuiltinMap[terms[0].Value.String()]; ok && bi.Infix != "" { + var lead *ast.Term + + switch len(terms) { + case bi.Decl.Arity() + 1: + lead = terms[1] + case bi.Decl.Arity() + 2: + lead = terms[len(terms)-1] + } + + if lead != nil && isUnionCall(lead) { + w.parenTerm = lead + } + } +} + // exprRendersBraceLead reports whether expr renders starting with a `{`. Such an // expression needs parens in an operand position, as bare braces there are read as // an explicit body. Mirrors rendersWithLeadingBrace in the ast package. @@ -2099,6 +2442,13 @@ func termRendersBraceLead(t *ast.Term) bool { return true case ast.Ref: return len(v) > 0 && termRendersBraceLead(v[0]) + case ast.Call: + // An infix call renders an operand first, so a brace-led operand of a + // nested call leads the whole rendering: `{1, 2} & s == set()`. + if bi, ok := ast.BuiltinMap[v[0].Value.String()]; ok && bi.Infix != "" && + len(v) == bi.Decl.Arity()+1 { + return termRendersBraceLead(v[1]) + } } return false diff --git a/v1/format/format_test.go b/v1/format/format_test.go index 2f09de14e9b..6f924dc133e 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, @@ -913,6 +922,123 @@ p if { not input.x == 1 }`, }, + { + note: "v0, and adds import if missing", + regoVersion: ast.RegoV0, + toFmt: ast.MustParseModuleWithOpts(`package test + p if { + input.a and input.b + }`, + ast.ParserOptions{ + FutureKeywords: []string{"and"}, + Capabilities: experimentalKeywordCapabilities(ast.RegoV1), + }), + expected: `package test + +import future.keywords.and + +p { + input.a and input.b +}`, + }, + { + note: "v1, and adds import if missing", + regoVersion: ast.RegoV1, + toFmt: ast.MustParseModuleWithOpts(`package test + p if { + input.a and input.b + }`, + ast.ParserOptions{ + FutureKeywords: []string{"and"}, + Capabilities: experimentalKeywordCapabilities(ast.RegoV1), + }), + expected: `package test + +import future.keywords.and + +p if { + input.a and input.b +}`, + }, + { + note: "v0, or adds import if missing", + regoVersion: ast.RegoV0, + toFmt: ast.MustParseModuleWithOpts(`package test + p if { + input.a or input.b + }`, + ast.ParserOptions{ + FutureKeywords: []string{"or"}, + Capabilities: experimentalKeywordCapabilities(ast.RegoV1), + }), + expected: `package test + +import future.keywords.or + +p { + input.a or input.b +}`, + }, + { + note: "v1, or adds import if missing", + regoVersion: ast.RegoV1, + toFmt: ast.MustParseModuleWithOpts(`package test + p if { + input.a or input.b + }`, + ast.ParserOptions{ + FutureKeywords: []string{"or"}, + Capabilities: experimentalKeywordCapabilities(ast.RegoV1), + }), + expected: `package test + +import future.keywords.or + +p if { + input.a or input.b +}`, + }, + { + note: "logical chain, no locations", + toFmt: ast.NewExpr(&ast.LogicalAnd{ + Lhs: ast.NewBody(ast.NewExpr(&ast.LogicalOr{ + Lhs: ast.NewBody(ast.MustParseExpr("input.a")), + Rhs: ast.NewBody(ast.MustParseExpr("input.b")), + })), + Rhs: ast.NewBody(ast.MustParseExpr("input.c")), + }), + expected: `(input.a or input.b) and input.c`, + }, + { + note: "logical chain, explicit operand bodies, no locations", + toFmt: ast.NewExpr(&ast.LogicalOr{ + Lhs: ast.Body{ + ast.MustParseExpr("input.a"), + ast.MustParseExpr("input.b"), + }, + ExplicitLhs: true, + Rhs: ast.NewBody(ast.MustParseExpr("input.c")), + ExplicitRhs: true, + }), + expected: `{ input.a; input.b } or { input.c }`, + }, + { + note: "logical chain, brace-led implicit operands, no locations", + toFmt: ast.NewExpr(&ast.LogicalAnd{ + Lhs: ast.NewBody(ast.NewExpr(ast.ObjectTerm( + [2]*ast.Term{ast.StringTerm("a"), ast.NumberTerm("1")}))), + Rhs: ast.NewBody(ast.NewExpr(ast.SetTerm(ast.VarTerm("x")))), + }), + expected: `({"a": 1}) and ({x})`, + }, + { + note: "negated logical expression, no locations", + toFmt: ast.NewExpr(&ast.LogicalOr{ + Lhs: ast.NewBody(ast.MustParseExpr("input.a")), + Rhs: ast.NewBody(ast.MustParseExpr("input.b")), + }).Complement(), + expected: `not (input.a or input.b)`, + }, } for _, tc := range cases { @@ -1229,8 +1355,8 @@ func TestFormatKeywordsInRefs(t *testing.T) { t.Fatalf("Failed to read rego source: %v", err) } - caps := ast.CapabilitiesForThisVersion(ast.CapabilitiesRegoVersion(regoVersion)) - feats := []string{} + caps := experimentalKeywordCapabilities(regoVersion) + feats := make([]string, 0, len(caps.Features)) for _, f := range caps.Features { if f != ast.FeatureKeywordsInRefs { feats = append(feats, f) @@ -1240,6 +1366,9 @@ func TestFormatKeywordsInRefs(t *testing.T) { popts := ast.ParserOptions{ RegoVersion: regoVersion, + // The source is parsed with keywords in refs allowed; it is + // only the formatting of refs that drops the feature. + Capabilities: experimentalKeywordCapabilities(regoVersion), } opts := Opts{ RegoVersion: regoVersion, diff --git a/v1/format/testfiles/v0/test_logical.rego b/v1/format/testfiles/v0/test_logical.rego new file mode 100644 index 00000000000..1559c3b3d55 --- /dev/null +++ b/v1/format/testfiles/v0/test_logical.rego @@ -0,0 +1,34 @@ +package test + +import future.keywords.and +import future.keywords.not +import future.keywords.or + +# in v0 a rule body keeps its braces, so an and/or expression is never inlined +p { input.a and input.b } + +q = 1 { input.a or input.b } + +r[x] { + x = input.a + input.b or input.c +} + +explicit_body_operand { {input.a} and input.b } + +parens { input.a and (input.b or input.c) } + +set_operand { ({input.a}) or input.b } + +negated { not (input.a or input.b) } + +multiline { + input.a and + input.b + + input.c or + input.d or + input.e +} + +with_modifier { input.a and input.b with input.x as 1 } diff --git a/v1/format/testfiles/v0/test_logical.rego.formatted b/v1/format/testfiles/v0/test_logical.rego.formatted new file mode 100644 index 00000000000..8090ff00d06 --- /dev/null +++ b/v1/format/testfiles/v0/test_logical.rego.formatted @@ -0,0 +1,48 @@ +package test + +import future.keywords.and +import future.keywords.not +import future.keywords.or + +# in v0 a rule body keeps its braces, so an and/or expression is never inlined +p { + input.a and input.b +} + +q = 1 { + input.a or input.b +} + +r[x] { + x = input.a + input.b or input.c +} + +explicit_body_operand { + { input.a } and input.b +} + +parens { + input.a and (input.b or input.c) +} + +set_operand { + ({input.a}) or input.b +} + +negated { + not (input.a or input.b) +} + +multiline { + input.a and + input.b + + input.c or + input.d or + input.e +} + +with_modifier { + input.a and input.b with input.x as 1 +} diff --git a/v1/format/testfiles/v1/test_logical_basic.rego b/v1/format/testfiles/v1/test_logical_basic.rego new file mode 100644 index 00000000000..f4278266b1a --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_basic.rego @@ -0,0 +1,63 @@ +package test + +import future.keywords.and +import future.keywords.or + +both if { input.a and input.b } + +either if { input.a or input.b } + +in_body if { + input.enabled + input.role=="admin" or input.role=="superuser" +} + +# `and` binds tighter than `or`, so no parens are needed or emitted +precedence if input.a or input.b and input.c + +chained if input.a or input.b or input.c or input.d + +mixed_chain if input.a and input.b or input.c and input.d + +operand_terms if { + x := input.n + x and 1 + x or "str" + x and [1, 2] + x and count([1]) == 1 + count([1]) == 1 or x +} + +in_comprehension if { + xs := [x | input.a[x]; input.b[x] and input.c[x]] + xs == [1] +} + +with_every if { + every x in [1, 2] { x > 0 } + input.a or input.b + + {every x in [1, 2] { x > 0 }} or input.c + + input.d and {every x in [1, 2] { x > 0 }} +} + +with_some if { + some x in input.xs + input.a[x] and input.b[x] + + {some y in input.ys;input.a[y]} and input.b + + input.c and {some z in input.zs;input.a[z]} +} + +multi_line_operand_bodies if { + { + some v in input.vs + input.a[v] + } and input.b + + input.c or { + every u in input.us { u > 0 } + } +} diff --git a/v1/format/testfiles/v1/test_logical_basic.rego.formatted b/v1/format/testfiles/v1/test_logical_basic.rego.formatted new file mode 100644 index 00000000000..f5e9738ff3c --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_basic.rego.formatted @@ -0,0 +1,63 @@ +package test + +import future.keywords.and +import future.keywords.or + +both if input.a and input.b + +either if input.a or input.b + +in_body if { + input.enabled + input.role == "admin" or input.role == "superuser" +} + +# `and` binds tighter than `or`, so no parens are needed or emitted +precedence if input.a or input.b and input.c + +chained if input.a or input.b or input.c or input.d + +mixed_chain if input.a and input.b or input.c and input.d + +operand_terms if { + x := input.n + x and 1 + x or "str" + x and [1, 2] + x and count([1]) == 1 + count([1]) == 1 or x +} + +in_comprehension if { + xs := [x | input.a[x]; input.b[x] and input.c[x]] + xs == [1] +} + +with_every if { + every x in [1, 2] { x > 0 } + input.a or input.b + + { every x in [1, 2] { x > 0 } } or input.c + + input.d and { every x in [1, 2] { x > 0 } } +} + +with_some if { + some x in input.xs + input.a[x] and input.b[x] + + { some y in input.ys; input.a[y] } and input.b + + input.c and { some z in input.zs; input.a[z] } +} + +multi_line_operand_bodies if { + { + some v in input.vs + input.a[v] + } and input.b + + input.c or { + every u in input.us { u > 0 } + } +} diff --git a/v1/format/testfiles/v1/test_logical_comments.rego b/v1/format/testfiles/v1/test_logical_comments.rego new file mode 100644 index 00000000000..e89263ee8b8 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_comments.rego @@ -0,0 +1,37 @@ +package test + +import future.keywords.and +import future.keywords.or + +between_operands if { + input.a and + # only admins + input.b +} + +after_operator if { + input.a and # first + input.b +} + +around_chain if { + # before + input.a or + # between + input.b or + # also + input.c + # after +} + +explicit_body_operand if { # open +# leading + input.a # trailing +# before close +} and input.b + +explicit_body_operand_rhs if input.a and { # open +# leading + input.b # trailing +# before close +} diff --git a/v1/format/testfiles/v1/test_logical_comments.rego.formatted b/v1/format/testfiles/v1/test_logical_comments.rego.formatted new file mode 100644 index 00000000000..41c5e8ff597 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_comments.rego.formatted @@ -0,0 +1,37 @@ +package test + +import future.keywords.and +import future.keywords.or + +between_operands if { + input.a and + # only admins + input.b +} + +after_operator if { + input.a and # first + input.b +} + +around_chain if { + # before + input.a or + # between + input.b or + # also + input.c + # after +} + +explicit_body_operand if { # open + # leading + input.a # trailing + # before close +} and input.b + +explicit_body_operand_rhs if input.a and { # open + # leading + input.b # trailing + # before close +} diff --git a/v1/format/testfiles/v1/test_logical_explicit_body.rego b/v1/format/testfiles/v1/test_logical_explicit_body.rego new file mode 100644 index 00000000000..d4df6dd9a46 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_explicit_body.rego @@ -0,0 +1,41 @@ +package test + +import future.keywords.and +import future.keywords.or + +# An explicit `{...}` operand is a body of its own, and is never collapsed +lhs_body if {input.a} and input.b + +rhs_body if input.a or {input.b} + +both_bodies if {input.a} and {input.b} + +multi_expr_body if { {x := input.a +x > 0} and input.b } + +multi_line_body if { + x := input.a + x > 0 +} and input.b + +body_on_operator_line if input.a and { +input.b or input.c } + +nested_bodies if { {input.a; input.b} and {input.c +input.d} } + +close_brace_shares_operator_line if { + input.a +} and { + input.b +} + +set_operand_keeps_parens if ({input.a}) and input.b + +set_operand_keeps_parens_rhs if input.a and ({input.b}) + +# a variable bound in an operand body does not escape it, so it is used there +body_with_comprehension if { + xs := [x | input.a[x]] + count(xs) > 0 +} and input.b diff --git a/v1/format/testfiles/v1/test_logical_explicit_body.rego.formatted b/v1/format/testfiles/v1/test_logical_explicit_body.rego.formatted new file mode 100644 index 00000000000..2d90bf571e8 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_explicit_body.rego.formatted @@ -0,0 +1,46 @@ +package test + +import future.keywords.and +import future.keywords.or + +# An explicit `{...}` operand is a body of its own, and is never collapsed +lhs_body if { input.a } and input.b + +rhs_body if input.a or { input.b } + +both_bodies if { input.a } and { input.b } + +multi_expr_body if { + x := input.a + x > 0 +} and input.b + +multi_line_body if { + x := input.a + x > 0 +} and input.b + +body_on_operator_line if input.a and { + input.b or input.c +} + +nested_bodies if { input.a; input.b } and { + input.c + input.d +} + +close_brace_shares_operator_line if { + input.a +} and { + input.b +} + +set_operand_keeps_parens if ({input.a}) and input.b + +set_operand_keeps_parens_rhs if input.a and ({input.b}) + +# a variable bound in an operand body does not escape it, so it is used there +body_with_comprehension if { + xs := [x | input.a[x]] + count(xs) > 0 +} and input.b diff --git a/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego b/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego new file mode 100644 index 00000000000..4fef79d805d --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego @@ -0,0 +1,16 @@ +package test + +import future.keywords.and +import future.keywords.or + +# when `and`/`or` are keywords, the author's bracket notation is preserved +quoted_refs if { + input["and"] == 1 + input["or"] == 2 + input.a["and"].b or input.c +} + +unquoted_refs if { + input.and == 1 + input.or == 2 +} diff --git a/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego.formatted b/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego.formatted new file mode 100644 index 00000000000..4fef79d805d --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego.formatted @@ -0,0 +1,16 @@ +package test + +import future.keywords.and +import future.keywords.or + +# when `and`/`or` are keywords, the author's bracket notation is preserved +quoted_refs if { + input["and"] == 1 + input["or"] == 2 + input.a["and"].b or input.c +} + +unquoted_refs if { + input.and == 1 + input.or == 2 +} diff --git a/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego.formatted_no_keywords_in_refs b/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego.formatted_no_keywords_in_refs new file mode 100644 index 00000000000..881bb67848e --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_keywords_in_refs.rego.formatted_no_keywords_in_refs @@ -0,0 +1,16 @@ +package test + +import future.keywords.and +import future.keywords.or + +# when `and`/`or` are keywords, the author's bracket notation is preserved +quoted_refs if { + input["and"] == 1 + input["or"] == 2 + input.a["and"].b or input.c +} + +unquoted_refs if { + input["and"] == 1 + input["or"] == 2 +} diff --git a/v1/format/testfiles/v1/test_logical_multiline.rego b/v1/format/testfiles/v1/test_logical_multiline.rego new file mode 100644 index 00000000000..b9e179f400f --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_multiline.rego @@ -0,0 +1,43 @@ +package test + +import future.keywords.and +import future.keywords.or + +# Line breaks are preserved; continuation lines get one extra indent level +broken_chain if { + input.a and + input.b and + input.c +} + +over_indented if { + input.a and + input.b and + input.c +} + +mixed_operators if { + input.a and input.b or + input.c and input.d +} + +partly_broken if { + input.a and input.b and + input.c +} + +nested_group_breaks if { + input.a and (input.b or + input.c) +} + +broken_group_lhs if { + (input.a or + input.b) and input.c +} + +already_formatted if { + input.a and + input.b or + input.c +} diff --git a/v1/format/testfiles/v1/test_logical_multiline.rego.formatted b/v1/format/testfiles/v1/test_logical_multiline.rego.formatted new file mode 100644 index 00000000000..8a90b79fb85 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_multiline.rego.formatted @@ -0,0 +1,43 @@ +package test + +import future.keywords.and +import future.keywords.or + +# Line breaks are preserved; continuation lines get one extra indent level +broken_chain if { + input.a and + input.b and + input.c +} + +over_indented if { + input.a and + input.b and + input.c +} + +mixed_operators if { + input.a and input.b or + input.c and input.d +} + +partly_broken if { + input.a and input.b and + input.c +} + +nested_group_breaks if { + input.a and (input.b or + input.c) +} + +broken_group_lhs if { + (input.a or + input.b) and input.c +} + +already_formatted if { + input.a and + input.b or + input.c +} diff --git a/v1/format/testfiles/v1/test_logical_nested.rego b/v1/format/testfiles/v1/test_logical_nested.rego new file mode 100644 index 00000000000..b729513aff2 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_nested.rego @@ -0,0 +1,33 @@ +package test + +import future.keywords.and +import future.keywords.not +import future.keywords.or + +deep_chain if { {a := input.a; a > 0} and (input.b or {input.c and input.d}) } + +nested_in_comprehension if { + xs := [x | input.a[x]; input.b[x] and (input.c[x] or input.d[x])] + ys := {x | input.a[x]; input.b[x] or {input.c[x]; input.d[x]}} + count(xs) == count(ys) +} + +nested_in_every if { + every x in input.xs { + x.a and (x.b or x.c) + } +} + +nested_in_not if not (input.a and (input.b or not input.c)) + +with_else := 1 if {input.a} and input.b else := 2 + +nested_body_in_body if { + { + input.a and {input.b + input.c } + } or { + input.d + input.e + } +} diff --git a/v1/format/testfiles/v1/test_logical_nested.rego.formatted b/v1/format/testfiles/v1/test_logical_nested.rego.formatted new file mode 100644 index 00000000000..d4639770eb2 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_nested.rego.formatted @@ -0,0 +1,37 @@ +package test + +import future.keywords.and +import future.keywords.not +import future.keywords.or + +deep_chain if { a := input.a; a > 0 } and (input.b or { input.c and input.d }) + +nested_in_comprehension if { + xs := [x | input.a[x]; input.b[x] and (input.c[x] or input.d[x])] + ys := {x | input.a[x]; input.b[x] or { input.c[x]; input.d[x] }} + count(xs) == count(ys) +} + +nested_in_every if { + every x in input.xs { + x.a and (x.b or x.c) + } +} + +nested_in_not if not (input.a and (input.b or not input.c)) + +with_else := 1 if { input.a } and input.b + +else := 2 + +nested_body_in_body if { + { + input.a and { + input.b + input.c + } + } or { + input.d + input.e + } +} diff --git a/v1/format/testfiles/v1/test_logical_not.rego b/v1/format/testfiles/v1/test_logical_not.rego new file mode 100644 index 00000000000..c82668704eb --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_not.rego @@ -0,0 +1,31 @@ +package test + +import future.keywords.and +import future.keywords.not +import future.keywords.or + +# `not` binds tighter than `and`/`or` +not_operand if { not input.a and input.b } + +not_rhs_operand if input.a or not input.b + +# a negated group is parenthesized +negated_group if { not (input.a or input.b) } + +negated_group_operand if not (input.a and input.b) or input.c + +# a negated body keeps its braces +negated_body if { not {input.a or input.b} } + +negated_body_rhs_operand if input.a and not {input.b} + +# a not-body operand keeps its braces on either side of the operator, and needs no +# parens: redundant ones are dropped +negated_body_lhs_operand if not {input.a} and input.b + +negated_body_lhs_operand_parens if (not {input.a}) and input.b + +negated_multi_expr_body_lhs_operand if (not {input.a; input.b}) or input.c + +# a `with` on a bare operand of `not` binds to the whole `not` expression +negated_with_operand if not (input.a with input.x as 1) diff --git a/v1/format/testfiles/v1/test_logical_not.rego.formatted b/v1/format/testfiles/v1/test_logical_not.rego.formatted new file mode 100644 index 00000000000..97e98e5fce9 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_not.rego.formatted @@ -0,0 +1,31 @@ +package test + +import future.keywords.and +import future.keywords.not +import future.keywords.or + +# `not` binds tighter than `and`/`or` +not_operand if not input.a and input.b + +not_rhs_operand if input.a or not input.b + +# a negated group is parenthesized +negated_group if not (input.a or input.b) + +negated_group_operand if not (input.a and input.b) or input.c + +# a negated body keeps its braces +negated_body if not { input.a or input.b } + +negated_body_rhs_operand if input.a and not { input.b } + +# a not-body operand keeps its braces on either side of the operator, and needs no +# parens: redundant ones are dropped +negated_body_lhs_operand if not { input.a } and input.b + +negated_body_lhs_operand_parens if not { input.a } and input.b + +negated_multi_expr_body_lhs_operand if not { input.a; input.b } or input.c + +# a `with` on a bare operand of `not` binds to the whole `not` expression +negated_with_operand if not (input.a with input.x as 1) diff --git a/v1/format/testfiles/v1/test_logical_parens.rego b/v1/format/testfiles/v1/test_logical_parens.rego new file mode 100644 index 00000000000..236cfc205ff --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_parens.rego @@ -0,0 +1,81 @@ +package test + +import future.keywords.and +import future.keywords.or + +redundant if { ((input.a or input.b)) } + +right_nested_or_keeps_parens if input.a or (input.b or input.c) + +left_nested_or_drops_parens if (input.a or input.b) or input.c + +right_nested_and_keeps_parens if input.a and (input.b and input.c) + +left_nested_and_drops_parens if (input.a and input.b) and input.c + +and_under_or_drops_parens if (input.a and input.b) or input.c + +and_under_or_drops_parens_rhs if input.c or (input.a and input.b) + +or_under_and_keeps_parens if input.c and (input.a or input.b) + +or_under_and_keeps_parens_lhs if (input.a or input.b) and input.c + +deeply_nested if input.a or (input.b and (input.c or input.d)) + +# A brace-led operand would be read back as an explicit body, so it keeps its parens +set_term_lhs if ({input.a}) or input.b + +set_term_rhs if input.b or ({input.a}) + +object_term_lhs if ({"b": 1}) or input.a + +object_term_rhs if input.a or ({"b": 1}) + +comprehension_term_lhs if ({x | input.a[x]}) or input.b + +comprehension_term_rhs if input.b or ({x | input.a[x]}) + +comparison_with_set_term_lhs if ({input.a} == input.b) and input.c + +comparison_with_set_term_rhs if input.c and ({input.a} == input.b) + +ref_into_set_term_lhs if ({input.a}[0]) and input.b + +ref_into_set_term_rhs if input.b and ({input.a}[0]) + +# a brace-led operand nested inside an infix call still leads the rendering +nested_brace_lead_lhs if ({1, 2}) & input.s == set() and input.a + +nested_brace_lead_rhs if input.a and ({1, 2}) & input.s == set() + +nested_brace_lead_ref_lhs if ({"b": 1}.b) == 1 and input.a + +nested_brace_lead_ref_rhs if input.a and ({"b": 1}.b) == 1 + +# `x | y` is a set union here, and needs no parens outside of braces +set_union_lhs if (input.a | input.b) or input.c + +set_union_rhs if input.c or (input.a | input.b) + +# inside braces it would read as a comprehension, so it keeps its parens +set_union_in_body_lhs if {(input.a | input.b)} or input.c + +set_union_in_body_rhs if input.c or {(input.a | input.b)} + +# in a rule body the leading `|` would make the braces read as a comprehension +set_union_in_rule_body if { + (input.a | input.b) or input.c +} + +set_union_in_rule_body_nested if { + ((input.a | input.b) == input.c) or input.d +} + +set_union_in_rule_body_with if { + (input.a | input.b) or input.c with input.x as 1 +} + +set_union_operand_with_body if input.c or {(input.a | input.b) with input.x as 1} + +set_union_operand_with_parens if input.c or ((input.a | input.b) with input.x as 1) diff --git a/v1/format/testfiles/v1/test_logical_parens.rego.formatted b/v1/format/testfiles/v1/test_logical_parens.rego.formatted new file mode 100644 index 00000000000..ee0af09b7ba --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_parens.rego.formatted @@ -0,0 +1,81 @@ +package test + +import future.keywords.and +import future.keywords.or + +redundant if input.a or input.b + +right_nested_or_keeps_parens if input.a or (input.b or input.c) + +left_nested_or_drops_parens if input.a or input.b or input.c + +right_nested_and_keeps_parens if input.a and (input.b and input.c) + +left_nested_and_drops_parens if input.a and input.b and input.c + +and_under_or_drops_parens if input.a and input.b or input.c + +and_under_or_drops_parens_rhs if input.c or input.a and input.b + +or_under_and_keeps_parens if input.c and (input.a or input.b) + +or_under_and_keeps_parens_lhs if (input.a or input.b) and input.c + +deeply_nested if input.a or input.b and (input.c or input.d) + +# A brace-led operand would be read back as an explicit body, so it keeps its parens +set_term_lhs if ({input.a}) or input.b + +set_term_rhs if input.b or ({input.a}) + +object_term_lhs if ({"b": 1}) or input.a + +object_term_rhs if input.a or ({"b": 1}) + +comprehension_term_lhs if ({x | input.a[x]}) or input.b + +comprehension_term_rhs if input.b or ({x | input.a[x]}) + +comparison_with_set_term_lhs if ({input.a} == input.b) and input.c + +comparison_with_set_term_rhs if input.c and ({input.a} == input.b) + +ref_into_set_term_lhs if ({input.a}[0]) and input.b + +ref_into_set_term_rhs if input.b and ({input.a}[0]) + +# a brace-led operand nested inside an infix call still leads the rendering +nested_brace_lead_lhs if ({1, 2} & input.s == set()) and input.a + +nested_brace_lead_rhs if input.a and ({1, 2} & input.s == set()) + +nested_brace_lead_ref_lhs if ({"b": 1}.b == 1) and input.a + +nested_brace_lead_ref_rhs if input.a and ({"b": 1}.b == 1) + +# `x | y` is a set union here, and needs no parens outside of braces +set_union_lhs if input.a | input.b or input.c + +set_union_rhs if input.c or input.a | input.b + +# inside braces it would read as a comprehension, so it keeps its parens +set_union_in_body_lhs if { (input.a | input.b) } or input.c + +set_union_in_body_rhs if input.c or { (input.a | input.b) } + +# in a rule body the leading `|` would make the braces read as a comprehension +set_union_in_rule_body if { + (input.a | input.b) or input.c +} + +set_union_in_rule_body_nested if { + (input.a | input.b) == input.c or input.d +} + +set_union_in_rule_body_with if { + (input.a | input.b) or input.c with input.x as 1 +} + +set_union_operand_with_body if input.c or { (input.a | input.b) with input.x as 1 } + +set_union_operand_with_parens if input.c or (input.a | input.b with input.x as 1) diff --git a/v1/format/testfiles/v1/test_logical_with.rego b/v1/format/testfiles/v1/test_logical_with.rego new file mode 100644 index 00000000000..4a788546b59 --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_with.rego @@ -0,0 +1,27 @@ +package test + +import future.keywords.and +import future.keywords.or + +# a trailing `with` applies to the whole and/or expression +whole_expression if { input.a and input.b with input.x as 1 } + +parens_around_expression if (input.a and input.b) with input.x as 1 + +multiple_modifiers if input.a or input.b with input.x as 1 with input.y as 2 + +indented_modifiers if input.a or input.b with input.x as 1 + with input.y as 2 + +# a `with` on a single operand is scoped by parens, which must be kept +lhs_operand if { (input.a with input.x as 1) and input.b } + +rhs_operand if input.a and (input.b with input.x as 1) + +both_operands if (input.a with input.x as 1) or (input.b with input.y as 2) + +operand_and_expression if input.a and (input.b with input.x as 1) with input.y as 2 + +explicit_body_operand_lhs if {input.a with input.x as 1} and input.b + +explicit_body_operand_rhs if input.a and {input.b with input.x as 1} diff --git a/v1/format/testfiles/v1/test_logical_with.rego.formatted b/v1/format/testfiles/v1/test_logical_with.rego.formatted new file mode 100644 index 00000000000..610f716100d --- /dev/null +++ b/v1/format/testfiles/v1/test_logical_with.rego.formatted @@ -0,0 +1,27 @@ +package test + +import future.keywords.and +import future.keywords.or + +# a trailing `with` applies to the whole and/or expression +whole_expression if input.a and input.b with input.x as 1 + +parens_around_expression if input.a and input.b with input.x as 1 + +multiple_modifiers if input.a or input.b with input.x as 1 with input.y as 2 + +indented_modifiers if input.a or input.b with input.x as 1 + with input.y as 2 + +# a `with` on a single operand is scoped by parens, which must be kept +lhs_operand if (input.a with input.x as 1) and input.b + +rhs_operand if input.a and (input.b with input.x as 1) + +both_operands if (input.a with input.x as 1) or (input.b with input.y as 2) + +operand_and_expression if input.a and (input.b with input.x as 1) with input.y as 2 + +explicit_body_operand_lhs if { input.a with input.x as 1 } and input.b + +explicit_body_operand_rhs if input.a and { input.b with input.x as 1 } diff --git a/v1/format/testfiles/v1/test_set_union_body.rego b/v1/format/testfiles/v1/test_set_union_body.rego new file mode 100644 index 00000000000..de7ef9b1a8c --- /dev/null +++ b/v1/format/testfiles/v1/test_set_union_body.rego @@ -0,0 +1,102 @@ +package test + +# A `|` leading the first expression of a `p if { ... }` body would make the +# braces read as a set comprehension, so the union keeps its parens there. +lone if { + (input.a | input.b) +} + +leading if { + (input.a | input.b) + input.c +} + +under_comparison if { + (input.a | input.b) == input.c +} + +under_unification if { + (input.a | input.b) = input.c +} + +with_modifier if { + (input.a | input.b) with input.x as 1 +} + +partial contains 1 if { + (input.a | input.b) +} + +function(_) if { + (input.a | input.b) +} + +value := 1 if { + (input.a | input.b) +} + +# No ambiguity when the union doesn't lead the braces +trailing if { + input.c + (input.a | input.b) +} + +assigned if { + x := (input.a | input.b) + x +} + +negated if { + not (input.a | input.b) +} + +rhs_operand if { + input.c == (input.a | input.b) +} + +nested_call if { + (input.a | input.b) & input.c == input.d +} + +# An `else` body, an `every` body and a comprehension body are never read as a term +else_body if { + input.z +} else if { + (input.a | input.b) +} + +every_body if { + every x in input.xs { + (input.a | input.b) + x + } +} + +comprehension_body := {y | + (input.a | input.b) + y := 1 +} + +# Sanity: comprehension syntax is untouched +set_comprehension := {x | some x in input.xs} + +object_comprehension := {k: v | some k, v in input.o} + +array_comprehension := [x | some x in input.xs] + +union_in_comprehension_head := {(input.a | input.b) | input.c} + +comprehension_term_body if { + {x | input.a[x]} +} + +comprehension_term_body_oneline if {x | input.a[x]} + +comprehension_in_expression if { + {x | input.a[x]} == input.b +} + +comprehension_statement if { + s := {x | some x in input.xs} + count(s) > 0 +} diff --git a/v1/format/testfiles/v1/test_set_union_body.rego.formatted b/v1/format/testfiles/v1/test_set_union_body.rego.formatted new file mode 100644 index 00000000000..eab1aa9b539 --- /dev/null +++ b/v1/format/testfiles/v1/test_set_union_body.rego.formatted @@ -0,0 +1,102 @@ +package test + +# A `|` leading the first expression of a `p if { ... }` body would make the +# braces read as a set comprehension, so the union keeps its parens there. +lone if { + (input.a | input.b) +} + +leading if { + (input.a | input.b) + input.c +} + +under_comparison if { + (input.a | input.b) == input.c +} + +under_unification if { + (input.a | input.b) = input.c +} + +with_modifier if { + (input.a | input.b) with input.x as 1 +} + +partial contains 1 if { + (input.a | input.b) +} + +function(_) if { + (input.a | input.b) +} + +value := 1 if { + (input.a | input.b) +} + +# No ambiguity when the union doesn't lead the braces +trailing if { + input.c + input.a | input.b +} + +assigned if { + x := input.a | input.b + x +} + +negated if { + not input.a | input.b +} + +rhs_operand if { + input.c == input.a | input.b +} + +nested_call if { + (input.a | input.b) & input.c == input.d +} + +# An `else` body, an `every` body and a comprehension body are never read as a term +else_body if { + input.z +} else if { + input.a | input.b +} + +every_body if { + every x in input.xs { + input.a | input.b + x + } +} + +comprehension_body := {y | + input.a | input.b + y := 1 +} + +# Sanity: comprehension syntax is untouched +set_comprehension := {x | some x in input.xs} + +object_comprehension := {k: v | some k, v in input.o} + +array_comprehension := [x | some x in input.xs] + +union_in_comprehension_head := {(input.a | input.b) | input.c} + +comprehension_term_body if { + {x | input.a[x]} +} + +comprehension_term_body_oneline if {x | input.a[x]} + +comprehension_in_expression if { + {x | input.a[x]} == input.b +} + +comprehension_statement if { + s := {x | some x in input.xs} + count(s) > 0 +}