Repository: gohugoio/hugo (language: go)

Issue:
tpl/collections: Allow querify to accept a map argument
The [`collections.Querify`](https://gohugo.io/functions/collections/querify/) template function accepts either of the following argument forms:

```text
{{ collections.Querify "a" 1 "b" 2 }}         --> a=1&b=2
{{ collections.Querify (slice "a" 1 "b" 2) }} --> a=1&b=2
```

But it would convenient if we could do:

```text
{{ collections.Querify (dict "a" 1 "b" 2) }}
```

For example, in our [embedded youtube shortcode] we:

[embedded youtube shortcode]: https://github.com/gohugoio/hugo/blob/989b299b55d3fb48f8447ea69e85e26ee9dc7795/tpl/tplimpl/embedded/templates/shortcodes/youtube.html#L76-L94

1. Build a map of parameters
2. Convert the map into a slice with this construct:

    ```text
    {{ range $k, $v := $params }}
      {{ $s = $s | append $k }}
      {{ $s = $s | append $v }}
    {{ end }}
    ```
3. Pass the slice to `collections.Querify`

It would be more elegant if we could remove step 2 above.


Discussion / hints:
>{{ collections.Querify (dict "a" 1 "b" 2) }}

We need to remember to sort the keys.
> We need to remember to sort the keys.

The existing code calls [Encode](https://pkg.go.dev/net/url#Values.Encode) before returning the result.

> Encode encodes the values into “URL encoded” form ("bar=baz&foo=quux") sorted by key.

So unless I'm missing something, I think that covers the sort.
>So unless I'm missing something, I think that covers the sort.

You're right, I didn't look at the code.

Fix patch:
diff --git a/tpl/collections/collections.go b/tpl/collections/collections.go
index a7e36f68928..c1e7286ceac 100644
--- a/tpl/collections/collections.go
+++ b/tpl/collections/collections.go
@@ -20,7 +20,6 @@ import (
 	"errors"
 	"fmt"
 	"math/rand"
-	"net/url"
 	"reflect"
 	"strings"
 	"time"
@@ -383,47 +382,6 @@ func (ns *Namespace) Last(limit any, l any) (any, error) {
 	return seqv.Slice(seqv.Len()-limitv, seqv.Len()).Interface(), nil
 }
 
-// Querify encodes the given params in URL-encoded form ("bar=baz&foo=quux") sorted by key.
-func (ns *Namespace) Querify(params ...any) (string, error) {
-	qs := url.Values{}
-
-	if len(params) == 1 {
-		switch v := params[0].(type) {
-		case []string:
-			if len(v)%2 != 0 {
-				return "", errors.New("invalid query")
-			}
-
-			for i := 0; i < len(v); i += 2 {
-				qs.Add(v[i], v[i+1])
-			}
-
-			return qs.Encode(), nil
-
-		case []any:
-			params = v
-
-		default:
-			return "", errors.New("query keys must be strings")
-		}
-	}
-
-	if len(params)%2 != 0 {
-		return "", errors.New("invalid query")
-	}
-
-	for i := 0; i < len(params); i += 2 {
-		switch v := params[i].(type) {
-		case string:
-			qs.Add(v, fmt.Sprintf("%v", params[i+1]))
-		default:
-			return "", errors.New("query keys must be strings")
-		}
-	}
-
-	return qs.Encode(), nil
-}
-
 // Reverse creates a copy of the list l and reverses it.
 func (ns *Namespace) Reverse(l any) (any, error) {
 	if l == nil {
diff --git a/tpl/collections/querify.go b/tpl/collections/querify.go
new file mode 100644
index 00000000000..19e6d8afedd
--- /dev/null
+++ b/tpl/collections/querify.go
@@ -0,0 +1,125 @@
+// Copyright 2024 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 collections
+
+import (
+	"errors"
+	"net/url"
+
+	"github.com/gohugoio/hugo/common/maps"
+	"github.com/spf13/cast"
+)
+
+var (
+	errWrongArgStructure = errors.New("expected a map, a slice with an even number of elements, or an even number of scalar values, and each key must be a string")
+	errKeyIsEmptyString  = errors.New("one of the keys is an empty string")
+)
+
+// Querify returns a URL query string composed of the given key-value pairs,
+// encoded and sorted by key.
+func (ns *Namespace) Querify(params ...any) (string, error) {
+	if len(params) == 0 {
+		return "", nil
+	}
+
+	if len(params) == 1 {
+		switch v := params[0].(type) {
+		case map[string]any: // created with collections.Dictionary
+			return mapToQueryString(v)
+		case maps.Params: // site configuration or page parameters
+			return mapToQueryString(v)
+		case []string:
+			return stringSliceToQueryString(v)
+		case []any:
+			s, err := interfaceSliceToStringSlice(v)
+			if err != nil {
+				return "", err
+			}
+			return stringSliceToQueryString(s)
+		default:
+			return "", errWrongArgStructure
+		}
+	}
+
+	if len(params)%2 != 0 {
+		return "", errWrongArgStructure
+	}
+
+	s, err := interfaceSliceToStringSlice(params)
+	if err != nil {
+		return "", err
+	}
+	return stringSliceToQueryString(s)
+}
+
+// mapToQueryString returns a URL query string derived from the given string
+// map, encoded and sorted by key. The function returns an error if it cannot
+// convert an element value to a string.
+func mapToQueryString[T map[string]any | maps.Params](m T) (string, error) {
+	if len(m) == 0 {
+		return "", nil
+	}
+
+	qs := url.Values{}
+	for k, v := range m {
+		if len(k) == 0 {
+			return "", errKeyIsEmptyString
+		}
+		vs, err := cast.ToStringE(v)
+		if err != nil {
+			return "", err
+		}
+		qs.Add(k, vs)
+	}
+	return qs.Encode(), nil
+}
+
+// sliceToQueryString returns a URL query string derived from the given slice
+// of strings, encoded and sorted by key. The function returns an error if
+// there are an odd number of elements.
+func stringSliceToQueryString(s []string) (string, error) {
+	if len(s) == 0 {
+		return "", nil
+	}
+	if len(s)%2 != 0 {
+		return "", errWrongArgStructure
+	}
+
+	qs := url.Values{}
+	for i := 0; i < len(s); i += 2 {
+		if len(s[i]) == 0 {
+			return "", errKeyIsEmptyString
+		}
+		qs.Add(s[i], s[i+1])
+	}
+	return qs.Encode(), nil
+}
+
+// interfaceSliceToStringSlice converts a slice of interfaces to a slice of
+// strings, returning an error if it cannot convert an element to a string.
+func interfaceSliceToStringSlice(s []any) ([]string, error) {
+	if len(s) == 0 {
+		return []string{}, nil
+	}
+
+	ss := make([]string, 0, len(s))
+	for _, v := range s {
+		vs, err := cast.ToStringE(v)
+		if err != nil {
+			return []string{}, err
+		}
+		ss = append(ss, vs)
+	}
+	return ss, nil
+}


Test patch:
diff --git a/tpl/collections/collections_test.go b/tpl/collections/collections_test.go
index 0f4bf82f5dc..2cd6bfc3f32 100644
--- a/tpl/collections/collections_test.go
+++ b/tpl/collections/collections_test.go
@@ -512,68 +512,6 @@ func TestLast(t *testing.T) {
 	}
 }
 
-func TestQuerify(t *testing.T) {
-	t.Parallel()
-	c := qt.New(t)
-	ns := newNs()
-
-	for i, test := range []struct {
-		params []any
-		expect any
-	}{
-		{[]any{"a", "b"}, "a=b"},
-		{[]any{"a", "b", "c", "d", "f", " &"}, `a=b&c=d&f=+%26`},
-		{[]any{[]string{"a", "b"}}, "a=b"},
-		{[]any{[]string{"a", "b", "c", "d", "f", " &"}}, `a=b&c=d&f=+%26`},
-		{[]any{[]any{"x", "y"}}, `x=y`},
-		{[]any{[]any{"x", 5}}, `x=5`},
-		// errors
-		{[]any{5, "b"}, false},
-		{[]any{"a", "b", "c"}, false},
-		{[]any{[]string{"a", "b", "c"}}, false},
-		{[]any{[]string{"a", "b"}, "c"}, false},
-		{[]any{[]any{"c", "d", "e"}}, false},
-	} {
-		errMsg := qt.Commentf("[%d] %v", i, test.params)
-
-		result, err := ns.Querify(test.params...)
-
-		if b, ok := test.expect.(bool); ok && !b {
-			c.Assert(err, qt.Not(qt.IsNil), errMsg)
-			continue
-		}
-
-		c.Assert(err, qt.IsNil, errMsg)
-		c.Assert(result, qt.Equals, test.expect, errMsg)
-	}
-}
-
-func BenchmarkQuerify(b *testing.B) {
-	ns := newNs()
-	params := []any{"a", "b", "c", "d", "f", " &"}
-
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		_, err := ns.Querify(params...)
-		if err != nil {
-			b.Fatal(err)
-		}
-	}
-}
-
-func BenchmarkQuerifySlice(b *testing.B) {
-	ns := newNs()
-	params := []string{"a", "b", "c", "d", "f", " &"}
-
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		_, err := ns.Querify(params)
-		if err != nil {
-			b.Fatal(err)
-		}
-	}
-}
-
 func TestSeq(t *testing.T) {
 	t.Parallel()
 	c := qt.New(t)
diff --git a/tpl/collections/querify_test.go b/tpl/collections/querify_test.go
new file mode 100644
index 00000000000..17556e4cbfb
--- /dev/null
+++ b/tpl/collections/querify_test.go
@@ -0,0 +1,121 @@
+// Copyright 2024 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 collections
+
+import (
+	"testing"
+
+	qt "github.com/frankban/quicktest"
+	"github.com/gohugoio/hugo/common/maps"
+)
+
+func TestQuerify(t *testing.T) {
+	t.Parallel()
+	c := qt.New(t)
+	ns := newNs()
+
+	for _, test := range []struct {
+		name   string
+		params []any
+		expect any
+	}{
+		// map
+		{"01", []any{maps.Params{"a": "foo", "b": "bar"}}, `a=foo&b=bar`},
+		{"02", []any{maps.Params{"a": 6, "b": 7}}, `a=6&b=7`},
+		{"03", []any{maps.Params{"a": "foo", "b": 7}}, `a=foo&b=7`},
+		{"04", []any{map[string]any{"a": "foo", "b": "bar"}}, `a=foo&b=bar`},
+		{"05", []any{map[string]any{"a": 6, "b": 7}}, `a=6&b=7`},
+		{"06", []any{map[string]any{"a": "foo", "b": 7}}, `a=foo&b=7`},
+		// slice
+		{"07", []any{[]string{"a", "foo", "b", "bar"}}, `a=foo&b=bar`},
+		{"08", []any{[]any{"a", 6, "b", 7}}, `a=6&b=7`},
+		{"09", []any{[]any{"a", "foo", "b", 7}}, `a=foo&b=7`},
+		// sequence of scalar values
+		{"10", []any{"a", "foo", "b", "bar"}, `a=foo&b=bar`},
+		{"11", []any{"a", 6, "b", 7}, `a=6&b=7`},
+		{"12", []any{"a", "foo", "b", 7}, `a=foo&b=7`},
+		// empty map
+		{"13", []any{map[string]any{}}, ``},
+		// empty slice
+		{"14", []any{[]string{}}, ``},
+		{"15", []any{[]any{}}, ``},
+		// no arguments
+		{"16", []any{}, ``},
+		// errors: zero key length
+		{"17", []any{maps.Params{"": "foo"}}, false},
+		{"18", []any{map[string]any{"": "foo"}}, false},
+		{"19", []any{[]string{"", "foo"}}, false},
+		{"20", []any{[]any{"", 6}}, false},
+		{"21", []any{"", "foo"}, false},
+		// errors: odd number of values
+		{"22", []any{[]string{"a", "foo", "b"}}, false},
+		{"23", []any{[]any{"a", 6, "b"}}, false},
+		{"24", []any{"a", "foo", "b"}, false},
+		// errors: value cannot be cast to string
+		{"25", []any{map[string]any{"a": "foo", "b": tstNoStringer{}}}, false},
+		{"26", []any{[]any{"a", "foo", "b", tstNoStringer{}}}, false},
+		{"27", []any{"a", "foo", "b", tstNoStringer{}}, false},
+	} {
+		errMsg := qt.Commentf("[%s] %v", test.name, test.params)
+
+		result, err := ns.Querify(test.params...)
+
+		if b, ok := test.expect.(bool); ok && !b {
+			c.Assert(err, qt.Not(qt.IsNil), errMsg)
+			continue
+		}
+
+		c.Assert(err, qt.IsNil, errMsg)
+		c.Assert(result, qt.Equals, test.expect, errMsg)
+	}
+}
+
+func BenchmarkQuerify(b *testing.B) {
+	ns := newNs()
+	params := []any{"a", "b", "c", "d", "f", " &"}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, err := ns.Querify(params...)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkQuerifySlice(b *testing.B) {
+	ns := newNs()
+	params := []string{"a", "b", "c", "d", "f", " &"}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, err := ns.Querify(params)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkQuerifyMap(b *testing.B) {
+	ns := newNs()
+	params := map[string]any{"a": "b", "c": "d", "f": " &"}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, err := ns.Querify(params)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}