Skip to content

Commit

Permalink
tpl/collections: Fix append when appending a slice to a slice of slices
Browse files Browse the repository at this point in the history
Fixes #11004
  • Loading branch information
bep committed Jun 13, 2023
1 parent 258884f commit 4d22ba2
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 2 deletions.
22 changes: 22 additions & 0 deletions common/collections/append.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,35 @@ func Append(to any, from ...any) (any, error) {
return Slice(from...), nil
}

// See issue #11004.
isToASliceOfSlices := tov.Len() > 0
for i := 0; i < tov.Len(); i++ {
v, isNil := indirect(tov.Index(i))
if isNil || v.Kind() != reflect.Slice {
isToASliceOfSlices = false
break
}
}

if isToASliceOfSlices {
fv := reflect.ValueOf(from)
if !fv.Type().AssignableTo(tot) {
// Fall back to a []interface{} slice.
tov, _ := indirect(reflect.ValueOf(to))
return appendToInterfaceSlice(tov, from)
}

return reflect.Append(tov, fv).Interface(), nil
}

for _, f := range from {
fv := reflect.ValueOf(f)
if !fv.Type().AssignableTo(tot) {
// Fall back to a []interface{} slice.
tov, _ := indirect(reflect.ValueOf(to))
return appendToInterfaceSlice(tov, from...)
}

tov = reflect.Append(tov, fv)
}

Expand Down
38 changes: 36 additions & 2 deletions common/collections/append_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestAppend(t *testing.T) {
t.Parallel()
c := qt.New(t)

for _, test := range []struct {
for i, test := range []struct {
start any
addend []any
expected any
Expand Down Expand Up @@ -66,6 +66,40 @@ func TestAppend(t *testing.T) {
[]any{&tstSlicer{"a"}},
[]any{"a", "b", &tstSlicer{"a"}},
},
// Issue ##11004
{
testSlicerInterfaces{testSlicerInterfaces{&tstSlicerIn1{"a"}, &tstSlicerIn1{"b"}}},
[]any{testSlicerInterfaces{testSlicerInterfaces{&tstSlicerIn1{"c"}}}},
testSlicerInterfaces{
testSlicerInterfaces{
&tstSlicerIn1{TheName: "a"},
&tstSlicerIn1{TheName: "b"},
},
testSlicerInterfaces{
&tstSlicerIn1{TheName: "c"},
},
},
},
{
[]any{[]string{"a", "b"}},
[]any{&tstSlicer{"a"}},
[]interface{}{
[]string{"a", "b"},
[]interface{}{
&tstSlicer{TheName: "a"},
},
},
},
{
[]any{[]string{"a", "b"}},
[]any{"c"},
[]interface{}{
[]string{"a", "b"},
[]interface{}{
"c",
},
},
},
// Errors
{"", []any{[]string{"a", "b"}}, false},
// No string concatenation.
Expand All @@ -85,6 +119,6 @@ func TestAppend(t *testing.T) {
}

c.Assert(err, qt.IsNil)
c.Assert(result, qt.DeepEquals, test.expected)
c.Assert(result, qt.DeepEquals, test.expected, qt.Commentf("[%d] %v", i, test.start))
}
}
4 changes: 4 additions & 0 deletions common/collections/slice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ type testSlicerInterface interface {

type testSlicerInterfaces []testSlicerInterface

func (t testSlicerInterfaces) Name() string {
return "testSlicerInterfaces"
}

type tstSlicerIn1 struct {
TheName string
}
Expand Down
5 changes: 5 additions & 0 deletions tpl/collections/append_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ func TestAppend(t *testing.T) {
{[]string{"a", "b"}, []any{"c"}, []string{"a", "b", "c"}},
{[]string{"a", "b"}, []any{"c", "d", "e"}, []string{"a", "b", "c", "d", "e"}},
{[]string{"a", "b"}, []any{[]string{"c", "d", "e"}}, []string{"a", "b", "c", "d", "e"}},
// Issue #11004
{[]any{[]any{"a"}}, []any{"b"}, []any{[]any{"a"}, []any{"b"}}},
{[]any{[]string{"a"}}, []any{"b"}, []any{[]string{"a"}, []any{"b"}}},
{[]any{[]any{"a"}, "b"}, []any{"c"}, []any{[]any{"a"}, "b", "c"}},

// Errors
{"", []any{[]string{"a", "b"}}, false},
{[]string{"a", "b"}, []any{}, false},
Expand Down

0 comments on commit 4d22ba2

Please sign in to comment.