fix(map): ensure string-to-string flags maintain stable pointers

example_test.go

@@ -34,3 +34,26 @@ func ExampleFlagSet_ShorthandLookup() {
 
 	fmt.Println(flag.Name)
 }
+
+func ExampleFlagSet_StringToString() {
+	args := []string{
+		"--arg", "a=1,b=2",
+		"--arg", "a=2",
+		"--arg=d=4",
+	}
+
+	fs := pflag.NewFlagSet("Example", pflag.ContinueOnError)
+	fs.StringToString("arg", make(map[string]string), "string-to-string arg accepting key=value pairs")
+
+	if err := fs.Parse(args); err != nil {
+		panic(err)
+	}
+
+	value, err := fs.GetStringToString("arg")
+	if err != nil {
+		panic(err)
+	}
+
+	fmt.Println(value)
+	// Output: map[a:2 b:2 d:4]
+}

flag.go

@@ -400,7 +400,12 @@ func (f *FlagSet) lookup(name NormalizedName) *Flag {
 	return f.formal[name]
 }
 
-// func to return a given type for a given flag name
+// getFlagType performs a lookup of a flag with the given name and ftype. The flag is stringified and passed through
+// convFunc before being returned to enforce flag immutablility.
+//
+// convFunc may be nil, in which case the raw flag value is returned directly and no immutability is enforced. This is
+// particularly useful when users need to access the pointer of the underlying flag value for manipulation (e.g.
+// resetting flag values in tests).
 func (f *FlagSet) getFlagType(name string, ftype string, convFunc func(sval string) (interface{}, error)) (interface{}, error) {
 	flag := f.Lookup(name)
 	if flag == nil {
@@ -413,6 +418,10 @@ func (f *FlagSet) getFlagType(name string, ftype string, convFunc func(sval stri
 		return nil, err
 	}
 
+	if convFunc == nil {
+		return flag.Value, nil
+	}
+
 	sval := flag.Value.String()
 	result, err := convFunc(sval)
 	if err != nil {

golangflag.go

@@ -158,4 +158,3 @@ func ParseSkippedFlags(osArgs []string, goFlagSet *goflag.FlagSet) error {
 	}
 	return goFlagSet.Parse(skippedFlags)
 }
-

string_slice.go

@@ -98,9 +98,12 @@ func (f *FlagSet) GetStringSlice(name string) ([]string, error) {
 // The argument p points to a []string variable in which to store the value of the flag.
 // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly.
 // For example:
-//   --ss="v1,v2" --ss="v3"
+//
+//	--ss="v1,v2" --ss="v3"
+//
 // will result in
-//   []string{"v1", "v2", "v3"}
+//
+//	[]string{"v1", "v2", "v3"}
 func (f *FlagSet) StringSliceVar(p *[]string, name string, value []string, usage string) {
 	f.VarP(newStringSliceValue(value, p), name, "", usage)
 }
@@ -114,9 +117,12 @@ func (f *FlagSet) StringSliceVarP(p *[]string, name, shorthand string, value []s
 // The argument p points to a []string variable in which to store the value of the flag.
 // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly.
 // For example:
-//   --ss="v1,v2" --ss="v3"
+//
+//	--ss="v1,v2" --ss="v3"
+//
 // will result in
-//   []string{"v1", "v2", "v3"}
+//
+//	[]string{"v1", "v2", "v3"}
 func StringSliceVar(p *[]string, name string, value []string, usage string) {
 	CommandLine.VarP(newStringSliceValue(value, p), name, "", usage)
 }
@@ -130,9 +136,12 @@ func StringSliceVarP(p *[]string, name, shorthand string, value []string, usage
 // The return value is the address of a []string variable that stores the value of the flag.
 // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly.
 // For example:
-//   --ss="v1,v2" --ss="v3"
+//
+//	--ss="v1,v2" --ss="v3"
+//
 // will result in
-//   []string{"v1", "v2", "v3"}
+//
+//	[]string{"v1", "v2", "v3"}
 func (f *FlagSet) StringSlice(name string, value []string, usage string) *[]string {
 	p := []string{}
 	f.StringSliceVarP(&p, name, "", value, usage)
@@ -150,9 +159,12 @@ func (f *FlagSet) StringSliceP(name, shorthand string, value []string, usage str
 // The return value is the address of a []string variable that stores the value of the flag.
 // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly.
 // For example:
-//   --ss="v1,v2" --ss="v3"
+//
+//	--ss="v1,v2" --ss="v3"
+//
 // will result in
-//   []string{"v1", "v2", "v3"}
+//
+//	[]string{"v1", "v2", "v3"}
 func StringSlice(name string, value []string, usage string) *[]string {
 	return CommandLine.StringSliceP(name, "", value, usage)
 }

string_to_string.go

@@ -21,7 +21,7 @@ func newStringToStringValue(val map[string]string, p *map[string]string) *string
 	return ssv
 }
 
-// Format: a=1,b=2
+// Set updates the flag value from the given string, adding additional mappings or updating existing ones.
 func (s *stringToStringValue) Set(val string) error {
 	var ss []string
 	n := strings.Count(val, "=")
@@ -47,13 +47,17 @@ func (s *stringToStringValue) Set(val string) error {
 		}
 		out[kv[0]] = kv[1]
 	}
+
+	// clear out any default flag values
 	if !s.changed {
-		*s.value = out
-	} else {
-		for k, v := range out {
-			(*s.value)[k] = v
+		for k := range *s.value {
+			delete(*s.value, k)
 		}
 	}
+
+	for k, v := range out {
+		(*s.value)[k] = v
+	}
 	s.changed = true
 	return nil
 }
@@ -84,85 +88,100 @@ func (s *stringToStringValue) String() string {
 	return "[" + strings.TrimSpace(buf.String()) + "]"
 }
 
-func stringToStringConv(val string) (interface{}, error) {
-	val = strings.Trim(val, "[]")
-	// An empty string would cause an empty map
-	if len(val) == 0 {
-		return map[string]string{}, nil
-	}
-	r := csv.NewReader(strings.NewReader(val))
-	ss, err := r.Read()
-	if err != nil {
-		return nil, err
-	}
-	out := make(map[string]string, len(ss))
-	for _, pair := range ss {
-		kv := strings.SplitN(pair, "=", 2)
-		if len(kv) != 2 {
-			return nil, fmt.Errorf("%s must be formatted as key=value", pair)
-		}
-		out[kv[0]] = kv[1]
-	}
-	return out, nil
-}
-
-// GetStringToString return the map[string]string value of a flag with the given name
+// GetStringToString return the map value of a flag with the given name from f. The returned map shares memory with the
+// internal flag value [Flag.Value].
 func (f *FlagSet) GetStringToString(name string) (map[string]string, error) {
-	val, err := f.getFlagType(name, "stringToString", stringToStringConv)
+	val, err := f.getFlagType(name, "stringToString", nil)
 	if err != nil {
 		return map[string]string{}, err
 	}
-	return val.(map[string]string), nil
-}
-
-// StringToStringVar defines a string flag with specified name, default value, and usage string.
-// The argument p points to a map[string]string variable in which to store the values of the multiple flags.
-// The value of each argument will not try to be separated by comma
-func (f *FlagSet) StringToStringVar(p *map[string]string, name string, value map[string]string, usage string) {
-	f.VarP(newStringToStringValue(value, p), name, "", usage)
-}
-
-// StringToStringVarP is like StringToStringVar, but accepts a shorthand letter that can be used after a single dash.
-func (f *FlagSet) StringToStringVarP(p *map[string]string, name, shorthand string, value map[string]string, usage string) {
-	f.VarP(newStringToStringValue(value, p), name, shorthand, usage)
-}
 
-// StringToStringVar defines a string flag with specified name, default value, and usage string.
-// The argument p points to a map[string]string variable in which to store the value of the flag.
-// The value of each argument will not try to be separated by comma
-func StringToStringVar(p *map[string]string, name string, value map[string]string, usage string) {
-	CommandLine.VarP(newStringToStringValue(value, p), name, "", usage)
-}
+	fv, ok := val.(*stringToStringValue)
+	if !ok {
+		panic(fmt.Errorf("illegal state: unspected internal type for stringToString flag '%s'", name))
+	}
+	if fv.value == nil {
+		return nil, nil
+	}
 
-// StringToStringVarP is like StringToStringVar, but accepts a shorthand letter that can be used after a single dash.
-func StringToStringVarP(p *map[string]string, name, shorthand string, value map[string]string, usage string) {
-	CommandLine.VarP(newStringToStringValue(value, p), name, shorthand, usage)
+	return *fv.value, nil
 }
 
-// StringToString defines a string flag with specified name, default value, and usage string.
-// The return value is the address of a map[string]string variable that stores the value of the flag.
-// The value of each argument will not try to be separated by comma
+// StringToString defines a map flag with specified name, default value, and usage string.
+//
+// StringToString flags are used to pass key=value pairs to applications. The same flag can be provided more than once
+// with all key=value pairs being merged into a final map. Multiple key=value pairs may be provided in a single arg,
+// separated by commas. A few simple examples include:
+//
+//	--arg a=1
+//	--arg a=1 --arg b=2
+//	--arg a=1,b=2
+//	--arg=a=1
+//
+// As a special case, a single key-value pair with a value containing a comma will be interpreted as a single pair:
+//
+//	--arg a=1,2
+//
+// Returns a pointer to the map which will be updated upon invocation of [FlagSet.Parse], [Flag.Value.Set], and others.
 func (f *FlagSet) StringToString(name string, value map[string]string, usage string) *map[string]string {
 	p := map[string]string{}
 	f.StringToStringVarP(&p, name, "", value, usage)
 	return &p
 }
 
-// StringToStringP is like StringToString, but accepts a shorthand letter that can be used after a single dash.
+// StringToStringP is like [FlagSet.StringToString], but also accepts a shorthand letter that can be used after a single
+// dash.
+//
+// See [FlagSet.StringToString].
 func (f *FlagSet) StringToStringP(name, shorthand string, value map[string]string, usage string) *map[string]string {
 	p := map[string]string{}
 	f.StringToStringVarP(&p, name, shorthand, value, usage)
 	return &p
 }
 
+// StringToStringVar is like [FlagSet.StringToString], but also accepts a map ppointer argument p which is updated with
+// the parsed key-value pairs.
+//
+// See [FlagSet.StringToString].
+func (f *FlagSet) StringToStringVar(p *map[string]string, name string, value map[string]string, usage string) {
+	f.VarP(newStringToStringValue(value, p), name, "", usage)
+}
+
+// StringToStringVarP is like [FlagSet.StringToString], but also accepts a map ppointer argument p which is updated with
+// the parsed key-value pairs, and a shorthand letter that can be used after a single dash.
+//
+// See [FlagSet.StringToString].
+func (f *FlagSet) StringToStringVarP(p *map[string]string, name, shorthand string, value map[string]string, usage string) {
+	f.VarP(newStringToStringValue(value, p), name, shorthand, usage)
+}
+
 // StringToString defines a string flag with specified name, default value, and usage string.
-// The return value is the address of a map[string]string variable that stores the value of the flag.
-// The value of each argument will not try to be separated by comma
+//
+// See [FlagSet.StringToString].
 func StringToString(name string, value map[string]string, usage string) *map[string]string {
 	return CommandLine.StringToStringP(name, "", value, usage)
 }
 
-// StringToStringP is like StringToString, but accepts a shorthand letter that can be used after a single dash.
+// StringToStringP is like [FlagSet.StringToString], but also accepts a shorthand letter that can be used after a single
+// dash.
+//
+// See [FlagSet.StringToString].
 func StringToStringP(name, shorthand string, value map[string]string, usage string) *map[string]string {
 	return CommandLine.StringToStringP(name, shorthand, value, usage)
 }
+
+// StringToStringVar is like [FlagSet.StringToString], but also accepts a map ppointer argument p which is updated with
+// the parsed key-value pairs.
+//
+// See [FlagSet.StringToString].
+func StringToStringVar(p *map[string]string, name string, value map[string]string, usage string) {
+	CommandLine.VarP(newStringToStringValue(value, p), name, "", usage)
+}
+
+// StringToStringVarP is like [FlagSet.StringToString], but also accepts a map ppointer argument p which is updated with
+// the parsed key-value pairs, and a shorthand letter that can be used after a single dash.
+//
+// See [FlagSet.StringToString].
+func StringToStringVarP(p *map[string]string, name, shorthand string, value map[string]string, usage string) {
+	CommandLine.VarP(newStringToStringValue(value, p), name, shorthand, usage)
+}