From c98b85d392f18d938a481adc6c933e43c79970ce Mon Sep 17 00:00:00 2001 From: AllyDale Date: Fri, 5 Feb 2021 15:16:50 +0800 Subject: [PATCH] bug fix #1235 : default value changes with parsed values on slice flags --- .gitignore | 2 ++ flag_float64_slice.go | 19 ++++++++++--- flag_int64_slice.go | 19 ++++++++++--- flag_int_slice.go | 19 ++++++++++--- flag_string_slice.go | 28 ++++++++++++------- flag_test.go | 65 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 22 deletions(-) diff --git a/.gitignore b/.gitignore index 2d5e149b43..afdca418ca 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ vendor .idea internal/*/built-example coverage.txt + +*.exe diff --git a/flag_float64_slice.go b/flag_float64_slice.go index 706ee6cd4b..b625ca1756 100644 --- a/flag_float64_slice.go +++ b/flag_float64_slice.go @@ -19,6 +19,16 @@ func NewFloat64Slice(defaults ...float64) *Float64Slice { return &Float64Slice{slice: append([]float64{}, defaults...)} } +// clone allocate a copy of self object +func (f *Float64Slice) clone() *Float64Slice { + n := &Float64Slice{ + slice: make([]float64, len(f.slice)), + hasBeenSet: f.hasBeenSet, + } + copy(n.slice, f.slice) + return n +} + // Set parses the value into a float64 and appends it to the list of values func (f *Float64Slice) Set(value string) error { if !f.hasBeenSet { @@ -133,11 +143,12 @@ func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error { } } + if f.Value == nil { + f.Value = &Float64Slice{} + } + copyValue := f.Value.clone() for _, name := range f.Names() { - if f.Value == nil { - f.Value = &Float64Slice{} - } - set.Var(f.Value, name, f.Usage) + set.Var(copyValue, name, f.Usage) } return nil diff --git a/flag_int64_slice.go b/flag_int64_slice.go index 2c9a15af3e..d2352c08fa 100644 --- a/flag_int64_slice.go +++ b/flag_int64_slice.go @@ -19,6 +19,16 @@ func NewInt64Slice(defaults ...int64) *Int64Slice { return &Int64Slice{slice: append([]int64{}, defaults...)} } +// clone allocate a copy of self object +func (i *Int64Slice) clone() *Int64Slice { + n := &Int64Slice{ + slice: make([]int64, len(i.slice)), + hasBeenSet: i.hasBeenSet, + } + copy(n.slice, i.slice) + return n +} + // Set parses the value into an integer and appends it to the list of values func (i *Int64Slice) Set(value string) error { if !i.hasBeenSet { @@ -132,11 +142,12 @@ func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error { f.HasBeenSet = true } + if f.Value == nil { + f.Value = &Int64Slice{} + } + copyValue := f.Value.clone() for _, name := range f.Names() { - if f.Value == nil { - f.Value = &Int64Slice{} - } - set.Var(f.Value, name, f.Usage) + set.Var(copyValue, name, f.Usage) } return nil diff --git a/flag_int_slice.go b/flag_int_slice.go index a73ca6b81c..018d56b625 100644 --- a/flag_int_slice.go +++ b/flag_int_slice.go @@ -19,6 +19,16 @@ func NewIntSlice(defaults ...int) *IntSlice { return &IntSlice{slice: append([]int{}, defaults...)} } +// clone allocate a copy of self object +func (i *IntSlice) clone() *IntSlice { + n := &IntSlice{ + slice: make([]int, len(i.slice)), + hasBeenSet: i.hasBeenSet, + } + copy(n.slice, i.slice) + return n +} + // TODO: Consistently have specific Set function for Int64 and Float64 ? // SetInt directly adds an integer to the list of values func (i *IntSlice) SetInt(value int) { @@ -143,11 +153,12 @@ func (f *IntSliceFlag) Apply(set *flag.FlagSet) error { f.HasBeenSet = true } + if f.Value == nil { + f.Value = &IntSlice{} + } + copyValue := f.Value.clone() for _, name := range f.Names() { - if f.Value == nil { - f.Value = &IntSlice{} - } - set.Var(f.Value, name, f.Usage) + set.Var(copyValue, name, f.Usage) } return nil diff --git a/flag_string_slice.go b/flag_string_slice.go index 35497032cb..0b45ab5044 100644 --- a/flag_string_slice.go +++ b/flag_string_slice.go @@ -18,6 +18,16 @@ func NewStringSlice(defaults ...string) *StringSlice { return &StringSlice{slice: append([]string{}, defaults...)} } +// clone allocate a copy of self object +func (s *StringSlice) clone() *StringSlice { + n := &StringSlice{ + slice: make([]string, len(s.slice)), + hasBeenSet: s.hasBeenSet, + } + copy(n.slice, s.slice) + return n +} + // Set appends the string value to the list of values func (s *StringSlice) Set(value string) error { if !s.hasBeenSet { @@ -144,17 +154,15 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error { f.HasBeenSet = true } + if f.Value == nil { + f.Value = &StringSlice{} + } + setValue := f.Destination + if f.Destination == nil { + setValue = f.Value.clone() + } for _, name := range f.Names() { - if f.Value == nil { - f.Value = &StringSlice{} - } - - if f.Destination != nil { - set.Var(f.Destination, name, f.Usage) - continue - } - - set.Var(f.Value, name, f.Usage) + set.Var(setValue, name, f.Usage) } return nil diff --git a/flag_test.go b/flag_test.go index b3b0d7c587..0f8e12adef 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1973,3 +1973,68 @@ func TestTimestampFlagApply_Fail_Parse_Wrong_Time(t *testing.T) { err := set.Parse([]string{"--time", "2006-01-02T15:04:05Z"}) expect(t, err, fmt.Errorf("invalid value \"2006-01-02T15:04:05Z\" for flag -time: parsing time \"2006-01-02T15:04:05Z\" as \"Jan 2, 2006 at 3:04pm (MST)\": cannot parse \"2006-01-02T15:04:05Z\" as \"Jan\"")) } + +type flagDefaultTestCase struct { + name string + flag Flag + toParse []string + expect string +} + +func TestFlagDefaultValue(t *testing.T) { + cases := []*flagDefaultTestCase{ + &flagDefaultTestCase{ + name: "stringSclice", + flag: &StringSliceFlag{Name: "flag", Value: NewStringSlice("default1", "default2")}, + toParse: []string{"--flag", "parsed"}, + expect: `--flag value (default: "default1", "default2") (accepts multiple inputs)`, + }, + &flagDefaultTestCase{ + name: "float64Sclice", + flag: &Float64SliceFlag{Name: "flag", Value: NewFloat64Slice(1.1, 2.2)}, + toParse: []string{"--flag", "13.3"}, + expect: `--flag value (default: 1.1, 2.2) (accepts multiple inputs)`, + }, + &flagDefaultTestCase{ + name: "int64Sclice", + flag: &Int64SliceFlag{Name: "flag", Value: NewInt64Slice(1, 2)}, + toParse: []string{"--flag", "13"}, + expect: `--flag value (default: 1, 2) (accepts multiple inputs)`, + }, + &flagDefaultTestCase{ + name: "intSclice", + flag: &IntSliceFlag{Name: "flag", Value: NewIntSlice(1, 2)}, + toParse: []string{"--flag", "13"}, + expect: `--flag value (default: 1, 2) (accepts multiple inputs)`, + }, + &flagDefaultTestCase{ + name: "string", + flag: &StringFlag{Name: "flag", Value: "default"}, + toParse: []string{"--flag", "parsed"}, + expect: `--flag value (default: "default")`, + }, + &flagDefaultTestCase{ + name: "bool", + flag: &BoolFlag{Name: "flag", Value: true}, + toParse: []string{"--flag", "false"}, + expect: `--flag (default: true)`, + }, + &flagDefaultTestCase{ + name: "uint64", + flag: &Uint64Flag{Name: "flag", Value: 1}, + toParse: []string{"--flag", "13"}, + expect: `--flag value (default: 1)`, + }, + } + for i, v := range cases { + set := flag.NewFlagSet("test", 0) + set.SetOutput(ioutil.Discard) + _ = v.flag.Apply(set) + if err := set.Parse(v.toParse); err != nil { + t.Error(err) + } + if got := v.flag.String(); got != v.expect { + t.Errorf("TestFlagDefaultValue %d %s\nexpect:%s\ngot:%s", i, v.name, v.expect, got) + } + } +}