feat(build): Support go "fmt" format strings as masked usage patterns (#12013)

This idea is perhaps a bit more far-fetched. It implements the ability in `lint-locale-usage` to basically fully handle "printf" invocations by transforming format strings to regexps when "%" wildcards are present.

Currently, it doesn't cache the transformation from format string to compiled regex because this doesn't make a performance difference (yet), given that most of these wildcards are only hit once or twice.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12013
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

build/lint-locale-usage/allowed-masked-usage.txt

@@ -6,32 +6,12 @@ translation_meta.test
 # this also gets instantiated as a Messenger once
 repo.migrate.migrating_failed.error
 
-# models/system/notice.go: func (n *Notice) TrStr() string
-admin.notices.type_1
-admin.notices.type_2
-
 # modules/setting/ui.go
 themes.names.
 
 # services/context/context.go
 relativetime.
 
-# templates/repo/issue/view_content.tmpl: indirection via $closeTranslationKey
-repo.issues.close
-repo.pulls.close
-
-# templates/repo/issue/view_content/comments.tmpl: indirection via $refTr
-repo.issues.ref_closing_from
-repo.issues.ref_issue_from
-repo.issues.ref_pull_from
-repo.issues.ref_reopening_from
-
-# templates/repo/issue/view_content/comments.tmpl: ctx.Locale.Tr (printf "projects.type-%d.display_name" .OldProject.Type)
-projects.
-projects.type-1.display_name
-projects.type-2.display_name
-projects.type-3.display_name
-
 # templates/repo/settings/webhook/link_menu.tmpl, templates/webhook/new.tmpl: repo.settings.web_hook_name_
 # tests/integration/repo_archive_text_test.go
 repo.settings.

build/lint-locale-usage/bin/handle-go.go

@@ -37,6 +37,13 @@ func HandleGoFile(handler llu.Handler, fname string, src any) error {
 	}
 
 	ast.Inspect(node, func(n ast.Node) bool {
+		return HandleGoNode(handler, fset, fname, n)
+	})
+
+	return nil
+}
+
+func HandleGoNode(handler llu.Handler, fset *token.FileSet, fname string, n ast.Node) bool {
 	// search for function calls of the form `anything.Tr(any-string-lit, ...)`
 
 	switch n2 := n.(type) {
@@ -77,8 +84,7 @@ func HandleGoFile(handler llu.Handler, fname string, src any) error {
 		}
 
 	case *ast.FuncDecl:
-			matchInsPrefix := handler.HandleGoCommentGroup(fset, n2.Doc, "llu:returnsTrKey")
+		if matchInsPrefix := handler.HandleGoCommentGroup(fset, n2.Doc, "llu:returnsTrKeyWeak"); matchInsPrefix != nil {
-			if matchInsPrefix != nil {
 			results := n2.Type.Results.List
 			if len(results) != 1 {
 				handler.OnWarning(fset, n2.Type.Func, fmt.Sprintf("function %s has unexpected return type; expected single return value", n2.Name.Name))
@@ -103,6 +109,31 @@ func HandleGoFile(handler llu.Handler, fname string, src any) error {
 			})
 		}
 
+		if matchInsPrefix := handler.HandleGoCommentGroup(fset, n2.Doc, "llu:returnsTrKey"); matchInsPrefix != nil {
+			results := n2.Type.Results.List
+			if len(results) != 1 {
+				handler.OnWarning(fset, n2.Type.Func, fmt.Sprintf("function %s has unexpected return type; expected single return value", n2.Name.Name))
+				return true
+			}
+
+			ast.Inspect(n2.Body, func(n ast.Node) bool {
+				// search for return stmts
+				if ret, ok := n.(*ast.ReturnStmt); ok {
+					for _, res := range ret.Results {
+						handler.HandleGoTrArgument(fset, res, *matchInsPrefix)
+					}
+					return false
+				} else if _, ok := n.(*ast.FuncDecl); ok {
+					ast.Inspect(n, func(n2 ast.Node) bool {
+						return HandleGoNode(handler, fset, fname, n2)
+					})
+					// don't search inside nested functions for return stmts
+					return false
+				}
+				return true
+			})
+		}
+
 		if strings.HasSuffix(fname, "services/migrations/migrate.go") {
 			lluMigrate.HandleMessengerInFunc(handler, fset, n2)
 		}
@@ -174,7 +205,4 @@ func HandleGoFile(handler llu.Handler, fname string, src any) error {
 	}
 
 	return true
-	})
-
-	return nil
 }

