From 15a2338ff24d8388781b6151b3eedf45d912f7e8 Mon Sep 17 00:00:00 2001
From: Ellen Emilia Anna Zscheile <fogti+devel@ytrizja.de>
Date: Wed, 2 Apr 2025 14:57:45 +0000
Subject: [PATCH] feat(build): teach lint-locale-usage about trPluralString
 (#7425)

This requires using the more complicated parsing from localestore.go

In order to avoid future code drift and code duplication,
localestore.go was refactored to call IterateMessagesContent instead of
essentially duplicating the code of RecursivelyAddTranslationsFromJSON
with small adjustments.

locale/utils.go was moved to translation/localeiter/utils.go
in order to avoid spreading translation-related routines among completely
different places.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7425
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Ellen Emilia Anna Zscheile <fogti+devel@ytrizja.de>
Co-committed-by: Ellen Emilia Anna Zscheile <fogti+devel@ytrizja.de>
---
 .deadcode-out                                 |  3 ++
 build/lint-locale-usage/lint-locale-usage.go  | 17 +++---
 build/lint-locale/lint-locale.go              | 12 +++--
 build/lint-locale/lint-locale_test.go         | 12 +++++
 modules/translation/i18n/localestore.go       | 53 ++++---------------
 .../localeiter}/utils.go                      | 42 +++++++++++----
 6 files changed, 73 insertions(+), 66 deletions(-)
 rename modules/{locale => translation/localeiter}/utils.go (56%)

diff --git a/.deadcode-out b/.deadcode-out
index 31184826ae..5b5698c5de 100644
--- a/.deadcode-out
+++ b/.deadcode-out
@@ -192,6 +192,9 @@ forgejo.org/modules/translation
 	MockLocale.HasKey
 	MockLocale.PrettyNumber
 
+forgejo.org/modules/translation/localeiter
+	IterateMessagesContent
+
 forgejo.org/modules/util
 	OptionalArg
 
diff --git a/build/lint-locale-usage/lint-locale-usage.go b/build/lint-locale-usage/lint-locale-usage.go
index edbd086d0e..88375c1c36 100644
--- a/build/lint-locale-usage/lint-locale-usage.go
+++ b/build/lint-locale-usage/lint-locale-usage.go
@@ -18,8 +18,8 @@ import (
 	tmplParser "text/template/parse"
 
 	"forgejo.org/modules/container"
-	"forgejo.org/modules/locale"
 	fjTemplates "forgejo.org/modules/templates"
+	"forgejo.org/modules/translation/localeiter"
 	"forgejo.org/modules/util"
 )
 
@@ -300,10 +300,6 @@ func main() {
 	}
 
 	msgids := make(container.Set[string])
-	onMsgid := func(trKey, trValue string) error {
-		msgids[trKey] = struct{}{}
-		return nil
-	}
 
 	localeFile := filepath.Join(filepath.Join("options", "locale"), "locale_en-US.ini")
 	localeContent, err := os.ReadFile(localeFile)
@@ -312,7 +308,10 @@ func main() {
 		os.Exit(2)
 	}
 
