Skip to content

Commit

Permalink
Add prealloc linter check + linter fixes (#5139)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
philipaconrad authored Sep 15, 2022
1 parent 0f47970 commit b2d92a3
Show file tree
Hide file tree
Showing 31 changed files with 63 additions and 53 deletions.
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ linters:
- structcheck
- staticcheck
- gosimple
- prealloc
# - gosec # too many false positives
3 changes: 2 additions & 1 deletion ast/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 3 additions & 1 deletion ast/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
2 changes: 1 addition & 1 deletion ast/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
6 changes: 3 additions & 3 deletions ast/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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")
}
Expand Down
5 changes: 3 additions & 2 deletions build/generate-cli-docs/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down
10 changes: 5 additions & 5 deletions bundle/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
Expand Down Expand Up @@ -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})
}
Expand Down Expand Up @@ -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})
}
Expand Down Expand Up @@ -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})
}
Expand Down Expand Up @@ -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})
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions cmd/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}

Expand Down
4 changes: 2 additions & 2 deletions compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions compile/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
4 changes: 2 additions & 2 deletions format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/bundle/inspect/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/compiler/wasm/wasm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion internal/gqlparser/validator/imported_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions internal/gqlparser/validator/rules/fields_on_correct_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 4 additions & 3 deletions internal/presentation/presentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions internal/report/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
}

Expand Down
2 changes: 1 addition & 1 deletion internal/strings/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
2 changes: 1 addition & 1 deletion loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion profiler/profiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion rego/rego_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions sdk/test/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions storage/disk/disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
Expand Down Expand Up @@ -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})
}
Expand Down
Loading

0 comments on commit b2d92a3

Please sign in to comment.