tpl/collections: Improve type handling in collections.Slice

Fixes #5188

common/collections/collections.go

@@ -19,3 +19,10 @@ package collections
 type Grouper interface {
 	Group(key interface{}, items interface{}) (interface{}, error)
 }
+
+// Slicer definse a very generic way to create a typed slice. This is used
+// in collections.Slice template func to get types such as Pages, PageGroups etc.
+// instead of the less useful []interface{}.
+type Slicer interface {
+	Slice(items []interface{}) (interface{}, error)
+}

hugolib/collections.go

@@ -0,0 +1,78 @@
+// Copyright 2018 The Hugo Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package hugolib
+
+import (
+	"fmt"
+
+	"github.com/gohugoio/hugo/common/collections"
+)
+
+var (
+	_ collections.Grouper = (*Page)(nil)
+	_ collections.Slicer  = (*Page)(nil)
+	_ collections.Slicer  = PageGroup{}
+	_ collections.Slicer  = WeightedPage{}
+)
+
+// collections.Slicer implementations below. We keep these bridge implementations
+// here as it makes it easier to get an idea of "type coverage". These
+// implementations have no value on their own.
+
+// Slice is not meant to be used externally. It's a bridge function
+// for the template functions. See collections.Slice.
+func (p *Page) Slice(items []interface{}) (interface{}, error) {
+	return toPages(items)
+}
+
+// Slice is not meant to be used externally. It's a bridge function
+// for the template functions. See collections.Slice.
+func (p PageGroup) Slice(items []interface{}) (interface{}, error) {
+	groups := make(PagesGroup, len(items))
+	for i, v := range items {
+		g, ok := v.(PageGroup)
+		if !ok {
+			return nil, fmt.Errorf("type %T is not a PageGroup", v)
+		}
+		groups[i] = g
+	}
+	return groups, nil
+}
+
+// Slice is not meant to be used externally. It's a bridge function
+// for the template functions. See collections.Slice.
+func (p WeightedPage) Slice(items []interface{}) (interface{}, error) {
+	weighted := make(WeightedPages, len(items))
+	for i, v := range items {
+		g, ok := v.(WeightedPage)
+		if !ok {
+			return nil, fmt.Errorf("type %T is not a WeightedPage", v)
+		}
+		weighted[i] = g
+	}
+	return weighted, nil
+}
+
+// collections.Grouper  implementations below
+
+// Group creates a PageGroup from a key and a Pages object
+// This method is not meant for external use. It got its non-typed arguments to satisfy
+// a very generic interface in the tpl package.
+func (p *Page) Group(key interface{}, in interface{}) (interface{}, error) {
+	pages, err := toPages(in)
+	if err != nil {
+		return nil, err
+	}
+	return PageGroup{Key: key, Pages: pages}, nil
+}

hugolib/collections_test.go

@@ -0,0 +1,88 @@
+// Copyright 2018 The Hugo Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package hugolib
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestGroupFunc(t *testing.T) {
+	assert := require.New(t)
+
+	pageContent := `
+---
+title: "Page"
+---
+
+`
+	b := newTestSitesBuilder(t)
+	b.WithSimpleConfigFile().
+		WithContent("page1.md", pageContent, "page2.md", pageContent).
+		WithTemplatesAdded("index.html", `
+{{ $cool := .Site.RegularPages | group "cool" }}
+{{ $cool.Key }}: {{ len $cool.Pages }}
+
+`)
+	b.CreateSites().Build(BuildCfg{})
+
+	assert.Equal(1, len(b.H.Sites))
+	require.Len(t, b.H.Sites[0].RegularPages, 2)
+
+	b.AssertFileContent("public/index.html", "cool: 2")
+}
+
+func TestSliceFunc(t *testing.T) {
+	assert := require.New(t)
+
+	pageContent := `
+---
+title: "Page"
+tags: ["blue", "green"]
+tags_weight: %d
+---
+
+`
+	b := newTestSitesBuilder(t)
+	b.WithSimpleConfigFile().
+		WithContent("page1.md", fmt.Sprintf(pageContent, 10), "page2.md", fmt.Sprintf(pageContent, 20)).
+		WithTemplatesAdded("index.html", `
+{{ $cool := first 1 .Site.RegularPages | group "cool" }}
+{{ $blue := after 1 .Site.RegularPages | group "blue" }}
+{{ $weightedPages := index (index .Site.Taxonomies "tags") "blue" }}
+
+{{ $p1 := index .Site.RegularPages 0 }}{{ $p2 := index .Site.RegularPages 1 }}
+{{ $wp1 := index $weightedPages 0 }}{{ $wp2 := index $weightedPages 1 }}
+
+{{ $pages := slice $p1 $p2 }}
+{{ $pageGroups := slice $cool $blue }}
+{{ $weighted := slice $wp1 $wp2 }}
+
+{{ printf "pages:%d:%T:%v/%v" (len $pages) $pages (index $pages 0) (index $pages 1) }}
+{{ printf "pageGroups:%d:%T:%v/%v" (len $pageGroups) $pageGroups (index (index $pageGroups 0).Pages 0) (index (index $pageGroups 1).Pages 0)}}
+{{ printf "weightedPages:%d::%T:%v" (len $weighted) $weighted $weighted | safeHTML }}
+
+`)
+	b.CreateSites().Build(BuildCfg{})
+
+	assert.Equal(1, len(b.H.Sites))
+	require.Len(t, b.H.Sites[0].RegularPages, 2)
+
+	b.AssertFileContent("public/index.html",
+		"pages:2:hugolib.Pages:Page(/page1.md)/Page(/page2.md)",
+		"pageGroups:2:hugolib.PagesGroup:Page(/page1.md)/Page(/page2.md)",
+		`weightedPages:2::hugolib.WeightedPages:[WeightedPage(10,"Page") WeightedPage(20,"Page")]`)
+}

