chore: early merge clibase foundations (#6542)

cli/clibase/clibase.go

@@ -5,6 +5,9 @@
 // We will extend its usage to the rest of our application, completely replacing
 // cobra/viper. It's also a candidate to be broken out into its own open-source
 // library, so we avoid deep coupling with Coder concepts.
+//
+// The Command interface is loosely based on the chi middleware pattern and
+// http.Handler/HandlerFunc.
 package clibase
 
 import (

cli/clibase/cmd.go

@@ -1,6 +1,15 @@
 package clibase
 
-import "strings"
+import (
+	"context"
+	"errors"
+	"io"
+	"os"
+	"strings"
+
+	"github.com/spf13/pflag"
+	"golang.org/x/xerrors"
+)
 
 // Cmd describes an executable command.
 type Cmd struct {
@@ -10,13 +19,40 @@ type Cmd struct {
 	Children []*Cmd
 	// Use is provided in form "command [flags] [args...]".
 	Use string
+
+	// Aliases is a list of alternative names for the command.
+	Aliases []string
+
 	// Short is a one-line description of the command.
 	Short string
+
+	// Hidden determines whether the command should be hidden from help.
+	Hidden bool
+
+	// RawArgs determines whether the command should receive unparsed arguments.
+	// No flags are parsed when set, and the command is responsible for parsing
+	// its own flags.
+	RawArgs bool
+
 	// Long is a detailed description of the command,
 	// presented on its help page. It may contain examples.
 	Long        string
 	Options     OptionSet
 	Annotations Annotations
+
+	// Middleware is called before the Handler.
+	// Use Chain() to combine multiple middlewares.
+	Middleware  MiddlewareFunc
+	Handler     HandlerFunc
+	HelpHandler HandlerFunc
+}
+
+// Walk calls fn for the command and all its children.
+func (c *Cmd) Walk(fn func(*Cmd)) {
+	fn(c)
+	for _, child := range c.Children {
+		child.Walk(fn)
+	}
 }
 
 // Name returns the first word in the Use string.
@@ -46,3 +82,336 @@ func (c *Cmd) FullUsage() string {
 	uses = append(uses, c.Use)
 	return strings.Join(uses, " ")
 }
+
+// Invoke creates a new invocation of the command, with
+// stdio discarded.
+//
+// The returned invocation is not live until Run() is called.
+func (c *Cmd) Invoke(args ...string) *Invocation {
+	return &Invocation{
+		Command: c,
+		Args:    args,
+		Stdout:  io.Discard,
+		Stderr:  io.Discard,
+		Stdin:   strings.NewReader(""),
+	}
+}
+
+// Invocation represents an instance of a command being executed.
+type Invocation struct {
+	ctx         context.Context
+	Command     *Cmd
+	parsedFlags *pflag.FlagSet
+	Args        []string
+	// Environ is a list of environment variables. Use EnvsWithPrefix to parse
+	// os.Environ.
+	Environ Environ
+	Stdout  io.Writer
+	Stderr  io.Writer
+	Stdin   io.Reader
+}
+
+// WithOS returns the invocation as a main package, filling in the invocation's unset
+// fields with OS defaults.
+func (i *Invocation) WithOS() *Invocation {
+	return i.with(func(i *Invocation) {
+		if i.Stdout == nil {
+			i.Stdout = os.Stdout
+		}
+		if i.Stderr == nil {
+			i.Stderr = os.Stderr
+		}
+		if i.Stdin == nil {
+			i.Stdin = os.Stdin
+		}
+		if i.Args == nil {
+			i.Args = os.Args[1:]
+		}
+		if i.Environ == nil {
+			i.Environ = ParseEnviron(os.Environ(), "")
+		}
+	})
+}
+
+func (i *Invocation) Context() context.Context {
+	if i.ctx == nil {
+		// Consider returning context.Background() instead?
+		panic("context not set, has WithContext() or Run() been called?")
+	}
+	return i.ctx
+}
+
+func (i *Invocation) ParsedFlags() *pflag.FlagSet {
+	if i.parsedFlags == nil {
+		panic("flags not parsed, has Run() been called?")
+	}
+	return i.parsedFlags
+}
+
+type runState struct {
+	allArgs      []string
+	commandDepth int
+
+	flagParseErr error
+}
+
+// run recursively executes the command and its children.
+// allArgs is wired through the stack so that global flags can be accepted
+// anywhere in the command invocation.
+func (i *Invocation) run(state *runState) error {
+	err := i.Command.Options.SetDefaults()
+	if err != nil {
+		return xerrors.Errorf("setting defaults: %w", err)
+	}
+
+	err = i.Command.Options.ParseEnv(i.Environ)
+	if err != nil {
+		return xerrors.Errorf("parsing env: %w", err)
+	}
+
+	// Now the fun part, argument parsing!
+
+	children := make(map[string]*Cmd)
+	for _, child := range i.Command.Children {
+		for _, name := range append(child.Aliases, child.Name()) {
+			if _, ok := children[name]; ok {
+				return xerrors.Errorf("duplicate command name: %s", name)
+			}
+			children[name] = child
+		}
+	}
+
+	if i.parsedFlags == nil {
+		i.parsedFlags = pflag.NewFlagSet(i.Command.Name(), pflag.ContinueOnError)
+		// We handle Usage ourselves.
+		i.parsedFlags.Usage = func() {}
+	}
+
+	i.parsedFlags.AddFlagSet(i.Command.Options.FlagSet())
+
+	var parsedArgs []string
+
+	if !i.Command.RawArgs {
+		// Flag parsing will fail on intermediate commands in the command tree,
+		// so we check the error after looking for a child command.
+		state.flagParseErr = i.parsedFlags.Parse(state.allArgs)
+		parsedArgs = i.parsedFlags.Args()
+	}
+
+	// Run child command if found (next child only)
+	// We must do subcommand detection after flag parsing so we don't mistake flag
+	// values for subcommand names.
+	if len(parsedArgs) > 0 {
+		nextArg := parsedArgs[0]
+		if child, ok := children[nextArg]; ok {
+			child.Parent = i.Command
+			i.Command = child
+			state.commandDepth++
+			err = i.run(state)
+			if err != nil {
+				return xerrors.Errorf(
+					"subcommand %s: %w", child.Name(), err,
+				)
+			}
+			return nil
+		}
+	}
+
+	// Flag parse errors are irrelevant for raw args commands.
+	if !i.Command.RawArgs && state.flagParseErr != nil && !errors.Is(state.flagParseErr, pflag.ErrHelp) {
+		return xerrors.Errorf(
+			"parsing flags (%v) for %q: %w",
+			state.allArgs,
+			i.Command.FullName(), state.flagParseErr,
+		)
+	}
+
+	if i.Command.RawArgs {
+		// If we're at the root command, then the name is omitted
+		// from the arguments, so we can just use the entire slice.
+		if state.commandDepth == 0 {
+			i.Args = state.allArgs
+		} else {
+			argPos, err := findArg(i.Command.Name(), state.allArgs, i.parsedFlags)
+			if err != nil {
+				panic(err)
+			}
+			i.Args = state.allArgs[argPos+1:]
+		}
+	} else {
+		// In non-raw-arg mode, we want to skip over flags.
+		i.Args = parsedArgs[state.commandDepth:]
+	}
+
+	mw := i.Command.Middleware
+	if mw == nil {
+		mw = Chain()
+	}
+
+	ctx := i.ctx
+	if ctx == nil {
+		ctx = context.Background()
+	}
+
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+	i = i.WithContext(ctx)
+
+	if i.Command.Handler == nil || errors.Is(state.flagParseErr, pflag.ErrHelp) {
+		if i.Command.HelpHandler == nil {
+			return xerrors.Errorf("no handler or help for command %s", i.Command.FullName())
+		}
+		return i.Command.HelpHandler(i)
+	}
+
+	err = mw(i.Command.Handler)(i)
+	if err != nil {
+		return xerrors.Errorf("running command %s: %w", i.Command.FullName(), err)
+	}
+	return nil
+}
+
+// findArg returns the index of the first occurrence of arg in args, skipping
+// over all flags.
+func findArg(want string, args []string, fs *pflag.FlagSet) (int, error) {
+	for i := 0; i < len(args); i++ {
+		arg := args[i]
+		if !strings.HasPrefix(arg, "-") {
+			if arg == want {
+				return i, nil
+			}
+			continue
+		}
+
+		// This is a flag!
+		if strings.Contains(arg, "=") {
+			// The flag contains the value in the same arg, just skip.
+			continue
+		}
+
+		// We need to check if NoOptValue is set, then we should not wait
+		// for the next arg to be the value.
+		f := fs.Lookup(strings.TrimLeft(arg, "-"))
+		if f == nil {
+			return -1, xerrors.Errorf("unknown flag: %s", arg)
+		}
+		if f.NoOptDefVal != "" {
+			continue
+		}
+
+		if i == len(args)-1 {
+			return -1, xerrors.Errorf("flag %s requires a value", arg)
+		}
+
+		// Skip the value.
+		i++
+	}
+
+	return -1, xerrors.Errorf("arg %s not found", want)
+}
+
+// Run executes the command.
+// If two command share a flag name, the first command wins.
+//
+//nolint:revive
+func (i *Invocation) Run() error {
+	return i.run(&runState{
+		allArgs: i.Args,
+	})
+}
+
+// WithContext returns a copy of the Invocation with the given context.
+func (i *Invocation) WithContext(ctx context.Context) *Invocation {
+	return i.with(func(i *Invocation) {
+		i.ctx = ctx
+	})
+}
+
+// with returns a copy of the Invocation with the given function applied.
+func (i *Invocation) with(fn func(*Invocation)) *Invocation {
+	i2 := *i
+	fn(&i2)
+	return &i2
+}
+
+// MiddlewareFunc returns the next handler in the chain,
+// or nil if there are no more.
+type MiddlewareFunc func(next HandlerFunc) HandlerFunc
+
+func chain(ms ...MiddlewareFunc) MiddlewareFunc {
+	return MiddlewareFunc(func(next HandlerFunc) HandlerFunc {
+		if len(ms) > 0 {
+			return chain(ms[1:]...)(ms[0](next))
+		}
+		return next
+	})
+}
+
+// Chain returns a Handler that first calls middleware in order.
+//
+//nolint:revive
+func Chain(ms ...MiddlewareFunc) MiddlewareFunc {
+	// We need to reverse the array to provide top-to-bottom execution
+	// order when defining a command.
+	reversed := make([]MiddlewareFunc, len(ms))
+	for i := range ms {
+		reversed[len(ms)-1-i] = ms[i]
+	}
+	return chain(reversed...)
+}
+
+func RequireNArgs(want int) MiddlewareFunc {
+	return RequireRangeArgs(want, want)
+}
+
+// RequireRangeArgs returns a Middleware that requires the number of arguments
+// to be between start and end (inclusive). If end is -1, then the number of
+// arguments must be at least start.
+func RequireRangeArgs(start, end int) MiddlewareFunc {
+	if start < 0 {
+		panic("start must be >= 0")
+	}
+	return func(next HandlerFunc) HandlerFunc {
+		return func(i *Invocation) error {
+			got := len(i.Args)
+			switch {
+			case start == end && got != start:
+				switch start {
+				case 0:
+					return xerrors.Errorf("wanted no args but got %v %v", got, i.Args)
+				default:
+					return xerrors.Errorf(
+						"wanted %v args but got %v %v",
+						start,
+						got,
+						i.Args,
+					)
+				}
+			case start > 0 && end == -1:
+				switch {
+				case got < start:
+					return xerrors.Errorf(
+						"wanted at least %v args but got %v",
+						start,
+						got,
+					)
+				default:
+					return next(i)
+				}
+			case start > end:
+				panic("start must be <= end")
+			case got < start || got > end:
+				return xerrors.Errorf(
+					"wanted between %v and %v args but got %v",
+					start, end,
+					got,
+				)
+			default:
+				return next(i)
+			}
+		}
+	}
+}
+
+// HandlerFunc handles an Invocation of a command.
+type HandlerFunc func(i *Invocation) error

cli/clibase/cmd_test.go

@@ -0,0 +1,368 @@
+package clibase_test
+
+import (
+	"bytes"
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/xerrors"
+
+	"github.com/coder/coder/cli/clibase"
+)
+
+// ioBufs is the standard input, output, and error for a command.
+type ioBufs struct {
+	Stdin  bytes.Buffer
+	Stdout bytes.Buffer
+	Stderr bytes.Buffer
+}
+
+// fakeIO sets Stdin, Stdout, and Stderr to buffers.
+func fakeIO(i *clibase.Invocation) *ioBufs {
+	var b ioBufs
+	i.Stdout = &b.Stdout
+	i.Stderr = &b.Stderr
+	i.Stdin = &b.Stdin
+	return &b
+}
+
+func TestCommand(t *testing.T) {
+	t.Parallel()
+
+	cmd := func() *clibase.Cmd {
+		var (
+			verbose bool
+			lower   bool
+			prefix  string
+		)
+		return &clibase.Cmd{
+			Use: "root [subcommand]",
+			Options: clibase.OptionSet{
+				clibase.Option{
+					Name:  "verbose",
+					Flag:  "verbose",
+					Value: clibase.BoolOf(&verbose),
+				},
+				clibase.Option{
+					Name:  "prefix",
+					Flag:  "prefix",
+					Value: clibase.StringOf(&prefix),
+				},
+			},
+			Children: []*clibase.Cmd{
+				{
+					Use:   "toupper [word]",
+					Short: "Converts a word to upper case",
+					Middleware: clibase.Chain(
+						clibase.RequireNArgs(1),
+					),
+					Aliases: []string{"up"},
+					Options: clibase.OptionSet{
+						clibase.Option{
+							Name:  "lower",
+							Flag:  "lower",
+							Value: clibase.BoolOf(&lower),
+						},
+					},
+					Handler: (func(i *clibase.Invocation) error {
+						i.Stdout.Write([]byte(prefix))
+						w := i.Args[0]
+						if lower {
+							w = strings.ToLower(w)
+						} else {
+							w = strings.ToUpper(w)
+						}
+						_, _ = i.Stdout.Write(
+							[]byte(
+								w,
+							),
+						)
+						if verbose {
+							i.Stdout.Write([]byte("!!!"))
+						}
+						return nil
+					}),
+				},
+			},
+		}
+	}
+
+	t.Run("SimpleOK", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke("toupper", "hello")
+		io := fakeIO(i)
+		i.Run()
+		require.Equal(t, "HELLO", io.Stdout.String())
+	})
+
+	t.Run("Alias", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"up", "hello",
+		)
+		io := fakeIO(i)
+		i.Run()
+		require.Equal(t, "HELLO", io.Stdout.String())
+	})
+
+	t.Run("NoSubcommand", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"na",
+		)
+		io := fakeIO(i)
+		err := i.Run()
+		require.Empty(t, io.Stdout.String())
+		require.Error(t, err)
+	})
+
+	t.Run("BadArgs", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"toupper",
+		)
+		io := fakeIO(i)
+		err := i.Run()
+		require.Empty(t, io.Stdout.String())
+		require.Error(t, err)
+	})
+
+	t.Run("UnknownFlags", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"toupper", "--unknown",
+		)
+		io := fakeIO(i)
+		err := i.Run()
+		require.Empty(t, io.Stdout.String())
+		require.Error(t, err)
+	})
+
+	t.Run("Verbose", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"--verbose", "toupper", "hello",
+		)
+		io := fakeIO(i)
+		require.NoError(t, i.Run())
+		require.Equal(t, "HELLO!!!", io.Stdout.String())
+	})
+
+	t.Run("Verbose=", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"--verbose=true", "toupper", "hello",
+		)
+		io := fakeIO(i)
+		require.NoError(t, i.Run())
+		require.Equal(t, "HELLO!!!", io.Stdout.String())
+	})
+
+	t.Run("PrefixSpace", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"--prefix", "conv: ", "toupper", "hello",
+		)
+		io := fakeIO(i)
+		require.NoError(t, i.Run())
+		require.Equal(t, "conv: HELLO", io.Stdout.String())
+	})
+
+	t.Run("GlobalFlagsAnywhere", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"toupper", "--prefix", "conv: ", "hello", "--verbose",
+		)
+		io := fakeIO(i)
+		require.NoError(t, i.Run())
+		require.Equal(t, "conv: HELLO!!!", io.Stdout.String())
+	})
+
+	t.Run("LowerVerbose", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"toupper", "--verbose", "hello", "--lower",
+		)
+		io := fakeIO(i)
+		require.NoError(t, i.Run())
+		require.Equal(t, "hello!!!", io.Stdout.String())
+	})
+
+	t.Run("ParsedFlags", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"toupper", "--verbose", "hello", "--lower",
+		)
+		_ = fakeIO(i)
+		require.NoError(t, i.Run())
+		require.Equal(t,
+			"true",
+			i.ParsedFlags().Lookup("verbose").Value.String(),
+		)
+	})
+
+	t.Run("NoDeepChild", func(t *testing.T) {
+		t.Parallel()
+		i := cmd().Invoke(
+			"root", "level", "level", "toupper", "--verbose", "hello", "--lower",
+		)
+		fio := fakeIO(i)
+		require.Error(t, i.Run(), fio.Stdout.String())
+	})
+}
+
+func TestCommand_MiddlewareOrder(t *testing.T) {
+	t.Parallel()
+
+	mw := func(letter string) clibase.MiddlewareFunc {
+		return func(next clibase.HandlerFunc) clibase.HandlerFunc {
+			return (func(i *clibase.Invocation) error {
+				_, _ = i.Stdout.Write([]byte(letter))
+				return next(i)
+			})
+		}
+	}
+
+	cmd := &clibase.Cmd{
+		Use:   "toupper [word]",
+		Short: "Converts a word to upper case",
+		Middleware: clibase.Chain(
+			mw("A"),
+			mw("B"),
+			mw("C"),
+		),
+		Handler: (func(i *clibase.Invocation) error {
+			return nil
+		}),
+	}
+
+	i := cmd.Invoke(
+		"hello", "world",
+	)
+	io := fakeIO(i)
+	require.NoError(t, i.Run())
+	require.Equal(t, "ABC", io.Stdout.String())
+}
+
+func TestCommand_RawArgs(t *testing.T) {
+	t.Parallel()
+
+	cmd := func() *clibase.Cmd {
+		return &clibase.Cmd{
+			Use: "root",
+			Options: []clibase.Option{
+				{
+					Name:  "password",
+					Flag:  "password",
+					Value: clibase.StringOf(new(string)),
+				},
+			},
+			Children: []*clibase.Cmd{
+				{
+					Use:     "sushi <args...>",
+					Short:   "Throws back raw output",
+					RawArgs: true,
+					Handler: (func(i *clibase.Invocation) error {
+						if v := i.ParsedFlags().Lookup("password").Value.String(); v != "codershack" {
+							return xerrors.Errorf("password %q is wrong!", v)
+						}
+						i.Stdout.Write([]byte(strings.Join(i.Args, " ")))
+						return nil
+					}),
+				},
+			},
+		}
+	}
+
+	t.Run("OK", func(t *testing.T) {
+		// Flag parsed before the raw arg command should still work.
+		t.Parallel()
+
+		i := cmd().Invoke(
+			"--password", "codershack", "sushi", "hello", "--verbose", "world",
+		)
+		io := fakeIO(i)
+		require.NoError(t, i.Run())
+		require.Equal(t, "hello --verbose world", io.Stdout.String())
+	})
+
+	t.Run("BadFlag", func(t *testing.T) {
+		// Verbose before the raw arg command should fail.
+		t.Parallel()
+
+		i := cmd().Invoke(
+			"--password", "codershack", "--verbose", "sushi", "hello", "world",
+		)
+		io := fakeIO(i)
+		require.Error(t, i.Run())
+		require.Empty(t, io.Stdout.String())
+	})
+
+	t.Run("NoPassword", func(t *testing.T) {
+		// Flag parsed before the raw arg command should still work.
+		t.Parallel()
+		i := cmd().Invoke(
+			"sushi", "hello", "--verbose", "world",
+		)
+		_ = fakeIO(i)
+		require.Error(t, i.Run())
+	})
+}
+
+func TestCommand_RootRaw(t *testing.T) {
+	t.Parallel()
+	cmd := &clibase.Cmd{
+		RawArgs: true,
+		Handler: func(i *clibase.Invocation) error {
+			i.Stdout.Write([]byte(strings.Join(i.Args, " ")))
+			return nil
+		},
+	}
+
+	inv := cmd.Invoke("hello", "--verbose", "--friendly")
+	stdio := fakeIO(inv)
+	err := inv.Run()
+	require.NoError(t, err)
+
+	require.Equal(t, "hello --verbose --friendly", stdio.Stdout.String())
+}
+
+func TestCommand_HyphenHyphen(t *testing.T) {
+	t.Parallel()
+	cmd := &clibase.Cmd{
+		Handler: (func(i *clibase.Invocation) error {
+			i.Stdout.Write([]byte(strings.Join(i.Args, " ")))
+			return nil
+		}),
+	}
+
+	inv := cmd.Invoke("--", "--verbose", "--friendly")
+	stdio := fakeIO(inv)
+	err := inv.Run()
+	require.NoError(t, err)
+
+	require.Equal(t, "--verbose --friendly", stdio.Stdout.String())
+}
+
+func TestCommand_ContextCancels(t *testing.T) {
+	t.Parallel()
+
+	var gotCtx context.Context
+
+	cmd := &clibase.Cmd{
+		Handler: (func(i *clibase.Invocation) error {
+			gotCtx = i.Context()
+			if err := gotCtx.Err(); err != nil {
+				return xerrors.Errorf("unexpected context error: %w", i.Context().Err())
+			}
+			return nil
+		}),
+	}
+
+	err := cmd.Invoke().Run()
+	require.NoError(t, err)
+
+	require.Error(t, gotCtx.Err())
+}

