Add perfsprint linter (open-policy-agent#7334)

And update code to conform to the rule.

- Replace unnecessary fmt.Sprintf with string concatenation
- Replace fmt.Sprint with more efficient strconv.Itoa
- Replace static fmt.Errorf calls with more efficient errors.New

Thanks @srenatus for pushing me down this rabbit hole!

Signed-off-by: Anders Eknert <[email protected]>

Author: anderseknert

.golangci.yaml

@@ -143,6 +143,15 @@ linters-settings:
   govet:
     enable:
       - deepequalerrors
+  perfsprint:
+    # only rule disabled by default, but it's a good one
+    err-error: true
+  revive:
+    rules:
+    # this mainly complains about us using min/max for variable names,
+    # which seems like an unlikely source of actual issues
+     - name: redefines-builtin-id
+       disabled: true
 
 linters:
   disable-all: true
@@ -164,4 +173,5 @@ linters:
     - prealloc
     - unconvert
     - copyloopvar
+    - perfsprint
     # - gosec # too many false positives

ast/parser_ext.go

@@ -5,6 +5,7 @@
 package ast
 
 import (
+	"errors"
 	"fmt"
 
 	v1 "github.com/open-policy-agent/opa/v1/ast"
@@ -279,7 +280,7 @@ func ParseStatement(input string) (Statement, error) {
 		return nil, err
 	}
 	if len(stmts) != 1 {
-		return nil, fmt.Errorf("expected exactly one statement")
+		return nil, errors.New("expected exactly one statement")
 	}
 	return stmts[0], nil
 }

build/generate-cli-docs/generate.go

@@ -88,7 +88,7 @@ func main() {
 			removed++
 			continue
 		}
-		document = append(document, fmt.Sprintf("%s\n", str))
+		document = append(document, str+"\n")
 	}
 
 	withHeader := fmt.Sprintf("%s%s", fileHeader, strings.Join(document, ""))

cmd/bench.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"math"
@@ -190,7 +191,7 @@ func benchMain(args []string, params benchmarkCommandParams, w io.Writer, r benc
 			if err != nil {
 				return err
 			} else if len(result) == 0 && params.fail {
-				return fmt.Errorf("undefined result")
+				return errors.New("undefined result")
 			}
 			return nil
 		}
@@ -207,7 +208,7 @@ func benchMain(args []string, params benchmarkCommandParams, w io.Writer, r benc
 			if err != nil {
 				return err
 			} else if len(result.Queries) == 0 && params.fail {
-				return fmt.Errorf("undefined result")
+				return errors.New("undefined result")
 			}
 			return nil
 		}
