Use slices.CompareFunc for imports and annotations

cmd/inspect.go

@@ -431,7 +431,7 @@ func truncateStr(s string, maxLen int) string {
 	if len(s) < maxLen {
 		return s
 	}
-	return fmt.Sprintf("%v...", s[:maxLen-3])
+	return s[:maxLen-3] + "..."
 }
 
 func removeNewLines(s string) string {

internal/versioncheck/versioncheck.go

@@ -153,11 +153,11 @@ func createReleaseInfo(ghResp GitHubRelease) (*DataResponse, error) {
 		ghResp.TagName, runtime.GOOS, runtime.GOARCH)
 
 	if runtime.GOARCH == "arm64" {
-		downloadLink = fmt.Sprintf("%v_static", downloadLink)
+		downloadLink += "_static"
 	}
 
 	if strings.HasPrefix(runtime.GOOS, "win") {
-		downloadLink = fmt.Sprintf("%v.exe", downloadLink)
+		downloadLink += ".exe"
 	}
 
 	return &DataResponse{
@@ -181,7 +181,6 @@ func (dr *DataResponse) IsSet() bool {
 
 // Slice returns the dr as a slice of key-value string pairs. If dr is nil, this function returns an empty slice.
 func (dr *DataResponse) Slice() [][2]string {
-
 	if !dr.IsSet() {
 		return nil
 	}

v1/ast/compare.go

@@ -244,38 +244,6 @@ func sortOrder(x any) int {
 	panic(fmt.Sprintf("illegal value: %T", x))
 }
 
-func importsCompare(a, b []*Import) int {
-	minLen := min(len(b), len(a))
-	for i := range minLen {
-		if cmp := a[i].Compare(b[i]); cmp != 0 {
-			return cmp
-		}
-	}
-	if len(a) < len(b) {
-		return -1
-	}
-	if len(b) < len(a) {
-		return 1
-	}
-	return 0
-}
-
-func annotationsCompare(a, b []*Annotations) int {
-	minLen := min(len(b), len(a))
-	for i := range minLen {
-		if cmp := a[i].Compare(b[i]); cmp != 0 {
-			return cmp
-		}
-	}
-	if len(a) < len(b) {
-		return -1
-	}
-	if len(b) < len(a) {
-		return 1
-	}
-	return 0
-}
-
 func rulesCompare(a, b []*Rule) int {
 	minLen := min(len(b), len(a))
 	for i := range minLen {

v1/ast/compile.go

@@ -29,7 +29,8 @@ const CompileErrorLimitDefault = 10
 var (
 	errLimitReached = newErrorString(CompileErr, nil, "error limit reached")
 
-	doubleEq = Equal.Ref()
+	doubleEq     = Equal.Ref()
+	emptyPackage = &Package{Path: Ref{VarTerm("")}}
 )
 
 // Compiler contains the state of a compilation process.
@@ -3795,15 +3796,14 @@ func (qc *queryCompiler) checkKeywordOverrides(_ *QueryContext, body Body) (Body
 }
 
 func (qc *queryCompiler) resolveRefs(qctx *QueryContext, body Body) (Body, error) {
-
 	var globals map[Var]*usedRef
 
 	if qctx != nil {
 		pkg := qctx.Package
 		// Query compiler ought to generate a package if one was not provided and one or more imports were provided.
 		// The generated package name could even be an empty string to avoid conflicts (it doesn't have to be valid syntactically)
 		if pkg == nil && len(qctx.Imports) > 0 {
-			pkg = &Package{Path: RefTerm(VarTerm("")).Value.(Ref)}
+			pkg = emptyPackage
 		}
 		if pkg != nil {
 			var ruleExports []Ref

v1/ast/parser_test.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"slices"
 	"strconv"
 	"strings"
 	"testing"
@@ -7319,7 +7320,7 @@ include if input.fruits.name == "banana"
 				t.Fatalf("Expected %v comments but got %v", tc.expNumComments, len(mod.Comments))
 			}
 
-			if annotationsCompare(tc.expAnnotations, mod.Annotations) != 0 {
+			if slices.CompareFunc(tc.expAnnotations, mod.Annotations, (*Annotations).Compare) != 0 {
 				t.Fatalf("expected %v but got %v", tc.expAnnotations, mod.Annotations)
 			}
 		})
@@ -7647,7 +7648,7 @@ rule[x] := true if x := 1
 					t.Fatalf("No annotations for rule on row %v", rule.Location.Row)
 				}
 
-				if annotationsCompare(annotations, rule.Annotations) != 0 {
+				if slices.CompareFunc(annotations, rule.Annotations, (*Annotations).Compare) != 0 {
 					t.Fatalf("expected rule on row %d to have annotations:\n\n%v\n\nbut got:\n\n%v",
 						rule.Location.Row, annotations, rule.Annotations)
 				}
@@ -7733,7 +7734,7 @@ q := 1`
 	expAnnotations := [][]*Annotations{a1, a2, a3}
 
 	for i, rule := range pm.Rules {
-		if annotationsCompare(expAnnotations[i], rule.Annotations) != 0 {
+		if slices.CompareFunc(expAnnotations[i], rule.Annotations, (*Annotations).Compare) != 0 {
 			t.Fatalf("expected %v but got %v", expAnnotations[i], rule.Annotations)
 		}
 	}
@@ -7817,7 +7818,7 @@ q := 1`
 	expAnnotations := [][]*Annotations{a1, a2, a3}
 
 	for i, rule := range pm.Rules {
-		if annotationsCompare(expAnnotations[i], rule.Annotations) != 0 {
+		if slices.CompareFunc(expAnnotations[i], rule.Annotations, (*Annotations).Compare) != 0 {
 			t.Fatalf("expected %v but got %v", expAnnotations[i], rule.Annotations)
 		}
 	}

v1/ast/policy.go

@@ -338,10 +338,10 @@ func (mod *Module) Compare(other *Module) int {
 	if cmp := mod.Package.Compare(other.Package); cmp != 0 {
 		return cmp
 	}
-	if cmp := importsCompare(mod.Imports, other.Imports); cmp != 0 {
+	if cmp := slices.CompareFunc(mod.Imports, other.Imports, (*Import).Compare); cmp != 0 {
 		return cmp
 	}
-	if cmp := annotationsCompare(mod.Annotations, other.Annotations); cmp != 0 {
+	if cmp := slices.CompareFunc(mod.Annotations, other.Annotations, (*Annotations).Compare); cmp != 0 {
 		return cmp
 	}
 	return rulesCompare(mod.Rules, other.Rules)
@@ -612,7 +612,7 @@ func (rule *Rule) Compare(other *Rule) int {
 		return cmp
 	}
 
-	if cmp := annotationsCompare(rule.Annotations, other.Annotations); cmp != 0 {
+	if cmp := slices.CompareFunc(rule.Annotations, other.Annotations, (*Annotations).Compare); cmp != 0 {
 		return cmp
 	}
 
@@ -979,7 +979,7 @@ func (body Body) Contains(x *Expr) bool {
 
 // Equal returns true if this Body is equal to the other Body.
 func (body Body) Equal(other Body) bool {
-	return body.Compare(other) == 0
+	return slices.EqualFunc(body, other, (*Expr).Equal)
 }
 
 // Hash returns the hash code for the Body.
@@ -1708,10 +1708,10 @@ func (w *With) Compare(other *With) int {
 	} else if other == nil {
 		return 1
 	}
-	if cmp := Compare(w.Target, other.Target); cmp != 0 {
+	if cmp := w.Target.Value.Compare(other.Target.Value); cmp != 0 {
 		return cmp
 	}
-	return Compare(w.Value, other.Value)
+	return w.Value.Value.Compare(other.Value.Value)
 }
 
 // Copy returns a deep copy of w.

v1/download/testharness.go

@@ -438,7 +438,7 @@ func (t *testServer) handle(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(404)
 				return
 			}
-			buf.WriteString(string(bs))
+			buf.Write(bs)
 			w.Write(buf.Bytes())
 			return
 		}

v1/plugins/discovery/discovery.go

@@ -786,7 +786,7 @@ func mergeValuesAndListOverrides(dest map[string]any, src map[string]any, prefix
 
 		fullKey := k
 		if prefix != "" {
-			fullKey = fmt.Sprintf("%v.%v", prefix, k)
+			fullKey = prefix + "." + k
 		}
 
 		nextMap, ok := v.(map[string]any)

v1/topdown/encoding_bench_test.go

@@ -6,43 +6,60 @@ import (
 	"github.com/open-policy-agent/opa/v1/ast"
 )
 
-var (
-	unmarshalled = ast.ObjectTerm(
+// 7447 ns/op	   22984 B/op	     142 allocs/op
+// 7243 ns/op	   22855 B/op	     139 allocs/op
+func BenchmarkYAMLMarshal(b *testing.B) {
+	obj := ast.ObjectTerm(
 		ast.Item(ast.InternedTerm("foo"), ast.ObjectTerm(
-			ast.Item(ast.InternedTerm("bar"), ast.ArrayTerm(ast.InternedTerm("baz"), ast.InternedTerm("qux"))),
+			ast.Item(ast.InternedTerm("bar"), ast.ArrayTerm(
+				ast.InternedTerm("baz"),
+				ast.InternedTerm("qux"),
+			)),
 			ast.Item(ast.InternedTerm("num"), ast.InternedTerm(42)),
 		)),
 	)
-	marshalled = `foo:
+	expect := ast.InternedTerm(`foo:
   bar:
   - baz
   - qux
   num: 42
-`
-)
+`)
 
-// 7447 ns/op	   22984 B/op	     142 allocs/op
-// 7343 ns/op	   22872 B/op	     140 allocs/op
-func BenchmarkYAMLMarshal(b *testing.B) {
-	expect := ast.InternedTerm(marshalled)
-	operands := []*ast.Term{unmarshalled}
+	operands := []*ast.Term{obj}
+	bctx := BuiltinContext{}
 	iter := eqIter(expect)
 
 	for b.Loop() {
-		if err := builtinYAMLMarshal(BuiltinContext{}, operands, iter); err != nil {
+		if err := builtinYAMLMarshal(bctx, operands, iter); err != nil {
 			b.Fatal(err)
 		}
 	}
 }
 
 // 5393 ns/op	   11066 B/op	     146 allocs/op
-// 5210 ns/op	   10980 B/op	     144 allocs/op
 func BenchmarkYAMLUnmarshal(b *testing.B) {
-	operands := []*ast.Term{ast.InternedTerm(marshalled)}
-	iter := eqIter(unmarshalled)
+	yamlTerm := ast.InternedTerm(`foo:
+  bar:
+  - baz
+  - qux
+  num: 42
+`)
+	expect := ast.ObjectTerm(
+		ast.Item(ast.InternedTerm("foo"), ast.ObjectTerm(
+			ast.Item(ast.InternedTerm("bar"), ast.ArrayTerm(
+				ast.InternedTerm("baz"),
+				ast.InternedTerm("qux"),
+			)),
+			ast.Item(ast.InternedTerm("num"), ast.InternedTerm(42)),
+		)),
+	)
+
+	operands := []*ast.Term{yamlTerm}
+	bctx := BuiltinContext{}
+	iter := eqIter(expect)
 
 	for b.Loop() {
-		if err := builtinYAMLUnmarshal(BuiltinContext{}, operands, iter); err != nil {
+		if err := builtinYAMLUnmarshal(bctx, operands, iter); err != nil {
 			b.Fatal(err)
 		}
 	}