Improve template helper functions: string/slice (#24266)

Follow #23328

The improvements:

1. The `contains` functions are covered by tests
2. The inconsistent behavior of `containGeneric` is replaced by
`StringUtils.Contains` and `SliceUtils.Contains`
3. In the future we can move more help functions into XxxUtils to
simplify the `helper.go` and reduce unnecessary global functions.

FAQ:

1. Why it's called `StringUtils.Contains` but not `strings.Contains`
like Golang?

Because our `StringUtils` is not Golang's `strings` package. There will
be our own string functions.

---------

Co-authored-by: silverwind <[email protected]>

modules/templates/helper.go

@@ -15,7 +15,6 @@ import (
 	"mime"
 	"net/url"
 	"path/filepath"
-	"reflect"
 	"regexp"
 	"strings"
 	"time"
@@ -68,11 +67,15 @@ func NewFuncMap() []template.FuncMap {
 		"PathEscape":         url.PathEscape,
 		"PathEscapeSegments": util.PathEscapeSegments,
 
+		// utils
+		"StringUtils": NewStringUtils,
+		"SliceUtils":  NewSliceUtils,
+
 		// -----------------------------------------------------------------
 		// string / json
+		// TODO: move string helper functions to StringUtils
 		"Join":           strings.Join,
 		"DotEscape":      DotEscape,
-		"HasPrefix":      strings.HasPrefix,
 		"EllipsisString": base.EllipsisString,
 		"DumpVar":        dumpVar,
 
@@ -144,35 +147,6 @@ func NewFuncMap() []template.FuncMap {
 			return fmt.Sprint(time.Since(startTime).Nanoseconds()/1e6) + "ms"
 		},
 
-		// -----------------------------------------------------------------
-		// slice
-		"containGeneric": func(arr, v interface{}) bool {
-			arrV := reflect.ValueOf(arr)
-			if arrV.Kind() == reflect.String && reflect.ValueOf(v).Kind() == reflect.String {
-				return strings.Contains(arr.(string), v.(string))
-			}
-			if arrV.Kind() == reflect.Slice {
-				for i := 0; i < arrV.Len(); i++ {
-					iV := arrV.Index(i)
-					if !iV.CanInterface() {
-						continue
-					}
-					if iV.Interface() == v {
-						return true
-					}
-				}
-			}
-			return false
-		},
-		"contain": func(s []int64, id int64) bool {
-			for i := 0; i < len(s); i++ {
-				if s[i] == id {
-					return true
-				}
-			}
-			return false
-		},
-
 		// -----------------------------------------------------------------
 		// setting
 		"AppName": func() string {

modules/templates/util_slice.go

@@ -0,0 +1,35 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package templates
+
+import (
+	"fmt"
+	"reflect"
+)
+
+type SliceUtils struct{}
+
+func NewSliceUtils() *SliceUtils {
+	return &SliceUtils{}
+}
+
+func (su *SliceUtils) Contains(s, v any) bool {
+	if s == nil {
+		return false
+	}
+	sv := reflect.ValueOf(s)
+	if sv.Kind() != reflect.Slice && sv.Kind() != reflect.Array {
+		panic(fmt.Sprintf("invalid type, expected slice or array, but got: %T", s))
+	}
+	for i := 0; i < sv.Len(); i++ {
+		it := sv.Index(i)
+		if !it.CanInterface() {
+			continue
+		}
+		if it.Interface() == v {
+			return true
+		}
+	}
+	return false
+}

modules/templates/util_string.go

@@ -0,0 +1,20 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package templates
+
+import "strings"
+
+type StringUtils struct{}
+
+func NewStringUtils() *StringUtils {
+	return &StringUtils{}
+}
+
+func (su *StringUtils) HasPrefix(s, prefix string) bool {
+	return strings.HasPrefix(s, prefix)
+}
+
+func (su *StringUtils) Contains(s, substr string) bool {
+	return strings.Contains(s, substr)
+}

modules/templates/util_test.go

@@ -4,6 +4,9 @@
 package templates
 
 import (
+	"html/template"
+	"io"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -41,3 +44,36 @@ func TestDict(t *testing.T) {
 		assert.Error(t, err)
 	}
 }
+
+func TestUtils(t *testing.T) {
+	execTmpl := func(code string, data any) string {
+		tmpl := template.New("test")
+		tmpl.Funcs(template.FuncMap{"SliceUtils": NewSliceUtils, "StringUtils": NewStringUtils})
+		template.Must(tmpl.Parse(code))
+		w := &strings.Builder{}
+		assert.NoError(t, tmpl.Execute(w, data))
+		return w.String()
+	}
+
+	actual := execTmpl("{{SliceUtils.Contains .Slice .Value}}", map[string]any{"Slice": []string{"a", "b"}, "Value": "a"})
+	assert.Equal(t, "true", actual)
+
+	actual = execTmpl("{{SliceUtils.Contains .Slice .Value}}", map[string]any{"Slice": []string{"a", "b"}, "Value": "x"})
+	assert.Equal(t, "false", actual)
+
+	actual = execTmpl("{{SliceUtils.Contains .Slice .Value}}", map[string]any{"Slice": []int64{1, 2}, "Value": int64(2)})
+	assert.Equal(t, "true", actual)
+
+	actual = execTmpl("{{StringUtils.Contains .String .Value}}", map[string]any{"String": "abc", "Value": "b"})
+	assert.Equal(t, "true", actual)
+
+	actual = execTmpl("{{StringUtils.Contains .String .Value}}", map[string]any{"String": "abc", "Value": "x"})
+	assert.Equal(t, "false", actual)
+
+	tmpl := template.New("test")
+	tmpl.Funcs(template.FuncMap{"SliceUtils": NewSliceUtils, "StringUtils": NewStringUtils})
+	template.Must(tmpl.Parse("{{SliceUtils.Contains .Slice .Value}}"))
+	// error is like this: `template: test:1:12: executing "test" at <SliceUtils.Contains>: error calling Contains: ...`
+	err := tmpl.Execute(io.Discard, map[string]any{"Slice": struct{}{}})
+	assert.ErrorContains(t, err, "invalid type, expected slice or array")
+}

templates/repo/graph/commits.tmpl

@@ -35,7 +35,7 @@
 						{{range $commit.Refs}}
 							{{$refGroup := .RefGroup}}
 							{{if eq $refGroup "pull"}}
-								{{if or (not $.HidePRRefs) (containGeneric $.SelectedBranches .Name)}}
+								{{if or (not $.HidePRRefs) (SliceUtils.Contains $.SelectedBranches .Name)}}
 									<!-- it's intended to use issues not pulls, if it's a pull you will get redirected -->
 									<a class="ui labelled icon button basic tiny gt-mr-2" href="{{$.RepoLink}}/{{if $.Repository.UnitEnabled $.Context $.UnitTypePullRequests}}pulls{{else}}issues{{end}}/{{.ShortName|PathEscape}}">
 										{{svg "octicon-git-pull-request" 16 "gt-mr-2"}}#{{.ShortName}}

templates/repo/header.tmpl

@@ -217,7 +217,7 @@
 				{{end}}
 
 				{{if or (.Permission.CanRead $.UnitTypeWiki) (.Permission.CanRead $.UnitTypeExternalWiki)}}
-					<a class="{{if .PageIsWiki}}active {{end}}item" href="{{.RepoLink}}/wiki" {{if and (.Permission.CanRead $.UnitTypeExternalWiki) (not (HasPrefix ((.Repository.MustGetUnit $.Context $.UnitTypeExternalWiki).ExternalWikiConfig.ExternalWikiURL) (.Repository.Link)))}} target="_blank" rel="noopener noreferrer" {{end}}>
+					<a class="{{if .PageIsWiki}}active {{end}}item" href="{{.RepoLink}}/wiki" {{if and (.Permission.CanRead $.UnitTypeExternalWiki) (not (StringUtils.HasPrefix ((.Repository.MustGetUnit $.Context $.UnitTypeExternalWiki).ExternalWikiConfig.ExternalWikiURL) (.Repository.Link)))}} target="_blank" rel="noopener noreferrer" {{end}}>
 						{{svg "octicon-book"}} {{.locale.Tr "repo.wiki"}}
 					</a>
 				{{end}}

templates/repo/issue/list.tmpl

@@ -227,7 +227,7 @@
 								{{end}}
 								{{$previousExclusiveScope = $exclusiveScope}}
 								<div class="item issue-action" data-action="toggle" data-element-id="{{.ID}}" data-url="{{$.RepoLink}}/issues/labels">
-									{{if contain $.SelLabelIDs .ID}}{{if $exclusiveScope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}{{end}} {{RenderLabel $.Context .}}
+									{{if SliceUtils.Contains $.SelLabelIDs .ID}}{{if $exclusiveScope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}{{end}} {{RenderLabel $.Context .}}
 								</div>
 							{{end}}
 						</div>

templates/repo/issue/milestone_issues.tmpl

@@ -65,7 +65,7 @@
 							<span class="info">{{.locale.Tr "repo.issues.filter_label_exclude" | Safe}}</span>
 							<a class="item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}">{{.locale.Tr "repo.issues.filter_label_no_select"}}</a>
 							{{range .Labels}}
-								<a class="item label-filter-item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&labels={{.QueryString}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}" data-label-id="{{.ID}}">{{if .IsExcluded}}{{svg "octicon-circle-slash"}}{{else if contain $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}}</a>
+								<a class="item label-filter-item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&labels={{.QueryString}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}" data-label-id="{{.ID}}">{{if .IsExcluded}}{{svg "octicon-circle-slash"}}{{else if SliceUtils.Contains $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}}</a>
 							{{end}}
 						</div>
 					</div>
@@ -171,7 +171,7 @@
 						<div class="menu">
 							{{range .Labels}}
 								<div class="item issue-action" data-action="toggle" data-element-id="{{.ID}}" data-url="{{$.RepoLink}}/issues/labels">
-									{{if contain $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}}
+									{{if SliceUtils.Contains $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}}
 								</div>
 							{{end}}
 						</div>

templates/repo/issue/view_content/attachments.tmpl

@@ -8,7 +8,7 @@
 			<div class="gt-f1 gt-p-3">
 				<a target="_blank" rel="noopener noreferrer" href="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
 					{{if FilenameIsImage .Name}}
-						{{if not (containGeneric $.Content .UUID)}}
+						{{if not (StringUtils.Contains $.Content .UUID)}}
 							{{$hasThumbnails = true}}
 						{{end}}
 						{{svg "octicon-file"}}
@@ -29,7 +29,7 @@
 		<div class="ui small thumbnails">
 			{{- range .Attachments -}}
 				{{if FilenameIsImage .Name}}
-					{{if not (containGeneric $.Content .UUID)}}
+					{{if not (StringUtils.Contains $.Content .UUID)}}
 					<a target="_blank" rel="noopener noreferrer" href="{{.DownloadURL}}">
 						<img alt="{{.Name}}" src="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
 					</a>

templates/repo/settings/tags.tmpl

@@ -92,14 +92,14 @@
 											{{if or .AllowlistUserIDs (and $.Owner.IsOrganization .AllowlistTeamIDs)}}
 												{{$userIDs := .AllowlistUserIDs}}
 												{{range $.Users}}
-													{{if contain $userIDs .ID}}
+													{{if SliceUtils.Contains $userIDs .ID}}
 														<a class="ui basic label" href="{{.HomeLink}}">{{avatar $.Context . 26}} {{.GetDisplayName}}</a>
 													{{end}}
 												{{end}}
 												{{if $.Owner.IsOrganization}}
 													{{$teamIDs := .AllowlistTeamIDs}}
 													{{range $.Teams}}
-														{{if contain $teamIDs .ID}}
+														{{if SliceUtils.Contains $teamIDs .ID}}
 															<a class="ui basic label" href="{{$.Owner.OrganisationLink}}/teams/{{PathEscape .LowerName}}">{{.Name}}</a>
 														{{end}}
 													{{end}}