Refactor locale&string&template related code (#29165)

Clarify when "string" should be used (and be escaped), and when
"template.HTML" should be used (no need to escape)

And help PRs like  #29059 , to render the error messages correctly.

3 files changed, 49 insertions, 41 deletions

diff --git a/modules/translation/i18n/i18n.go b/modules/translation/i18n/i18n.go
index 42475545b3..1555cd961e 100644
--- a/modules/translation/i18n/i18n.go
+++ b/modules/translation/i18n/i18n.go
@@ -4,26 +4,25 @@
 package i18n
 
 import (
+	"html/template"
 	"io"
 )
 
 var DefaultLocales = NewLocaleStore()
 
 type Locale interface {
-	// Tr translates a given key and arguments for a language
-	Tr(trKey string, trArgs ...any) string
-	// Has reports if a locale has a translation for a given key
-	Has(trKey string) bool
+	// TrString translates a given key and arguments for a language
+	TrString(trKey string, trArgs ...any) string
+	// TrHTML translates a given key and arguments for a language, string arguments are escaped to HTML
+	TrHTML(trKey string, trArgs ...any) template.HTML
+	// HasKey reports if a locale has a translation for a given key
+	HasKey(trKey string) bool
 }
 
 // LocaleStore provides the functions common to all locale stores
 type LocaleStore interface {
 	io.Closer
 
-	// Tr translates a given key and arguments for a language
-	Tr(lang, trKey string, trArgs ...any) string
-	// Has reports if a locale has a translation for a given key
-	Has(lang, trKey string) bool
 	// SetDefaultLang sets the default language to fall back to
 	SetDefaultLang(lang string)
 	// ListLangNameDesc provides paired slices of language names to descriptors
@@ -45,7 +44,7 @@ func ResetDefaultLocales() {
 	DefaultLocales = NewLocaleStore()
 }
 
-// GetLocales returns the locale from the default locales
+// GetLocale returns the locale from the default locales
 func GetLocale(lang string) (Locale, bool) {
 	return DefaultLocales.Locale(lang)
 }
diff --git a/modules/translation/i18n/i18n_test.go b/modules/translation/i18n/i18n_test.go
index 1d1be43318..ffe69a74df 100644
--- a/modules/translation/i18n/i18n_test.go
+++ b/modules/translation/i18n/i18n_test.go
@@ -17,7 +17,7 @@ fmt = %[1]s %[2]s
 
 [section]
 sub = Sub String
-mixed = test value; <span style="color: red\; background: none;">more text</span>
+mixed = test value; <span style="color: red\; background: none;">%s</span>
 `)
 
 	testData2 := []byte(`
@@ -32,29 +32,33 @@ sub = Changed Sub String
 	assert.NoError(t, ls.AddLocaleByIni("lang2", "Lang2", testData2, nil))
 	ls.SetDefaultLang("lang1")
 
-	result := ls.Tr("lang1", "fmt", "a", "b")
+	lang1, _ := ls.Locale("lang1")
+	lang2, _ := ls.Locale("lang2")
+
+	result := lang1.TrString("fmt", "a", "b")
 	assert.Equal(t, "a b", result)
 
-	result = ls.Tr("lang2", "fmt", "a", "b")
+	result = lang2.TrString("fmt", "a", "b")
 	assert.Equal(t, "b a", result)
 
-	result = ls.Tr("lang1", "section.sub")
+	result = lang1.TrString("section.sub")
 	assert.Equal(t, "Sub String", result)
 
-	result = ls.Tr("lang2", "section.sub")
+	result = lang2.TrString("section.sub")
 	assert.Equal(t, "Changed Sub String", result)
 
-	result = ls.Tr("", ".dot.name")
+	langNone, _ := ls.Locale("none")
+	result = langNone.TrString(".dot.name")
 	assert.Equal(t, "Dot Name", result)
 
-	result = ls.Tr("lang2", "section.mixed")
-	assert.Equal(t, `test value; <span style="color: red; background: none;">more text</span>`, result)
+	result2 := lang2.TrHTML("section.mixed", "a&b")
+	assert.EqualValues(t, `test value; <span style="color: red; background: none;">a&amp;b</span>`, result2)
 
 	langs, descs := ls.ListLangNameDesc()
 	assert.ElementsMatch(t, []string{"lang1", "lang2"}, langs)
 	assert.ElementsMatch(t, []string{"Lang1", "Lang2"}, descs)
 
-	found := ls.Has("lang1", "no-such")
+	found := lang1.HasKey("no-such")
 	assert.False(t, found)
 	assert.NoError(t, ls.Close())
 }
@@ -72,9 +76,10 @@ c=22
 
 	ls := NewLocaleStore()
 	assert.NoError(t, ls.AddLocaleByIni("lang1", "Lang1", testData1, testData2))
-	assert.Equal(t, "11", ls.Tr("lang1", "a"))
-	assert.Equal(t, "21", ls.Tr("lang1", "b"))
-	assert.Equal(t, "22", ls.Tr("lang1", "c"))
+	lang1, _ := ls.Locale("lang1")
+	assert.Equal(t, "11", lang1.TrString("a"))
+	assert.Equal(t, "21", lang1.TrString("b"))
+	assert.Equal(t, "22", lang1.TrString("c"))
 }
 
 func TestLocaleStoreQuirks(t *testing.T) {
@@ -110,8 +115,9 @@ func TestLocaleStoreQuirks(t *testing.T) {
 	for _, testData := range testDataList {
 		ls := NewLocaleStore()
 		err := ls.AddLocaleByIni("lang1", "Lang1", []byte("a="+testData.in), nil)
+		lang1, _ := ls.Locale("lang1")
 		assert.NoError(t, err, testData.hint)
-		assert.Equal(t, testData.out, ls.Tr("lang1", "a"), testData.hint)
+		assert.Equal(t, testData.out, lang1.TrString("a"), testData.hint)
 		assert.NoError(t, ls.Close())
 	}
 
diff --git a/modules/translation/i18n/localestore.go b/modules/translation/i18n/localestore.go
index f5a951a79f..69cc9fd91d 100644
--- a/modules/translation/i18n/localestore.go
+++ b/modules/translation/i18n/localestore.go
@@ -5,6 +5,8 @@ package i18n
 
 import (
 	"fmt"
+	"html/template"
+	"slices"
 
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -18,6 +20,8 @@ type locale struct {
 	idxToMsgMap map[int]string // the map idx is generated by store's trKeyToIdxMap
 }
 
+var _ Locale = (*locale)(nil)
+
 type localeStore struct {
 	// After initializing has finished, these fields are read-only.
 	langNames []string
@@ -85,20 +89,6 @@ func (store *localeStore) SetDefaultLang(lang string) {
 	store.defaultLang = lang
 }
 
-// Tr translates content to target language. fall back to default language.
-func (store *localeStore) Tr(lang, trKey string, trArgs ...any) string {
-	l, _ := store.Locale(lang)
-
-	return l.Tr(trKey, trArgs...)
-}
-
-// Has returns whether the given language has a translation for the provided key
-func (store *localeStore) Has(lang, trKey string) bool {
-	l, _ := store.Locale(lang)
-
-	return l.Has(trKey)
-}
-
 // Locale returns the locale for the lang or the default language
 func (store *localeStore) Locale(lang string) (Locale, bool) {
 	l, found := store.localeMap[lang]
@@ -113,13 +103,11 @@ func (store *localeStore) Locale(lang string) (Locale, bool) {
 	return l, found
 }
 
-// Close implements io.Closer
 func (store *localeStore) Close() error {
 	return nil
 }
 
-// Tr translates content to locale language. fall back to default language.
-func (l *locale) Tr(trKey string, trArgs ...any) string {
+func (l *locale) TrString(trKey string, trArgs ...any) string {
 	format := trKey
 
 	idx, ok := l.store.trKeyToIdxMap[trKey]
@@ -141,8 +129,23 @@ func (l *locale) Tr(trKey string, trArgs ...any) string {
 	return msg
 }
 
-// Has returns whether a key is present in this locale or not
-func (l *locale) Has(trKey string) bool {
+func (l *locale) TrHTML(trKey string, trArgs ...any) template.HTML {
+	args := slices.Clone(trArgs)
+	for i, v := range args {
+		switch v := v.(type) {
+		case string:
+			args[i] = template.HTML(template.HTMLEscapeString(v))
+		case fmt.Stringer:
+			args[i] = template.HTMLEscapeString(v.String())
+		default: // int, float, include template.HTML
+			// do nothing, just use it
+		}
+	}
+	return template.HTML(l.TrString(trKey, args...))
+}
+
+// HasKey returns whether a key is present in this locale or not
+func (l *locale) HasKey(trKey string) bool {
 	idx, ok := l.store.trKeyToIdxMap[trKey]
 	if !ok {
 		return false