cli/clibase/env.go

@@ -25,9 +25,38 @@ type EnvVar struct {
 	Value string
 }
 
-// EnvsWithPrefix returns all environment variables starting with
+type Environ []EnvVar
+
+func (e Environ) ToOS() []string {
+	var env []string
+	for _, v := range e {
+		env = append(env, v.Name+"="+v.Value)
+	}
+	return env
+}
+
+func (e Environ) Lookup(name string) (string, bool) {
+	for _, v := range e {
+		if v.Name == name {
+			return v.Value, true
+		}
+	}
+	return "", false
+}
+
+func (e *Environ) Set(name, value string) {
+	for i, v := range *e {
+		if v.Name == name {
+			(*e)[i].Value = value
+			return
+		}
+	}
+	*e = append(*e, EnvVar{Name: name, Value: value})
+}
+
+// ParseEnviron returns all environment variables starting with
 // prefix without said prefix.
-func EnvsWithPrefix(environ []string, prefix string) []EnvVar {
+func ParseEnviron(environ []string, prefix string) Environ {
 	var filtered []EnvVar
 	for _, line := range environ {
 		name := envName(line)

cli/clibase/env_test.go

@@ -16,7 +16,7 @@ func TestFilterNamePrefix(t *testing.T) {
 	tests := []struct {
 		name string
 		args args
-		want []clibase.EnvVar
+		want clibase.Environ
 	}{
 		{"empty", args{[]string{}, "SHIRE"}, nil},
 		{
@@ -36,8 +36,8 @@ func TestFilterNamePrefix(t *testing.T) {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			if got := clibase.EnvsWithPrefix(tt.args.environ, tt.args.prefix); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("EnvsWithPrefix() = %v, want %v", got, tt.want)
+			if got := clibase.ParseEnviron(tt.args.environ, tt.args.prefix); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("FilterNamePrefix() = %v, want %v", got, tt.want)
 			}
 		})
 	}

cli/clibase/option.go

@@ -58,6 +58,10 @@ func (s *OptionSet) Add(opts ...Option) {
 
 // FlagSet returns a pflag.FlagSet for the OptionSet.
 func (s *OptionSet) FlagSet() *pflag.FlagSet {
+	if s == nil {
+		return &pflag.FlagSet{}
+	}
+
 	fs := pflag.NewFlagSet("", pflag.ContinueOnError)
 	for _, opt := range *s {
 		if opt.Flag == "" {
@@ -71,11 +75,16 @@ func (s *OptionSet) FlagSet() *pflag.FlagSet {
 			}
 		}
 
+		val := opt.Value
+		if val == nil {
+			val = &DiscardValue{}
+		}
+
 		fs.AddFlag(&pflag.Flag{
 			Name:        opt.Flag,
 			Shorthand:   opt.FlagShorthand,
 			Usage:       opt.Description,
-			Value:       opt.Value,
+			Value:       val,
 			DefValue:    "",
 			Changed:     false,
 			Deprecated:  "",
@@ -90,14 +99,19 @@ func (s *OptionSet) FlagSet() *pflag.FlagSet {
 }
 
 // ParseEnv parses the given environment variables into the OptionSet.
-func (s *OptionSet) ParseEnv(globalPrefix string, environ []string) error {
+// Use EnvsWithPrefix to filter out prefixes.
+func (s *OptionSet) ParseEnv(vs []EnvVar) error {
+	if s == nil {
+		return nil
+	}
+
 	var merr *multierror.Error
 
 	// We parse environment variables first instead of using a nested loop to
 	// avoid N*M complexity when there are a lot of options and environment
 	// variables.
 	envs := make(map[string]string)
-	for _, v := range EnvsWithPrefix(environ, globalPrefix) {
+	for _, v := range vs {
 		envs[v.Name] = v.Value
 	}
 
@@ -129,7 +143,12 @@ func (s *OptionSet) ParseEnv(globalPrefix string, environ []string) error {
 // SetDefaults sets the default values for each Option.
 // It should be called before all parsing (e.g. ParseFlags, ParseEnv).
 func (s *OptionSet) SetDefaults() error {
+	if s == nil {
+		return nil
+	}
+
 	var merr *multierror.Error
+
 	for _, opt := range *s {
 		if opt.Default == "" {
 			continue

cli/clibase/option_test.go

@@ -87,7 +87,9 @@ func TestOptionSet_ParseEnv(t *testing.T) {
 			},
 		}
 
-		err := os.ParseEnv("CODER_", []string{"CODER_WORKSPACE_NAME=foo"})
+		err := os.ParseEnv([]clibase.EnvVar{
+			{Name: "WORKSPACE_NAME", Value: "foo"},
+		})
 		require.NoError(t, err)
 		require.EqualValues(t, "foo", workspaceName)
 	})
@@ -109,7 +111,7 @@ func TestOptionSet_ParseEnv(t *testing.T) {
 		err := os.SetDefaults()
 		require.NoError(t, err)
 
-		err = os.ParseEnv("CODER_", []string{"CODER_WORKSPACE_NAME="})
+		err = os.ParseEnv(clibase.ParseEnviron([]string{"CODER_WORKSPACE_NAME="}, "CODER_"))
 		require.NoError(t, err)
 		require.EqualValues(t, "defname", workspaceName)
 	})

cli/clibase/values.go

@@ -28,6 +28,10 @@ type NoOptDefValuer interface {
 
 type Int64 int64
 
+func Int64Of(i *int64) *Int64 {
+	return (*Int64)(i)
+}
+
 func (i *Int64) Set(s string) error {
 	ii, err := strconv.ParseInt(s, 10, 64)
 	*i = Int64(ii)
@@ -48,6 +52,10 @@ func (Int64) Type() string {
 
 type Bool bool
 
+func BoolOf(b *bool) *Bool {
+	return (*Bool)(b)
+}
+
 func (b *Bool) Set(s string) error {
 	if s == "" {
 		*b = Bool(false)
@@ -76,6 +84,10 @@ func (Bool) Type() string {
 
 type String string
 
+func StringOf(s *string) *String {
+	return (*String)(s)
+}
+
 func (*String) NoOptDefValue() string {
 	return ""
 }
@@ -102,6 +114,10 @@ var _ pflag.SliceValue = &Strings{}
 // Strings is a slice of strings that implements pflag.Value and pflag.SliceValue.
 type Strings []string
 
+func StringsOf(ss *[]string) *Strings {
+	return (*Strings)(ss)
+}
+
 func (s *Strings) Append(v string) error {
 	*s = append(*s, v)
 	return nil
@@ -152,6 +168,10 @@ func (Strings) Type() string {
 
 type Duration time.Duration
 
+func DurationOf(d *time.Duration) *Duration {
+	return (*Duration)(d)
+}
+
 func (d *Duration) Set(v string) error {
 	dd, err := time.ParseDuration(v)
 	*d = Duration(dd)
@@ -185,6 +205,10 @@ func (Duration) Type() string {
 
 type URL url.URL
 
+func URLOf(u *url.URL) *URL {
+	return (*URL)(u)
+}
+
 func (u *URL) Set(v string) error {
 	uu, err := url.Parse(v)
 	if err != nil {

cli/server.go

@@ -95,7 +95,7 @@ func ReadGitAuthProvidersFromEnv(environ []string) ([]codersdk.GitAuthConfig, er
 	sort.Strings(environ)
 
 	var providers []codersdk.GitAuthConfig
-	for _, v := range clibase.EnvsWithPrefix(environ, envPrefix+"GITAUTH_") {
+	for _, v := range clibase.ParseEnviron(environ, envPrefix+"GITAUTH_") {
 		tokens := strings.SplitN(v.Name, "_", 2)
 		if len(tokens) != 2 {
 			return nil, xerrors.Errorf("invalid env var: %s", v.Name)
@@ -185,7 +185,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 				return xerrors.Errorf("set defaults: %w", err)
 			}
 
-			err = cliOpts.ParseEnv(envPrefix, os.Environ())
+			err = cliOpts.ParseEnv(clibase.ParseEnviron(os.Environ(), envPrefix))
 			if err != nil {
 				return xerrors.Errorf("parse env: %w", err)
 			}