diff --git a/.golangci.yaml b/.golangci.yaml index 7d31264c51..dd01c2f6f0 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -21,6 +21,10 @@ linters: - usetesting # - gosec # too many false positives settings: + errcheck: + exclude-functions: + - github.com/open-policy-agent/opa/v1/util.WriteAppender + - github.com/open-policy-agent/opa/v1/util.WriteInt gocritic: enabled-checks: # NOTE that these are rules enabled in addition to the default set diff --git a/cmd/exec_test.go b/cmd/exec_test.go index 53fa30e901..974bf8219d 100644 --- a/cmd/exec_test.go +++ b/cmd/exec_test.go @@ -852,9 +852,7 @@ main contains "hello" if { // runExec to hang indefinitely. WithContext allows us to cancel the context // when we have the required errors logged. var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { err := runExecWithContext(ctx, params) // we cancelled the context, so we expect that error if err != nil && err.Error() != "context canceled" { @@ -868,7 +866,7 @@ main contains "hello" if { t.Error(err) return } - }() + }) test.EventuallyOrFatal(t, 5*time.Second, func() bool { for _, expErr := range tc.expErrs { diff --git a/cmd/test.go b/cmd/test.go index 8e59705a43..12840d6a23 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "maps" "os" "os/signal" goRuntime "runtime" @@ -125,9 +126,7 @@ func opaTest(args []string, testParams testCommandParams) int { if err == nil && testParams.coverage { modules = make(map[string]*ast.Module) for name, b := range bundles { - for k, v := range b.ParsedModules(name) { - modules[k] = v - } + maps.Copy(modules, b.ParsedModules(name)) } } } else { diff --git a/internal/cmd/genmanifestschema/main.go b/internal/cmd/genmanifestschema/main.go index 074a656d1b..a57eb8a66f 100644 --- a/internal/cmd/genmanifestschema/main.go +++ b/internal/cmd/genmanifestschema/main.go @@ -37,8 +37,8 @@ func reflectSchema() ([]byte, error) { // DisallowUnknownFields), and embedders rely on this to attach custom // configuration alongside the documented fields. Keep the schema in // step with that contract; sub-records like WasmResolver stay strict. - b.AllowAdditionalProperties(reflect.TypeOf(bundle.Manifest{})) - rootRef, err := b.AddStruct(reflect.TypeOf(bundle.Manifest{})) + b.AllowAdditionalProperties(reflect.TypeFor[bundle.Manifest]()) + rootRef, err := b.AddStruct(reflect.TypeFor[bundle.Manifest]()) if err != nil { return nil, err } diff --git a/internal/cmd/genplanschema/main.go b/internal/cmd/genplanschema/main.go index 0d2fd5cd45..410b6c6f31 100644 --- a/internal/cmd/genplanschema/main.go +++ b/internal/cmd/genplanschema/main.go @@ -42,7 +42,7 @@ func reflectSchema() ([]byte, error) { return nil, err } - rootRef, err := b.AddStruct(reflect.TypeOf(ir.Policy{})) + rootRef, err := b.AddStruct(reflect.TypeFor[ir.Policy]()) if err != nil { return nil, err } @@ -74,13 +74,13 @@ func planResolver(b *genjsonschema.Builder, t reflect.Type) (any, bool, error) { switch t.Kind() { case reflect.Struct: switch { - case t == reflect.TypeOf(ir.Operand{}): + case t == reflect.TypeFor[ir.Operand](): ref, err := addOperand(b) if err != nil { return nil, false, err } return genjsonschema.Map("$ref", ref), true, nil - case t == reflect.TypeOf(ir.Block{}): + case t == reflect.TypeFor[ir.Block](): ref, err := addBlock(b) if err != nil { return nil, false, err @@ -96,13 +96,13 @@ func planResolver(b *genjsonschema.Builder, t reflect.Type) (any, bool, error) { } case reflect.Interface: switch { - case t == reflect.TypeOf((*ir.Stmt)(nil)).Elem(): + case t == reflect.TypeFor[ir.Stmt](): ref, err := addStmtUnion(b) if err != nil { return nil, false, err } return genjsonschema.Map("$ref", ref), true, nil - case t == reflect.TypeOf((*ir.Val)(nil)).Elem(): + case t == reflect.TypeFor[ir.Val](): ref, err := addValUnion(b) if err != nil { return nil, false, err diff --git a/internal/genjsonschema/genjsonschema.go b/internal/genjsonschema/genjsonschema.go index 973358a574..5ee1932aac 100644 --- a/internal/genjsonschema/genjsonschema.go +++ b/internal/genjsonschema/genjsonschema.go @@ -15,6 +15,7 @@ import ( "errors" "fmt" "reflect" + "slices" "sort" "strings" ) @@ -327,10 +328,8 @@ func MakeNullable(schema any) any { out[i] = Entry{"type", []string{v, "null"}} return out case []string: - for _, s := range v { - if s == "null" { - return m - } + if slices.Contains(v, "null") { + return m } out := cloneOrderedMap(m) widened := make([]string, len(v)+1) diff --git a/internal/genjsonschema/genjsonschema_test.go b/internal/genjsonschema/genjsonschema_test.go index 4a300fd053..34de23c58b 100644 --- a/internal/genjsonschema/genjsonschema_test.go +++ b/internal/genjsonschema/genjsonschema_test.go @@ -175,7 +175,7 @@ type primitives struct { func TestReflectStructPrimitives(t *testing.T) { b := NewBuilder(nil) - if _, err := b.AddStruct(reflect.TypeOf(primitives{})); err != nil { + if _, err := b.AddStruct(reflect.TypeFor[primitives]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -196,7 +196,7 @@ type omitFields struct { func TestOmitEmptyAndNullability(t *testing.T) { b := NewBuilder(nil) - if _, err := b.AddStruct(reflect.TypeOf(omitFields{})); err != nil { + if _, err := b.AddStruct(reflect.TypeFor[omitFields]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -216,7 +216,7 @@ type withMaps struct { func TestMapHandling(t *testing.T) { b := NewBuilder(nil) - if _, err := b.AddStruct(reflect.TypeOf(withMaps{})); err != nil { + if _, err := b.AddStruct(reflect.TypeFor[withMaps]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -238,7 +238,7 @@ type outer struct { func TestNestedStructsAndPointer(t *testing.T) { b := NewBuilder(nil) - if _, err := b.AddStruct(reflect.TypeOf(outer{})); err != nil { + if _, err := b.AddStruct(reflect.TypeFor[outer]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -260,7 +260,7 @@ type embedder struct { func TestEmbeddedStructFieldsArePromoted(t *testing.T) { b := NewBuilder(nil) - if _, err := b.AddStruct(reflect.TypeOf(embedder{})); err != nil { + if _, err := b.AddStruct(reflect.TypeFor[embedder]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -281,13 +281,13 @@ type withInterface struct { func TestResolverInterceptsTypes(t *testing.T) { resolver := func(_ *Builder, t reflect.Type) (any, bool, error) { - if t == reflect.TypeOf((*marker)(nil)).Elem() { + if t == reflect.TypeFor[marker]() { return OrderedMap{{"description", "opaque marker"}}, true, nil } return nil, false, nil } b := NewBuilder(resolver) - if _, err := b.AddStruct(reflect.TypeOf(withInterface{})); err != nil { + if _, err := b.AddStruct(reflect.TypeFor[withInterface]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -305,13 +305,13 @@ func TestResolverResultIsNullableWhenFieldCanBeNull(t *testing.T) { // resolver's bare schema gets wrapped in a nullable form so the // generated schema admits null in addition to the resolved shape. resolver := func(_ *Builder, t reflect.Type) (any, bool, error) { - if t == reflect.TypeOf((*marker)(nil)).Elem() { + if t == reflect.TypeFor[marker]() { return OrderedMap{{"description", "opaque marker"}}, true, nil } return nil, false, nil } b := NewBuilder(resolver) - if _, err := b.AddStruct(reflect.TypeOf(withRequiredInterface{})); err != nil { + if _, err := b.AddStruct(reflect.TypeFor[withRequiredInterface]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -323,7 +323,7 @@ func TestResolverResultIsNullableWhenFieldCanBeNull(t *testing.T) { func TestInterfaceWithoutResolverErrors(t *testing.T) { b := NewBuilder(nil) - _, err := b.AddStruct(reflect.TypeOf(withInterface{})) + _, err := b.AddStruct(reflect.TypeFor[withInterface]()) if err == nil { t.Fatal("expected error for unresolved non-empty interface") } @@ -334,13 +334,13 @@ func TestResolverHandledWithNilSchemaErrors(t *testing.T) { // otherwise the field would marshal as JSON null, silently corrupting // output. resolver := func(_ *Builder, t reflect.Type) (any, bool, error) { - if t == reflect.TypeOf((*marker)(nil)).Elem() { + if t == reflect.TypeFor[marker]() { return nil, true, nil } return nil, false, nil } b := NewBuilder(resolver) - _, err := b.AddStruct(reflect.TypeOf(withInterface{})) + _, err := b.AddStruct(reflect.TypeFor[withInterface]()) if err == nil || !strings.Contains(err.Error(), "nil schema") { t.Fatalf("expected nil-schema error, got: %v", err) } @@ -386,7 +386,7 @@ type unsupportedKind struct { func TestUnsupportedKindReportsFieldPath(t *testing.T) { b := NewBuilder(nil) - _, err := b.AddStruct(reflect.TypeOf(unsupportedKind{})) + _, err := b.AddStruct(reflect.TypeFor[unsupportedKind]()) if err == nil || !strings.Contains(err.Error(), "unsupportedKind.Ch") { t.Fatalf("expected error mentioning field path, got: %v", err) } @@ -398,7 +398,7 @@ type intKeyMap struct { func TestNonStringMapKeyErrors(t *testing.T) { b := NewBuilder(nil) - _, err := b.AddStruct(reflect.TypeOf(intKeyMap{})) + _, err := b.AddStruct(reflect.TypeFor[intKeyMap]()) if err == nil || !strings.Contains(err.Error(), "map key") { t.Fatalf("expected map-key error, got: %v", err) } @@ -406,8 +406,8 @@ func TestNonStringMapKeyErrors(t *testing.T) { func TestAllowAdditionalPropertiesSkipsClosedClause(t *testing.T) { b := NewBuilder(nil) - b.AllowAdditionalProperties(reflect.TypeOf(primitives{})) - if _, err := b.AddStruct(reflect.TypeOf(primitives{})); err != nil { + b.AllowAdditionalProperties(reflect.TypeFor[primitives]()) + if _, err := b.AddStruct(reflect.TypeFor[primitives]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) @@ -416,8 +416,8 @@ func TestAllowAdditionalPropertiesSkipsClosedClause(t *testing.T) { } // Sanity check: opting in by pointer type also works. b2 := NewBuilder(nil) - b2.AllowAdditionalProperties(reflect.TypeOf(&primitives{})) - if _, err := b2.AddStruct(reflect.TypeOf(primitives{})); err != nil { + b2.AllowAdditionalProperties(reflect.TypeFor[*primitives]()) + if _, err := b2.AddStruct(reflect.TypeFor[primitives]()); err != nil { t.Fatalf("AddStruct: %v", err) } got2 := mustMarshal(t, b2.DefsOrdered()) @@ -431,8 +431,8 @@ func TestAllowAdditionalPropertiesIsPerType(t *testing.T) { // `inner` def — additionalProperties:false is the right default for // sub-records. b := NewBuilder(nil) - b.AllowAdditionalProperties(reflect.TypeOf(outer{})) - if _, err := b.AddStruct(reflect.TypeOf(outer{})); err != nil { + b.AllowAdditionalProperties(reflect.TypeFor[outer]()) + if _, err := b.AddStruct(reflect.TypeFor[outer]()); err != nil { t.Fatalf("AddStruct: %v", err) } got := mustMarshal(t, b.DefsOrdered()) diff --git a/internal/gojsonschema/schemaLoader.go b/internal/gojsonschema/schemaLoader.go index a27113d83f..c3ef0f86bb 100644 --- a/internal/gojsonschema/schemaLoader.go +++ b/internal/gojsonschema/schemaLoader.go @@ -32,11 +32,8 @@ type SchemaLoader struct { // NewSchemaLoader creates a new NewSchemaLoader func NewSchemaLoader() *SchemaLoader { - ps := &SchemaLoader{ - pool: &schemaPool{ - schemaPoolDocuments: make(map[string]*schemaPoolDocument), - }, + pool: &schemaPool{schemaPoolDocuments: make(map[string]*schemaPoolDocument)}, AutoDetect: true, Validate: false, Draft: Hybrid, @@ -46,15 +43,10 @@ func NewSchemaLoader() *SchemaLoader { return ps } -func (sl *SchemaLoader) validateMetaschema(documentNode any) error { - - var ( - schema string - err error - ) +func (sl *SchemaLoader) validateMetaschema(documentNode any) (err error) { + var schema string if sl.AutoDetect { - schema, _, err = parseSchemaURL(documentNode) - if err != nil { + if schema, _, err = parseSchemaURL(documentNode); err != nil { return err } } @@ -71,7 +63,6 @@ func (sl *SchemaLoader) validateMetaschema(documentNode any) error { sl.Validate = false metaSchema, err := sl.Compile(NewReferenceLoader(schema)) - if err != nil { return err } @@ -84,7 +75,7 @@ func (sl *SchemaLoader) validateMetaschema(documentNode any) error { var res bytes.Buffer for _, err := range result.Errors() { res.WriteString(err.String()) - res.WriteString("\n") + res.WriteByte('\n') } return errors.New(res.String()) } @@ -99,7 +90,6 @@ func (sl *SchemaLoader) AddSchemas(loaders ...JSONLoader) error { for _, loader := range loaders { doc, err := loader.LoadJSON() - if err != nil { return err } @@ -122,15 +112,12 @@ func (sl *SchemaLoader) AddSchemas(loaders ...JSONLoader) error { // AddSchema adds a schema under the provided URL to the schema cache func (sl *SchemaLoader) AddSchema(url string, loader JSONLoader) error { - ref, err := gojsonreference.NewJsonReference(url) - if err != nil { return err } doc, err := loader.LoadJSON() - if err != nil { return err } @@ -146,9 +133,7 @@ func (sl *SchemaLoader) AddSchema(url string, loader JSONLoader) error { // Compile loads and compiles a schema func (sl *SchemaLoader) Compile(rootSchema JSONLoader) (*Schema, error) { - ref, err := rootSchema.JSONReference() - if err != nil { return nil, err } @@ -170,14 +155,12 @@ func (sl *SchemaLoader) Compile(rootSchema JSONLoader) (*Schema, error) { doc = spd.Document } else { // Load JSON directly - doc, err = rootSchema.LoadJSON() - if err != nil { + if doc, err = rootSchema.LoadJSON(); err != nil { return nil, err } // References need only be parsed if loading JSON directly - // as pool.GetDocument already does this for us if loading by reference - err = sl.pool.parseReferences(doc, ref, true) - if err != nil { + // as pool.GetDocument already does this for us if loading by reference + if err = sl.pool.parseReferences(doc, ref, true); err != nil { return nil, err } } @@ -199,8 +182,7 @@ func (sl *SchemaLoader) Compile(rootSchema JSONLoader) (*Schema, error) { } } - err = d.parse(doc, draft) - if err != nil { + if err = d.parse(doc, draft); err != nil { return nil, err } diff --git a/internal/logging/logging.go b/internal/logging/logging.go index fd09205be3..18492467d8 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -98,7 +98,7 @@ func (*prettyFormatter) Format(e *logrus.Entry) ([]byte, error) { b.WriteString(" = ") } b.WriteString(stringVal) - b.WriteString("\n") + b.WriteByte('\n') } b.WriteByte('\n') return b.Bytes(), nil diff --git a/internal/methodlesstemplate/funcs.go b/internal/methodlesstemplate/funcs.go index 4d733135fe..a313a97c85 100644 --- a/internal/methodlesstemplate/funcs.go +++ b/internal/methodlesstemplate/funcs.go @@ -100,14 +100,6 @@ func addValueFuncs(out map[string]reflect.Value, in FuncMap) { } } -// addFuncs adds to values the functions in funcs. It does no checking of the input - -// call addValueFuncs first. -func addFuncs(out, in FuncMap) { - for name, fn := range in { - out[name] = fn - } -} - // goodFunc reports whether the function or method has the right result signature. func goodFunc(name string, typ reflect.Type) error { // We allow functions with 1 result or 2 results where the second is an error. diff --git a/internal/methodlesstemplate/template.go b/internal/methodlesstemplate/template.go index 9ae5a6ca5b..5ed6225fcd 100644 --- a/internal/methodlesstemplate/template.go +++ b/internal/methodlesstemplate/template.go @@ -175,9 +175,9 @@ func (t *Template) Delims(left, right string) *Template { func (t *Template) Funcs(funcMap FuncMap) *Template { t.init() t.muFuncs.Lock() - defer t.muFuncs.Unlock() addValueFuncs(t.execFuncs, funcMap) - addFuncs(t.parseFuncs, funcMap) + maps.Copy(t.parseFuncs, funcMap) + t.muFuncs.Unlock() return t } diff --git a/internal/providers/aws/v4/host.go b/internal/providers/aws/v4/host.go index bf93659a43..19c689da45 100644 --- a/internal/providers/aws/v4/host.go +++ b/internal/providers/aws/v4/host.go @@ -28,48 +28,37 @@ func getHost(r *http.Request) string { // If Host is an IPv6 literal with a port number, Hostname returns the // IPv6 literal without the square brackets. IPv6 literals may include // a zone identifier. -// -// Copied from the Go 1.8 standard library (net/url) func stripPort(hostport string) string { - colon := strings.IndexByte(hostport, ':') - if colon == -1 { + before, _, ok := strings.Cut(hostport, ":") + if !ok { return hostport } - if i := strings.IndexByte(hostport, ']'); i != -1 { - return strings.TrimPrefix(hostport[:i], "[") + if before, _, ok := strings.Cut(hostport, "]"); ok { + return strings.TrimPrefix(before, "[") } - return hostport[:colon] + return before } // Port returns the port part of u.Host, without the leading colon. // If u.Host doesn't contain a port, Port returns an empty string. -// -// Copied from the Go 1.8 standard library (net/url) func portOnly(hostport string) string { - colon := strings.IndexByte(hostport, ':') - if colon == -1 { + _, after, ok := strings.Cut(hostport, ":") + if !ok { return "" } - if i := strings.Index(hostport, "]:"); i != -1 { - return hostport[i+len("]:"):] + if _, after, ok := strings.Cut(hostport, "]:"); ok { + return after } if strings.Contains(hostport, "]") { return "" } - return hostport[colon+len(":"):] + return after } // Returns true if the specified URI is using the standard port // (i.e. port 80 for HTTP URIs or 443 for HTTPS URIs) func isDefaultPort(scheme, port string) bool { - if port == "" { - return true - } - - lowerCaseScheme := strings.ToLower(scheme) - if (lowerCaseScheme == "http" && port == "80") || (lowerCaseScheme == "https" && port == "443") { - return true - } - - return false + return port == "" || + (strings.EqualFold(scheme, "http") && port == "80") || + (strings.EqualFold(scheme, "https") && port == "443") } diff --git a/internal/version/version.go b/internal/version/version.go index 2655c9d716..8d1237bce3 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -7,7 +7,6 @@ package version import ( "context" - "fmt" "runtime" "github.com/open-policy-agent/opa/v1/storage" @@ -19,7 +18,6 @@ var versionPath = storage.MustParsePath("/system/version") // Write the build version information into storage. This makes the // version information available to the REPL and the HTTP server. func Write(ctx context.Context, store storage.Store, txn storage.Transaction) error { - if err := storage.MakeDir(ctx, store, txn, versionPath); err != nil { return err } @@ -33,4 +31,4 @@ func Write(ctx context.Context, store storage.Store, txn storage.Transaction) er } // UserAgent defines the current OPA instances User-Agent default header value. -var UserAgent = fmt.Sprintf("Open-Policy-Agent/%s (%s, %s)", version.Version, runtime.GOOS, runtime.GOARCH) +var UserAgent = "Open-Policy-Agent/" + version.Version + " (" + runtime.GOOS + ", " + runtime.GOARCH + ")" diff --git a/internal/wasm/sdk/internal/wasm/bindings.go b/internal/wasm/sdk/internal/wasm/bindings.go index 390acafd44..05e7dfadb7 100644 --- a/internal/wasm/sdk/internal/wasm/bindings.go +++ b/internal/wasm/sdk/internal/wasm/bindings.go @@ -135,20 +135,14 @@ func (d *builtinDispatcher) opaAbort(_ context.Context, addr int32) { } func (d *builtinDispatcher) opaPrintln(_ context.Context, addr int32) { - uaddr := uint32(addr) - size := d.mem.Size() - if uaddr >= size { - return + uaddr, size := uint32(addr), d.mem.Size() + if uaddr < size { + if data, ok := d.mem.Read(uaddr, size-uaddr); ok { + if before, _, ok := bytes.Cut(data, []byte{0}); ok { + os.Stderr.Write(append(before, '\n')) + } + } } - data, ok := d.mem.Read(uaddr, size-uaddr) - if !ok { - return - } - n := bytes.IndexByte(data, 0) - if n < 0 { - return - } - fmt.Fprintln(os.Stderr, string(data[:n])) } func (d *builtinDispatcher) opaBuiltin0(ctx context.Context, id, _ int32) int32 { @@ -229,11 +223,11 @@ func (d *builtinDispatcher) fromWasmValue(ctx context.Context, addr int32) (*ast if !ok { return nil, errors.New("invalid serialized value address") } - n := bytes.IndexByte(data, 0) - if n < 0 { + before, _, ok := bytes.Cut(data, []byte{0}) + if !ok { return nil, errors.New("unterminated serialized value") } - return ast.ParseTerm(string(data[:n])) + return ast.ParseTerm(string(before)) } // toWasmValue serialises term, writes the bytes into Wasm memory via diff --git a/internal/wasm/sdk/internal/wasm/vm.go b/internal/wasm/sdk/internal/wasm/vm.go index c77bf61829..2cd957afe3 100644 --- a/internal/wasm/sdk/internal/wasm/vm.go +++ b/internal/wasm/sdk/internal/wasm/vm.go @@ -305,10 +305,7 @@ func (i *VM) Eval(ctx context.Context, if !ok { return nil, fmt.Errorf("read result from memory at %d", resultAddr) } - n := bytes.IndexByte(data, 0) - if n < 0 { - n = 0 - } + n := max(bytes.IndexByte(data, 0), 0) // Skip free'ing input and result JSON as the heap will be reset next round anyway. return data[:n], nil @@ -386,10 +383,7 @@ func (i *VM) evalCompat(ctx context.Context, if !ok { return nil, fmt.Errorf("read result from memory at %d", serialized) } - n := bytes.IndexByte(data, 0) - if n < 0 { - n = 0 - } + n := max(bytes.IndexByte(data, 0), 0) metrics.Timer("wasm_vm_eval_prepare_result").Stop() return data[:n], nil @@ -593,10 +587,7 @@ func (i *VM) fromRegoJSON(ctx context.Context, addr int32, free bool) (any, erro if !ok { return nil, fmt.Errorf("read memory at %d", serialized) } - n := bytes.IndexByte(data, 0) - if n < 0 { - n = 0 - } + n := max(bytes.IndexByte(data, 0), 0) // Parse the result into go types. decoder := json.NewDecoder(bytes.NewReader(data[:n])) diff --git a/internal/wasm/sdk/opa/capabilities/capabilities_nowasm.go b/internal/wasm/sdk/opa/capabilities/capabilities_nowasm.go index 6b17984bb8..93fd1c5e80 100644 --- a/internal/wasm/sdk/opa/capabilities/capabilities_nowasm.go +++ b/internal/wasm/sdk/opa/capabilities/capabilities_nowasm.go @@ -3,7 +3,6 @@ // license that can be found in the LICENSE file. //go:build !opa_wasm && !generate -// +build !opa_wasm,!generate package capabilities diff --git a/v1/ast/compile.go b/v1/ast/compile.go index c642c508a8..268334773c 100644 --- a/v1/ast/compile.go +++ b/v1/ast/compile.go @@ -4278,13 +4278,13 @@ func (n *ModuleTreeNode) DepthFirst(f func(*ModuleTreeNode) bool) { // TreeNode represents a node in the rule tree. The rule tree is keyed by // rule path. type TreeNode struct { + Values []*Rule + Sorted []Value Key Value External *ExternalIndex - Values []*Rule - Children map[Value]*TreeNode - Sorted []Value - Hide bool Index RuleIndex + Children map[Value]*TreeNode + Hide bool } func (n *TreeNode) String() string { diff --git a/v1/ast/compile_stage_test.go b/v1/ast/compile_stage_test.go index b61e62c52e..0c876eabbc 100644 --- a/v1/ast/compile_stage_test.go +++ b/v1/ast/compile_stage_test.go @@ -5,6 +5,7 @@ package ast import ( + "slices" "testing" ) @@ -275,13 +276,7 @@ func TestCompilerStageSkippingWithAfterStages(t *testing.T) { c.Compile(map[string]*Module{}) stages := c.StagesToRun() - found := false - for _, s := range stages { - if s == "CustomAfterCheckTypes" { - found = true - break - } - } + found := slices.Contains(stages, "CustomAfterCheckTypes") if !found { t.Error("after stage should be in StagesToRun()") diff --git a/v1/ast/compile_test.go b/v1/ast/compile_test.go index c5b78cfa51..ed654ff4a8 100644 --- a/v1/ast/compile_test.go +++ b/v1/ast/compile_test.go @@ -9145,8 +9145,8 @@ p := $"{walk(["a", "b"])}"`, t.Fatal("expected error, got none") } if c.Errors[0].Message != tc.exp { - if strings.HasPrefix(tc.exp, "contains:") { - if exp := strings.TrimPrefix(tc.exp, "contains:"); !strings.Contains(c.Errors[0].Message, exp) { + if after, ok := strings.CutPrefix(tc.exp, "contains:"); ok { + if exp := after; !strings.Contains(c.Errors[0].Message, exp) { t.Fatalf("expected error containing:\n\n%s\n\ngot:\n\n%s", tc.exp, c.Errors[0].Message) } } else { diff --git a/v1/ast/external_source_test.go b/v1/ast/external_source_test.go index 02a11e7dcb..6e516aa122 100644 --- a/v1/ast/external_source_test.go +++ b/v1/ast/external_source_test.go @@ -2,6 +2,7 @@ package ast import ( "context" + "slices" "sync/atomic" "testing" ) @@ -81,10 +82,8 @@ type fakeEvalResolver struct { } func (r fakeEvalResolver) Resolve(ref Ref) (Value, error) { - for _, u := range r.unknowns { - if ref.HasPrefix(u) { - return nil, UnknownValueErr{} - } + if slices.ContainsFunc(r.unknowns, ref.HasPrefix) { + return nil, UnknownValueErr{} } if ref.HasPrefix(InputRootRef) { if r.input == nil { diff --git a/v1/ast/index_debug.go b/v1/ast/index_debug.go index 88d451b175..c707dea5b5 100644 --- a/v1/ast/index_debug.go +++ b/v1/ast/index_debug.go @@ -8,6 +8,8 @@ import ( "fmt" "sort" "strings" + + "github.com/open-policy-agent/opa/v1/util" ) func (node *trieNode) mermaid() string { @@ -159,18 +161,23 @@ func (node *trieNode) format(sb *strings.Builder, depth int) { } if len(node.rules) > 0 { - fmt.Fprintf(sb, " [%d rule(s)]", len(node.rules)) + sb.WriteString(" [") + util.WriteInt(sb, len(node.rules)) + sb.WriteString(" rule(s)]") } if len(node.mappers) > 0 { - fmt.Fprintf(sb, " [%d mapper(s)]", len(node.mappers)) + sb.WriteString(" [") + util.WriteInt(sb, len(node.mappers)) + sb.WriteString(" mapper(s)]") } if node.value != nil { - fmt.Fprintf(sb, " value=%v", node.value) + sb.WriteString(" value=") + sb.WriteString(node.value.String()) } if node.multiple { sb.WriteString(" [multiple]") } - sb.WriteString("\n") + sb.WriteByte('\n') if node.undefined != nil { sb.WriteString(indent) @@ -197,7 +204,9 @@ func (node *trieNode) format(sb *strings.Builder, depth int) { }) for i := range scalars { sb.WriteString(indent) - fmt.Fprintf(sb, " %v:\n", scalars[i]) + sb.WriteString(" ") + sb.WriteString(scalars[i].String()) + sb.WriteString(":\n") for j := range nodes { if ValueEqual(scalars[i], scalars[j]) { nodes[j].format(sb, depth+2) diff --git a/v1/ast/policy.go b/v1/ast/policy.go index c5592d99b8..5ba0e4434a 100644 --- a/v1/ast/policy.go +++ b/v1/ast/policy.go @@ -1485,17 +1485,19 @@ func (d *SomeDecl) Hash() int { } func (q *Every) String() string { + b := bytes.NewBufferString("every ") if q.Key != nil { - return fmt.Sprintf("every %s, %s in %s { %s }", - q.Key, - q.Value, - q.Domain, - q.Body) + util.WriteAppender(b, q.Key) + b.WriteString(", ") } - return fmt.Sprintf("every %s in %s { %s }", - q.Value, - q.Domain, - q.Body) + util.WriteAppender(b, q.Value) + b.WriteString(" in ") + util.WriteAppender(b, q.Domain) + b.WriteString(" { ") + util.WriteAppender(b, q.Body) + b.WriteString(" }") + + return b.String() } func (q *Every) Loc() *Location { diff --git a/v1/ast/treenode_dump.go b/v1/ast/treenode_dump.go index f22367bc81..e91a35d6c6 100644 --- a/v1/ast/treenode_dump.go +++ b/v1/ast/treenode_dump.go @@ -4,6 +4,8 @@ import ( "fmt" "sort" "strings" + + "github.com/open-policy-agent/opa/v1/util" ) // Dump returns a string representation of the tree structure rooted at this node. @@ -24,9 +26,10 @@ func (n *TreeNode) dumpRecursive(sb *strings.Builder, prefix, childPrefix string fmt.Fprintf(sb, " ext:%v", n.External.Ref) } if len(n.Values) > 0 { - fmt.Fprintf(sb, " rules:%d", len(n.Values)) + sb.WriteString(" rules:") + util.WriteInt(sb, len(n.Values)) } - sb.WriteString("\n") + sb.WriteByte('\n') if len(n.Children) == 0 { return diff --git a/v1/bundle/bundle.go b/v1/bundle/bundle.go index f697e336d5..6028303d8e 100644 --- a/v1/bundle/bundle.go +++ b/v1/bundle/bundle.go @@ -1815,10 +1815,8 @@ func preProcessBundle(loader DirectoryLoader, skipVerify bool, sizeLimitBytes in base := filepath.Base(f.Path()) if base == patchFile { - - var b bytes.Buffer - tee := io.TeeReader(f.reader, &b) - f.reader = tee + b := new(bytes.Buffer) + f.reader = io.TeeReader(f.reader, b) buf, err := readFile(f, sizeLimitBytes) if err != nil { @@ -1829,7 +1827,7 @@ func preProcessBundle(loader DirectoryLoader, skipVerify bool, sizeLimitBytes in return bundle, nil, fmt.Errorf("bundle load failed on patch decode: %w", err) } - f.reader = &b + f.reader = b } } } diff --git a/v1/config/config.go b/v1/config/config.go index b42f7dd12e..2ff620a8de 100644 --- a/v1/config/config.go +++ b/v1/config/config.go @@ -230,7 +230,7 @@ func unmarshalConfig(raw []byte) (*Config, error) { func knownConfigFields(objValue reflect.Value) map[string]reflect.Value { knownFields := map[string]reflect.Value{} for i := 0; i != objValue.NumField(); i++ { - jsonName := strings.Split(objValue.Type().Field(i).Tag.Get("json"), ",")[0] + jsonName, _, _ := strings.Cut(objValue.Type().Field(i).Tag.Get("json"), ",") if jsonName == "" || jsonName == "-" { continue } diff --git a/v1/config/spec.go b/v1/config/spec.go index 39217570a7..cfa64ab7fd 100644 --- a/v1/config/spec.go +++ b/v1/config/spec.go @@ -59,7 +59,7 @@ func structKeys(specs *[]ConfigSpec, pattern []string, t reflect.Type) []string for i := range t.NumField() { field := t.Field(i) - name := strings.Split(field.Tag.Get("json"), ",")[0] + name, _, _ := strings.Cut(field.Tag.Get("json"), ",") if name == "-" { continue } diff --git a/v1/config/validate_test.go b/v1/config/validate_test.go index d980afb240..7dbed5b023 100644 --- a/v1/config/validate_test.go +++ b/v1/config/validate_test.go @@ -89,9 +89,9 @@ func TestParseConfigNonStringDecisionErrors(t *testing.T) { // updating validate.rego (or vice versa), this test fails. func TestCoreValidationRootSpecMatchesConfigStruct(t *testing.T) { structKeys := map[string]struct{}{} - objType := reflect.TypeOf(Config{}) + objType := reflect.TypeFor[Config]() for i := range objType.NumField() { - name := strings.Split(objType.Field(i).Tag.Get("json"), ",")[0] + name, _, _ := strings.Cut(objType.Field(i).Tag.Get("json"), ",") if name == "" || name == "-" { continue } diff --git a/v1/metrics/metrics.go b/v1/metrics/metrics.go index 481f27337e..119af78aab 100644 --- a/v1/metrics/metrics.go +++ b/v1/metrics/metrics.go @@ -311,19 +311,19 @@ type Counter interface { } type counter struct { - c uint64 + c atomic.Uint64 } func (c *counter) Incr() { - atomic.AddUint64(&c.c, 1) + c.c.Add(1) } func (c *counter) Add(n uint64) { - atomic.AddUint64(&c.c, n) + c.c.Add(n) } func (c *counter) Value() any { - return atomic.LoadUint64(&c.c) + return c.c.Load() } func Statistics(num ...int64) any { diff --git a/v1/plugins/rest/rest.go b/v1/plugins/rest/rest.go index 5c741f4eec..e95ddb921f 100644 --- a/v1/plugins/rest/rest.go +++ b/v1/plugins/rest/rest.go @@ -385,7 +385,7 @@ func (c Client) Do(ctx context.Context, method, path string) (*http.Response, er if len(dump) < defaultResponseSizeLimitBytes { c.loggerFields["response"] = string(dump) } else { - c.loggerFields["response"] = fmt.Sprintf("%v...", string(dump[:defaultResponseSizeLimitBytes])) + c.loggerFields["response"] = string(dump[:defaultResponseSizeLimitBytes]) + "..." } } c.logger.WithFields(c.loggerFields).Debug("Received response.") diff --git a/v1/rego/rego.go b/v1/rego/rego.go index 097b80e3d8..e9ca152596 100644 --- a/v1/rego/rego.go +++ b/v1/rego/rego.go @@ -447,8 +447,12 @@ func EvalEvaluatedRuleTracker(t *topdown.EvaluatedRuleTracker) EvalOption { } func (pq preparedQuery) Modules() map[string]*ast.Module { - mods := make(map[string]*ast.Module) + size := len(pq.r.parsedModules) + for _, b := range pq.r.bundles { + size += len(b.Modules) + } + mods := make(map[string]*ast.Module, size) maps.Copy(mods, pq.r.parsedModules) for _, b := range pq.r.bundles { @@ -597,13 +601,16 @@ func (errs Errors) Error() string { return "no error" } if len(errs) == 1 { - return fmt.Sprintf("1 error occurred: %v", errs[0].Error()) + return "1 error occurred: " + errs[0].Error() } - buf := []string{fmt.Sprintf("%v errors occurred", len(errs))} + bb := new(bytes.Buffer) + util.WriteInt(bb, len(errs)) + bb.WriteString(" errors occurred") for _, err := range errs { - buf = append(buf, err.Error()) + bb.WriteByte('\n') + bb.WriteString(err.Error()) } - return strings.Join(buf, "\n") + return bb.String() } var errPartialEvaluationNotEffective = errors.New("partial evaluation not effective") @@ -1639,7 +1646,6 @@ func CompilePartial(yes bool) CompileOption { // Compile returns a compiled policy query. func (r *Rego) Compile(ctx context.Context, opts ...CompileOption) (*CompileResult, error) { var cfg CompileContext - for _, opt := range opts { opt(&cfg) } @@ -1648,7 +1654,6 @@ func (r *Rego) Compile(ctx context.Context, opts ...CompileOption) (*CompileResu modules := make([]*ast.Module, 0, len(r.compiler.Modules)) if cfg.partial { - pq, err := r.Partial(ctx) if err != nil { return nil, err diff --git a/v1/runtime/check_user_unix.go b/v1/runtime/check_user_unix.go index 7b398c195f..084a3743b2 100644 --- a/v1/runtime/check_user_unix.go +++ b/v1/runtime/check_user_unix.go @@ -1,5 +1,4 @@ //go:build !linux && !windows -// +build !linux,!windows // Copyright 2022 The OPA Authors. All rights reserved. // Use of this source code is governed by an Apache2 diff --git a/v1/server/server.go b/v1/server/server.go index 12ef8edee5..b1af6a462a 100644 --- a/v1/server/server.go +++ b/v1/server/server.go @@ -266,12 +266,16 @@ func (s *Server) Shutdown(ctx context.Context) error { } if len(errorList) > 0 { - errMsg := "error while shutting down: " + errMsg := new(strings.Builder) + errMsg.WriteString("error while shutting down: ") for i, err := range errorList { - //nolint:perfsprint - errMsg += fmt.Sprintf("(%d) %s. ", i, err.Error()) + errMsg.WriteByte('(') + util.WriteInt(errMsg, i) + errMsg.WriteString(") ") + errMsg.WriteString(err.Error()) + errMsg.WriteString(". ") } - return errors.New(errMsg) + return errors.New(errMsg.String()) } return nil } @@ -869,7 +873,7 @@ func (s *Server) initRouters(ctx context.Context) { for _, router := range []*http.ServeMux{mainRouter, diagRouter} { if s.metrics != nil { s.metrics.RegisterEndpoints(func(path, method string, handler http.Handler) { - router.Handle(fmt.Sprintf("%s %s", method, path), handler) + router.Handle(method+" "+path, handler) }) } diff --git a/v1/server/types/types.go b/v1/server/types/types.go index 5dc145df2a..f1f27f8dd1 100644 --- a/v1/server/types/types.go +++ b/v1/server/types/types.go @@ -26,7 +26,7 @@ var jsonFields = map[reflect.Type]map[string]bool{} // UnmarshalExtras and MarshalExtras can distinguish known fields from extras. // Call from an init() function in the package that defines T. func RegisterJSONFields[T any]() { - t := reflect.TypeOf((*T)(nil)).Elem() + t := reflect.TypeFor[T]() if t.Kind() != reflect.Struct { panic(fmt.Sprintf("types: RegisterJSONFields[%s]: not a struct", t)) } @@ -47,7 +47,7 @@ func RegisterJSONFields[T any]() { } func knownJSONFields[T any]() map[string]bool { - t := reflect.TypeOf((*T)(nil)).Elem() + t := reflect.TypeFor[T]() f, ok := jsonFields[t] if !ok { panic(fmt.Sprintf("types: %s not registered with RegisterJSONFields", t)) diff --git a/v1/storage/disk/disk.go b/v1/storage/disk/disk.go index 106ddb66bb..7358d2eb3c 100644 --- a/v1/storage/disk/disk.go +++ b/v1/storage/disk/disk.go @@ -97,7 +97,7 @@ type Options struct { // Store provides a disk-based implementation of the storage.Store interface. type Store struct { db *badger.DB // underlying key-value store - xid uint64 // next transaction id + xid atomic.Uint64 // next transaction id rmu sync.RWMutex // reader-writer lock wmu sync.Mutex // writer lock pm *pathMapper // maps logical storage paths to underlying store keys @@ -239,7 +239,7 @@ func (db *Store) NewTransaction(_ context.Context, params ...storage.Transaction context = params[0].Context } - xid := atomic.AddUint64(&db.xid, uint64(1)) + xid := db.xid.Add(1) if write { db.wmu.Lock() // only one concurrent write txn } else { @@ -274,7 +274,7 @@ func (db *Store) Truncate(ctx context.Context, txn storage.Transaction, params s // write new bundle policy and data into the existing DB underlying := db.db.NewTransaction(true) - xid := atomic.AddUint64(&db.xid, uint64(1)) + xid := db.xid.Add(uint64(1)) underlyingTxn := newTransaction(xid, true, underlying, params.Context, db.pm, db.partitions, db) // For backwards compatibility, check if `RootOverwrite` was configured. @@ -314,7 +314,7 @@ func (db *Store) Truncate(ctx context.Context, txn storage.Transaction, params s } underlying = db.db.NewTransaction(true) - xid = atomic.AddUint64(&db.xid, uint64(1)) + xid = db.xid.Add(uint64(1)) underlyingTxn = newTransaction(xid, true, underlying, params.Context, db.pm, db.partitions, db) if err = underlyingTxn.UpsertPolicy(ctx, strings.TrimLeft(update.Path.String(), "/"), update.Value); err != nil { @@ -397,7 +397,7 @@ func (db *Store) doTruncateData(ctx context.Context, underlying *transaction, ba } txn := badgerdb.NewTransaction(true) - xid := atomic.AddUint64(&db.xid, uint64(1)) + xid := db.xid.Add(1) sTxn := newTransaction(xid, true, txn, params.Context, db.pm, db.partitions, db) if err = sTxn.Write(ctx, storage.AddOp, path, value); err != nil { @@ -480,8 +480,7 @@ func (db *Store) Commit(ctx context.Context, txn storage.Transaction) error { } write := false // read only txn readOnly := db.db.NewTransaction(write) - xid := atomic.AddUint64(&db.xid, uint64(1)) - readTxn := newTransaction(xid, write, readOnly, nil, db.pm, db.partitions, db) + readTxn := newTransaction(db.xid.Add(1), write, readOnly, nil, db.pm, db.partitions, db) for h := range db.triggers { h.cb(ctx, readTxn, event) } diff --git a/v1/storage/errors.go b/v1/storage/errors.go index a3d1c00737..95bf25a156 100644 --- a/v1/storage/errors.go +++ b/v1/storage/errors.go @@ -4,10 +4,6 @@ package storage -import ( - "fmt" -) - const ( // InternalErr indicates an unknown, internal error has occurred. InternalErr = "storage_internal_error" @@ -49,7 +45,7 @@ type Error struct { func (err *Error) Error() string { if err.Message != "" { - return fmt.Sprintf("%v: %v", err.Code, err.Message) + return err.Code + ": " + err.Message } return err.Code } diff --git a/v1/storage/inmem/inmem.go b/v1/storage/inmem/inmem.go index 8aa1fc9e42..7aaccb425a 100644 --- a/v1/storage/inmem/inmem.go +++ b/v1/storage/inmem/inmem.go @@ -116,7 +116,7 @@ func NewFromASTObject(data ast.Object) storage.Store { type store struct { rmu sync.RWMutex // reader-writer lock wmu sync.Mutex // writer lock - xid uint64 // last generated transaction id + xid atomic.Uint64 // last generated transaction id data any // raw or AST data policies map[string][]byte // raw policies triggers map[*handle]storage.TriggerConfig // registered triggers @@ -137,7 +137,7 @@ type handle struct { func (db *store) NewTransaction(_ context.Context, params ...storage.TransactionParams) (storage.Transaction, error) { txn := &transaction{ - xid: atomic.AddUint64(&db.xid, uint64(1)), + xid: db.xid.Add(1), db: db, } diff --git a/v1/test/e2e/authz/nodisk.go b/v1/test/e2e/authz/nodisk.go index b14774a9ca..e0a3fedf3f 100644 --- a/v1/test/e2e/authz/nodisk.go +++ b/v1/test/e2e/authz/nodisk.go @@ -3,7 +3,6 @@ // license that can be found in the LICENSE file. //go:build !bench_disk -// +build !bench_disk // nolint: unused // build tags confuse these linters package authz diff --git a/v1/test/e2e/wasm/authz/nodisk.go b/v1/test/e2e/wasm/authz/nodisk.go index b14774a9ca..e0a3fedf3f 100644 --- a/v1/test/e2e/wasm/authz/nodisk.go +++ b/v1/test/e2e/wasm/authz/nodisk.go @@ -3,7 +3,6 @@ // license that can be found in the LICENSE file. //go:build !bench_disk -// +build !bench_disk // nolint: unused // build tags confuse these linters package authz diff --git a/v1/topdown/builtins.go b/v1/topdown/builtins.go index b5a8a6714d..1f5cfb7f33 100644 --- a/v1/topdown/builtins.go +++ b/v1/topdown/builtins.go @@ -7,7 +7,6 @@ package topdown import ( "context" "encoding/binary" - "fmt" "io" "math/rand" @@ -73,7 +72,6 @@ type ( // function. If a random number generator cannot be created, an error is // returned. func (bctx *BuiltinContext) Rand() (*rand.Rand, error) { - if bctx.rand != nil { return bctx.rand, nil } @@ -180,26 +178,18 @@ func functionalWrapper4(name string, fn FunctionalBuiltin4) BuiltinFunc { } func handleBuiltinErr(name string, loc *ast.Location, err error) error { + var code string switch err := err.(type) { case BuiltinEmpty: return nil case *Error, Halt: return err case builtins.ErrOperand: - e := &Error{ - Code: TypeErr, - Message: fmt.Sprintf("%v: %v", name, err.Error()), - Location: loc, - } - return e.Wrap(err) + code = TypeErr default: - e := &Error{ - Code: BuiltinErr, - Message: fmt.Sprintf("%v: %v", name, err.Error()), - Location: loc, - } - return e.Wrap(err) + code = BuiltinErr } + return (&Error{Code: code, Message: name + ": " + err.Error(), Location: loc}).Wrap(err) } func readInt64(r io.Reader) (int64, error) { diff --git a/v1/topdown/cache/cache_test.go b/v1/topdown/cache/cache_test.go index 8f595d6119..1d8be3497b 100644 --- a/v1/topdown/cache/cache_test.go +++ b/v1/topdown/cache/cache_test.go @@ -626,14 +626,12 @@ func TestConcurrentInsert(t *testing.T) { wg := sync.WaitGroup{} for range 5 { - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { cacheValue2 := newInterQueryCacheValue(ast.String("bar2"), 5) cache.Insert(ast.String("foo2"), cacheValue2) - }() + }) } wg.Wait() diff --git a/v1/topdown/enumerate_bench_test.go b/v1/topdown/enumerate_bench_test.go index a2a93b4142..a174cfa1ea 100644 --- a/v1/topdown/enumerate_bench_test.go +++ b/v1/topdown/enumerate_bench_test.go @@ -120,7 +120,7 @@ func generateNestedDataset(size int) map[string]any { for i := range size { // Random nested object with 3-5 levels of nesting permissions := make([]any, rng.Intn(10)+1) - for j := 0; j < len(permissions); j++ { + for j := range permissions { permissions[j] = map[string]any{ "name": fmt.Sprintf("perm_%d", j), "level": rng.Intn(10), diff --git a/v1/topdown/glob_bench_test.go b/v1/topdown/glob_bench_test.go index 3bb33f414c..7de197a61a 100644 --- a/v1/topdown/glob_bench_test.go +++ b/v1/topdown/glob_bench_test.go @@ -66,8 +66,7 @@ func BenchmarkBuiltinGlobMatchAsync(b *testing.B) { wg := sync.WaitGroup{} for i := range clientCount { clientID := i - wg.Add(1) - go func() { + wg.Go(func() { for j := range patternCount { var operands []*ast.Term if reusePattern { @@ -88,8 +87,7 @@ func BenchmarkBuiltinGlobMatchAsync(b *testing.B) { return } } - wg.Done() - }() + }) } wg.Wait() } diff --git a/v1/topdown/http.go b/v1/topdown/http.go index 79d49f33ca..3c47ebc4e0 100644 --- a/v1/topdown/http.go +++ b/v1/topdown/http.go @@ -394,7 +394,7 @@ func verifyURLHost(bctx BuiltinContext, unverifiedURL string) error { return err } - host := strings.Split(parsedURL.Host, ":")[0] + host, _, _ := strings.Cut(parsedURL.Host, ":") return verifyHost(bctx, host) } diff --git a/v1/topdown/http_test.go b/v1/topdown/http_test.go index 5f52f77d7a..3bea7397ea 100644 --- a/v1/topdown/http_test.go +++ b/v1/topdown/http_test.go @@ -806,7 +806,7 @@ func TestHTTPRedirectAllowNet(t *testing.T) { if err != nil { t.Fatal(err) } - serverHost := strings.Split(serverURL.Host, ":")[0] + serverHost, _, _ := strings.Cut(serverURL.Host, ":") // expected result expectedResult := make(map[string]any) @@ -3712,7 +3712,7 @@ func TestHTTPGetRequestAllowNet(t *testing.T) { if err != nil { t.Fatal(err) } - serverHost := strings.Split(serverURL.Host, ":")[0] + serverHost, _, _ := strings.Cut(serverURL.Host, ":") // expected result expectedResult := make(map[string]any) @@ -3820,7 +3820,7 @@ func TestHTTPWithCustomTransport(t *testing.T) { if err != nil { t.Fatal(err) } - serverHost := strings.Split(serverURL.Host, ":")[0] + serverHost, _, _ := strings.Cut(serverURL.Host, ":") // expected result expectedResult := make(map[string]any) diff --git a/v1/topdown/net_test.go b/v1/topdown/net_test.go index 0743e6380a..23a1476523 100644 --- a/v1/topdown/net_test.go +++ b/v1/topdown/net_test.go @@ -3,7 +3,6 @@ // license that can be found in the LICENSE file. //go:build !race -// +build !race package topdown diff --git a/v1/topdown/parse_bytes.go b/v1/topdown/parse_bytes.go index f912d276c2..73e2d940eb 100644 --- a/v1/topdown/parse_bytes.go +++ b/v1/topdown/parse_bytes.go @@ -37,7 +37,7 @@ func parseNumBytesError(msg string) error { } func errBytesUnitNotRecognized(unit string) error { - return parseNumBytesError(fmt.Sprintf("byte unit %s not recognized", unit)) + return parseNumBytesError("byte unit " + unit + " not recognized") } var ( @@ -116,9 +116,7 @@ func builtinNumBytes(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term // Makes the string lower case and removes quotation marks func formatString(s ast.String) string { - str := string(s) - lower := strings.ToLower(str) - return strings.ReplaceAll(lower, "\"", "") + return strings.ReplaceAll(strings.ToLower(string(s)), "\"", "") } // Splits the string into a number string à la "10" or "10.2" and a unit diff --git a/v1/topdown/regex_bench_test.go b/v1/topdown/regex_bench_test.go index aec4ff6ef6..7b979a5071 100644 --- a/v1/topdown/regex_bench_test.go +++ b/v1/topdown/regex_bench_test.go @@ -66,8 +66,7 @@ func BenchmarkBuiltinRegexMatchAsync(b *testing.B) { wg := sync.WaitGroup{} for i := range clientCount { clientID := i - wg.Add(1) - go func() { + wg.Go(func() { for j := range patternCount { var operands []*ast.Term if reusePattern { @@ -83,8 +82,7 @@ func BenchmarkBuiltinRegexMatchAsync(b *testing.B) { return } } - wg.Done() - }() + }) } wg.Wait() } diff --git a/v1/topdown/strings.go b/v1/topdown/strings.go index 4ffd307c78..0f80875a83 100644 --- a/v1/topdown/strings.go +++ b/v1/topdown/strings.go @@ -583,20 +583,13 @@ func builtinSplitN(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) limit = -1 } parts := strings.SplitN(text, delim, limit) - end := n - if end > len(parts) { - end = len(parts) - } - result = make([]*ast.Term, end) + result = make([]*ast.Term, min(n, len(parts))) for i := range result { result[i] = ast.InternedTerm(parts[i]) } } else { parts := strings.Split(text, delim) - start := len(parts) + n - if start < 0 { - start = 0 - } + start := max(len(parts)+n, 0) result = make([]*ast.Term, len(parts)-start) for i, p := range parts[start:] { result[i] = ast.InternedTerm(p) diff --git a/v1/topdown/topdown_bench_test.go b/v1/topdown/topdown_bench_test.go index 81386f4dfe..eae98ff6e8 100644 --- a/v1/topdown/topdown_bench_test.go +++ b/v1/topdown/topdown_bench_test.go @@ -175,43 +175,39 @@ func BenchmarkConcurrency8Writers(b *testing.B) { } func benchmarkConcurrency(b *testing.B, params []storage.TransactionParams) { - mod, data := test.GenerateConcurrencyBenchmarkData() ctx := b.Context() store := inmem.NewFromObject(data) - mods := map[string]*ast.Module{"module": ast.MustParseModule(mod)} - compiler := ast.NewCompiler() + body := ast.MustParseBody("data.test.p = x") + wg := &sync.WaitGroup{} - if compiler.Compile(mods); compiler.Failed() { - b.Fatalf("Unexpected compiler error: %v", compiler.Errors) + compiler, err := ast.CompileModules(map[string]string{"module": mod}) + if err != nil { + b.Fatalf("Unexpected compiler error: %v", err) } for b.Loop() { - wg := new(sync.WaitGroup) - queriesPerCore := 1000 / len(params) - for j := range params { - param := params[j] // capture j'th params before goroutine - wg.Add(1) - go func() { - defer wg.Done() - for range queriesPerCore { - txn := storage.NewTransactionOrDie(ctx, store, param) - query := NewQuery(ast.MustParseBody("data.test.p = x")). + for _, param := range params { + wg.Go(func() { + for range 1000 / len(params) { + txn, err := store.NewTransaction(ctx, param) + if err != nil { + b.Fatalf("Unexpected transaction error: %v", err) + } + rs, err := NewQuery(body). WithCompiler(compiler). WithStore(store). - WithTransaction(txn) - rs, err := query.Run(ctx) + WithTransaction(txn). + Run(ctx) if err != nil { - b.Errorf("Unexpected topdown query error: %v", err) - return + b.Fatalf("Unexpected topdown query error: %v", err) } if len(rs) != 1 || !rs[0][ast.Var("x")].Equal(ast.BooleanTerm(true)) { - b.Errorf("Unexpected undefined/extra/bad result: %v", rs) - return + b.Fatalf("Unexpected undefined/extra/bad result: %v", rs) } store.Abort(ctx, txn) } - }() + }) } wg.Wait() @@ -548,14 +544,16 @@ func BenchmarkWalk(b *testing.B) { if err != nil { b.Fatal(err) } - err = storage.Txn(b.Context(), store, storage.TransactionParams{}, func(txn storage.Transaction) error { + ctx := b.Context() + + err = storage.Txn(ctx, store, storage.TransactionParams{}, func(txn storage.Transaction) error { q := NewQuery(compiledQuery). WithStore(store). WithCompiler(compiler). WithTransaction(txn) for b.Loop() { - rs, err := q.Run(b.Context()) + rs, err := q.Run(ctx) if err != nil || len(rs) != 1 || !rs[0][ast.Var("x")].Equal(ast.IntNumberTerm(n-1)) { b.Fatal("Unexpected result:", rs, "err:", err) } diff --git a/v1/topdown/trace.go b/v1/topdown/trace.go index 8b672da54e..0f366accdd 100644 --- a/v1/topdown/trace.go +++ b/v1/topdown/trace.go @@ -845,7 +845,7 @@ func PrettyEvent(w io.Writer, e *Event, opts PrettyEventOpts) error { func printPrettyVars(w *bytes.Buffer, exprVars map[string]varInfo) { containsTabs := false - varRows := make(map[int]any) + varRows := make(map[int]any, len(exprVars)) for _, info := range exprVars { if len(info.exprLoc.Tabs) > 0 { containsTabs = true @@ -891,10 +891,10 @@ func printPrettyVars(w *bytes.Buffer, exprVars map[string]varInfo) { return } - w.WriteString("\n") + w.WriteByte('\n') printArrows(w, byCol, -1) for i := len(byCol) - 1; i >= 0; i-- { - w.WriteString("\n") + w.WriteByte('\n') printArrows(w, byCol, i) } } @@ -909,7 +909,6 @@ func printArrows(w *bytes.Buffer, l []varInfo, printValueAt int) { } isFirst := true for i, info := range slice { - isLast := i >= len(slice)-1 col := info.col @@ -926,11 +925,11 @@ func printArrows(w *bytes.Buffer, l []varInfo, printValueAt int) { for j := range spaces { tab := false if slices.Contains(info.exprLoc.Tabs, j+prevCol+1) { - w.WriteString("\t") + w.WriteByte('\t') tab = true } if !tab { - w.WriteString(" ") + w.WriteByte(' ') } } @@ -943,7 +942,7 @@ func printArrows(w *bytes.Buffer, l []varInfo, printValueAt int) { w.WriteString(valueStr) } } else { - w.WriteString("|") + w.WriteByte('|') } prevCol = col isFirst = false diff --git a/v1/types/types.go b/v1/types/types.go index aff4fde39e..030ca150f8 100644 --- a/v1/types/types.go +++ b/v1/types/types.go @@ -345,7 +345,7 @@ func (p *DynamicProperty) MarshalJSON() ([]byte, error) { } func (p *DynamicProperty) String() string { - return fmt.Sprintf("%s: %s", Sprint(p.Key), Sprint(p.Value)) + return Sprint(p.Key) + ": " + Sprint(p.Value) } // Object represents the object type. diff --git a/v1/util/performance.go b/v1/util/performance.go index f269a2e1d7..44987c086b 100644 --- a/v1/util/performance.go +++ b/v1/util/performance.go @@ -1,6 +1,9 @@ package util import ( + "bytes" + "encoding" + "io" "slices" "strconv" "strings" @@ -8,6 +11,18 @@ import ( "unsafe" ) +type ( + Signed interface { + ~int | ~int8 | ~int16 | ~int32 | ~int64 + } + Unsigned interface { + ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr + } + Integer interface { + Signed | Unsigned + } +) + // SyncPool is a generic sync.Pool for type T, providing some convenience // over sync.Pool directly: [SyncPool.Put] ensures that nil values are not // put into the pool, and [SyncPool.Get] returns a pointer to T without having @@ -147,10 +162,32 @@ func NumDigitsUint(n uint64) int { } // AppendInt is a less messy version of strconv.AppendInt for base 10 ints. -func AppendInt(buf []byte, n int) []byte { +func AppendInt[T Integer](buf []byte, n T) []byte { return strconv.AppendInt(buf, int64(n), 10) } +// WriteInt writes the string form of n to out. +func WriteInt[T Integer](out io.Writer, n T) (int, error) { + var buf []byte + if b, ok := out.(*bytes.Buffer); ok { + buf = b.AvailableBuffer() + } + return out.Write(AppendInt(buf, n)) +} + +// WriteAppender writes the appended text of appender to out. +func WriteAppender[T encoding.TextAppender](out io.Writer, appender T) (int, error) { + var buf []byte + if b, ok := out.(*bytes.Buffer); ok { + buf = b.AvailableBuffer() + } + b, err := appender.AppendText(buf) + if err != nil { + return 0, err + } + return out.Write(b) +} + // Atoi is a convenience function for [Atoi64] where an int is preferable to an int64. // See the documentation of [Atoi64] for details on the performance benefits of this // function over strconv.Atoi. diff --git a/v1/util/test/populate.go b/v1/util/test/populate.go index 011d19a8a2..4db74053fb 100644 --- a/v1/util/test/populate.go +++ b/v1/util/test/populate.go @@ -52,7 +52,7 @@ func populateDefaultTypes(t *testing.T, fieldType reflect.Type, fieldValue refle switch fieldType.Kind() { case reflect.Slice: - if fieldType == reflect.TypeOf(json.RawMessage{}) { + if fieldType == reflect.TypeFor[json.RawMessage]() { fieldValue.Set(reflect.ValueOf(fmt.Appendf(nil, `{"test": "bar-%d"}`, index))) return true } @@ -116,7 +116,7 @@ func populateDefaultTypes(t *testing.T, fieldType reflect.Type, fieldValue refle return true case fieldType.Key().Kind() == reflect.String && - fieldType.Elem() == reflect.TypeOf(json.RawMessage{}): + fieldType.Elem() == reflect.TypeFor[json.RawMessage](): fieldValue.Set(reflect.ValueOf(map[string]json.RawMessage{ "key1": fmt.Appendf(nil, `{"test": "bar-%d"}`, index),