@@ -307,7 +308,7 @@ func benchE2E(ctx context.Context, args []string, params benchmarkCommandParams,
 	// We fix the issue here by binding port 0; this will result in the OS
 	// allocating us an open port.
 	rtParams := runtime.Params{
-		Addrs:                  &[]string{fmt.Sprintf("%s:0", host)},
+		Addrs:                  &[]string{host + ":0"},
 		Paths:                  paths,
 		Logger:                 logger,
 		BundleMode:             params.bundlePaths.isFlagSet(),
@@ -365,7 +366,7 @@ func benchE2E(ctx context.Context, args []string, params benchmarkCommandParams,
 	}
 	// Check for port still being unbound after retry loop.
 	if port == 0 {
-		return fmt.Errorf("unable to bind a port for bench testing")
+		return errors.New("unable to bind a port for bench testing")
 	}
 
 	query, err := readQuery(params, args)
@@ -399,7 +400,7 @@ func benchE2E(ctx context.Context, args []string, params benchmarkCommandParams,
 	} else {
 		_, err := ast.ParseBody(query)
 		if err != nil {
-			return fmt.Errorf("error occurred while parsing query")
+			return errors.New("error occurred while parsing query")
 		}
 
 		if strings.HasPrefix(query, "data.") {
@@ -536,7 +537,7 @@ func e2eQuery(params benchmarkCommandParams, url string, input map[string]interf
 		}
 
 		if result.Result == nil && params.fail {
-			return nil, fmt.Errorf("undefined result")
+			return nil, errors.New("undefined result")
 		}
 
 		return result.Metrics, nil
@@ -549,31 +550,31 @@ func e2eQuery(params benchmarkCommandParams, url string, input map[string]interf
 
 	if params.fail {
 		if result.Result == nil {
-			return nil, fmt.Errorf("undefined result")
+			return nil, errors.New("undefined result")
 		}
 
 		i := *result.Result
 
 		peResult, ok := i.(map[string]interface{})
 		if !ok {
-			return nil, fmt.Errorf("invalid result for compile response")
+			return nil, errors.New("invalid result for compile response")
 		}
 
 		if len(peResult) == 0 {
-			return nil, fmt.Errorf("undefined result")
+			return nil, errors.New("undefined result")
 		}
 
 		if val, ok := peResult["queries"]; ok {
 			queries, ok := val.([]interface{})
 			if !ok {
-				return nil, fmt.Errorf("invalid result for output of partial evaluation")
+				return nil, errors.New("invalid result for output of partial evaluation")
 			}
 
 			if len(queries) == 0 {
-				return nil, fmt.Errorf("undefined result")
+				return nil, errors.New("undefined result")
 			}
 		} else {
-			return nil, fmt.Errorf("invalid result for output of partial evaluation")
+			return nil, errors.New("invalid result for output of partial evaluation")
 		}
 	}
 
@@ -606,14 +607,14 @@ func renderBenchmarkResult(params benchmarkCommandParams, br testing.BenchmarkRe
 		fmt.Fprintf(w, "\n")
 	default:
 		data := [][]string{
-			{"samples", fmt.Sprintf("%d", br.N)},
+			{"samples", strconv.Itoa(br.N)},
 			{"ns/op", prettyFormatFloat(float64(br.T.Nanoseconds()) / float64(br.N))},
 		}
 		if params.benchMem {
 			data = append(data, []string{
-				"B/op", fmt.Sprintf("%d", br.AllocedBytesPerOp()),
+				"B/op", strconv.FormatInt(br.AllocedBytesPerOp(), 10),
 			}, []string{
-				"allocs/op", fmt.Sprintf("%d", br.AllocsPerOp()),
+				"allocs/op", strconv.FormatInt(br.AllocsPerOp(), 10),
 			})
 		}
 

cmd/bench_test.go

@@ -498,7 +498,7 @@ func validateBenchMainPrep(t *testing.T, args []string, params benchmarkCommandP
 		}
 
 		if len(rs) == 0 {
-			return testing.BenchmarkResult{}, fmt.Errorf("expected result, got none")
+			return testing.BenchmarkResult{}, errors.New("expected result, got none")
 		}
 
 		return testing.BenchmarkResult{}, nil

cmd/build.go

@@ -7,6 +7,7 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -232,7 +233,7 @@ against OPA v0.22.0:
 `,
 		PreRunE: func(Cmd *cobra.Command, args []string) error {
 			if len(args) == 0 {
-				return fmt.Errorf("expected at least one path")
+				return errors.New("expected at least one path")
 			}
 			return env.CmdFlags.CheckEnvironmentVariables(Cmd)
 		},
@@ -293,7 +294,7 @@ func dobuild(params buildParams, args []string) error {
 	}
 
 	if (bvc != nil || bsc != nil) && !params.bundleMode {
-		return fmt.Errorf("enable bundle mode (ie. --bundle) to verify or sign bundle files or directories")
+		return errors.New("enable bundle mode (ie. --bundle) to verify or sign bundle files or directories")
 	}
 
 	var capabilities *ast.Capabilities

cmd/build_test.go

@@ -4,12 +4,14 @@ import (
 	"archive/tar"
 	"compress/gzip"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path"
 	"path/filepath"
 	"reflect"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -637,7 +639,7 @@ p contains 1
 f(x) if { p[x] }
 `,
 			},
-			err: fmt.Errorf("plan compilation requires at least one entrypoint"),
+			err: errors.New("plan compilation requires at least one entrypoint"),
 		},
 	}
 	for _, tc := range tests {
@@ -942,7 +944,7 @@ p2 := 2
 				tr := tar.NewReader(gr)
 
 				expManifest := strings.ReplaceAll(tc.manifest, "%REGO_VERSION%",
-					fmt.Sprintf("%d", ast.DefaultRegoVersion.Int()))
+					strconv.Itoa(ast.DefaultRegoVersion.Int()))
 
 				found := false
 				for {
@@ -1868,7 +1870,7 @@ q contains 1 if {
 								}
 								expVal = strings.ReplaceAll(expVal, "%ROOT%", root)
 								expVal = strings.ReplaceAll(expVal, "%DEFAULT_REGO_VERSION%",
-									fmt.Sprintf("%d", ast.DefaultRegoVersion.Int()))
+									strconv.Itoa(ast.DefaultRegoVersion.Int()))
 								if string(b) != expVal {
 									t.Fatalf("expected %v:\n\n%v\n\nbut got:\n\n%v", expName, expVal, string(b))
 								}

cmd/check.go

@@ -5,6 +5,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"io/fs"
@@ -174,7 +175,7 @@ func init() {
 
 		PreRunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) == 0 {
-				return fmt.Errorf("specify at least one file")
+				return errors.New("specify at least one file")
 			}
 			return env.CmdFlags.CheckEnvironmentVariables(cmd)
 		},

cmd/eval.go

@@ -155,7 +155,7 @@ func validateEvalParams(p *evalCommandParams, cmdArgs []string) error {
 
 	if p.optimizationLevel > 0 {
 		if len(p.dataPaths.v) > 0 && p.bundlePaths.isFlagSet() {
-			return fmt.Errorf("specify either --data or --bundle flag with optimization level greater than 0")
+			return errors.New("specify either --data or --bundle flag with optimization level greater than 0")
 		}
 	}
 
@@ -699,7 +699,7 @@ func setupEval(args []string, params evalCommandParams) (*evalContext, error) {
 	if params.strictBuiltinErrors {
 		regoArgs = append(regoArgs, rego.StrictBuiltinErrors(true))
 		if params.showBuiltinErrors {
-			return nil, fmt.Errorf("cannot use --show-builtin-errors with --strict-builtin-errors, --strict-builtin-errors will return the first built-in error encountered immediately")
+			return nil, errors.New("cannot use --show-builtin-errors with --strict-builtin-errors, --strict-builtin-errors will return the first built-in error encountered immediately")
 		}
 	}
 

cmd/eval_test.go

@@ -619,17 +619,17 @@ func testReadParamWithSchemaDir(input string, inputSchema string) error {
 		}
 
 		if schemaSet == nil {
-			err = fmt.Errorf("Schema set is empty")
+			err = errors.New("Schema set is empty")
 			return
 		}
 
 		if schemaSet.Get(ast.MustParseRef("schema.input")) == nil {
-			err = fmt.Errorf("Expected schema for input in schemaSet but got none")
+			err = errors.New("Expected schema for input in schemaSet but got none")
 			return
 		}
 
 		if schemaSet.Get(ast.MustParseRef(`schema.kubernetes["data-schema"]`)) == nil {
-			err = fmt.Errorf("Expected schemas for data in schemaSet but got none")
+			err = errors.New("Expected schemas for data in schemaSet but got none")
 			return
 		}