From b2d92a33c14e5b5320250b8ef32fe5fe0026c628 Mon Sep 17 00:00:00 2001 From: Philip Conrad Date: Thu, 15 Sep 2022 15:09:54 -0400 Subject: [PATCH] Add `prealloc` linter check + linter fixes (#5139) This commit adds the `prealloc` linter to the list of linters for OPA, and fixes up the miscellaneous locations in the code that the linter found where we could easily preallocate slices. Signed-off-by: Philip Conrad --- .golangci.yaml | 1 + ast/annotations.go | 3 ++- ast/check_test.go | 4 +++- ast/compile_test.go | 2 +- ast/policy.go | 6 +++--- build/generate-cli-docs/generate.go | 5 +++-- bundle/file_test.go | 10 +++++----- cmd/inspect.go | 4 ++-- cmd/version_test.go | 5 +++-- compile/compile.go | 4 ++-- compile/compile_test.go | 4 ++-- format/format.go | 4 ++-- internal/bundle/inspect/inspect.go | 2 +- internal/compiler/wasm/wasm.go | 2 +- internal/gqlparser/validator/imported_test.go | 2 +- .../validator/rules/fields_on_correct_type.go | 7 ++++--- internal/presentation/presentation.go | 7 ++++--- internal/report/report.go | 5 +++-- internal/strings/strings.go | 2 +- loader/loader.go | 2 +- profiler/profiler.go | 2 +- rego/rego_test.go | 2 +- sdk/test/test.go | 4 ++-- storage/disk/disk_test.go | 4 ++-- storage/inmem/inmem_test.go | 8 ++++---- test/wasm/cmd/wasm-rego-testgen/main.go | 2 +- tester/reporter.go | 5 +++-- topdown/cidr.go | 2 +- topdown/strings.go | 2 +- topdown/tokens.go | 2 +- types/types.go | 2 +- 31 files changed, 63 insertions(+), 53 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 6286815ef3..ff84cb3317 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -25,4 +25,5 @@ linters: - structcheck - staticcheck - gosimple + - prealloc # - gosec # too many false positives diff --git a/ast/annotations.go b/ast/annotations.go index 68f9f645cf..d255d2f0bf 100644 --- a/ast/annotations.go +++ b/ast/annotations.go @@ -639,7 +639,8 @@ func (as *AnnotationSet) GetPackageScope(pkg *Package) *Annotations { // Flatten returns a flattened list view of this AnnotationSet. // The returned slice is sorted, first by the annotations' target path, then by their target location func (as *AnnotationSet) Flatten() []*AnnotationsRef { - var refs []*AnnotationsRef + // This preallocation often won't be optimal, but it's superior to starting with a nil slice. + refs := make([]*AnnotationsRef, 0, len(as.byPath.Children)+len(as.byRule)+len(as.byPackage)) refs = as.byPath.flatten(refs) diff --git a/ast/check_test.go b/ast/check_test.go index 4a0f77bddd..33a51ca063 100644 --- a/ast/check_test.go +++ b/ast/check_test.go @@ -1255,7 +1255,9 @@ func newTestEnv(rs []string) *TypeEnv { package test `) - var elems []util.T + // We preallocate enough for at least the base rules. + // Else cases will cause reallocs, but that's okay. + elems := make([]util.T, 0, len(rs)) for i := range rs { rule := MustParseRule(rs[i]) diff --git a/ast/compile_test.go b/ast/compile_test.go index aa3165114a..5f112e1137 100644 --- a/ast/compile_test.go +++ b/ast/compile_test.go @@ -676,7 +676,7 @@ func TestCompilerErrorLimit(t *testing.T) { "rego_compile_error: error limit reached", } - var result []string + result := make([]string, 0, len(errs)) for _, err := range errs { result = append(result, err.Error()) } diff --git a/ast/policy.go b/ast/policy.go index 938ee0f9e9..04ebe56f10 100644 --- a/ast/policy.go +++ b/ast/policy.go @@ -787,7 +787,7 @@ func (a Args) Copy() Args { } func (a Args) String() string { - var buf []string + buf := make([]string, 0, len(a)) for _, t := range a { buf = append(buf, t.String()) } @@ -931,7 +931,7 @@ func (body Body) SetLoc(loc *Location) { } func (body Body) String() string { - var buf []string + buf := make([]string, 0, len(body)) for _, v := range body { buf = append(buf, v.String()) } @@ -1238,7 +1238,7 @@ func (expr *Expr) SetLoc(loc *Location) { } func (expr *Expr) String() string { - var buf []string + buf := make([]string, 0, 2+len(expr.With)) if expr.Negated { buf = append(buf, "not") } diff --git a/build/generate-cli-docs/generate.go b/build/generate-cli-docs/generate.go index aae3179ecd..66141c1880 100644 --- a/build/generate-cli-docs/generate.go +++ b/build/generate-cli-docs/generate.go @@ -72,13 +72,14 @@ func main() { } heading := regexp.MustCompile(`^[\\-]+$`) - var document []string + lines := strings.Split(builder.String(), "\n") + document := make([]string, 0, len(lines)) removed := 0 // The document may contain "----" for headings, which will be converted to h1 // elements in markdown. This is undesirable, so let's remove them and prepend // the line before that with ### to instead create a h3 - for line, str := range strings.Split(builder.String(), "\n") { + for line, str := range lines { if heading.Match([]byte(str)) { document[line-1-removed] = fmt.Sprintf("### %s\n", document[line-1-removed]) removed++ diff --git a/bundle/file_test.go b/bundle/file_test.go index bbd5a0c144..809610f3d9 100644 --- a/bundle/file_test.go +++ b/bundle/file_test.go @@ -49,7 +49,7 @@ func TestTarballLoader(t *testing.T) { func TestIterator(t *testing.T) { - var files [][2]string + files := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { files = append(files, [2]string{name, content}) } @@ -91,7 +91,7 @@ func TestIteratorOrder(t *testing.T) { "/roles/policy.rego": "package bar\n p = 1", } - var files [][2]string + files := make([][2]string, 0, len(archFiles)) for name, content := range archFiles { files = append(files, [2]string{name, content}) } @@ -170,7 +170,7 @@ func TestTarballLoaderWithFilter(t *testing.T) { t.Fatalf("Unexpected error: %s", err) } - var gzFiles [][2]string + gzFiles := make([][2]string, 0, len(files)) for name, content := range files { gzFiles = append(gzFiles, [2]string{name, content}) } @@ -222,7 +222,7 @@ func TestTarballLoaderWithFilterDir(t *testing.T) { t.Fatalf("Unexpected error: %s", err) } - var gzFiles [][2]string + gzFiles := make([][2]string, 0, len(files)) for name, content := range files { gzFiles = append(gzFiles, [2]string{name, content}) } @@ -389,7 +389,7 @@ func testGetTarballFile(t *testing.T, root string) *os.File { t.Fatalf("Unexpected error: %s", err) } - var gzFiles [][2]string + gzFiles := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { gzFiles = append(gzFiles, [2]string{name, content}) } diff --git a/cmd/inspect.go b/cmd/inspect.go index 34423a459c..af1890a18d 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -177,7 +177,7 @@ func populateNamespaces(out io.Writer, n map[string][]string) error { t.SetAutoMergeCellsByColumnIndex([]int{0}) var lines [][]string - var keys []string + keys := make([]string, 0, len(n)) for k := range n { keys = append(keys, k) } @@ -334,7 +334,7 @@ func printTitle(out io.Writer, ref *ast.AnnotationsRef) { func generateTableWithKeys(writer io.Writer, keys ...string) *tablewriter.Table { table := tablewriter.NewWriter(writer) aligns := []int{} - var hdrs []string + hdrs := make([]string, 0, len(keys)) for _, k := range keys { hdrs = append(hdrs, strings.Title(k)) //nolint:staticcheck // SA1019, no unicode aligns = append(aligns, tablewriter.ALIGN_LEFT) diff --git a/cmd/version_test.go b/cmd/version_test.go index cd30d8c2f4..aeaab19db3 100644 --- a/cmd/version_test.go +++ b/cmd/version_test.go @@ -77,9 +77,10 @@ func TestCheckOPAUpdateBadURL(t *testing.T) { func expectOutputKeys(t *testing.T, stdout string, expectedKeys []string) { t.Helper() - var gotKeys []string + lines := strings.Split(strings.Trim(stdout, "\n"), "\n") + gotKeys := make([]string, 0, len(lines)) - for _, line := range strings.Split(strings.Trim(stdout, "\n"), "\n") { + for _, line := range lines { gotKeys = append(gotKeys, strings.Split(line, ":")[0]) } diff --git a/compile/compile.go b/compile/compile.go index 7e8825d0bf..1e8b99ae8a 100644 --- a/compile/compile.go +++ b/compile/compile.go @@ -372,7 +372,7 @@ func (c *Compiler) initBundle() error { result.Manifest.Init() result.Data = load.Files.Documents - var modules []string + modules := make([]string, 0, len(load.Files.Modules)) for k := range load.Files.Modules { modules = append(modules, k) @@ -824,7 +824,7 @@ func (o *optimizer) findRequiredDocuments(ref *ast.Term) []string { }) } - var result []string + result := make([]string, 0, len(keep)) for k := range keep { result = append(result, k) diff --git a/compile/compile_test.go b/compile/compile_test.go index 303ee6c4c1..1262c64b18 100644 --- a/compile/compile_test.go +++ b/compile/compile_test.go @@ -1525,14 +1525,14 @@ func getOptimizer(modules map[string]string, data string, entries []string, root func getModuleFiles(src map[string]string, includeRaw bool) []bundle.ModuleFile { - var keys []string + keys := make([]string, 0, len(src)) for k := range src { keys = append(keys, k) } sort.Strings(keys) - var modules []bundle.ModuleFile + modules := make([]bundle.ModuleFile, 0, len(keys)) for _, k := range keys { module, err := ast.ParseModule(k, src[k]) diff --git a/format/format.go b/format/format.go index 706dbf74cb..10406bfff6 100644 --- a/format/format.go +++ b/format/format.go @@ -854,7 +854,7 @@ func (w *writer) writeComprehension(open, close byte, term *ast.Term, body ast.B } func (w *writer) writeComprehensionBody(open, close byte, body ast.Body, term, compr *ast.Location, comments []*ast.Comment) []*ast.Comment { - var exprs []interface{} + exprs := make([]interface{}, 0, len(body)) for _, expr := range body { exprs = append(exprs, expr) } @@ -1004,7 +1004,7 @@ func groupIterable(elements []interface{}, last *ast.Location) [][]interface{} { }) var lines [][]interface{} - var cur []interface{} + cur := make([]interface{}, 0, len(elements)) for i, t := range elements { elem := t loc := getLoc(elem) diff --git a/internal/bundle/inspect/inspect.go b/internal/bundle/inspect/inspect.go index 708dca9cef..58d8438e1c 100644 --- a/internal/bundle/inspect/inspect.go +++ b/internal/bundle/inspect/inspect.go @@ -39,7 +39,7 @@ func File(path string, includeAnnotations bool) (*Info, error) { bi := &Info{Manifest: b.Manifest} namespaces := make(map[string][]string, len(b.Modules)) - var modules []*ast.Module + modules := make([]*ast.Module, 0, len(b.Modules)) for _, m := range b.Modules { namespaces[m.Parsed.Package.Path.String()] = append(namespaces[m.Parsed.Package.Path.String()], filepath.Clean(m.Path)) modules = append(modules, m.Parsed) diff --git a/internal/compiler/wasm/wasm.go b/internal/compiler/wasm/wasm.go index 8951169ef2..21fe41358e 100644 --- a/internal/compiler/wasm/wasm.go +++ b/internal/compiler/wasm/wasm.go @@ -897,7 +897,7 @@ func mapFunc(mapping ast.Object, fn *ir.Func, index int) (ast.Object, bool) { } func (c *Compiler) emitMappingAndStartFunc() error { - var indices []uint32 + indices := make([]uint32, 0, len(c.policy.Funcs.Funcs)) var ok bool mapping := ast.NewObject() diff --git a/internal/gqlparser/validator/imported_test.go b/internal/gqlparser/validator/imported_test.go index 157e73c5d7..41c20e9502 100644 --- a/internal/gqlparser/validator/imported_test.go +++ b/internal/gqlparser/validator/imported_test.go @@ -43,7 +43,7 @@ func TestValidation(t *testing.T) { d.pattern = regexp.MustCompile("^" + d.Rule + "$") } - var schemas []*ast.Schema + schemas := make([]*ast.Schema, 0, len(rawSchemas)) for i, schema := range rawSchemas { schema, err := gqlparser.LoadSchema(&ast.Source{Input: schema, Name: fmt.Sprintf("schemas.yml[%d]", i)}) if err != nil { diff --git a/internal/gqlparser/validator/rules/fields_on_correct_type.go b/internal/gqlparser/validator/rules/fields_on_correct_type.go index c8e2f7b257..946c9fea67 100644 --- a/internal/gqlparser/validator/rules/fields_on_correct_type.go +++ b/internal/gqlparser/validator/rules/fields_on_correct_type.go @@ -43,11 +43,12 @@ func getSuggestedTypeNames(walker *Walker, parent *ast.Definition, name string) return nil } - var suggestedObjectTypes []string + possibleTypes := walker.Schema.GetPossibleTypes(parent) + suggestedObjectTypes := make([]string, 0, len(possibleTypes)) var suggestedInterfaceTypes []string interfaceUsageCount := map[string]int{} - for _, possibleType := range walker.Schema.GetPossibleTypes(parent) { + for _, possibleType := range possibleTypes { field := possibleType.Fields.ForName(name) if field == nil { continue @@ -87,7 +88,7 @@ func getSuggestedFieldNames(parent *ast.Definition, name string) []string { return nil } - var possibleFieldNames []string + possibleFieldNames := make([]string, 0, len(parent.Fields)) for _, field := range parent.Fields { possibleFieldNames = append(possibleFieldNames, field.Name) } diff --git a/internal/presentation/presentation.go b/internal/presentation/presentation.go index 5277cb31db..18c0e102a5 100644 --- a/internal/presentation/presentation.go +++ b/internal/presentation/presentation.go @@ -223,7 +223,8 @@ func (e OutputErrors) Error() string { prefix = fmt.Sprintf("%d errors occurred:\n", len(e)) } - var s []string + // We preallocate for at least the minimum number of strings. + s := make([]string, 0, len(e)) for _, err := range e { s = append(s, err.Error()) if l, ok := err.Details.(string); ok { @@ -597,8 +598,8 @@ func generateTableMetrics(writer io.Writer) *tablewriter.Table { func generateTableWithKeys(writer io.Writer, keys ...string) *tablewriter.Table { table := tablewriter.NewWriter(writer) - aligns := []int{} - var hdrs []string + aligns := make([]int, 0, len(keys)) + hdrs := make([]string, 0, len(keys)) for _, k := range keys { hdrs = append(hdrs, strings.Title(k)) //nolint:staticcheck // SA1019, no unicode here aligns = append(aligns, tablewriter.ALIGN_LEFT) diff --git a/internal/report/report.go b/internal/report/report.go index f80915dc83..13a7d08942 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -137,9 +137,10 @@ func (dr *DataResponse) Pretty() string { return "" } - var lines []string + pairs := dr.Slice() + lines := make([]string, 0, len(pairs)) - for _, pair := range dr.Slice() { + for _, pair := range pairs { lines = append(lines, fmt.Sprintf("%v: %v", pair[0], pair[1])) } diff --git a/internal/strings/strings.go b/internal/strings/strings.go index 63a71be7da..76fb9ee8c8 100644 --- a/internal/strings/strings.go +++ b/internal/strings/strings.go @@ -16,7 +16,7 @@ import ( // "ideal" width and returns the shortened paths by replacing the middle parts of paths // with "...", ex: bundle1/.../a/b/policy.rego func TruncateFilePaths(maxIdealWidth, maxWidth int, path ...string) (map[string]string, int) { - var canShorten [][]byte + canShorten := make([][]byte, 0, len(path)) for _, p := range path { canShorten = append(canShorten, []byte(getPathFromFirstSeparator(p))) diff --git a/loader/loader.go b/loader/loader.go index 3022ac37f5..7371e8b5c9 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -501,7 +501,7 @@ func Dirs(paths []string) []string { unique[dir] = struct{}{} } - var u []string + u := make([]string, 0, len(unique)) for k := range unique { u = append(u, k) } diff --git a/profiler/profiler.go b/profiler/profiler.go index 6908f9f4b5..fc6bc21b0f 100644 --- a/profiler/profiler.go +++ b/profiler/profiler.go @@ -240,7 +240,7 @@ func aggregate(stats ...ExprStats) ExprStatsAggregated { NumRedo: stats[0].NumRedo, Location: stats[0].Location, } - var timeNs []int64 + timeNs := make([]int64, 0, len(stats)) for _, s := range stats { timeNs = append(timeNs, s.ExprTimeNs) } diff --git a/rego/rego_test.go b/rego/rego_test.go index 40dd15f58a..daad545251 100644 --- a/rego/rego_test.go +++ b/rego/rego_test.go @@ -1571,7 +1571,7 @@ func TestPreparedQueryGetModules(t *testing.T) { "c.rego": "package c\nr = 1", } - var regoArgs []func(r *Rego) + regoArgs := make([]func(r *Rego), 0, len(mods)+1) for name, mod := range mods { regoArgs = append(regoArgs, Module(name, mod)) diff --git a/sdk/test/test.go b/sdk/test/test.go index 6a157a9922..130d2a1405 100644 --- a/sdk/test/test.go +++ b/sdk/test/test.go @@ -111,7 +111,7 @@ func (s *Server) URL() string { // Builds the tarball from the supplied policies and prepares the layers in a temporary directory func (s *Server) buildBundles(ref string, policies map[string]string) error { // Prepare the modules to include in the bundle. Sort them so bundles are deterministic. - var modules []bundle.ModuleFile + modules := make([]bundle.ModuleFile, 0, len(policies)) for url, str := range policies { module, err := ast.ParseModule(url, str) if err != nil { @@ -364,7 +364,7 @@ func (s *Server) handleBundles(w http.ResponseWriter, r *http.Request) { } // Prepare the modules to include in the bundle. Sort them so bundles are deterministic. - var modules []bundle.ModuleFile + modules := make([]bundle.ModuleFile, 0, len(b)) for url, str := range b { module, err := ast.ParseModule(url, str) if err != nil { diff --git a/storage/disk/disk_test.go b/storage/disk/disk_test.go index 695dcff87b..6524edeb2a 100644 --- a/storage/disk/disk_test.go +++ b/storage/disk/disk_test.go @@ -202,7 +202,7 @@ func runTruncateTest(t *testing.T, dir string) { "/roles/policy.rego": "package bar\n p = 1", } - var files [][2]string + files := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { files = append(files, [2]string{name, content}) } @@ -334,7 +334,7 @@ func TestTruncateMultipleTxn(t *testing.T) { // additional data file at root archiveFiles["/data.json"] = `{"a": {"b": {"z": true}}}}` - var files [][2]string + files := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { files = append(files, [2]string{name, content}) } diff --git a/storage/inmem/inmem_test.go b/storage/inmem/inmem_test.go index 57df115b8a..0999c13934 100644 --- a/storage/inmem/inmem_test.go +++ b/storage/inmem/inmem_test.go @@ -334,7 +334,7 @@ func TestTruncateNoExistingPath(t *testing.T) { "/a/b/c/data.json": "[1,2,3]", } - var files [][2]string + files := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { files = append(files, [2]string{name, content}) } @@ -396,7 +396,7 @@ func TestTruncate(t *testing.T) { "/roles/policy.rego": "package bar\n p = 1", } - var files [][2]string + files := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { files = append(files, [2]string{name, content}) } @@ -490,7 +490,7 @@ func TestTruncateDataMergeError(t *testing.T) { "/data.json": `{"a": {"b": {"c": "bar"}}}`, } - var files [][2]string + files := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { files = append(files, [2]string{name, content}) } @@ -525,7 +525,7 @@ func TestTruncateBadRootWrite(t *testing.T) { "/roles/policy.rego": "package bar\n p = 1", } - var files [][2]string + files := make([][2]string, 0, len(archiveFiles)) for name, content := range archiveFiles { files = append(files, [2]string{name, content}) } diff --git a/test/wasm/cmd/wasm-rego-testgen/main.go b/test/wasm/cmd/wasm-rego-testgen/main.go index 53d2f6b218..2b6d8cf4a0 100644 --- a/test/wasm/cmd/wasm-rego-testgen/main.go +++ b/test/wasm/cmd/wasm-rego-testgen/main.go @@ -42,7 +42,7 @@ type compiledTestCase struct { } func compileTestCases(ctx context.Context, tests cases.Set) (*compiledTestCaseSet, error) { - var result []compiledTestCase + result := make([]compiledTestCase, 0, len(tests.Cases)) for _, tc := range tests.Cases { var numExpects int diff --git a/tester/reporter.go b/tester/reporter.go index 00038afb8a..74a7f4802d 100644 --- a/tester/reporter.go +++ b/tester/reporter.go @@ -38,7 +38,8 @@ func (r PrettyReporter) Report(ch chan *Result) error { dirty := false var pass, fail, skip, errs int - var results, failures []*Result + results := make([]*Result, 0, len(ch)) + var failures []*Result for tr := range ch { if tr.Pass() { @@ -161,7 +162,7 @@ type JSONReporter struct { // Report prints the test report to the reporter's output. func (r JSONReporter) Report(ch chan *Result) error { - var report []*Result + report := make([]*Result, 0, len(ch)) for tr := range ch { report = append(report, tr) } diff --git a/topdown/cidr.go b/topdown/cidr.go index e2dbfed479..e203021799 100644 --- a/topdown/cidr.go +++ b/topdown/cidr.go @@ -300,7 +300,7 @@ func evalNetCIDRMerge(networks []*net.IPNet) []*net.IPNet { return nil } - var ranges cidrBlockRanges + ranges := make(cidrBlockRanges, 0, len(networks)) // For each CIDR, create an IP range. Sort them and merge when possible. for _, network := range networks { diff --git a/topdown/strings.go b/topdown/strings.go index 718b747662..1a3d3d564c 100644 --- a/topdown/strings.go +++ b/topdown/strings.go @@ -405,7 +405,7 @@ func builtinReplaceN(a, b ast.Value) (ast.Value, error) { return nil, err } - var oldnewArr []string + oldnewArr := make([]string, 0, len(keys)*2) for _, k := range keys { keyVal, ok := k.Value.(ast.String) if !ok { diff --git a/topdown/tokens.go b/topdown/tokens.go index c23fa1b254..d01dde6197 100644 --- a/topdown/tokens.go +++ b/topdown/tokens.go @@ -309,7 +309,7 @@ func getKeysFromCertOrJWK(certificate string) ([]verificationKey, error) { return nil, fmt.Errorf("failed to parse a JWK key (set): %w", err) } - var keys []verificationKey + keys := make([]verificationKey, 0, len(jwks.Keys)) for _, k := range jwks.Keys { key, err := k.Materialize() if err != nil { diff --git a/types/types.go b/types/types.go index cd7138245c..a4b0ad383e 100644 --- a/types/types.go +++ b/types/types.go @@ -684,7 +684,7 @@ type FuncArgs struct { } func (a FuncArgs) String() string { - var buf []string + buf := make([]string, 0, len(a.Args)+1) for i := range a.Args { buf = append(buf, Sprint(a.Args[i])) }