From 3087f28d56e3081e1623901bdce9ffe6f98035b2 Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Thu, 13 Aug 2026 19:38:56 +0200 Subject: [PATCH] Various io.Writer improvements And avoid having init() functions in tests, as those run even for unrelated tests, and make debugging confusing as breakpoints get hit for things called from them. Signed-off-by: Anders Eknert --- .golangci.yaml | 1 + cmd/parse.go | 3 +- internal/presentation/presentation.go | 12 +- v1/ast/policy_appenders_test.go | 365 +++++++++++++------------- v1/ast/term_appenders_test.go | 297 ++++++++++----------- v1/rego/rego_metadata_test.go | 5 +- v1/repl/repl.go | 2 +- v1/server/server_test.go | 4 +- v1/topdown/http.go | 12 +- v1/topdown/json.go | 4 +- v1/topdown/providers.go | 6 +- v1/topdown/regex_template.go | 12 +- v1/topdown/time.go | 26 +- v1/topdown/topdown_test.go | 18 ++ v1/topdown/trace.go | 15 +- 15 files changed, 398 insertions(+), 384 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index a283ba14ce7..865b4755d6e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -26,6 +26,7 @@ linters: exclude-functions: - github.com/open-policy-agent/opa/v1/util.WriteAppender - github.com/open-policy-agent/opa/v1/util.WriteInt + - (io.Writer).Write forbidigo: forbid: - pattern: '^sort\.[A-Z][a-zA-Z]+$' diff --git a/cmd/parse.go b/cmd/parse.go index 7b2e69ee759..63f9b48eb13 100644 --- a/cmd/parse.go +++ b/cmd/parse.go @@ -107,7 +107,8 @@ func parse(args []string, params *parseParams, stdout io.Writer, stderr io.Write _ = pr.JSON(stderr, pr.Output{Errors: pr.NewOutputErrors(err)}) return 1 } - _, _ = stdout.Write(append(bs, '\n')) + + stdout.Write(append(bs, '\n')) default: ast.Pretty(stdout, result.Parsed) } diff --git a/internal/presentation/presentation.go b/internal/presentation/presentation.go index 1b3d7bbec3c..153fb1b6579 100644 --- a/internal/presentation/presentation.go +++ b/internal/presentation/presentation.go @@ -55,7 +55,6 @@ func (o DepAnalysisOutput) JSON(w io.Writer) error { // Pretty outputs o to w in a human-readable format. func (o DepAnalysisOutput) Pretty(w io.Writer) error { - var headers []string var rows [][]string @@ -374,7 +373,7 @@ func Source(w io.Writer, errW io.Writer, r Output) error { if err != nil { return err } - fmt.Fprintln(w, string(bs)) + w.Write(append(bs, '\n')) } for i := range r.Partial.Support { @@ -383,7 +382,7 @@ func Source(w io.Writer, errW io.Writer, r Output) error { if err != nil { return err } - fmt.Fprint(w, string(bs)) + w.Write(bs) } return nil @@ -407,14 +406,13 @@ func Raw(w io.Writer, errW io.Writer, r Output) error { if err != nil { return err } - - fmt.Fprint(w, string(bytes)) + w.Write(bytes) } if i+1 >= len(rs.Expressions) { - fmt.Fprintln(w, "") + w.Write([]byte{'\n'}) } else { - fmt.Fprint(w, " ") + w.Write([]byte{' '}) } } } diff --git a/v1/ast/policy_appenders_test.go b/v1/ast/policy_appenders_test.go index aa55565479e..21b7ab71057 100644 --- a/v1/ast/policy_appenders_test.go +++ b/v1/ast/policy_appenders_test.go @@ -2,225 +2,230 @@ package ast_test import ( "encoding" + "sync" "testing" "github.com/open-policy-agent/opa/v1/ast" ) -var policyAppenderTests = []struct { +type appenderTest struct { name string node ast.StringLengther want string nolen bool -}{ - { - name: "module", - node: &ast.Module{ - Package: &ast.Package{ - Path: ast.Ref{ast.DefaultRootDocument, ast.InternedTerm("a"), ast.InternedTerm("b")}, - }, - Imports: ast.MustParseImports(` +} + +var policyAppenderTests = sync.OnceValue(func() []appenderTest { + return []appenderTest{ + { + name: "module", + node: &ast.Module{ + Package: &ast.Package{ + Path: ast.Ref{ast.DefaultRootDocument, ast.InternedTerm("a"), ast.InternedTerm("b")}, + }, + Imports: ast.MustParseImports(` import data.foo.bar as baz import input.a.b.c `), - }, - want: `package a.b + }, + want: `package a.b import data.foo.bar as baz import input.a.b.c `, - }, - { - name: "module annotated", - node: ast.MustParseModuleWithOpts(`# METADATA + }, + { + name: "module annotated", + node: ast.MustParseModuleWithOpts(`# METADATA # title: p package p # METADATA # title: r r = true`, - ast.ParserOptions{ProcessAnnotation: true}), - want: "# METADATA\n# {\"scope\":\"package\",\"title\":\"p\"}\npackage p\n\n# METADATA\n# {\"scope\":\"rule\",\"title\":\"r\"}\nr = true if { true }", - // We don't count this correctly for annoations currently. - nolen: true, - }, - { - name: "package", - node: &ast.Package{ - Path: ast.MustParseRef("data.example.foo"), + ast.ParserOptions{ProcessAnnotation: true}), + want: "# METADATA\n# {\"scope\":\"package\",\"title\":\"p\"}\npackage p\n\n# METADATA\n# {\"scope\":\"rule\",\"title\":\"r\"}\nr = true if { true }", + // We don't count this correctly for annoations currently. + nolen: true, }, - want: `package example.foo`, - }, - { - name: "package with special chars", - node: &ast.Package{ - Path: ast.Ref{ - ast.DefaultRootDocument, - ast.StringTerm("pkg"), - ast.StringTerm("with-a"), - ast.StringTerm("dash"), + { + name: "package", + node: &ast.Package{ + Path: ast.MustParseRef("data.example.foo"), }, + want: `package example.foo`, }, - want: `package pkg["with-a"].dash`, - }, - { - name: "import", - node: &ast.Import{ - Path: ast.NewTerm(ast.MustParseRef("data.example.foo")), - Alias: ast.Var("bar"), + { + name: "package with special chars", + node: &ast.Package{ + Path: ast.Ref{ + ast.DefaultRootDocument, + ast.StringTerm("pkg"), + ast.StringTerm("with-a"), + ast.StringTerm("dash"), + }, + }, + want: `package pkg["with-a"].dash`, }, - want: `import data.example.foo as bar`, - }, - { - name: "head", - node: &ast.Head{ - Reference: ast.Ref{ast.VarTerm("allow")}, - Value: ast.InternedTerm(true), + { + name: "import", + node: &ast.Import{ + Path: ast.NewTerm(ast.MustParseRef("data.example.foo")), + Alias: ast.Var("bar"), + }, + want: `import data.example.foo as bar`, }, - want: `allow = true`, - }, - { - name: "head assign", - node: &ast.Head{ - Reference: ast.Ref{ast.VarTerm("allow")}, - Value: ast.InternedTerm(false), - Assign: true, + { + name: "head", + node: &ast.Head{ + Reference: ast.Ref{ast.VarTerm("allow")}, + Value: ast.InternedTerm(true), + }, + want: `allow = true`, }, - want: `allow := false`, - }, - { - name: "head with key", - node: &ast.Head{ - Reference: ast.Ref{ast.VarTerm("deny")}, - Key: ast.InternedTerm("reason"), + { + name: "head assign", + node: &ast.Head{ + Reference: ast.Ref{ast.VarTerm("allow")}, + Value: ast.InternedTerm(false), + Assign: true, + }, + want: `allow := false`, }, - want: `deny contains "reason"`, - }, - { - name: "ref head with value", - node: &ast.Head{ - Reference: ast.Ref{ast.VarTerm("authz"), ast.StringTerm("deny"), ast.VarTerm("user")}, - Value: ast.InternedTerm("violation"), - Assign: true, + { + name: "head with key", + node: &ast.Head{ + Reference: ast.Ref{ast.VarTerm("deny")}, + Key: ast.InternedTerm("reason"), + }, + want: `deny contains "reason"`, }, - want: `authz.deny[user] := "violation"`, - }, - { - name: "body", - node: ast.Body{ - ast.MustParseExpr("input.foo == 1"), - ast.MustParseExpr("input.bar != 2"), + { + name: "ref head with value", + node: &ast.Head{ + Reference: ast.Ref{ast.VarTerm("authz"), ast.StringTerm("deny"), ast.VarTerm("user")}, + Value: ast.InternedTerm("violation"), + Assign: true, + }, + want: `authz.deny[user] := "violation"`, }, - want: `equal(input.foo, 1); neq(input.bar, 2)`, - }, - { - name: "expr", - node: ast.MustParseExpr(`input.foo[_][1][baz] == "bar"`), - want: `equal(input.foo[_][1][baz], "bar")`, - }, - { - name: "with", - node: &ast.With{ - Target: ast.MustParseTerm("input.foo"), - Value: ast.MustParseTerm(`"bar"`), + { + name: "body", + node: ast.Body{ + ast.MustParseExpr("input.foo == 1"), + ast.MustParseExpr("input.bar != 2"), + }, + want: `equal(input.foo, 1); neq(input.bar, 2)`, }, - want: `with input.foo as "bar"`, - }, - { - name: "every", - node: &ast.Every{ - Key: ast.MustParseTerm("k"), - Value: ast.MustParseTerm("v"), - Domain: ast.MustParseTerm("input.map"), - Body: ast.Body{ - ast.MustParseExpr("v > 0"), + { + name: "expr", + node: ast.MustParseExpr(`input.foo[_][1][baz] == "bar"`), + want: `equal(input.foo[_][1][baz], "bar")`, + }, + { + name: "with", + node: &ast.With{ + Target: ast.MustParseTerm("input.foo"), + Value: ast.MustParseTerm(`"bar"`), }, + want: `with input.foo as "bar"`, }, - want: `every k, v in input.map { gt(v, 0) }`, - }, - { - name: "some decl", - node: &ast.SomeDecl{ - Symbols: []*ast.Term{ - ast.MustParseTerm("x"), - ast.MustParseTerm("y"), + { + name: "every", + node: &ast.Every{ + Key: ast.MustParseTerm("k"), + Value: ast.MustParseTerm("v"), + Domain: ast.MustParseTerm("input.map"), + Body: ast.Body{ + ast.MustParseExpr("v > 0"), + }, }, + want: `every k, v in input.map { gt(v, 0) }`, }, - want: `some x, y`, - }, - { - name: "some in decl", - node: ast.MustParseExpr("some x, y in input.map"), - want: `some x, y in input.map`, - }, - { - name: "and, implicit", - node: ast.MustParseExprWithOpts("x and y", ast.ParserOptions{ - Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), - AllFutureKeywords: true, - }), - want: `x and y`, - }, - { - name: "and, implicit, expanded", - node: &ast.LogicalAnd{ - Lhs: ast.NewBody( - ast.NewExpr(ast.VarTerm("a")), - ast.NewExpr(ast.VarTerm("x")), - ), - Rhs: ast.NewBody( - ast.NewExpr(ast.VarTerm("b")), - ast.NewExpr(ast.VarTerm("y")), - ), - ExplicitLhs: false, - ExplicitRhs: false, + { + name: "some decl", + node: &ast.SomeDecl{ + Symbols: []*ast.Term{ + ast.MustParseTerm("x"), + ast.MustParseTerm("y"), + }, + }, + want: `some x, y`, }, - want: `{ a; x } and { b; y }`, - }, - { - name: "and, explicit", - node: ast.MustParseExprWithOpts("{ x } and { y }", ast.ParserOptions{ - Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), - AllFutureKeywords: true, - }), - want: `{ x } and { y }`, - }, - { - name: "or, implicit", - node: ast.MustParseExprWithOpts("x or y", ast.ParserOptions{ - Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), - AllFutureKeywords: true, - }), - want: `x or y`, - }, - { - name: "or, implicit, expanded", - node: &ast.LogicalOr{ - Lhs: ast.NewBody( - ast.NewExpr(ast.VarTerm("a")), - ast.NewExpr(ast.VarTerm("x")), - ), - Rhs: ast.NewBody( - ast.NewExpr(ast.VarTerm("b")), - ast.NewExpr(ast.VarTerm("y")), - ), - ExplicitLhs: false, - ExplicitRhs: false, + { + name: "some in decl", + node: ast.MustParseExpr("some x, y in input.map"), + want: `some x, y in input.map`, }, - want: `{ a; x } or { b; y }`, - }, - { - name: "or, explicit", - node: ast.MustParseExprWithOpts("{ x } or { y }", ast.ParserOptions{ - Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), - AllFutureKeywords: true, - }), - want: `{ x } or { y }`, - }, -} + { + name: "and, implicit", + node: ast.MustParseExprWithOpts("x and y", ast.ParserOptions{ + Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), + AllFutureKeywords: true, + }), + want: `x and y`, + }, + { + name: "and, implicit, expanded", + node: &ast.LogicalAnd{ + Lhs: ast.NewBody( + ast.NewExpr(ast.VarTerm("a")), + ast.NewExpr(ast.VarTerm("x")), + ), + Rhs: ast.NewBody( + ast.NewExpr(ast.VarTerm("b")), + ast.NewExpr(ast.VarTerm("y")), + ), + ExplicitLhs: false, + ExplicitRhs: false, + }, + want: `{ a; x } and { b; y }`, + }, + { + name: "and, explicit", + node: ast.MustParseExprWithOpts("{ x } and { y }", ast.ParserOptions{ + Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), + AllFutureKeywords: true, + }), + want: `{ x } and { y }`, + }, + { + name: "or, implicit", + node: ast.MustParseExprWithOpts("x or y", ast.ParserOptions{ + Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), + AllFutureKeywords: true, + }), + want: `x or y`, + }, + { + name: "or, implicit, expanded", + node: &ast.LogicalOr{ + Lhs: ast.NewBody( + ast.NewExpr(ast.VarTerm("a")), + ast.NewExpr(ast.VarTerm("x")), + ), + Rhs: ast.NewBody( + ast.NewExpr(ast.VarTerm("b")), + ast.NewExpr(ast.VarTerm("y")), + ), + ExplicitLhs: false, + ExplicitRhs: false, + }, + want: `{ a; x } or { b; y }`, + }, + { + name: "or, explicit", + node: ast.MustParseExprWithOpts("{ x } or { y }", ast.ParserOptions{ + Capabilities: ast.CapabilitiesForThisVersion(ast.CapabilitiesExperimentalKeywords(true)), + AllFutureKeywords: true, + }), + want: `{ x } or { y }`, + }, + } +}) func TestASTNodeTextAppendersAndLengthAllocation(t *testing.T) { - for _, tc := range policyAppenderTests { + for _, tc := range policyAppenderTests() { t.Run(tc.name, func(t *testing.T) { var buf []byte res, err := tc.node.(encoding.TextAppender).AppendText(buf) @@ -241,7 +246,7 @@ func TestASTNodeTextAppendersAndLengthAllocation(t *testing.T) { } func BenchmarkNoNodeTypeAllocatesOnAppend(b *testing.B) { - for _, tc := range policyAppenderTests { + for _, tc := range policyAppenderTests() { b.Run(tc.name, func(b *testing.B) { buf := make([]byte, 0, tc.node.StringLength()) for b.Loop() { diff --git a/v1/ast/term_appenders_test.go b/v1/ast/term_appenders_test.go index d27c9c3762b..8ff6b559601 100644 --- a/v1/ast/term_appenders_test.go +++ b/v1/ast/term_appenders_test.go @@ -2,165 +2,170 @@ package ast_test import ( "encoding" + "sync" "testing" "github.com/open-policy-agent/opa/v1/ast" ) -var valueAppenderTests = []struct { +type valueAppenderTest struct { name string term ast.StringLengther want string -}{ - { - name: "var", - term: ast.MustParseTerm("input.foo.bar"), - want: "input.foo.bar", - }, - { - name: "string", - term: ast.StringTerm(`foo bar baz`), - want: `"foo bar baz"`, - }, - { - name: "string with escapes", - term: ast.StringTerm(`"foo" "bar" "qux"`), - want: `"\"foo\" \"bar\" \"qux\""`, - }, - { - name: "string with newlines", - term: ast.StringTerm("line1\nline2\nline3"), - want: `"line1\nline2\nline3"`, - }, - { - name: "number", - term: ast.MustParseTerm("3.14"), - want: "3.14", - }, - { - name: "boolean", - term: ast.MustParseTerm("true"), - want: "true", - }, - { - name: "null", - term: ast.MustParseTerm("null"), - want: "null", - }, - { - name: "array", - term: ast.MustParseTerm(`[1, "two", false]`), - want: `[1, "two", false]`, - }, - { - name: "set", - term: ast.MustParseTerm(`{1, 2, 3}`), - want: `{1, 2, 3}`, - }, - { - name: "object", - term: ast.MustParseTerm(`{"a": 1, "b": "two"}`), - want: `{"a": 1, "b": "two"}`, - }, - { - name: "call", - term: ast.MustParseExpr(`foo(input.bar, "baz")`), - want: `foo(input.bar, "baz")`, - }, - { - name: "template string", - term: ast.MustParseTerm(`$"Hello, {input.name}!"`), - want: `$"Hello, {input.name}!"`, - }, - { - name: "ref", - term: ast.MustParseTerm(`data.foo["b a r"].allow`), - want: `data.foo["b a r"].allow`, - }, - { - name: "object comprehension", - term: ast.MustParseTerm(`{k: v | +} + +var valueAppenderTests = sync.OnceValue(func() []valueAppenderTest { + return []valueAppenderTest{ + { + name: "var", + term: ast.MustParseTerm("input.foo.bar"), + want: "input.foo.bar", + }, + { + name: "string", + term: ast.StringTerm(`foo bar baz`), + want: `"foo bar baz"`, + }, + { + name: "string with escapes", + term: ast.StringTerm(`"foo" "bar" "qux"`), + want: `"\"foo\" \"bar\" \"qux\""`, + }, + { + name: "string with newlines", + term: ast.StringTerm("line1\nline2\nline3"), + want: `"line1\nline2\nline3"`, + }, + { + name: "number", + term: ast.MustParseTerm("3.14"), + want: "3.14", + }, + { + name: "boolean", + term: ast.MustParseTerm("true"), + want: "true", + }, + { + name: "null", + term: ast.MustParseTerm("null"), + want: "null", + }, + { + name: "array", + term: ast.MustParseTerm(`[1, "two", false]`), + want: `[1, "two", false]`, + }, + { + name: "set", + term: ast.MustParseTerm(`{1, 2, 3}`), + want: `{1, 2, 3}`, + }, + { + name: "object", + term: ast.MustParseTerm(`{"a": 1, "b": "two"}`), + want: `{"a": 1, "b": "two"}`, + }, + { + name: "call", + term: ast.MustParseExpr(`foo(input.bar, "baz")`), + want: `foo(input.bar, "baz")`, + }, + { + name: "template string", + term: ast.MustParseTerm(`$"Hello, {input.name}!"`), + want: `$"Hello, {input.name}!"`, + }, + { + name: "ref", + term: ast.MustParseTerm(`data.foo["b a r"].allow`), + want: `data.foo["b a r"].allow`, + }, + { + name: "object comprehension", + term: ast.MustParseTerm(`{k: v | some k, v data.items[k] == v v > 10 }`), - want: `{k: v | some k, v; equal(data.items[k], v); gt(v, 10)}`, - }, - { - name: "object comprehension infix value", - term: ast.MustParseTerm(`{k: (v * 100) | some k, v; v > 0}`), - want: `{k: (v * 100) | some k, v; gt(v, 0)}`, - }, - { - name: "object comprehension infix key", - term: ast.MustParseTerm(`{(k + 1): v | some k, v; v > 0}`), - want: `{(k + 1): v | some k, v; gt(v, 0)}`, - }, - { - name: "array comprehension", - term: ast.MustParseTerm(`[(x * 2) | x := data.numbers[_]; x > 5]`), - want: `[(x * 2) | assign(x, data.numbers[_]); gt(x, 5)]`, - }, - { - name: "array comprehension nested infix operators", - term: ast.MustParseTerm(`[((x + 1) * 2) | x := data.numbers[_]]`), - want: `[((x + 1) * 2) | assign(x, data.numbers[_])]`, - }, - { - name: "array comprehension non-infix head", - term: ast.MustParseTerm(`[count(x) | x := data.lists[_]]`), - want: `[count(x) | assign(x, data.lists[_])]`, - }, - { - name: "array comprehension nested infix with function call", - term: ast.MustParseTerm(`[(to_number(cpu_str[i])/1000) | cpu_str[i]]`), - want: `[(to_number(cpu_str[i]) / 1000) | cpu_str[i]]`, - }, - { - name: "set comprehension", - term: ast.MustParseTerm(`{x | x := data.values[_]; x < 100}`), - want: `{x | assign(x, data.values[_]); lt(x, 100)}`, - }, - { - name: "not", - term: &ast.Not{ - Body: ast.NewBody(ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1")))), - }, - want: `not lt(100, 1)`, - }, - { - name: "not, explicit body, one-line", - term: &ast.Not{ - ExplicitBody: true, - Body: ast.NewBody(ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1")))), - }, - want: `not {lt(100, 1)}`, - }, - { - name: "not, implicit body, multi-line", - term: &ast.Not{ - Body: ast.NewBody( - ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1"))), - ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("99"), ast.NumberTerm("1"))), - ), - }, - want: `not {lt(100, 1); lt(99, 1)}`, - }, - { - name: "not, explicit body, multi-line", - term: &ast.Not{ - ExplicitBody: true, - Body: ast.NewBody( - ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1"))), - ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("99"), ast.NumberTerm("1"))), - ), - }, - want: `not {lt(100, 1); lt(99, 1)}`, - }, -} + want: `{k: v | some k, v; equal(data.items[k], v); gt(v, 10)}`, + }, + { + name: "object comprehension infix value", + term: ast.MustParseTerm(`{k: (v * 100) | some k, v; v > 0}`), + want: `{k: (v * 100) | some k, v; gt(v, 0)}`, + }, + { + name: "object comprehension infix key", + term: ast.MustParseTerm(`{(k + 1): v | some k, v; v > 0}`), + want: `{(k + 1): v | some k, v; gt(v, 0)}`, + }, + { + name: "array comprehension", + term: ast.MustParseTerm(`[(x * 2) | x := data.numbers[_]; x > 5]`), + want: `[(x * 2) | assign(x, data.numbers[_]); gt(x, 5)]`, + }, + { + name: "array comprehension nested infix operators", + term: ast.MustParseTerm(`[((x + 1) * 2) | x := data.numbers[_]]`), + want: `[((x + 1) * 2) | assign(x, data.numbers[_])]`, + }, + { + name: "array comprehension non-infix head", + term: ast.MustParseTerm(`[count(x) | x := data.lists[_]]`), + want: `[count(x) | assign(x, data.lists[_])]`, + }, + { + name: "array comprehension nested infix with function call", + term: ast.MustParseTerm(`[(to_number(cpu_str[i])/1000) | cpu_str[i]]`), + want: `[(to_number(cpu_str[i]) / 1000) | cpu_str[i]]`, + }, + { + name: "set comprehension", + term: ast.MustParseTerm(`{x | x := data.values[_]; x < 100}`), + want: `{x | assign(x, data.values[_]); lt(x, 100)}`, + }, + { + name: "not", + term: &ast.Not{ + Body: ast.NewBody(ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1")))), + }, + want: `not lt(100, 1)`, + }, + { + name: "not, explicit body, one-line", + term: &ast.Not{ + ExplicitBody: true, + Body: ast.NewBody(ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1")))), + }, + want: `not {lt(100, 1)}`, + }, + { + name: "not, implicit body, multi-line", + term: &ast.Not{ + Body: ast.NewBody( + ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1"))), + ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("99"), ast.NumberTerm("1"))), + ), + }, + want: `not {lt(100, 1); lt(99, 1)}`, + }, + { + name: "not, explicit body, multi-line", + term: &ast.Not{ + ExplicitBody: true, + Body: ast.NewBody( + ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("100"), ast.NumberTerm("1"))), + ast.NewExpr(ast.LessThan.Call(ast.NumberTerm("99"), ast.NumberTerm("1"))), + ), + }, + want: `not {lt(100, 1); lt(99, 1)}`, + }, + } +}) func TestASTValueTextAppendersAndStringLength(t *testing.T) { - for _, tc := range valueAppenderTests { + for _, tc := range valueAppenderTests() { t.Run(tc.name, func(t *testing.T) { var buf []byte var err error @@ -184,7 +189,7 @@ func TestASTValueTextAppendersAndStringLength(t *testing.T) { // Ensure no appender allocates when appending to a pre-sized buffer. func BenchmarkNoASTTypeAllocatesOnAppendToBufferOfStringLength(b *testing.B) { - for _, tc := range valueAppenderTests { + for _, tc := range valueAppenderTests() { b.Run(tc.name, func(b *testing.B) { buf := make([]byte, 0, tc.term.StringLength()) for b.Loop() { diff --git a/v1/rego/rego_metadata_test.go b/v1/rego/rego_metadata_test.go index 82d3c7ec256..e8261fe0280 100644 --- a/v1/rego/rego_metadata_test.go +++ b/v1/rego/rego_metadata_test.go @@ -5,6 +5,7 @@ package rego import ( + "os" "testing" "github.com/open-policy-agent/opa/v1/ast" @@ -12,7 +13,7 @@ import ( "github.com/open-policy-agent/opa/v1/types" ) -func init() { +func TestMain(m *testing.M) { ast.RegisterBuiltin(&ast.Builtin{ Name: "test.transform_metadata", Decl: types.NewFunction(nil, types.B), @@ -30,6 +31,8 @@ func init() { return iter(ast.BooleanTerm(true)) }, ) + + os.Exit(m.Run()) } func TestEvalMetadataTransformViaBuiltin(t *testing.T) { diff --git a/v1/repl/repl.go b/v1/repl/repl.go index 5b98cd1e6de..5fbc00fc575 100644 --- a/v1/repl/repl.go +++ b/v1/repl/repl.go @@ -612,7 +612,7 @@ func (r *REPL) cmdShow(args []string) error { if err != nil { return err } - fmt.Fprint(r.output, string(bs)) + r.output.Write(bs) return nil } else if args[0] == "debug" { debug := replDebugState{ diff --git a/v1/server/server_test.go b/v1/server/server_test.go index aa3ad0fa3e1..68f22ec733a 100644 --- a/v1/server/server_test.go +++ b/v1/server/server_test.go @@ -68,7 +68,7 @@ import ( prom "github.com/prometheus/client_golang/prometheus" ) -func init() { +func TestMain(m *testing.M) { ast.RegisterBuiltin(&ast.Builtin{ Name: "test.set_outgoing", Decl: astTypes.NewFunction(nil, astTypes.B), @@ -83,6 +83,8 @@ func init() { return iter(ast.BooleanTerm(true)) }, ) + + os.Exit(m.Run()) } type tr struct { diff --git a/v1/topdown/http.go b/v1/topdown/http.go index c402f85854c..62fd92c3513 100644 --- a/v1/topdown/http.go +++ b/v1/topdown/http.go @@ -104,7 +104,6 @@ var ( http.StatusRequestURITooLong, http.StatusNotImplemented, } - httpSendNetworkErrTerm, httpSendInternalErrTerm *ast.Term allowedKeys = ast.NewSet() cacheableCodes = ast.NewSet() @@ -204,12 +203,12 @@ func generateRaiseErrorResult(err error) *ast.Term { switch err.(type) { case *url.Error: errObj = ast.NewObject( - ast.Item(ast.InternedTerm("code"), httpSendNetworkErrTerm), + ast.Item(ast.InternedTerm("code"), ast.InternedTerm(HTTPSendNetworkErr)), ast.Item(ast.InternedTerm("message"), ast.StringTerm(err.Error())), ) default: errObj = ast.NewObject( - ast.Item(ast.InternedTerm("code"), httpSendInternalErrTerm), + ast.Item(ast.InternedTerm("code"), ast.InternedTerm(HTTPSendInternalErr)), ast.Item(ast.InternedTerm("message"), ast.StringTerm(err.Error())), ) } @@ -292,15 +291,12 @@ func getKeyFromRequest(req ast.Object) (ast.Object, error) { } func init() { + ast.InternStringTerm(HTTPSendNetworkErr, HTTPSendInternalErr) + ast.InternStringTerm(allowedKeyNames[:]...) for _, element := range allowedKeyNames { - ast.InternStringTerm(element) allowedKeys.Add(ast.InternedTerm(element)) } - ast.InternStringTerm(HTTPSendNetworkErr, HTTPSendInternalErr) - httpSendNetworkErrTerm = ast.InternedTerm(HTTPSendNetworkErr) - httpSendInternalErrTerm = ast.InternedTerm(HTTPSendInternalErr) - createCacheableHTTPStatusCodes() initDefaults() RegisterBuiltinFunc(ast.HTTPSend.Name, builtinHTTPSend) diff --git a/v1/topdown/json.go b/v1/topdown/json.go index aeccac825fc..bb8cc4925f8 100644 --- a/v1/topdown/json.go +++ b/v1/topdown/json.go @@ -378,9 +378,7 @@ func builtinJSONPatch(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Ter } func init() { - for _, key := range []string{"op", "path", "from", "value", "add", "remove", "replace", "move", "copy", "test"} { - ast.InternStringTerm(key) - } + ast.InternStringTerm("op", "path", "from", "value", "add", "remove", "replace", "move", "copy", "test") RegisterBuiltinFunc(ast.JSONFilter.Name, builtinJSONFilter) RegisterBuiltinFunc(ast.JSONRemove.Name, builtinJSONRemove) diff --git a/v1/topdown/providers.go b/v1/topdown/providers.go index 29d721e4b2d..511797ac8ee 100644 --- a/v1/topdown/providers.go +++ b/v1/topdown/providers.go @@ -203,11 +203,7 @@ func builtinAWSSigV4SignReq(_ BuiltinContext, operands []*ast.Term, iter func(*a } func init() { - for _, key := range []string{ - "aws_service", "aws_access_key", "aws_secret_access_key", "aws_region", "disable_payload_signing", - } { - ast.InternStringTerm(key) - } + ast.InternStringTerm("aws_service", "aws_access_key", "aws_secret_access_key", "aws_region", "disable_payload_signing") awsRequiredConfigKeyNames = ast.NewSet( ast.InternedTerm("aws_service"), diff --git a/v1/topdown/regex_template.go b/v1/topdown/regex_template.go index a1d946fd59e..0c1f698bfe8 100644 --- a/v1/topdown/regex_template.go +++ b/v1/topdown/regex_template.go @@ -82,10 +82,7 @@ func compileRegexTemplate(tpl string, delimiterStart, delimiterEnd byte) (*regex return nil, errBraces } varsR := make([]*regexp.Regexp, len(idxs)/2) - pattern := bytes.NewBufferString("") - - // WriteByte's error value is always nil for bytes.Buffer, no need to check it. - pattern.WriteByte('^') + pattern := bytes.NewBufferString("^") var end int var err error @@ -113,10 +110,5 @@ func compileRegexTemplate(tpl string, delimiterStart, delimiterEnd byte) (*regex pattern.WriteByte('$') // Compile full regexp. - reg, errCompile := regexp.Compile(pattern.String()) - if errCompile != nil { - return nil, errCompile - } - - return reg, nil + return regexp.Compile(pattern.String()) } diff --git a/v1/topdown/time.go b/v1/topdown/time.go index 7171d5c0b20..294adfa1daa 100644 --- a/v1/topdown/time.go +++ b/v1/topdown/time.go @@ -21,20 +21,22 @@ import ( "github.com/open-policy-agent/opa/v1/topdown/durationparser" ) -var tzCache map[string]*time.Location -var tzCacheMutex *sync.Mutex +var ( + tzCache = make(map[string]*time.Location) + tzCacheMutex = &sync.Mutex{} -// 1677-09-21T00:12:43.145224192-00:00 -var minDateAllowedForNsConversion = time.Unix(0, math.MinInt64) + // 1677-09-21T00:12:43.145224192-00:00 + minDateAllowedForNsConversion = time.Unix(0, math.MinInt64) -// 2262-04-11T23:47:16.854775807-00:00 -var maxDateAllowedForNsConversion = time.Unix(0, math.MaxInt64) + // 2262-04-11T23:47:16.854775807-00:00 + maxDateAllowedForNsConversion = time.Unix(0, math.MaxInt64) -var durationCoefficients = map[string]int{ - "d": 24, - "w": 7 * 24, - "y": 365 * 24, -} + durationCoefficients = map[string]int{ + "d": 24, + "w": 7 * 24, + "y": 365 * 24, + } +) // parseExtendedDuration parses a duration string that may contain extended // units (d, w, y) mixed with standard Go duration units (h, m, s, ms, us, ns). @@ -420,6 +422,4 @@ func init() { RegisterBuiltinFunc(ast.Weekday.Name, builtinWeekday) RegisterBuiltinFunc(ast.AddDate.Name, builtinAddDate) RegisterBuiltinFunc(ast.Diff.Name, builtinDiff) - tzCacheMutex = &sync.Mutex{} - tzCache = make(map[string]*time.Location) } diff --git a/v1/topdown/topdown_test.go b/v1/topdown/topdown_test.go index 01bc5bd624d..321a60409c7 100644 --- a/v1/topdown/topdown_test.go +++ b/v1/topdown/topdown_test.go @@ -32,6 +32,24 @@ import ( "github.com/open-policy-agent/opa/v1/util" ) +func TestMain(m *testing.M) { + ast.RegisterBuiltin(&ast.Builtin{ + Name: "test.sleep", + Decl: types.NewFunction( + types.Args(types.S), + types.Nl, + ), + }) + + RegisterBuiltinFunc("test.sleep", func(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) error) error { + d, _ := time.ParseDuration(string(operands[0].Value.(ast.String))) + time.Sleep(d) + return iter(ast.NullTerm()) + }) + + os.Exit(m.Run()) +} + func TestTopDownQueryIDsUnique(t *testing.T) { t.Parallel() diff --git a/v1/topdown/trace.go b/v1/topdown/trace.go index ba2c8dde55d..d6742acb4e2 100644 --- a/v1/topdown/trace.go +++ b/v1/topdown/trace.go @@ -840,7 +840,7 @@ func PrettyEvent(w io.Writer, e *Event, opts PrettyEventOpts) error { } printPrettyVars(buf, exprVars) - _, _ = fmt.Fprint(w, buf.String()) + w.Write(buf.Bytes()) return nil } @@ -924,21 +924,20 @@ func printArrows(w *bytes.Buffer, l []varInfo, printValueAt int) { } for j := range spaces { - tab := false + var space byte = ' ' if slices.Contains(info.exprLoc.Tabs, j+prevCol+1) { - w.WriteByte('\t') - tab = true - } - if !tab { - w.WriteByte(' ') + space = '\t' } + w.WriteByte(space) } if isLast && printValueAt >= 0 { valueStr := iStrs.Truncate(info.Value(), maxPrettyExprVarWidth) if (i > 0 && col == l[i-1].col) || (i < len(l)-1 && col == l[i+1].col) { // There is another var on this column, so we need to include the name to differentiate them. - fmt.Fprintf(w, "%s: %s", info.Title(), valueStr) + w.WriteString(info.Title()) + w.WriteString(": ") + w.WriteString(valueStr) } else { w.WriteString(valueStr) }