build/lint-locale-usage/bin/lint-locale-usage.go

@@ -13,6 +13,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strings"
 
@@ -44,12 +45,57 @@ type StringTrie interface {
 
 type StringTrieMap map[string]StringTrie
 
+func printfPatternToRegex(key string) (string, bool) {
+	parts := strings.Split(key, "%")
+	if len(parts) < 2 {
+		return key, false
+	}
+	var pattern strings.Builder
+	pattern.WriteString("^")
+	pattern.WriteString(parts[0])
+	skip := false
+	for _, part := range parts[1:] {
+		if skip {
+			skip = false
+			continue
+		}
+		if len(part) == 0 {
+			// "%%"
+			pattern.WriteString("%")
+			continue
+		}
+		switch part[0] {
+		case 'd':
+			pattern.WriteString("[0-9]+")
+		default:
+			pattern.WriteString("[A-Za-z0-9]*")
+		}
+		pattern.WriteString(part[1:])
+	}
+	pattern.WriteString("$")
+	return pattern.String(), true
+}
+
 func (m StringTrieMap) Matches(key []string) bool {
 	if len(key) == 0 || m == nil {
 		return true
 	}
 	value, ok := m[key[0]]
 	if !ok {
+		for altKey, value := range m {
+			// TODO: cache mapping $printfFormatString -> $regexpCompileOutput
+			pattern, found := printfPatternToRegex(altKey)
+			if !found {
+				continue
+			}
+			matched, err := regexp.MatchString(pattern, key[0])
+			if err != nil {
+				panic(fmt.Sprintf("unable to compile regexp '%s': %s", pattern, err.Error()))
+			}
+			if matched && (value == nil || value.Matches(key[1:])) {
+				return true
+			}
+		}
 		return false
 	}
 	if value == nil {
@@ -101,7 +147,7 @@ func ParseAllowedMaskedUsages(fname string, usedMsgids container.Set[string], al
 		if line == "" || strings.HasPrefix(line, "#") {
 			continue
 		}
-		if linePrefix, found := strings.CutSuffix(line, "."); found {
+		if linePrefix, found := strings.CutSuffix(line, "."); found || strings.Contains(line, "%") {
 			allowedMaskedPrefixes.Insert(strings.Split(linePrefix, "."))
 		} else {
 			if !chkMsgid(line) {
@@ -145,9 +191,14 @@ func Usage() {
 
 	fmt.Fprintf(outp, "\nSpecial Go doc comments:\n")
 	for _, i := range []string{
+		"//llu:returnsTrKeyWeak",
+		"\tcan be used in front of functions to indicate",
+		"\tthat the function returns message IDs (allows nesting inside complicated function calls)",
+		"\tWARNING: this currently doesn't support nested functions properly",
+		"",
 		"//llu:returnsTrKey",
 		"\tcan be used in front of functions to indicate",
-		"\tthat the function returns message IDs",
+		"\tthat the function returns message IDs (doesn't allow nesting inside complicated function calls)",
 		"\tWARNING: this currently doesn't support nested functions properly",
 		"",
 		"//llu:returnsTrKeySuffix prefix.",
@@ -260,6 +311,10 @@ func main() {
 	}
 
 	handler := llu.Handler{
+		OnMsgidPattern: func(fset *token.FileSet, pos token.Pos, msgidPattern string) {
+			msgidPatternSplit := strings.Split(msgidPattern, ".")
+			allowedMaskedPrefixes.Insert(msgidPatternSplit)
+		},
 		OnMsgidPrefix: func(fset *token.FileSet, pos token.Pos, msgidPrefix string, truncated bool) {
 			msgidPrefixSplit := strings.Split(msgidPrefix, ".")
 			if !truncated {
@@ -270,6 +325,10 @@ func main() {
 			}
 		},
 		OnMsgid: func(fset *token.FileSet, pos token.Pos, msgid string, weak bool) {
+			if strings.Contains(msgid, "%") {
+				fmt.Printf("%s:\tunexpected msgid pattern: %s\n", fset.Position(pos).String(), msgid)
+				return
+			}
 			if !msgids.Contains(msgid) {
 				if weak && allowWeakMissingMsgids {
 					return

build/lint-locale-usage/handle-go.go

@@ -34,12 +34,14 @@ func (handler Handler) HandleGoTrBasicLit(fset *token.FileSet, argLit *ast.Basic
 }
 
 func (handler Handler) HandleGoTrArgument(fset *token.FileSet, n ast.Expr, prefix string) {
-	if argLit, ok := n.(*ast.BasicLit); ok {
+	switch n := n.(type) {
-		handler.HandleGoTrBasicLit(fset, argLit, prefix)
+	case *ast.BasicLit:
-	} else if argBinExpr, ok := n.(*ast.BinaryExpr); ok {
+		handler.HandleGoTrBasicLit(fset, n, prefix)
-		if argBinExpr.Op != token.ADD {
+
+	case *ast.BinaryExpr:
+		if n.Op != token.ADD {
 			// pass
-		} else if argLit, ok := argBinExpr.X.(*ast.BasicLit); ok && argLit.Kind == token.STRING {
+		} else if argLit, ok := n.X.(*ast.BasicLit); ok && argLit.Kind == token.STRING {
 			// extract string content
 			arg, err := strconv.Unquote(argLit.Value)
 			if err != nil {
@@ -53,6 +55,39 @@ func (handler Handler) HandleGoTrArgument(fset *token.FileSet, n ast.Expr, prefi
 			}
 			handler.OnMsgidPrefix(fset, argLit.ValuePos, prep, trunc)
 		}
+
+	case *ast.CallExpr:
+		if selExpr, ok := n.Fun.(*ast.SelectorExpr); ok {
+			if xIdent, xok := selExpr.X.(*ast.Ident); !xok || xIdent.Name != "fmt" {
+				return
+			}
+			if selExpr.Sel.Name != "Sprintf" {
+				handler.OnWarning(fset, selExpr.Sel.NamePos, fmt.Sprintf("unexpected formatting function encountered: %s", selExpr.Sel.Name))
+				return
+			}
+			if len(n.Args) == 0 {
+				handler.OnWarning(fset, selExpr.Sel.NamePos, fmt.Sprintf("unexpected formatting function invocation (no arguments) of '%s'", selExpr.Sel.Name))
+				return
+			}
+
+			if argLit, ok := n.Args[0].(*ast.BasicLit); ok && argLit.Kind == token.STRING {
+				// extract string content
+				arg, err := strconv.Unquote(argLit.Value)
+				if err != nil {
+					return
+				}
+				if strings.Contains(arg, " ") {
+					handler.OnWarning(fset, argLit.ValuePos, fmt.Sprintf(
+						"formatting function invocation of '%s' with weird msgid format string: %s",
+						selExpr.Sel.Name,
+						arg,
+					))
+					return
+				}
+				// found interesting strings
+				handler.OnMsgidPattern(fset, argLit.ValuePos, prefix+arg)
+			}
+		}
 	}
 }
 

build/lint-locale-usage/handle-tmpl.go

@@ -150,16 +150,12 @@ func (handler Handler) handleTemplateMsgid(fset *token.FileSet, node tmplParser.
 			handler.OnMsgid(fset, stringPos, msgidPrefix, false)
 		} else {
 			if nodeIdent.Ident == "printf" {
-				parts := strings.SplitN(msgidPrefix, "%", 2)
+				// found interesting strings
-				if len(parts) != 2 {
+				if !(strings.HasSuffix(msgidPrefix, ".%s") && strings.Count(msgidPrefix, "%") == 1) {
-					handler.OnWarning(
+					handler.OnMsgidPattern(fset, stringPos, msgidPrefix)
-						fset,
-						stringPos,
-						fmt.Sprintf("unsupported invocation of locate function (format string doesn't match \"prefix%%smth\" pattern): %s", nodeString.String()),
-					)
 					return
 				}
-				msgidPrefix = parts[0]
+				msgidPrefix = strings.TrimSuffix(msgidPrefix, "%s")
 			}
 
 			msgidPrefixFin, truncated := PrepareMsgidPrefix(msgidPrefix)

build/lint-locale-usage/handler.go

@@ -47,6 +47,7 @@ func InitLocaleTrFunctions() map[string][]uint {
 type Handler struct {
 	OnMsgid            func(fset *token.FileSet, pos token.Pos, msgid string, weak bool)
 	OnMsgidPrefix      func(fset *token.FileSet, pos token.Pos, msgidPrefix string, truncated bool)
+	OnMsgidPattern     func(fset *token.FileSet, pos token.Pos, msgidPattern string)
 	OnUnexpectedInvoke func(fset *token.FileSet, pos token.Pos, funcname string, argc int)
 	OnWarning          func(fset *token.FileSet, pos token.Pos, msg string)
 	LocaleTrFunctions  map[string][]uint

models/project/project.go

@@ -184,7 +184,7 @@ func init() {
 
 // GetCardConfig retrieves the types of configurations project column cards could have
 //
-//llu:returnsTrKey
+//llu:returnsTrKeyWeak
 func GetCardConfig() []CardConfig {
 	return []CardConfig{
 		{CardTypeTextOnly, "repo.projects.card_type.text_only"},

models/project/template.go

@@ -27,7 +27,7 @@ const (
 
 // GetTemplateConfigs retrieves the template configs of configurations project columns could have
 //
-//llu:returnsTrKey
+//llu:returnsTrKeyWeak
 func GetTemplateConfigs() []TemplateConfig {
 	return []TemplateConfig{
 		{TemplateTypeNone, "repo.projects.type.none"},

models/system/notice.go

@@ -38,6 +38,8 @@ func init() {
 }
 
 // TrStr returns a translation format string.
+//
+//llu:returnsTrKey
 func (n *Notice) TrStr() string {
 	return fmt.Sprintf("admin.notices.type_%d", n.Type)
 }

templates/repo/issue/view_content.tmpl

@@ -110,14 +110,14 @@
 												</span>
 											</button>
 										{{else}}
-											{{$closeTranslationKey := "repo.issues.close"}}
+											{{$closeTranslation := ctx.Locale.Tr "repo.issues.close"}}
 											{{if .Issue.IsPull}}
-												{{$closeTranslationKey = "repo.pulls.close"}}
+												{{$closeTranslation = ctx.Locale.Tr "repo.pulls.close"}}
 											{{end}}
-											<button id="status-button" class="secondary button" data-status="{{ctx.Locale.Tr $closeTranslationKey}}" data-status-and-comment="{{ctx.Locale.Tr "repo.issues.close_comment_issue"}}" name="status" value="close">
+											<button id="status-button" class="secondary button" data-status="{{$closeTranslation}}" data-status-and-comment="{{ctx.Locale.Tr "repo.issues.close_comment_issue"}}" name="status" value="close">
 												{{svg "octicon-issue-closed"}}
 												<span>
-													{{ctx.Locale.Tr $closeTranslationKey}}
+													{{$closeTranslation}}
 												</span>
 											</button>
 										{{end}}

templates/repo/issue/view_content/comments.tmpl

@@ -129,13 +129,13 @@
 			{{if ne .RefRepoID .Issue.RepoID}}
 				{{$refFrom = ctx.Locale.Tr "repo.issues.ref_from" .RefRepo.FullName}}
 			{{end}}
-			{{$refTr := "repo.issues.ref_issue_from"}}
+			{{$refTr := "issue"}}
 			{{if .Issue.IsPull}}
-				{{$refTr = "repo.issues.ref_pull_from"}}
+				{{$refTr = "pull"}}
 			{{else if eq .RefAction 1}}
-				{{$refTr = "repo.issues.ref_closing_from"}}
+				{{$refTr = "closing"}}
 			{{else if eq .RefAction 2}}
-				{{$refTr = "repo.issues.ref_reopening_from"}}
+				{{$refTr = "reopening"}}
 			{{end}}
 			<div class="timeline-item event" id="{{.HashTag}}">
 				<span class="badge">{{svg "octicon-bookmark"}}</span>
@@ -143,7 +143,7 @@
 				{{if eq .RefAction 3}}<del>{{end}}
 				<span class="text grey muted-links">
 					{{template "shared/user/authorlink" .Poster}}
-					{{ctx.Locale.Tr $refTr $createdStr (.RefCommentLink ctx) $refFrom}}
+					{{ctx.Locale.Tr (printf "repo.issues.ref_%s_from" $refTr) $createdStr (.RefCommentLink ctx) $refFrom}}
 				</span>
 				{{if eq .RefAction 3}}</del>{{end}}