fix(clibase): allow empty values to unset defaults (#6832)

ammario · web-flow · commit 164528176acc · 2023-03-28T01:58:06.000Z
diff --git a/cli/clibase/cmd.go b/cli/clibase/cmd.go
@@ -293,11 +293,19 @@ func (i *Invocation) run(state *runState) error {
 				return
 			}
 
+			// If flag was changed, we need to remove the default values.
 			sv, ok := f.Value.(pflag.SliceValue)
 			if !ok {
 				panic("defaulted array option is not a slice value")
 			}
-			err := sv.Replace(sv.GetSlice()[i:])
+			ss := sv.GetSlice()
+			if len(ss) == 0 {
+				// Slice likely zeroed by a flag.
+				// E.g. "--fruit" may default to "apples,oranges" but the user
+				// provided "--fruit=""".
+				return
+			}
+			err := sv.Replace(ss[i:])
 			if err != nil {
 				panic(err)
 			}
diff --git a/cli/clibase/cmd_test.go b/cli/clibase/cmd_test.go
@@ -503,3 +503,35 @@ func TestCommand_SliceFlags(t *testing.T) {
 	err = cmd("bad", "bad", "bad").Invoke().Run()
 	require.NoError(t, err)
 }
+
+func TestCommand_EmptySlice(t *testing.T) {
+	t.Parallel()
+
+	cmd := func(want ...string) *clibase.Cmd {
+		var got []string
+		return &clibase.Cmd{
+			Use: "root",
+			Options: clibase.OptionSet{
+				{
+					Name:    "arr",
+					Flag:    "arr",
+					Default: "bad,bad,bad",
+					Env:     "ARR",
+					Value:   clibase.StringArrayOf(&got),
+				},
+			},
+			Handler: (func(i *clibase.Invocation) error {
+				require.Equal(t, want, got)
+				return nil
+			}),
+		}
+	}
+
+	// Base-case
+	err := cmd("bad", "bad", "bad").Invoke().Run()
+	require.NoError(t, err)
+
+	inv := cmd().Invoke("--arr", "")
+	err = inv.Run()
+	require.NoError(t, err)
+}
diff --git a/cli/clibase/option.go b/cli/clibase/option.go
@@ -126,6 +126,9 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
 		// way for a user to change a Default value to an empty string from
 		// the environment. Unfortunately, we have old configuration files
 		// that rely on the faulty behavior.
+		//
+		// TODO: We should remove this hack in May 2023, when deployments
+		// have had months to migrate to the new behavior.
 		if !ok || envVal == "" {
 			continue
 		}
diff --git a/cli/clibase/values.go b/cli/clibase/values.go
@@ -146,6 +146,10 @@ func writeAsCSV(vals []string) string {
 }
 
 func (s *StringArray) Set(v string) error {
+	if v == "" {
+		*s = nil
+		return nil
+	}
 	ss, err := readAsCSV(v)
 	if err != nil {
 		return err
