Skip to content

Commit eddb896

Browse files
allenfluxallenflux
committed
fix: initialize parsers in collection value constructors
1 parent 192ed0b commit eddb896

3 files changed

Lines changed: 186 additions & 1 deletion

File tree

‎constructor_values_test.go‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
package cli_test
2+
3+
import (
4+
"flag"
5+
"reflect"
6+
"testing"
7+
8+
"github.com/urfave/cli/v3"
9+
)
10+
11+
func TestConstructedValues(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
newValue func() cli.Value
15+
defaults any
16+
input string
17+
want any
18+
next string
19+
wantNext any
20+
}{
21+
{"string slice", func() cli.Value { return cli.NewStringSlice("old") }, []string{"old"}, "new, next", []string{"new", " next"}, "last", []string{"new", " next", "last"}},
22+
{"int slice", func() cli.Value { return cli.NewIntSlice(1) }, []int{1}, "2,3", []int{2, 3}, "4", []int{2, 3, 4}},
23+
{"int8 slice", func() cli.Value { return cli.NewInt8Slice(1) }, []int8{1}, "2,3", []int8{2, 3}, "4", []int8{2, 3, 4}},
24+
{"int16 slice", func() cli.Value { return cli.NewInt16Slice(1) }, []int16{1}, "2,3", []int16{2, 3}, "4", []int16{2, 3, 4}},
25+
{"int32 slice", func() cli.Value { return cli.NewInt32Slice(1) }, []int32{1}, "2,3", []int32{2, 3}, "4", []int32{2, 3, 4}},
26+
{"int64 slice", func() cli.Value { return cli.NewInt64Slice(1) }, []int64{1}, "2,3", []int64{2, 3}, "4", []int64{2, 3, 4}},
27+
{"uint slice", func() cli.Value { return cli.NewUintSlice(1) }, []uint{1}, "2,3", []uint{2, 3}, "4", []uint{2, 3, 4}},
28+
{"uint8 slice", func() cli.Value { return cli.NewUint8Slice(1) }, []uint8{1}, "2,3", []uint8{2, 3}, "4", []uint8{2, 3, 4}},
29+
{"uint16 slice", func() cli.Value { return cli.NewUint16Slice(1) }, []uint16{1}, "2,3", []uint16{2, 3}, "4", []uint16{2, 3, 4}},
30+
{"uint32 slice", func() cli.Value { return cli.NewUint32Slice(1) }, []uint32{1}, "2,3", []uint32{2, 3}, "4", []uint32{2, 3, 4}},
31+
{"uint64 slice", func() cli.Value { return cli.NewUint64Slice(1) }, []uint64{1}, "2,3", []uint64{2, 3}, "4", []uint64{2, 3, 4}},
32+
{"float slice", func() cli.Value { return cli.NewFloatSlice(1) }, []float64{1}, "2.5,3.5", []float64{2.5, 3.5}, "4.5", []float64{2.5, 3.5, 4.5}},
33+
{"float32 slice", func() cli.Value { return cli.NewFloat32Slice(1) }, []float32{1}, "2.5,3.5", []float32{2.5, 3.5}, "4.5", []float32{2.5, 3.5, 4.5}},
34+
{"float64 slice", func() cli.Value { return cli.NewFloat64Slice(1) }, []float64{1}, "2.5,3.5", []float64{2.5, 3.5}, "4.5", []float64{2.5, 3.5, 4.5}},
35+
{"string map", func() cli.Value { return cli.NewStringMap(map[string]string{"old": "value"}) }, map[string]string{"old": "value"}, "key=value,other= entry ", map[string]string{"key": "value", "other": " entry "}, "key=last", map[string]string{"key": "last", "other": " entry "}},
36+
}
37+
38+
for _, test := range tests {
39+
t.Run(test.name, func(t *testing.T) {
40+
t.Run("defaults", func(t *testing.T) {
41+
if got := test.newValue().Get(); !reflect.DeepEqual(got, test.defaults) {
42+
t.Fatalf("Get() = %#v, want %#v", got, test.defaults)
43+
}
44+
})
45+
t.Run("set", func(t *testing.T) {
46+
defer func() {
47+
if recovered := recover(); recovered != nil {
48+
t.Fatalf("Set panicked: %v", recovered)
49+
}
50+
}()
51+
value := test.newValue()
52+
flags := flag.NewFlagSet(test.name, flag.ContinueOnError)
53+
flags.Var(value, "value", "")
54+
if err := flags.Set("value", test.input); err != nil {
55+
t.Fatal(err)
56+
}
57+
if got := value.Get(); !reflect.DeepEqual(got, test.want) {
58+
t.Fatalf("Get() = %#v, want %#v", got, test.want)
59+
}
60+
if err := flags.Parse([]string{"-value", test.next}); err != nil {
61+
t.Fatal(err)
62+
}
63+
if got := value.Get(); !reflect.DeepEqual(got, test.wantNext) {
64+
t.Fatalf("Get() = %#v, want %#v", got, test.wantNext)
65+
}
66+
})
67+
})
68+
}
69+
}
70+
71+
func TestConstructedValuesEmptyDefaults(t *testing.T) {
72+
tests := []struct {
73+
name string
74+
value cli.Value
75+
defaults any
76+
input string
77+
want any
78+
}{
79+
{"nil slice", cli.NewStringSlice(), []string(nil), "value", []string{"value"}},
80+
{"empty slice", cli.NewStringSlice([]string{}...), []string{}, "value", []string{"value"}},
81+
{"nil map", cli.NewStringMap(nil), map[string]string(nil), "key=value", map[string]string{"key": "value"}},
82+
}
83+
for _, test := range tests {
84+
t.Run(test.name, func(t *testing.T) {
85+
if got := test.value.Get(); !reflect.DeepEqual(got, test.defaults) {
86+
t.Fatalf("Get() = %#v, want %#v", got, test.defaults)
87+
}
88+
if err := test.value.Set(test.input); err != nil {
89+
t.Fatal(err)
90+
}
91+
if got := test.value.Get(); !reflect.DeepEqual(got, test.want) {
92+
t.Fatalf("Get() = %#v, want %#v", got, test.want)
93+
}
94+
})
95+
}
96+
}
97+
98+
func TestConstructedValuesParseErrors(t *testing.T) {
99+
tests := []struct {
100+
name string
101+
value cli.Value
102+
input string
103+
}{
104+
{"int8 overflow", cli.NewInt8Slice(), "128"},
105+
{"negative uint", cli.NewUintSlice(), "-1"},
106+
{"invalid float", cli.NewFloatSlice(), "not-a-float"},
107+
{"missing map separator", cli.NewStringMap(nil), "key"},
108+
}
109+
for _, test := range tests {
110+
t.Run(test.name, func(t *testing.T) {
111+
if err := test.value.Set(test.input); err == nil {
112+
t.Fatalf("Set(%q) unexpectedly succeeded", test.input)
113+
}
114+
})
115+
}
116+
}
117+
118+
func TestConstructedValuesSerializedThenSet(t *testing.T) {
119+
t.Run("slice", func(t *testing.T) {
120+
value := cli.NewStringSlice("old")
121+
if err := value.Set(cli.NewStringSlice("new").Serialize()); err != nil {
122+
t.Fatal(err)
123+
}
124+
if err := value.Set("last"); err != nil {
125+
t.Fatal(err)
126+
}
127+
if got, want := value.Value(), []string{"new", "last"}; !reflect.DeepEqual(got, want) {
128+
t.Fatalf("Value() = %#v, want %#v", got, want)
129+
}
130+
})
131+
t.Run("map", func(t *testing.T) {
132+
value := cli.NewStringMap(map[string]string{"old": "value"})
133+
if err := value.Set(cli.NewStringMap(map[string]string{"new": "value"}).Serialize()); err != nil {
134+
t.Fatal(err)
135+
}
136+
if err := value.Set("last=entry"); err != nil {
137+
t.Fatal(err)
138+
}
139+
if got, want := value.Value(), map[string]string{"new": "value", "last": "entry"}; !reflect.DeepEqual(got, want) {
140+
t.Fatalf("Value() = %#v, want %#v", got, want)
141+
}
142+
})
143+
}
144+
145+
func TestConstructedValuesDefaultStorage(t *testing.T) {
146+
t.Run("slice", func(t *testing.T) {
147+
defaults := []string{"old"}
148+
value := cli.NewStringSlice(defaults...)
149+
defaults[0] = "changed"
150+
if got, want := value.Value(), []string{"changed"}; !reflect.DeepEqual(got, want) {
151+
t.Fatalf("Value() = %#v, want %#v", got, want)
152+
}
153+
if err := value.Set("new"); err != nil {
154+
t.Fatal(err)
155+
}
156+
if got := defaults[0]; got != "changed" {
157+
t.Fatalf("Set modified defaults: %q", got)
158+
}
159+
})
160+
t.Run("map", func(t *testing.T) {
161+
defaults := map[string]string{"old": "value"}
162+
value := cli.NewStringMap(defaults)
163+
defaults["old"] = "changed"
164+
if got, want := value.Value(), map[string]string{"old": "changed"}; !reflect.DeepEqual(got, want) {
165+
t.Fatalf("Value() = %#v, want %#v", got, want)
166+
}
167+
if err := value.Set("new=entry"); err != nil {
168+
t.Fatal(err)
169+
}
170+
if got := defaults["old"]; got != "changed" {
171+
t.Fatalf("Set modified defaults: %q", got)
172+
}
173+
if _, ok := defaults["new"]; ok {
174+
t.Fatal("Set added an entry to defaults")
175+
}
176+
})
177+
}

‎flag_map_impl.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,12 @@ func (i MapBase[T, C, VC]) Create(val map[string]T, p *map[string]T, c C) Value
4040

4141
// NewMapBase makes a *MapBase with default values
4242
func NewMapBase[T any, C any, VC ValueCreator[T, C]](defaults map[string]T) *MapBase[T, C, VC] {
43+
var t T
44+
var c C
45+
var vc VC
4346
return &MapBase[T, C, VC]{
44-
dict: &defaults,
47+
dict: &defaults,
48+
value: vc.Create(t, new(T), c),
4549
}
4650
}
4751

‎flag_slice_base.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,12 @@ func (i SliceBase[T, C, VC]) Create(val []T, p *[]T, c C) Value {
3030

3131
// NewSliceBase makes a *SliceBase with default values
3232
func NewSliceBase[T any, C any, VC ValueCreator[T, C]](defaults ...T) *SliceBase[T, C, VC] {
33+
var t T
34+
var c C
35+
var vc VC
3336
return &SliceBase[T, C, VC]{
3437
slice: &defaults,
38+
value: vc.Create(t, new(T), c),
3539
}
3640
}
3741

0 commit comments

Comments
 (0)