Refactor factory.Executable() to be a method rather than a func

This way, factory can satisfy an interface that requires `Executable()`.

pkg/cmd/auth/login/login_test.go

@@ -147,8 +147,7 @@ func Test_NewCmdLogin(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			io, stdin, _, _ := iostreams.Test()
 			f := &cmdutil.Factory{
-				IOStreams:  io,
-				Executable: func() string { return "/path/to/gh" },
+				IOStreams: io,
 			}
 
 			io.SetStdoutTTY(true)

pkg/cmd/auth/refresh/refresh_test.go

@@ -91,8 +91,7 @@ func Test_NewCmdRefresh(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			io, _, _, _ := iostreams.Test()
 			f := &cmdutil.Factory{
-				IOStreams:  io,
-				Executable: func() string { return "/path/to/gh" },
+				IOStreams: io,
 			}
 			io.SetStdinTTY(tt.tty)
 			io.SetStdoutTTY(tt.tty)

pkg/cmd/factory/default.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net/http"
 	"os"
-	"path/filepath"
 	"time"
 
 	"github.com/cli/cli/v2/api"
@@ -19,17 +18,10 @@ import (
 )
 
 func New(appVersion string) *cmdutil.Factory {
-	var exe string
 	f := &cmdutil.Factory{
-		Config: configFunc(), // No factory dependencies
-		Branch: branchFunc(), // No factory dependencies
-		Executable: func() string {
-			if exe != "" {
-				return exe
-			}
-			exe = executable("gh")
-			return exe
-		},
+		Config:         configFunc(), // No factory dependencies
+		Branch:         branchFunc(), // No factory dependencies
+		ExecutableName: "gh",
 	}
 
 	f.IOStreams = ioStreams(f)                   // Depends on Config
@@ -121,52 +113,6 @@ func browserLauncher(f *cmdutil.Factory) string {
 	return os.Getenv("BROWSER")
 }
 
-// Finds the location of the executable for the current process as it's found in PATH, respecting symlinks.
-// If the process couldn't determine its location, return fallbackName. If the executable wasn't found in
-// PATH, return the absolute location to the program.
-//
-// The idea is that the result of this function is callable in the future and refers to the same
-// installation of gh, even across upgrades. This is needed primarily for Homebrew, which installs software
-// under a location such as `/usr/local/Cellar/gh/1.13.1/bin/gh` and symlinks it from `/usr/local/bin/gh`.
-// When the version is upgraded, Homebrew will often delete older versions, but keep the symlink. Because of
-// this, we want to refer to the `gh` binary as `/usr/local/bin/gh` and not as its internal Homebrew
-// location.
-//
-// None of this would be needed if we could just refer to GitHub CLI as `gh`, i.e. without using an absolute
-// path. However, for some reason Homebrew does not include `/usr/local/bin` in PATH when it invokes git
-// commands to update its taps. If `gh` (no path) is being used as git credential helper, as set up by `gh
-// auth login`, running `brew update` will print out authentication errors as git is unable to locate
-// Homebrew-installed `gh`.
-func executable(fallbackName string) string {
-	exe, err := os.Executable()
-	if err != nil {
-		return fallbackName
-	}
-
-	base := filepath.Base(exe)
-	path := os.Getenv("PATH")
-	for _, dir := range filepath.SplitList(path) {
-		p, err := filepath.Abs(filepath.Join(dir, base))
-		if err != nil {
-			continue
-		}
-		f, err := os.Stat(p)
-		if err != nil {
-			continue
-		}
-
-		if p == exe {
-			return p
-		} else if f.Mode()&os.ModeSymlink != 0 {
-			if t, err := os.Readlink(p); err == nil && t == exe {
-				return p
-			}
-		}
-	}
-
-	return exe
-}
-
 func configFunc() func() (config.Config, error) {
 	var cachedConfig config.Config
 	var configError error

pkg/cmdutil/factory.go

@@ -2,6 +2,9 @@ package cmdutil
 
 import (
 	"net/http"
+	"os"
+	"path/filepath"
+	"strings"
 
 	"github.com/cli/cli/v2/context"
 	"github.com/cli/cli/v2/internal/config"
@@ -25,7 +28,59 @@ type Factory struct {
 	Branch     func() (string, error)
 
 	ExtensionManager extensions.ExtensionManager
-
-	// Executable is the path to the currently invoked gh binary
-	Executable func() string
+	ExecutableName   string
+}
+
+// Executable is the path to the currently invoked binary
+func (f *Factory) Executable() string {
+	if !strings.ContainsRune(f.ExecutableName, os.PathSeparator) {
+		f.ExecutableName = executable(f.ExecutableName)
+	}
+	return f.ExecutableName
+}
+
+// Finds the location of the executable for the current process as it's found in PATH, respecting symlinks.
+// If the process couldn't determine its location, return fallbackName. If the executable wasn't found in
+// PATH, return the absolute location to the program.
+//
+// The idea is that the result of this function is callable in the future and refers to the same
+// installation of gh, even across upgrades. This is needed primarily for Homebrew, which installs software
+// under a location such as `/usr/local/Cellar/gh/1.13.1/bin/gh` and symlinks it from `/usr/local/bin/gh`.
+// When the version is upgraded, Homebrew will often delete older versions, but keep the symlink. Because of
+// this, we want to refer to the `gh` binary as `/usr/local/bin/gh` and not as its internal Homebrew
+// location.
+//
+// None of this would be needed if we could just refer to GitHub CLI as `gh`, i.e. without using an absolute
+// path. However, for some reason Homebrew does not include `/usr/local/bin` in PATH when it invokes git
+// commands to update its taps. If `gh` (no path) is being used as git credential helper, as set up by `gh
+// auth login`, running `brew update` will print out authentication errors as git is unable to locate
+// Homebrew-installed `gh`.
+func executable(fallbackName string) string {
+	exe, err := os.Executable()
+	if err != nil {
+		return fallbackName
+	}
+
+	base := filepath.Base(exe)
+	path := os.Getenv("PATH")
+	for _, dir := range filepath.SplitList(path) {
+		p, err := filepath.Abs(filepath.Join(dir, base))
+		if err != nil {
+			continue
+		}
+		f, err := os.Stat(p)
+		if err != nil {
+			continue
+		}
+
+		if p == exe {
+			return p
+		} else if f.Mode()&os.ModeSymlink != 0 {
+			if t, err := os.Readlink(p); err == nil && t == exe {
+				return p
+			}
+		}
+	}
+
+	return exe
 }