hugolib/page.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/gohugoio/hugo/media"
 
-	"github.com/gohugoio/hugo/common/collections"
 	"github.com/gohugoio/hugo/common/maps"
 
 	"github.com/gohugoio/hugo/langs"
@@ -71,8 +70,6 @@ var (
 
 	// Assert that it implements the interface needed for related searches.
 	_ related.Document = (*Page)(nil)
-
-	_ collections.Grouper = Page{}
 )
 
 const (

hugolib/pageGroup.go

@@ -296,14 +296,3 @@ func (p Pages) GroupByParamDate(key string, format string, order ...string) (Pag
 	}
 	return p.groupByDateField(sorter, formatter, order...)
 }
-
-// Group creates a PageGroup from a key and a Pages object
-// This method is not meant for external use. It got its non-typed arguments to satisfy
-// a very generic interface in the tpl package.
-func (p Page) Group(key interface{}, in interface{}) (interface{}, error) {
-	pages, err := toPages(in)
-	if err != nil {
-		return nil, err
-	}
-	return PageGroup{Key: key, Pages: pages}, nil
-}

hugolib/pageGroup_test.go

@@ -1,4 +1,4 @@
-// Copyright 2015 The Hugo Authors. All rights reserved.
+// Copyright 2018 The Hugo Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -20,7 +20,6 @@ import (
 	"testing"
 
 	"github.com/spf13/cast"
-	"github.com/stretchr/testify/require"
 )
 
 type pageGroupTestObject struct {
@@ -456,28 +455,3 @@ func TestGroupByParamDateWithEmptyPages(t *testing.T) {
 		t.Errorf("PagesGroup isn't empty. It should be %#v, got %#v", nil, groups)
 	}
 }
-
-func TestGroupFunc(t *testing.T) {
-	assert := require.New(t)
-
-	pageContent := `
----
-title: "Page"
----
-
-`
-	b := newTestSitesBuilder(t)
-	b.WithSimpleConfigFile().
-		WithContent("page1.md", pageContent, "page2.md", pageContent).
-		WithTemplatesAdded("index.html", `
-{{ $cool := .Site.RegularPages | group "cool" }}
-{{ $cool.Key }}: {{ len $cool.Pages }}
-
-`)
-	b.CreateSites().Build(BuildCfg{})
-
-	assert.Equal(1, len(b.H.Sites))
-	require.Len(t, b.H.Sites[0].RegularPages, 2)
-
-	b.AssertFileContent("public/index.html", "cool: 2")
-}

hugolib/pagination.go

@@ -453,20 +453,34 @@ func toPages(seq interface{}) (Pages, error) {
 		return Pages{}, nil
 	}
 
-	switch seq.(type) {
+	switch v := seq.(type) {
 	case Pages:
-		return seq.(Pages), nil
+		return v, nil
 	case *Pages:
-		return *(seq.(*Pages)), nil
+		return *(v), nil
 	case []*Page:
-		return Pages(seq.([]*Page)), nil
+		return Pages(v), nil
 	case WeightedPages:
-		return (seq.(WeightedPages)).Pages(), nil
+		return v.Pages(), nil
 	case PageGroup:
-		return (seq.(PageGroup)).Pages, nil
+		return v.Pages, nil
-	default:
+	case []interface{}:
-		return nil, fmt.Errorf("unsupported type in paginate, got %T", seq)
+		pages := make(Pages, len(v))
+		success := true
+		for i, vv := range v {
+			p, ok := vv.(*Page)
+			if !ok {
+				success = false
+				break
 			}
+			pages[i] = p
+		}
+		if success {
+			return pages, nil
+		}
+	}
+
+	return nil, fmt.Errorf("cannot convert type %T to Pages", seq)
 }
 
 // probablyEqual checks page lists for probable equality.

tpl/collections/collections.go

@@ -319,18 +319,10 @@ func (ns *Namespace) Group(key interface{}, items interface{}) (interface{}, err
 		return nil, errors.New("nil is not a valid key to group by")
 	}
 
