Skip to content

fix: replace hashicorp/go-version with golang.org/x/mod/semver #532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mwbrooks merged 2 commits into from
May 8, 2026
Merged

fix: replace hashicorp/go-version with golang.org/x/mod/semver #532

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd5075f
fix: replace hashicorp/go-version with golang.org/x/mod/semver
mwbrooks May 8, 2026
f152325
fix: include invalid value in semver error messages
mwbrooks May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ require (
github.com/cli/safeexec v1.0.1
github.com/google/uuid v1.6.0
github.com/gorilla/websocket v1.5.3
github.com/hashicorp/go-version v1.9.0
github.com/joho/godotenv v1.5.1
github.com/kyokomi/emoji/v2 v2.2.13
github.com/logrusorgru/aurora/v4 v4.0.0
Expand Down Expand Up @@ -303,6 +302,7 @@ require (
github.com/hashicorp/go-immutable-radix/v2 v2.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/hashicorp/go-retryablehttp v0.7.8 // indirect
github.com/hashicorp/go-version v1.9.0 // indirect
github.com/hashicorp/golang-lru v1.0.2 // indirect
github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
github.com/hexops/gotextdiff v1.0.3 // indirect
Expand Down
38 changes: 23 additions & 15 deletions internal/update/semver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,40 @@
package update

import (
"github.com/hashicorp/go-version"
"golang.org/x/mod/semver"

"github.com/slackapi/slack-cli/internal/slackerror"
)

// SemVerGreaterThan returns true if release is greater than current
func SemVerGreaterThan(release string, current string) (bool, error) {
releaseVersion, err := version.NewVersion(release)
if err != nil {
return false, slackerror.New(slackerror.ErrInvalidSemVer).WithRootCause(err)
r := ensureVPrefix(release)
c := ensureVPrefix(current)
if !semver.IsValid(r) {
return false, slackerror.New(slackerror.ErrInvalidSemVer)
Copy link
Copy Markdown
Member

@zimeg zimeg May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ thought: This might be surfaced in verbose logs too but I'm concerned that not wrapping this error can cause a less meaningful message for debugging?

Copy link
Copy Markdown
Member Author

@mwbrooks mwbrooks May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧠 I could update this with a verbose log.

I had the same concern about not wrapping the error, but there is no error returned anymore. We could manually create a descriptive error, I suppose. But the ErrInvalidSemVer already captures the core essence.

Copy link
Copy Markdown
Member

@zimeg zimeg May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mwbrooks Not blocking at all! I meant to suggest a "message" change with this but was also stumped about what'd be meaningful to include:

Suggested change
return false, slackerror.New(slackerror.ErrInvalidSemVer)
return false, slackerror.New(slackerror.ErrInvalidSemVer).
WithMessage("Value %s is not a semantic versioning", r)

Copy link
Copy Markdown
Member Author

@mwbrooks mwbrooks May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! Commit f152325 adds the .WithMessage to each clause.

}
currentVersion, err := version.NewVersion(current)
if err != nil {
return false, slackerror.New(slackerror.ErrInvalidSemVer).WithRootCause(err)
if !semver.IsValid(c) {
return false, slackerror.New(slackerror.ErrInvalidSemVer)
}
return releaseVersion.GreaterThan(currentVersion), nil
return semver.Compare(r, c) > 0, nil
Copy link
Copy Markdown
Member

@zimeg zimeg May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 praise: RC? Release candidate?

}

// SemVerLessThan returns true if release is less than current
func SemVerLessThan(release string, current string) (bool, error) {
releaseVersion, err := version.NewVersion(release)
if err != nil {
return false, slackerror.New(slackerror.ErrInvalidSemVer).WithRootCause(err)
r := ensureVPrefix(release)
c := ensureVPrefix(current)
if !semver.IsValid(r) {
return false, slackerror.New(slackerror.ErrInvalidSemVer)
}
if !semver.IsValid(c) {
return false, slackerror.New(slackerror.ErrInvalidSemVer)
}
currentVersion, err := version.NewVersion(current)
if err != nil {
return false, slackerror.New(slackerror.ErrInvalidSemVer).WithRootCause(err)
return semver.Compare(r, c) < 0, nil
}

func ensureVPrefix(v string) string {
if len(v) > 0 && v[0] != 'v' {
return "v" + v
}
return releaseVersion.LessThan(currentVersion), nil
return v
}
10 changes: 6 additions & 4 deletions test/testutil/testutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,21 @@ package testutil
import (
"regexp"

"github.com/hashicorp/go-version"
"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/spf13/cobra"
"golang.org/x/mod/semver"
)

// package + .test for root command
var rootName string = "cmd.test"

// ContainsSemVer checks if a string contains valid semver
func ContainsSemVer(s string) bool {
matcher := regexp.MustCompile(version.SemverRegexpRaw)
match := matcher.MatchString(s)
return match
if semver.IsValid("v"+s) || semver.IsValid(s) {
return true
}
matcher := regexp.MustCompile(`[0-9]+\.[0-9]+\.[0-9]+`)
return matcher.MatchString(s)
}

// Set the command's IOStream to the mocked IOStream
Expand Down
Loading