common/collections: Allow a mix of slice types in append/Scratch.Add

The type handling in these was improved in Hugo 0.49, but this also meant that it was no longer possible to start out with a string slice and later append `Page` etc. to it.

This commit makes sure that the old behaviour is now possible again by falling back to a `[]interface{}` as a last resort.

Fixes #5361
This commit is contained in:
Bjørn Erik Pedersen 2018-10-27 11:10:39 +02:00
parent b27ccf34bf
commit dac7092a9c
3 changed files with 51 additions and 4 deletions

View file

@ -48,6 +48,10 @@ func Append(to interface{}, from ...interface{}) (interface{}, error) {
// If we get []string []string, we append the from slice to to // If we get []string []string, we append the from slice to to
if tot == fromt { if tot == fromt {
return reflect.AppendSlice(tov, fromv).Interface(), nil return reflect.AppendSlice(tov, fromv).Interface(), nil
} else if !fromt.AssignableTo(tot) {
// Fall back to a []interface{} slice.
return appendToInterfaceSliceFromValues(tov, fromv)
} }
} }
} }
@ -60,7 +64,8 @@ func Append(to interface{}, from ...interface{}) (interface{}, error) {
for _, f := range from { for _, f := range from {
fv := reflect.ValueOf(f) fv := reflect.ValueOf(f)
if !fv.Type().AssignableTo(tot) { if !fv.Type().AssignableTo(tot) {
return nil, fmt.Errorf("append element type mismatch: expected %v, got %v", tot, fv.Type()) // Fall back to a []interface{} slice.
return appendToInterfaceSlice(tov, from...)
} }
tov = reflect.Append(tov, fv) tov = reflect.Append(tov, fv)
} }
@ -68,6 +73,32 @@ func Append(to interface{}, from ...interface{}) (interface{}, error) {
return tov.Interface(), nil return tov.Interface(), nil
} }
func appendToInterfaceSliceFromValues(slice1, slice2 reflect.Value) ([]interface{}, error) {
var tos []interface{}
for _, slice := range []reflect.Value{slice1, slice2} {
for i := 0; i < slice.Len(); i++ {
tos = append(tos, slice.Index(i).Interface())
}
}
return tos, nil
}
func appendToInterfaceSlice(tov reflect.Value, from ...interface{}) ([]interface{}, error) {
var tos []interface{}
for i := 0; i < tov.Len(); i++ {
tos = append(tos, tov.Index(i).Interface())
}
for _, v := range from {
tos = append(tos, v)
}
return tos, nil
}
// indirect is borrowed from the Go stdlib: 'text/template/exec.go' // indirect is borrowed from the Go stdlib: 'text/template/exec.go'
// TODO(bep) consolidate // TODO(bep) consolidate
func indirect(v reflect.Value) (rv reflect.Value, isNil bool) { func indirect(v reflect.Value) (rv reflect.Value, isNil bool) {

View file

@ -46,10 +46,12 @@ func TestAppend(t *testing.T) {
{testSlicerInterfaces{&tstSlicerIn1{"a"}, &tstSlicerIn1{"b"}}, {testSlicerInterfaces{&tstSlicerIn1{"a"}, &tstSlicerIn1{"b"}},
[]interface{}{&tstSlicerIn1{"c"}}, []interface{}{&tstSlicerIn1{"c"}},
testSlicerInterfaces{&tstSlicerIn1{"a"}, &tstSlicerIn1{"b"}, &tstSlicerIn1{"c"}}}, testSlicerInterfaces{&tstSlicerIn1{"a"}, &tstSlicerIn1{"b"}, &tstSlicerIn1{"c"}}},
//https://github.com/gohugoio/hugo/issues/5361
{[]string{"a", "b"}, []interface{}{tstSlicers{&tstSlicer{"a"}, &tstSlicer{"b"}}},
[]interface{}{"a", "b", &tstSlicer{"a"}, &tstSlicer{"b"}}},
{[]string{"a", "b"}, []interface{}{&tstSlicer{"a"}},
[]interface{}{"a", "b", &tstSlicer{"a"}}},
// Errors // Errors
{testSlicerInterfaces{&tstSlicerIn1{"a"}, &tstSlicerIn1{"b"}},
[]interface{}{"c"},
false},
{"", []interface{}{[]string{"a", "b"}}, false}, {"", []interface{}{[]string{"a", "b"}}, false},
// No string concatenation. // No string concatenation.
{"ab", {"ab",

View file

@ -96,6 +96,20 @@ func TestScratchAddTypedSliceToInterfaceSlice(t *testing.T) {
} }
// https://github.com/gohugoio/hugo/issues/5361
func TestScratchAddDifferentTypedSliceToInterfaceSlice(t *testing.T) {
t.Parallel()
assert := require.New(t)
scratch := NewScratch()
scratch.Set("slice", []string{"foo"})
_, err := scratch.Add("slice", []int{1, 2})
assert.NoError(err)
assert.Equal([]interface{}{"foo", 1, 2}, scratch.Get("slice"))
}
func TestScratchSet(t *testing.T) { func TestScratchSet(t *testing.T) {
t.Parallel() t.Parallel()
assert := require.New(t) assert := require.New(t)