fix(internal/semver): replace comparisons with x/mod/semver APIs (#3165)

Author: noahdietz

go.mod

@@ -22,6 +22,7 @@ require (
 	github.com/walle/targz v0.0.0-20140417120357-57fe4206da5a
 	github.com/yuin/goldmark v1.7.13
 	golang.org/x/exp v0.0.0-20250911091902-df9299821621
+	golang.org/x/mod v0.30.0
 	google.golang.org/genproto v0.0.0-20251103181224-f26f9409b101
 	google.golang.org/genproto/googleapis/api v0.0.0-20251029180050-ab9386a59fda
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda
@@ -261,7 +262,6 @@ require (
 	go.uber.org/zap v1.27.0 // indirect
 	golang.org/x/crypto v0.45.0 // indirect
 	golang.org/x/exp/typeparams v0.0.0-20251002181428-27f1f14c8bb9 // indirect
-	golang.org/x/mod v0.30.0 // indirect
 	golang.org/x/net v0.47.0 // indirect
 	golang.org/x/oauth2 v0.31.0 // indirect
 	golang.org/x/sync v0.18.0 // indirect

internal/semver/semver.go

@@ -18,10 +18,11 @@ package semver
 
 import (
 	"fmt"
-	"log/slog"
 	"regexp"
 	"strconv"
 	"strings"
+
+	"golang.org/x/mod/semver"
 )
 
 // Version represents a semantic version.
@@ -109,61 +110,6 @@ func Parse(versionString string) (*Version, error) {
 	return v, nil
 }
 
-// Compare returns an integer comparing two versions.
-// The result is -1, 0, or 1 depending on whether v is less than, equal to, or greater than other.
-func (v *Version) Compare(other *Version) int {
-	if v.Major < other.Major {
-		return -1
-	}
-	if v.Major > other.Major {
-		return 1
-	}
-	if v.Minor < other.Minor {
-		return -1
-	}
-	if v.Minor > other.Minor {
-		return 1
-	}
-	if v.Patch < other.Patch {
-		return -1
-	}
-	if v.Patch > other.Patch {
-		return 1
-	}
-	// a pre-release version is less than a non-pre-release version
-	if v.Prerelease != "" && other.Prerelease == "" {
-		return -1
-	}
-	if v.Prerelease == "" && other.Prerelease != "" {
-		return 1
-	}
-	// lexical comparison between prerelease type (e.g. "alpha" vs "beta")
-	if v.Prerelease < other.Prerelease {
-		return -1
-	}
-	if v.Prerelease > other.Prerelease {
-		return 1
-	}
-	// prerelease number (e.g. "alpha1" vs "alpha2")
-	// Note: Lack of prerelease number is considered lower precedence than
-	// the same prerelease when it has a number - https://semver.org/#spec-item-11.
-	if v.PrereleaseNumber == nil && other.PrereleaseNumber != nil {
-		return -1
-	}
-	if v.PrereleaseNumber != nil && other.PrereleaseNumber == nil {
-		return 1
-	}
-	if v.PrereleaseNumber != nil && other.PrereleaseNumber != nil {
-		if *v.PrereleaseNumber < *other.PrereleaseNumber {
-			return -1
-		}
-		if *v.PrereleaseNumber > *other.PrereleaseNumber {
-			return 1
-		}
-	}
-	return 0
-}
-
 // String formats a Version struct into a string.
 func (v *Version) String() string {
 	version := fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Patch)
@@ -219,22 +165,30 @@ func MaxVersion(versionStrings ...string) string {
 	if len(versionStrings) == 0 {
 		return ""
 	}
-	versions := make([]*Version, 0)
+	versions := make([]string, 0)
 	for _, versionString := range versionStrings {
-		v, err := Parse(versionString)
-		if err != nil {
-			slog.Warn("invalid version string", "version", v)
+		// Our client versions must not have a "v" prefix.
+		if strings.HasPrefix(versionString, "v") {
 			continue
 		}
-		versions = append(versions, v)
-	}
-	largest := versions[0]
-	for i := 1; i < len(versions); i++ {
-		if largest.Compare(versions[i]) < 0 {
-			largest = versions[i]
+
+		// Prepend "v" internally so that we can use [semver.IsValid] and
+		// [semver.Sort].
+		vPrefixedString := "v" + versionString
+		if !semver.IsValid(vPrefixedString) {
+			continue
 		}
+		versions = append(versions, vPrefixedString)
+	}
+
+	if len(versions) == 0 {
+		return ""
 	}
-	return largest.String()
+
+	semver.Sort(versions)
+
+	// Trim the "v" we prepended to make use of [semver].
+	return strings.TrimPrefix(versions[len(versions)-1], "v")
 }
 
 // ChangeLevel represents the level of change, corresponding to semantic versioning.

internal/semver/semver_test.go

@@ -252,158 +252,6 @@ func TestDeriveNext(t *testing.T) {
 	}
 }
 
-func TestCompare(t *testing.T) {
-	for _, test := range []struct {
-		name     string
-		versionA string
-		versionB string
-		want     int
-	}{
-		{
-			name:     "equal",
-			versionA: "1.2.3",
-			versionB: "1.2.3",
-			want:     0,
-		},
-		{
-			name:     "equal with pre-release",
-			versionA: "1.2.3-alpha",
-			versionB: "1.2.3-alpha",
-			want:     0,
-		},
-		{
-			name:     "equal with pre-release and number",
-			versionA: "1.2.3-alpha4",
-			versionB: "1.2.3-alpha4",
-			want:     0,
-		},
-		{
-			name:     "equal with pre-release and number, different separator",
-			versionA: "1.2.3-alpha4",
-			versionB: "1.2.3-alpha.4",
-			want:     0,
-		},
-		{
-			name:     "less than patch",
-			versionA: "1.2.3",
-			versionB: "1.2.4",
-			want:     -1,
-		},
-		{
-			name:     "less than minor",
-			versionA: "1.2.3",
-			versionB: "1.3.0",
-			want:     -1,
-		},
-		{
-			name:     "less than major",
-			versionA: "1.2.3",
-			versionB: "2.0.0",
-			want:     -1,
-		},
-		{
-			name:     "less than prerelease",
-			versionA: "1.2.3-alpha",
-			versionB: "1.2.3-beta",
-			want:     -1,
-		},
-		{
-			name:     "less than prerelease number",
-			versionA: "1.2.3-alpha1",
-			versionB: "1.2.3-alpha2",
-			want:     -1,
-		},
-		{
-			name:     "less than prerelease number with separator",
-			versionA: "1.2.3-alpha.1",
-			versionB: "1.2.3-alpha.2",
-			want:     -1,
-		},
-		{
-			name:     "less than prerelease against stable",
-			versionA: "1.2.3-alpha1",
-			versionB: "1.2.3",
-			want:     -1,
-		},
-		{
-			name:     "less than prerelease without number",
-			versionA: "1.2.3-alpha",
-			versionB: "1.2.3-alpha1",
-			want:     -1,
-		},
-		{
-			name:     "greater than patch",
-			versionA: "1.2.4",
-			versionB: "1.2.3",
-			want:     1,
-		},
-		{
-			name:     "greater than minor",
-			versionA: "1.3.0",
-			versionB: "1.2.3",
-			want:     1,
-		},
-		{
-			name:     "greater than major",
-			versionA: "2.0.0",
-			versionB: "1.2.3",
-			want:     1,
-		},
-		{
-			name:     "greater than prerelease",
-			versionA: "1.2.3-beta",
-			versionB: "1.2.3-alpha",
-			want:     1,
-		},
-		{
-			name:     "greater than prerelease number",
-			versionA: "1.2.3-alpha2",
-			versionB: "1.2.3-alpha1",
-			want:     1,
-		},
-		{
-			name:     "greater than prerelease number with separator",
-			versionA: "1.2.3-alpha.2",
-			versionB: "1.2.3-alpha.1",
-			want:     1,
-		},
-		{
-			name:     "greater than prerelease against stable",
-			versionA: "1.2.3",
-			versionB: "1.2.3-alpha1",
-			want:     1,
-		},
-		{
-			name:     "greater than prerelease without number",
-			versionA: "1.2.3-alpha1",
-			versionB: "1.2.3-alpha",
-			want:     1,
-		},
-		{
-			// Note: This is SemVer compliant, but may seem odd in practice.
-			name:     "greater prerelease version core than non-prerelease version core",
-			versionA: "1.2.3-alpha1",
-			versionB: "1.2.0",
-			want:     1,
-		},
-	} {
-		t.Run(test.name, func(t *testing.T) {
-			a, err := Parse(test.versionA)
-			if err != nil {
-				t.Fatalf("Parse() returned an error: %v", err)
-			}
-			b, err := Parse(test.versionB)
-			if err != nil {
-				t.Fatalf("Parse() returned an error: %v", err)
-			}
-			got := a.Compare(b)
-			if diff := cmp.Diff(test.want, got); diff != "" {
-				t.Errorf("mismatch (-want +got):\n%s", diff)
-			}
-		})
-	}
-}
-
 func TestMaxVersion(t *testing.T) {
 	for _, test := range []struct {
 		name     string