From d6b0749ea2f55ab1e00883ae98275d6316194894 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Fri, 16 Jul 2021 14:52:05 +0200
Subject: [PATCH] Tweak error messages and add more tests for extension install
 name check

---
 pkg/cmd/extensions/command.go      | 40 +++++++++------
 pkg/cmd/extensions/command_test.go | 79 +++++++++++++++++++++++++++++-
 2 files changed, 102 insertions(+), 17 deletions(-)

diff --git a/pkg/cmd/extensions/command.go b/pkg/cmd/extensions/command.go
index d06d9dba8..2bdf75b41 100644
--- a/pkg/cmd/extensions/command.go
+++ b/pkg/cmd/extensions/command.go
@@ -10,6 +10,7 @@ import (
 	"github.com/cli/cli/git"
 	"github.com/cli/cli/internal/ghrepo"
 	"github.com/cli/cli/pkg/cmdutil"
+	"github.com/cli/cli/pkg/extensions"
 	"github.com/cli/cli/utils"
 	"github.com/spf13/cobra"
 )
@@ -73,28 +74,14 @@ func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command {
 					}
 					return m.InstallLocal(wd)
 				}
+
 				repo, err := ghrepo.FromFullName(args[0])
 				if err != nil {
 					return err
 				}
-				if !strings.HasPrefix(repo.RepoName(), "gh-") {
-					return errors.New("the repository name must start with `gh-`")
-				}
-
-				commandName := strings.TrimPrefix(repo.RepoName(), "gh-")
-				rootCmd := cmd.Root()
-				c, _, err := rootCmd.Traverse([]string{commandName})
-				if err != nil {
+				if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil {
 					return err
 				}
-				if c != rootCmd {
-					return fmt.Errorf("could not install %s: %s is already a command", args[0], commandName)
-				}
-				for _, ext := range m.List() {
-					if ext.Name() == commandName {
-						return fmt.Errorf("could not install %s: %s is already an installed extension", args[0], commandName)
-					}
-				}
 
 				cfg, err := f.Config()
 				if err != nil {
@@ -145,3 +132,24 @@ func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command {
 	extCmd.Hidden = true
 	return &extCmd
 }
+
+func checkValidExtension(rootCmd *cobra.Command, m extensions.ExtensionManager, extName string) error {
+	if !strings.HasPrefix(extName, "gh-") {
+		return errors.New("extension repository name must start with `gh-`")
+	}
+
+	commandName := strings.TrimPrefix(extName, "gh-")
+	if c, _, err := rootCmd.Traverse([]string{commandName}); err != nil {
+		return err
+	} else if c != rootCmd {
+		return fmt.Errorf("%q matches the name of a built-in command", commandName)
+	}
+
+	for _, ext := range m.List() {
+		if ext.Name() == commandName {
+			return fmt.Errorf("there is already an installed extension that provides the %q command", commandName)
+		}
+	}
+
+	return nil
+}
diff --git a/pkg/cmd/extensions/command_test.go b/pkg/cmd/extensions/command_test.go
index 9e7243ef1..41eae92b2 100644
--- a/pkg/cmd/extensions/command_test.go
+++ b/pkg/cmd/extensions/command_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/cli/cli/pkg/cmdutil"
 	"github.com/cli/cli/pkg/extensions"
 	"github.com/cli/cli/pkg/iostreams"
+	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -60,7 +61,7 @@ func TestNewCmdExtensions(t *testing.T) {
 				}
 			},
 			wantErr: true,
-			errMsg:  "could not install owner/gh-existing-ext: existing-ext is already an installed extension",
+			errMsg:  "there is already an installed extension that provides the \"existing-ext\" command",
 		},
 		{
 			name: "install local extension",
@@ -166,3 +167,79 @@ func TestNewCmdExtensions(t *testing.T) {
 func normalizeDir(d string) string {
 	return strings.TrimPrefix(d, "/private")
 }
+
+func Test_checkValidExtension(t *testing.T) {
+	rootCmd := &cobra.Command{}
+	rootCmd.AddCommand(&cobra.Command{Use: "help"})
+	rootCmd.AddCommand(&cobra.Command{Use: "auth"})
+
+	m := &extensions.ExtensionManagerMock{
+		ListFunc: func() []extensions.Extension {
+			return []extensions.Extension{
+				&extensions.ExtensionMock{
+					NameFunc: func() string { return "screensaver" },
+				},
+				&extensions.ExtensionMock{
+					NameFunc: func() string { return "triage" },
+				},
+			}
+		},
+	}
+
+	type args struct {
+		rootCmd *cobra.Command
+		manager extensions.ExtensionManager
+		extName string
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantError string
+	}{
+		{
+			name: "valid extension",
+			args: args{
+				rootCmd: rootCmd,
+				manager: m,
+				extName: "gh-hello",
+			},
+		},
+		{
+			name: "invalid extension name",
+			args: args{
+				rootCmd: rootCmd,
+				manager: m,
+				extName: "gherkins",
+			},
+			wantError: "extension repository name must start with `gh-`",
+		},
+		{
+			name: "clashes with built-in command",
+			args: args{
+				rootCmd: rootCmd,
+				manager: m,
+				extName: "gh-auth",
+			},
+			wantError: "\"auth\" matches the name of a built-in command",
+		},
+		{
+			name: "clashes with an installed extension",
+			args: args{
+				rootCmd: rootCmd,
+				manager: m,
+				extName: "gh-triage",
+			},
+			wantError: "there is already an installed extension that provides the \"triage\" command",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := checkValidExtension(tt.args.rootCmd, tt.args.manager, tt.args.extName)
+			if tt.wantError == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.EqualError(t, err, tt.wantError)
+			}
+		})
+	}
+}