From cefe74dd5169e5d641439d5e8236d02c65449526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 14 Jun 2023 09:00:28 +0200 Subject: [PATCH 1/2] tpl/collections: Fix append when appending a slice to a slice of slices Fixes #11093 --- common/collections/append.go | 29 +++++++++++++++++-- common/collections/append_test.go | 45 +++++++++++++++++++++++++++-- tpl/collections/integration_test.go | 31 ++++++++++++++++++++ 3 files changed, 101 insertions(+), 4 deletions(-) diff --git a/common/collections/append.go b/common/collections/append.go index a9c14c1aa1d..fe8792fc421 100644 --- a/common/collections/append.go +++ b/common/collections/append.go @@ -22,6 +22,9 @@ import ( // If length of from is one and the only element is a slice of same type as to, // it will be appended. func Append(to any, from ...any) (any, error) { + if len(from) == 0 { + return to, nil + } tov, toIsNil := indirect(reflect.ValueOf(to)) toIsNil = toIsNil || to == nil @@ -33,18 +36,39 @@ func Append(to any, from ...any) (any, error) { } tot = tov.Type().Elem() + if tot.Kind() == reflect.Slice { + totvt := tot.Elem() + fromvs := make([]reflect.Value, len(from)) + for i, f := range from { + fromv := reflect.ValueOf(f) + fromt := fromv.Type() + if fromt.Kind() == reflect.Slice { + fromt = fromt.Elem() + } + if totvt != fromt { + return nil, fmt.Errorf("cannot append slice of %s to slice of %s", fromt, totvt) + } else { + fromvs[i] = fromv + } + } + return reflect.Append(tov, fromvs...).Interface(), nil + + } + toIsNil = tov.Len() == 0 if len(from) == 1 { fromv := reflect.ValueOf(from[0]) + fromt := fromv.Type() + if fromt.Kind() == reflect.Slice { + fromt = fromt.Elem() + } if fromv.Kind() == reflect.Slice { if toIsNil { // If we get nil []string, we just return the []string return from[0], nil } - fromt := reflect.TypeOf(from[0]).Elem() - // If we get []string []string, we append the from slice to to if tot == fromt { return reflect.AppendSlice(tov, fromv).Interface(), nil @@ -52,6 +76,7 @@ func Append(to any, from ...any) (any, error) { // Fall back to a []interface{} slice. return appendToInterfaceSliceFromValues(tov, fromv) } + } } } diff --git a/common/collections/append_test.go b/common/collections/append_test.go index 6df32fee68d..f997e7a20b0 100644 --- a/common/collections/append_test.go +++ b/common/collections/append_test.go @@ -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 @@ -85,6 +85,47 @@ 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("test: [%d] %v", i, test)) } } + +// #11093 +func TestAppendToMultiDimensionalSlice(t *testing.T) { + t.Parallel() + c := qt.New(t) + + for _, test := range []struct { + to any + from []any + expected any + }{ + {[][]string{{"a", "b"}}, + []any{[]string{"c", "d"}}, + [][]string{ + {"a", "b"}, + {"c", "d"}, + }, + }, + {[][]string{{"a", "b"}}, + []any{[]string{"c", "d"}, []string{"e", "f"}}, + [][]string{ + {"a", "b"}, + {"c", "d"}, + {"e", "f"}, + }, + }, + {[][]string{{"a", "b"}}, + []any{[]int{1, 2}}, + false, + }, + } { + result, err := Append(test.to, test.from...) + if b, ok := test.expected.(bool); ok && !b { + c.Assert(err, qt.Not(qt.IsNil)) + } else { + c.Assert(err, qt.IsNil) + c.Assert(result, qt.DeepEquals, test.expected) + } + } + +} diff --git a/tpl/collections/integration_test.go b/tpl/collections/integration_test.go index 225eab9faca..da1d6e488a6 100644 --- a/tpl/collections/integration_test.go +++ b/tpl/collections/integration_test.go @@ -73,3 +73,34 @@ Desc: [map[a:3 b:3] map[a:3 b:1] map[a:3 b:1] map[a:3 b:1] map[a:3 b:0] map[a:3 } } + +// Issue #11004. +func TestAppendSliceToASliceOfSlices(t *testing.T) { + t.Parallel() + + files := ` +-- hugo.toml -- +-- layouts/index.html -- +{{ $obj := slice (slice "a") }} +{{ $obj = $obj | append (slice "b") }} +{{ $obj = $obj | append (slice "c") }} + +{{ $obj }} + + + ` + + for i := 0; i < 4; i++ { + + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).Build() + + b.AssertFileContent("public/index.html", "[[a] [b] [c]]") + + } + +} From bfcc2a0c8283a9580493f9d88794e4326ae50945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 14 Jun 2023 09:44:18 +0200 Subject: [PATCH 2/2] common/collections: Always make a copy of the input slice in Append Fixes #10458. --- common/collections/append.go | 7 +++++++ common/collections/append_test.go | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/common/collections/append.go b/common/collections/append.go index fe8792fc421..91abc46d348 100644 --- a/common/collections/append.go +++ b/common/collections/append.go @@ -31,6 +31,13 @@ func Append(to any, from ...any) (any, error) { var tot reflect.Type if !toIsNil { + if tov.Kind() == reflect.Slice { + // Create a copy of tov, so we don't modify the original. + c := reflect.MakeSlice(tov.Type(), tov.Len(), tov.Len()+len(from)) + reflect.Copy(c, tov) + tov = c + } + if tov.Kind() != reflect.Slice { return nil, fmt.Errorf("expected a slice, got %T", to) } diff --git a/common/collections/append_test.go b/common/collections/append_test.go index f997e7a20b0..415eb2f257e 100644 --- a/common/collections/append_test.go +++ b/common/collections/append_test.go @@ -129,3 +129,15 @@ func TestAppendToMultiDimensionalSlice(t *testing.T) { } } + +func TestAppendShouldMakeACopyOfTheInputSlice(t *testing.T) { + t.Parallel() + c := qt.New(t) + slice := make([]string, 0, 100) + slice = append(slice, "a", "b") + result, err := Append(slice, "c") + c.Assert(err, qt.IsNil) + slice[0] = "d" + c.Assert(result, qt.DeepEquals, []string{"a", "b", "c"}) + c.Assert(slice, qt.DeepEquals, []string{"d", "b"}) +}