mod/module: add Version.Compare

Which in turn allows us to use it directly in call sites instead of
module.Sort over a slice of Version. In some cases we still just do
slices.SortFunc, but in others we can combine with other operations
to do slices.SortedFunc to make the code a bit nicer.

No need to delete the func and break any existing users,
but we can mark it as deprecated and hint to gopls that it should
be inlined as an easy fix for any users.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I2e782cfa699cab1d551d6493c3d859cceb8f9c6e
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1210875
Reviewed-by: Roger Peppe <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mvdan

Diff for: internal/mod/modload/tidy.go

@@ -243,8 +243,7 @@ func (ld *loader) resolveDependencies(ctx context.Context, rootPkgPaths []string
 		for m, p := range modAddedBy {
 			logf("added: %v (by %v)", m, p.ImportPath())
 		}
-		toAdd := slices.Collect(maps.Keys(modAddedBy))
-		module.Sort(toAdd) // to make errors deterministic
+		toAdd := slices.SortedFunc(maps.Keys(modAddedBy), module.Version.Compare) // to make errors deterministic
 		oldRs := rs
 		var err error
 		rs, err = ld.updateRoots(ctx, rs, pkgs, toAdd)
@@ -406,7 +405,7 @@ func (ld *loader) updateRoots(ctx context.Context, rs *modrequirements.Requireme
 		}
 	}
 	if needSort {
-		module.Sort(roots)
+		slices.SortFunc(roots, module.Version.Compare)
 	}
 
 	// "Each root appears only once, at the selected version of its path ….”
@@ -627,7 +626,7 @@ func (ld *loader) tidyRoots(ctx context.Context, old *modrequirements.Requiremen
 		queue = append(queue, pkg)
 		queued[pkg] = true
 	}
-	module.Sort(roots)
+	slices.SortFunc(roots, module.Version.Compare)
 	tidy := modrequirements.NewRequirements(ld.mainModule.Path(), ld.registry, roots, old.DefaultMajorVersions())
 
 	for len(queue) > 0 {
@@ -659,7 +658,7 @@ func (ld *loader) tidyRoots(ctx context.Context, old *modrequirements.Requiremen
 		}
 
 		if tidyRoots := tidy.RootModules(); len(roots) > len(tidyRoots) {
-			module.Sort(roots)
+			slices.SortFunc(roots, module.Version.Compare)
 			tidy = modrequirements.NewRequirements(ld.mainModule.Path(), ld.registry, roots, tidy.DefaultMajorVersions())
 		}
 	}

Diff for: internal/mod/modload/update.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"fmt"
 	"io/fs"
+	"maps"
 	"runtime"
+	"slices"
 	"strings"
 	"sync/atomic"
 
@@ -70,10 +72,8 @@ func UpdateVersions(ctx context.Context, fsys fs.FS, modRoot string, reg Registr
 			newVersions = append(newVersions, v)
 		}
 	}
-	for _, v := range mversionsMap {
-		newVersions = append(newVersions, v)
-	}
-	module.Sort(newVersions)
+	newVersions = slices.AppendSeq(newVersions, maps.Values(mversionsMap))
+	slices.SortFunc(newVersions, module.Version.Compare)
 	rs = modrequirements.NewRequirements(mf.QualifiedModule(), reg, newVersions, mf.DefaultMajorVersions())
 	g, err = rs.Graph(ctx)
 	if err != nil {

Diff for: mod/modfile/modfile.go

@@ -504,7 +504,7 @@ func parse(modfile []byte, filename string, strict bool) (*File, error) {
 		mf.defaultMajorVersions = defaultMajorVersions
 	}
 	mf.versions = versions[:len(versions):len(versions)]
-	module.Sort(mf.versions)
+	slices.SortFunc(mf.versions, module.Version.Compare)
 	return mf, nil
 }
 

Diff for: mod/modregistrytest/registry.go

@@ -345,11 +345,7 @@ func writeError(w http.ResponseWriter, err error) {
 func pushContent(ctx context.Context, client *modregistry.Client, mods map[module.Version]*moduleContent) error {
 	pushed := make(map[module.Version]bool)
 	// Iterate over modules in deterministic order.
-	// TODO(mvdan): if Version.Compare is exposed, then here we could just do:
-	// for _, v := range slices.SortedFunc(maps.Keys(mods), module.Version.Compare) {
-	vs := slices.Collect(maps.Keys(mods))
-	module.Sort(vs)
-	for _, v := range vs {
+	for _, v := range slices.SortedFunc(maps.Keys(mods), module.Version.Compare) {
 		err := visitDepthFirst(mods, v, func(v module.Version, m *moduleContent) error {
 			if pushed[v] {
 				return nil

Diff for: mod/module/module.go

@@ -113,6 +113,21 @@ func (m Version) Equal(m1 Version) bool {
 	return m.path == m1.path && m.version == m1.version
 }
 
+func (m Version) Compare(m1 Version) int {
+	if c := cmp.Compare(m.path, m1.path); c != 0 {
+		return c
+	}
+	// To help go.sum formatting, allow version/file.
+	// Compare semver prefix by semver rules,
+	// file by string order.
+	va, fa, _ := strings.Cut(m.version, "/")
+	vb, fb, _ := strings.Cut(m1.version, "/")
+	if c := semver.Compare(va, vb); c != 0 {
+		return c
+	}
+	return cmp.Compare(fa, fb)
+}
+
 // BasePath returns the path part of m without its major version suffix.
 func (m Version) BasePath() string {
 	if m.IsLocal() {
@@ -246,19 +261,10 @@ func NewVersion(path string, version string) (Version, error) {
 // The Version fields are interpreted as semantic versions (using semver.Compare)
 // optionally followed by a tie-breaking suffix introduced by a slash character,
 // like in "v0.0.1/module.cue".
+//
+// Deprecated: use [slices.SortFunc] with [Version.Compare].
+//
+//go:fix inline
 func Sort(list []Version) {
-	slices.SortFunc(list, func(a, b Version) int {
-		if c := cmp.Compare(a.path, b.path); c != 0 {
-			return c
-		}
-		// To help go.sum formatting, allow version/file.
-		// Compare semver prefix by semver rules,
-		// file by string order.
-		va, fa, _ := strings.Cut(a.version, "/")
-		vb, fb, _ := strings.Cut(b.version, "/")
-		if c := semver.Compare(va, vb); c != 0 {
-			return c
-		}
-		return cmp.Compare(fa, fb)
-	})
+	slices.SortFunc(list, Version.Compare)
 }