gopls/internal/util/moremaps: use Sorted throughout gopls

This CL (written in passing while thinking about modernizers)
replaces each place that allocates a slice of map keys, sorts them,
and ranges over the slice, with "for k, v := range moremaps.Sorted(m)".

It also adds moremaps.SortedFunc for symmetry.

Updates golang/go#68598

Change-Id: I526ddf2398e7cee77f8369fc7a977e6f64727f27
Reviewed-on: https://go-review.googlesource.com/c/tools/+/639135
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Commit-Queue: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/internal/cache/analysis.go

@@ -490,14 +490,7 @@ func (an *analysisNode) summaryHash() file.Hash {
 		fmt.Fprintf(hasher, "compiles: %t\n", an.summary.Compiles)
 
 		// action results: errors and facts
-		actions := an.summary.Actions
-		names := make([]string, 0, len(actions))
-		for name := range actions {
-			names = append(names, name)
-		}
-		sort.Strings(names)
-		for _, name := range names {
-			summary := actions[name]
+		for name, summary := range moremaps.Sorted(an.summary.Actions) {
 			fmt.Fprintf(hasher, "action %s\n", name)
 			if summary.Err != "" {
 				fmt.Fprintf(hasher, "error %s\n", summary.Err)

gopls/internal/cache/check.go

@@ -33,6 +33,7 @@ import (
 	"golang.org/x/tools/gopls/internal/label"
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/util/bug"
+	"golang.org/x/tools/gopls/internal/util/moremaps"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	"golang.org/x/tools/internal/analysisinternal"
 	"golang.org/x/tools/internal/event"
@@ -1314,13 +1315,7 @@ func typerefsKey(id PackageID, imports map[ImportPath]*metadata.Package, compile
 
 	fmt.Fprintf(hasher, "typerefs: %s\n", id)
 
-	importPaths := make([]string, 0, len(imports))
-	for impPath := range imports {
-		importPaths = append(importPaths, string(impPath))
-	}
-	sort.Strings(importPaths)
-	for _, importPath := range importPaths {
-		imp := imports[ImportPath(importPath)]
+	for importPath, imp := range moremaps.Sorted(imports) {
 		// TODO(rfindley): strength reduce the typerefs.Export API to guarantee
 		// that it only depends on these attributes of dependencies.
 		fmt.Fprintf(hasher, "import %s %s %s", importPath, imp.ID, imp.Name)
@@ -1431,13 +1426,8 @@ func localPackageKey(inputs *typeCheckInputs) file.Hash {
 	fmt.Fprintf(hasher, "go %s\n", inputs.goVersion)
 
 	// import map
-	importPaths := make([]string, 0, len(inputs.depsByImpPath))
-	for impPath := range inputs.depsByImpPath {
-		importPaths = append(importPaths, string(impPath))
-	}
-	sort.Strings(importPaths)
-	for _, impPath := range importPaths {
-		fmt.Fprintf(hasher, "import %s %s", impPath, string(inputs.depsByImpPath[ImportPath(impPath)]))
+	for impPath, depID := range moremaps.Sorted(inputs.depsByImpPath) {
+		fmt.Fprintf(hasher, "import %s %s", impPath, depID)
 	}
 
 	// file names and contents

gopls/internal/cache/typerefs/packageset.go

@@ -7,11 +7,11 @@ package typerefs
 import (
 	"fmt"
 	"math/bits"
-	"sort"
 	"strings"
 	"sync"
 
 	"golang.org/x/tools/gopls/internal/cache/metadata"
+	"golang.org/x/tools/gopls/internal/util/moremaps"
 )
 
 // PackageIndex stores common data to enable efficient representation of
@@ -123,13 +123,7 @@ func (s *PackageSet) Contains(id metadata.PackageID) bool {
 
 // Elems calls f for each element of the set in ascending order.
 func (s *PackageSet) Elems(f func(IndexID)) {
-	blockIndexes := make([]int, 0, len(s.sparse))
-	for k := range s.sparse {
-		blockIndexes = append(blockIndexes, k)
-	}
-	sort.Ints(blockIndexes)
-	for _, i := range blockIndexes {
-		v := s.sparse[i]
+	for i, v := range moremaps.Sorted(s.sparse) {
 		for b := 0; b < blockSize; b++ {
 			if (v & (1 << b)) != 0 {
 				f(IndexID(i*blockSize + b))

gopls/internal/debug/template_test.go

@@ -15,7 +15,6 @@ import (
 	"html/template"
 	"os"
 	"runtime"
-	"sort"
 	"strings"
 	"testing"
 
@@ -24,6 +23,7 @@ import (
 	"golang.org/x/tools/gopls/internal/cache"
 	"golang.org/x/tools/gopls/internal/debug"
 	"golang.org/x/tools/gopls/internal/file"
+	"golang.org/x/tools/gopls/internal/util/moremaps"
 	"golang.org/x/tools/internal/testenv"
 )
 
@@ -110,13 +110,7 @@ func TestTemplates(t *testing.T) {
 		}
 	}
 	// now check all the known templates, in alphabetic order, for determinacy
-	keys := []string{}
-	for k := range templates {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	for _, k := range keys {
-		v := templates[k]
+	for k, v := range moremaps.Sorted(templates) {
 		// the FuncMap is an annoyance; should not be necessary
 		if err := templatecheck.CheckHTML(v.tmpl, v.data); err != nil {
 			t.Errorf("%s: %v", k, err)

gopls/internal/fuzzy/input_test.go

@@ -6,6 +6,7 @@ package fuzzy_test
 
 import (
 	"bytes"
+	"slices"
 	"sort"
 	"testing"
 
@@ -83,17 +84,9 @@ func TestWordSplit(t *testing.T) {
 }
 
 func diffStringLists(a, b []string) bool {
-	if len(a) != len(b) {
-		return false
-	}
 	sort.Strings(a)
 	sort.Strings(b)
-	for i := range a {
-		if a[i] != b[i] {
-			return false
-		}
-	}
-	return true
+	return slices.Equal(a, b)
 }
 
 var lastSegmentSplitTests = []struct {

gopls/internal/golang/addtest.go

@@ -17,7 +17,6 @@ import (
 	"go/types"
 	"os"
 	"path/filepath"
-	"sort"
 	"strconv"
 	"strings"
 	"text/template"
@@ -29,6 +28,7 @@ import (
 	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/protocol"
 	goplsastutil "golang.org/x/tools/gopls/internal/util/astutil"
+	"golang.org/x/tools/gopls/internal/util/moremaps"
 	"golang.org/x/tools/internal/imports"
 	"golang.org/x/tools/internal/typesinternal"
 )
@@ -727,15 +727,10 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
 			}
 		} else {
 			importsBuffer.WriteString("\nimport(")
-			// Loop over the map in sorted order ensures deterministic outcome.
-			paths := make([]string, 0, len(extraImports))
-			for key := range extraImports {
-				paths = append(paths, key)
-			}
-			sort.Strings(paths)
-			for _, path := range paths {
+			// Sort for determinism.
+			for path, name := range moremaps.Sorted(extraImports) {
 				importsBuffer.WriteString("\n\t")
-				if name := extraImports[path]; name != "" {
+				if name != "" {
 					importsBuffer.WriteString(name + " ")
 				}
 				importsBuffer.WriteString(fmt.Sprintf("\"%s\"", path))

gopls/internal/util/moremaps/maps.go

@@ -42,21 +42,30 @@ func ValueSlice[M ~map[K]V, K comparable, V any](m M) []V {
 
 // SameKeys reports whether x and y have equal sets of keys.
 func SameKeys[K comparable, V1, V2 any](x map[K]V1, y map[K]V2) bool {
-	if len(x) != len(y) {
-		return false
-	}
-	for k := range x {
-		if _, ok := y[k]; !ok {
-			return false
-		}
-	}
-	return true
+	ignoreValues := func(V1, V2) bool { return true }
+	return maps.EqualFunc(x, y, ignoreValues)
 }
 
 // Sorted returns an iterator over the entries of m in key order.
 func Sorted[M ~map[K]V, K cmp.Ordered, V any](m M) iter.Seq2[K, V] {
+	// TODO(adonovan): use maps.Sorted if proposal #68598 is accepted.
+	return func(yield func(K, V) bool) {
+		keys := KeySlice(m)
+		slices.Sort(keys)
+		for _, k := range keys {
+			if !yield(k, m[k]) {
+				break
+			}
+		}
+	}
+}
+
+// SortedFunc returns an iterator over the entries of m in key order.
+func SortedFunc[M ~map[K]V, K comparable, V any](m M, cmp func(x, y K) int) iter.Seq2[K, V] {
+	// TODO(adonovan): use maps.SortedFunc if proposal #68598 is accepted.
 	return func(yield func(K, V) bool) {
-		keys := slices.Sorted(maps.Keys(m))
+		keys := KeySlice(m)
+		slices.SortFunc(keys, cmp)
 		for _, k := range keys {
 			if !yield(k, m[k]) {
 				break