-	tp := reflect.TypeOf(items)
+	in := newSliceElement(items)
-	switch tp.Kind() {
+
-	case reflect.Array, reflect.Slice:
+	if g, ok := in.(collections.Grouper); ok {
-		tp = tp.Elem()
+		return g.Group(key, items)
-		if tp.Kind() == reflect.Ptr {
-			tp = tp.Elem()
-		}
-		in := reflect.New(tp).Interface()
-		switch vv := in.(type) {
-		case collections.Grouper:
-			return vv.Group(key, items)
-		}
 	}
 
 	return nil, fmt.Errorf("grouping not supported for type %T", items)
@@ -514,7 +506,33 @@ func (ns *Namespace) Shuffle(seq interface{}) (interface{}, error) {
 }
 
 // Slice returns a slice of all passed arguments.
-func (ns *Namespace) Slice(args ...interface{}) []interface{} {
+func (ns *Namespace) Slice(args ...interface{}) interface{} {
+	if len(args) == 0 {
+		return args
+	}
+
+	first := args[0]
+	allTheSame := true
+	if len(args) > 1 {
+		// This can be a mix of types.
+		firstType := reflect.TypeOf(first)
+		for i := 1; i < len(args); i++ {
+			if firstType != reflect.TypeOf(args[i]) {
+				allTheSame = false
+				break
+			}
+		}
+	}
+
+	if allTheSame {
+		if g, ok := first.(collections.Slicer); ok {
+			v, err := g.Slice(args)
+			if err == nil {
+				return v
+			}
+		}
+	}
+
 	return args
 }
 

tpl/collections/collections_test.go

@@ -25,6 +25,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gohugoio/hugo/common/collections"
 	"github.com/gohugoio/hugo/config"
 	"github.com/gohugoio/hugo/deps"
 	"github.com/gohugoio/hugo/helpers"
@@ -110,6 +111,8 @@ func TestGroup(t *testing.T) {
 		{"b", []tstGrouper2{tstGrouper2{}, tstGrouper2{}}, "b(2)"},
 		{"a", []*tstGrouper{}, "a(0)"},
 		{"a", []string{"a", "b"}, false},
+		{"a", "asdf", false},
+		{"a", nil, false},
 		{nil, []*tstGrouper{&tstGrouper{}, &tstGrouper{}}, false},
 	} {
 		errMsg := fmt.Sprintf("[%d] %v", i, test)
@@ -633,6 +636,22 @@ func TestShuffleRandomising(t *testing.T) {
 	}
 }
 
+var _ collections.Slicer = (*tstSlicer)(nil)
+
+type tstSlicer struct {
+	name string
+}
+
+func (p *tstSlicer) Slice(items []interface{}) (interface{}, error) {
+	result := make(tstSlicers, len(items))
+	for i, v := range items {
+		result[i] = v.(*tstSlicer)
+	}
+	return result, nil
+}
+
+type tstSlicers []*tstSlicer
+
 func TestSlice(t *testing.T) {
 	t.Parallel()
 
@@ -640,18 +659,24 @@ func TestSlice(t *testing.T) {
 
 	for i, test := range []struct {
 		args     []interface{}
+		expected interface{}
 	}{
-		{[]interface{}{"a", "b"}},
+		{[]interface{}{"a", "b"}, []interface{}{"a", "b"}},
-		// errors
+		{[]interface{}{&tstSlicer{"a"}, &tstSlicer{"b"}}, tstSlicers{&tstSlicer{"a"}, &tstSlicer{"b"}}},
-		{[]interface{}{5, "b"}},
+		{[]interface{}{&tstSlicer{"a"}, "b"}, []interface{}{&tstSlicer{"a"}, "b"}},
-		{[]interface{}{tstNoStringer{}}},
+		{[]interface{}{}, []interface{}{}},
+		{[]interface{}{nil}, []interface{}{nil}},
+		{[]interface{}{5, "b"}, []interface{}{5, "b"}},
+		{[]interface{}{tstNoStringer{}}, []interface{}{tstNoStringer{}}},
 	} {
 		errMsg := fmt.Sprintf("[%d] %v", i, test.args)
 
 		result := ns.Slice(test.args...)
 
-		assert.Equal(t, test.args, result, errMsg)
+		assert.Equal(t, test.expected, result, errMsg)
 	}
+
+	assert.Len(t, ns.Slice(), 0)
 }
 
 func TestUnion(t *testing.T) {

tpl/collections/reflect_helpers.go

@@ -102,6 +102,23 @@ func convertNumber(v reflect.Value, to reflect.Kind) (reflect.Value, error) {
 
 }
 
+func newSliceElement(items interface{}) interface{} {
+	tp := reflect.TypeOf(items)
+	if tp == nil {
+		return nil
+	}
+	switch tp.Kind() {
+	case reflect.Array, reflect.Slice:
+		tp = tp.Elem()
+		if tp.Kind() == reflect.Ptr {
+			tp = tp.Elem()
+		}
+
+		return reflect.New(tp).Interface()
+	}
+	return nil
+}
+
 func isNumber(kind reflect.Kind) bool {
 	return isInt(kind) || isUint(kind) || isFloat(kind)
 }