-	if err = locale.IterateMessagesContent(localeContent, onMsgid); err != nil {
+	if err = localeiter.IterateMessagesContent(localeContent, func(trKey, trValue string) error {
+		msgids[trKey] = struct{}{}
+		return nil
+	}); err != nil {
 		fmt.Printf("%s:\tERROR: %s\n", localeFile, err.Error())
 		os.Exit(2)
 	}
@@ -324,7 +323,11 @@ func main() {
 		os.Exit(2)
 	}
 
-	if err := locale.IterateMessagesNextContent(localeContent, onMsgid); err != nil {
+	if err := localeiter.IterateMessagesNextContent(localeContent, func(trKey, pluralForm, trValue string) error {
+		// ignore plural form
+		msgids[trKey] = struct{}{}
+		return nil
+	}); err != nil {
 		fmt.Printf("%s:\tERROR: %s\n", localeFile, err.Error())
 		os.Exit(2)
 	}
diff --git a/build/lint-locale/lint-locale.go b/build/lint-locale/lint-locale.go
index 94ce941e62..0d80ffa4b0 100644
--- a/build/lint-locale/lint-locale.go
+++ b/build/lint-locale/lint-locale.go
@@ -14,7 +14,7 @@ import (
 	"slices"
 	"strings"
 
-	"forgejo.org/modules/locale"
+	"forgejo.org/modules/translation/localeiter"
 
 	"github.com/microcosm-cc/bluemonday"
 	"github.com/sergi/go-diff/diffmatchpatch"
@@ -100,7 +100,7 @@ func checkValue(trKey, value string) []string {
 func checkLocaleContent(localeContent []byte) []string {
 	errors := []string{}
 
-	if err := locale.IterateMessagesContent(localeContent, func(trKey, trValue string) error {
+	if err := localeiter.IterateMessagesContent(localeContent, func(trKey, trValue string) error {
 		errors = append(errors, checkValue(trKey, trValue)...)
 		return nil
 	}); err != nil {
@@ -113,8 +113,12 @@ func checkLocaleContent(localeContent []byte) []string {
 func checkLocaleNextContent(localeContent []byte) []string {
 	errors := []string{}
 
-	if err := locale.IterateMessagesNextContent(localeContent, func(trKey, trValue string) error {
-		errors = append(errors, checkValue(trKey, trValue)...)
+	if err := localeiter.IterateMessagesNextContent(localeContent, func(trKey, pluralForm, trValue string) error {
+		fullKey := trKey
+		if pluralForm != "" {
+			fullKey = trKey + "." + pluralForm
+		}
+		errors = append(errors, checkValue(fullKey, trValue)...)
 		return nil
 	}); err != nil {
 		panic(err)
diff --git a/build/lint-locale/lint-locale_test.go b/build/lint-locale/lint-locale_test.go
index 37407461c7..d481a80e37 100644
--- a/build/lint-locale/lint-locale_test.go
+++ b/build/lint-locale/lint-locale_test.go
@@ -91,4 +91,16 @@ func TestNextLocalizationPolicy(t *testing.T) {
 			"settings.hidden_comment_types_description": "\"<not-an-allowed-key> <label>\""
 		}`)))
 	})
+
+	t.Run("Plural form", func(t *testing.T) {
+		assert.Equal(t, []string{"repo.pulls.title_desc: key = \x1b[31m<a href=\"https://example.com\">\x1b[0m"}, checkLocaleNextContent([]byte(`{"repo.pulls.title_desc": {
+                       "few": "key = <a href=\"%s\">",
+                       "other": "key = <a href=\"https://example.com\">"
+    }}`)))
+
+		assert.Equal(t, []string{"repo.pulls.title_desc.few: key = \x1b[31m<a href=\"https://example.com\">\x1b[0m"}, checkLocaleNextContent([]byte(`{"repo.pulls.title_desc": {
+                       "few": "key = <a href=\"https://example.com\">",
+                       "other": "key = <a href=\"%s\">"
+    }}`)))
+	})
 }
diff --git a/modules/translation/i18n/localestore.go b/modules/translation/i18n/localestore.go
index a9c1f440bc..bf98b7b5da 100644
--- a/modules/translation/i18n/localestore.go
+++ b/modules/translation/i18n/localestore.go
@@ -8,9 +8,9 @@ import (
 	"html/template"
 	"slices"
 
-	"forgejo.org/modules/json"
 	"forgejo.org/modules/log"
 	"forgejo.org/modules/setting"
+	"forgejo.org/modules/translation/localeiter"
 	"forgejo.org/modules/util"
 )
 
@@ -94,55 +94,20 @@ func (store *localeStore) AddLocaleByIni(langName, langDesc string, pluralRule P
 	return nil
 }
 
-func RecursivelyAddTranslationsFromJSON(locale *locale, object map[string]any, prefix string) error {
-	for key, value := range object {
-		var fullkey string
-		if prefix != "" {
-			fullkey = prefix + "." + key
-		} else {
-			fullkey = key
-		}
-
-		switch v := value.(type) {
-		case string:
-			// Check whether we are adding a plural form to the parent object, or a new nested JSON object.
-
-			switch key {
-			case "zero", "one", "two", "few", "many":
-				locale.newStyleMessages[prefix+PluralFormSeparator+key] = v
-			case "other":
-				locale.newStyleMessages[prefix] = v
-			default:
-				locale.newStyleMessages[fullkey] = v
-			}
-
-		case map[string]any:
-			err := RecursivelyAddTranslationsFromJSON(locale, v, fullkey)
-			if err != nil {
-				return err
-			}
-
-		case nil:
-		default:
-			return fmt.Errorf("Unrecognized JSON value '%s'", value)
-		}
-	}
-
-	return nil
-}
-
 func (store *localeStore) AddToLocaleFromJSON(langName string, source []byte) error {
 	locale, ok := store.localeMap[langName]
 	if !ok {
 		return ErrLocaleDoesNotExist
 	}
 
-	var result map[string]any
-	if err := json.Unmarshal(source, &result); err != nil {
-		return err
-	}
-
-	return RecursivelyAddTranslationsFromJSON(locale, result, "")
+	return localeiter.IterateMessagesNextContent(source, func(key, pluralForm, value string) error {
+		msgKey := key
+		if pluralForm != "" {
+			msgKey = key + PluralFormSeparator + pluralForm
+		}
+		locale.newStyleMessages[msgKey] = value
+		return nil
+	})
 }
 
 func (l *locale) LookupNewStyleMessage(trKey string) string {
diff --git a/modules/locale/utils.go b/modules/translation/localeiter/utils.go
similarity index 56%
rename from modules/locale/utils.go
rename to modules/translation/localeiter/utils.go
index 726dc92adc..669b816837 100644
--- a/modules/locale/utils.go
+++ b/modules/translation/localeiter/utils.go
@@ -4,7 +4,7 @@
 
 // extracted from `/build/lint-locale.go`, `/build/lint-locale-usage.go`
 
-package locale
+package localeiter
 
 import (
 	"encoding/json" //nolint:depguard
@@ -27,6 +27,9 @@ func IterateMessagesContent(localeContent []byte, onMsgid func(string, string) e
 	for _, section := range cfg.Sections() {
 		for _, key := range section.Keys() {
 			var trKey string
+			// see https://codeberg.org/forgejo/discussions/issues/104
+			//     https://github.com/WeblateOrg/weblate/issues/10831
+			// for an explanation of why "common" is an alternative
 			if section.Name() == "" || section.Name() == "DEFAULT" || section.Name() == "common" {
 				trKey = key.Name()
 			} else {
@@ -41,34 +44,51 @@ func IterateMessagesContent(localeContent []byte, onMsgid func(string, string) e
 	return nil
 }
 
-func iterateMessagesNextInner(onMsgid func(string, string) error, data map[string]any, trKey ...string) error {
+func iterateMessagesNextInner(onMsgid func(string, string, string) error, data map[string]any, trKey string) error {
 	for key, value := range data {
-		currentKey := key
-		if len(trKey) == 1 {
-			currentKey = trKey[0] + "." + key
+		fullKey := key
+		if trKey != "" {
+			fullKey = trKey + "." + key
 		}
-
 		switch value := value.(type) {
 		case string:
-			if err := onMsgid(currentKey, value); err != nil {
+			// Check whether we are adding a plural form to the parent object, or a new nested JSON object.
+			realKey := trKey
+			pluralSuffix := ""
+
+			switch key {
+			case "zero", "one", "two", "few", "many":
+				pluralSuffix = key
+			case "other":
+				// do nothing
+			default:
+				realKey = fullKey
+			}
+
+			if err := onMsgid(realKey, pluralSuffix, value); err != nil {
 				return err
 			}
+
 		case map[string]any:
-			if err := iterateMessagesNextInner(onMsgid, value, currentKey); err != nil {
+			if err := iterateMessagesNextInner(onMsgid, value, fullKey); err != nil {
 				return err
 			}
+
+		case nil:
+			// do nothing
+
 		default:
-			return fmt.Errorf("unexpected type: %s - %T", currentKey, value)
+			return fmt.Errorf("Unexpected JSON type: %s - %T", fullKey, value)
 		}
 	}
 
 	return nil
 }
 
-func IterateMessagesNextContent(localeContent []byte, onMsgid func(string, string) error) error {
+func IterateMessagesNextContent(localeContent []byte, onMsgid func(string, string, string) error) error {
 	var localeData map[string]any
 	if err := json.Unmarshal(localeContent, &localeData); err != nil {
 		return err
 	}
-	return iterateMessagesNextInner(onMsgid, localeData)
+	return iterateMessagesNextInner(onMsgid, localeData, "")
 }