refactor: remove 'version.Get()' and only use 'version.Raw()' (#445)

* refactor: remove version.Get() and use only version.Raw()

Move the v-prefix enforcement from Get() into init(), ensuring Version
is normalized at startup. This eliminates the confusing Get() vs Raw()
distinction — Raw() is now the single way to access the version.

* refactor: use strings.HasPrefix instead of regexp in ensurePrefix

Author: mwbrooks

cmd/doctor/doctor_test.go

@@ -38,7 +38,7 @@ import (
 )
 
 func TestDoctorCommand(t *testing.T) {
-	expectedCLIVersion := version.Get()
+	expectedCLIVersion := version.Raw()
 	expectedCredentials := types.SlackAuth{
 		TeamDomain: "team123",
 		TeamID:     "T123",

cmd/root.go

@@ -125,14 +125,14 @@ func NewRootCommand(clients *shared.ClientFactory, updateNotification *update.Up
 			})
 
 			// Check for an CLI update in the background while the command runs
-			updateNotification = update.New(clients, version.Get(), "SLACK_SKIP_UPDATE")
+			updateNotification = update.New(clients, version.Raw(), "SLACK_SKIP_UPDATE")
 			updateNotification.CheckForUpdateInBackground(ctx, false)
 			return nil
 		},
 		PersistentPostRunE: func(cmd *cobra.Command, args []string) error {
 			// TODO: since commands are moving to `*E` cobra lifecycle methods, this method may not be invoked if those earlier lifecycle methods return an error. Maybe move this to the cleanup() method below? but maybe this is OK, no need to prompt users to update if they encounter an error?
 			// when the command is `slack update`
-			return updateNotification.PrintAndPromptUpdates(cmd, version.Get())
+			return updateNotification.PrintAndPromptUpdates(cmd, version.Raw())
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return cmd.Help()
@@ -153,7 +153,7 @@ func Init(ctx context.Context) (*cobra.Command, *shared.ClientFactory) {
 
 	// Support `--version` by setting root command's `Version` and custom template.
 	// Add a newline to `SetVersionTemplate` to display correctly on terminals.
-	rootCmd.Version = version.Get()
+	rootCmd.Version = version.Raw()
 	rootCmd.SetVersionTemplate(versioncmd.Template() + "\n")
 
 	// Add subcommands (each subcommand may add their own child subcommands)

cmd/upgrade/upgrade.go

@@ -55,7 +55,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command {
 // When there are updates, the function will *not* print a message because the root command handles printing update notifications.
 func checkForUpdates(clients *shared.ClientFactory, cmd *cobra.Command) error {
 	ctx := cmd.Context()
-	updateNotification := update.New(clients, version.Get(), "SLACK_SKIP_UPDATE")
+	updateNotification := update.New(clients, version.Raw(), "SLACK_SKIP_UPDATE")
 
 	// TODO(@mbrooks) This update check is happening at the same time as the root command's `CheckForUpdateInBackground`.
 	//                The difference between the two is that this update check is forced while the root command runs every 24 hours.

cmd/version/version.go

@@ -81,7 +81,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command {
 
 func Template() string {
 	processName := cmdutil.GetProcessName()
-	version := version.Get()
+	version := version.Raw()
 
 	return fmt.Sprintf(style.Secondary("Using %s %s"), processName, version)
 }

internal/pkg/auth/login.go

@@ -147,7 +147,7 @@ func createNewAuth(ctx context.Context, apiClient api.APIInterface, authClient a
 		return types.SlackAuth{}, "", err
 	}
 
-	authExchangeRes, err := apiClient.ExchangeAuthTicket(ctx, authTicket, challengeCode, version.Get())
+	authExchangeRes, err := apiClient.ExchangeAuthTicket(ctx, authTicket, challengeCode, version.Raw())
 	if err != nil {
 		return types.SlackAuth{}, "", err
 	}
@@ -162,7 +162,7 @@ func requestAuthTicket(ctx context.Context, apiClient api.APIInterface, io iostr
 	span, ctx = opentracing.StartSpanFromContext(ctx, "requestAuthTicket")
 	defer span.Finish()
 
-	cliVersion := semver.MajorMinor(version.Get())
+	cliVersion := semver.MajorMinor(version.Raw())
 
 	// Get a ticket from Slack
 	if authTicketResult, err := apiClient.GenerateAuthTicket(ctx, cliVersion, noRotation); err != nil {
@@ -246,7 +246,7 @@ func LoginNoPrompt(ctx context.Context, clients *shared.ClientFactory, ticketArg
 
 	// existing ticket request, try to exchange
 	if ticketArg != "" && challengeCodeArg != "" {
-		authExchangeRes, err := clients.API().ExchangeAuthTicket(ctx, ticketArg, challengeCodeArg, version.Get())
+		authExchangeRes, err := clients.API().ExchangeAuthTicket(ctx, ticketArg, challengeCodeArg, version.Raw())
 		if err != nil || !authExchangeRes.IsReady {
 			return types.SlackAuth{}, "", err
 		}

internal/version/version.go

@@ -16,7 +16,6 @@ package version
 
 import (
 	"os"
-	"regexp"
 	"strings"
 )
 
@@ -33,23 +32,24 @@ func init() {
 	if envVersion := getVersionFromEnv(); envVersion != "" {
 		Version = envVersion
 	}
+	Version = ensurePrefix(Version)
 }
 
 // getVersionFromEnv will return the formatted version from EnvTestVersion otherwise "".
 func getVersionFromEnv() string {
 	return strings.Trim(os.Getenv(EnvTestVersion), " ")
 }
 
-// Get the version and format it (e.g. `v1.0.0`)
-func Get() string {
-	version := Version
-	if match, _ := regexp.MatchString(`^[^v]`, version); match {
-		version = "v" + version
+// ensurePrefix ensures that the version string has a "v" prefix.
+// Empty strings are returned as-is to avoid producing a bare "v".
+func ensurePrefix(v string) string {
+	if v != "" && !strings.HasPrefix(v, "v") {
+		return "v" + v
 	}
-	return version
+	return v
 }
 
-// Raw returns the raw, unformatted version
+// Raw returns the version
 func Raw() string {
 	return Version
 }

internal/version/version_test.go

@@ -27,8 +27,7 @@ func TestVersion(t *testing.T) {
 	assert.True(t, len(v) > 0, "some default value exists")
 }
 
-// Test overriding the Version with an environment variable
-func Test_Get(t *testing.T) {
+func Test_ensurePrefix(t *testing.T) {
 	tests := map[string]struct {
 		version  string
 		expected string
@@ -52,10 +51,7 @@ func Test_Get(t *testing.T) {
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			original := Version
-			defer func() { Version = original }()
-			Version = tc.version
-			assert.Equal(t, tc.expected, Get())
+			assert.Equal(t, tc.expected, ensurePrefix(tc.version))
 		})
 	}
 }