From d40c444d079e99b8e678578efc56bdee7a524012 Mon Sep 17 00:00:00 2001 From: Ellen Emilia Anna Zscheile Date: Fri, 14 Mar 2025 15:50:30 +0000 Subject: [PATCH] feat(build): linter for missing msgid definitions (#7109) Add a new linter that checks that basic usages (those with an constant string) of the `Tr` function in Go and template files are referring to an existing translation value. Add it to the CI stack but not make it fail yet. Signed-off-by: Ellen Emilia Anna Zscheile Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7109 Reviewed-by: Gusted Co-authored-by: Ellen Emilia Anna Zscheile Co-committed-by: Ellen Emilia Anna Zscheile --- Makefile | 8 +- build/lint-locale-usage/lint-locale-usage.go | 331 ++++++++++++++++++ .../lint-locale-usage_test.go | 44 +++ build/{ => lint-locale}/lint-locale.go | 58 +-- build/{ => lint-locale}/lint-locale_test.go | 32 +- modules/locale/utils.go | 74 ++++ 6 files changed, 488 insertions(+), 59 deletions(-) create mode 100644 build/lint-locale-usage/lint-locale-usage.go create mode 100644 build/lint-locale-usage/lint-locale-usage_test.go rename build/{ => lint-locale}/lint-locale.go (79%) rename build/{ => lint-locale}/lint-locale_test.go (88%) create mode 100644 modules/locale/utils.go diff --git a/Makefile b/Makefile index 483c66348a..e61e08bcc9 100644 --- a/Makefile +++ b/Makefile @@ -413,7 +413,7 @@ lint-frontend: lint-js lint-css lint-frontend-fix: lint-js-fix lint-css-fix .PHONY: lint-backend -lint-backend: lint-go lint-go-vet lint-editorconfig lint-renovate lint-locale lint-disposable-emails +lint-backend: lint-go lint-go-vet lint-editorconfig lint-renovate lint-locale lint-locale-usage lint-disposable-emails .PHONY: lint-backend-fix lint-backend-fix: lint-go-fix lint-go-vet lint-editorconfig lint-disposable-emails-fix @@ -458,7 +458,11 @@ lint-renovate: node_modules .PHONY: lint-locale lint-locale: - $(GO) run build/lint-locale.go + $(GO) run build/lint-locale/lint-locale.go + +.PHONY: lint-locale-usage +lint-locale-usage: + $(GO) run build/lint-locale-usage/lint-locale-usage.go --allow-missing-msgids .PHONY: lint-md lint-md: node_modules diff --git a/build/lint-locale-usage/lint-locale-usage.go b/build/lint-locale-usage/lint-locale-usage.go new file mode 100644 index 0000000000..f42bc59cbb --- /dev/null +++ b/build/lint-locale-usage/lint-locale-usage.go @@ -0,0 +1,331 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package main + +import ( + "fmt" + "go/ast" + goParser "go/parser" + "go/token" + "io/fs" + "os" + "path/filepath" + "strconv" + "strings" + "text/template" + tmplParser "text/template/parse" + + "code.gitea.io/gitea/modules/container" + "code.gitea.io/gitea/modules/locale" + fjTemplates "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/util" +) + +// this works by first gathering all valid source string IDs from `en-US` reference files +// and then checking if all used source strings are actually defined + +type OnMsgidHandler func(fset *token.FileSet, pos token.Pos, msgid string) + +type LocatedError struct { + Location string + Kind string + Err error +} + +func (e LocatedError) Error() string { + var sb strings.Builder + + sb.WriteString(e.Location) + sb.WriteString(":\t") + if e.Kind != "" { + sb.WriteString(e.Kind) + sb.WriteString(": ") + } + sb.WriteString("ERROR: ") + sb.WriteString(e.Err.Error()) + + return sb.String() +} + +func isLocaleTrFunction(funcname string) bool { + return funcname == "Tr" || funcname == "TrN" +} + +// the `Handle*File` functions follow the following calling convention: +// * `fname` is the name of the input file +// * `src` is either `nil` (then the function invokes `ReadFile` to read the file) +// or the contents of the file as {`[]byte`, or a `string`} + +func (omh OnMsgidHandler) HandleGoFile(fname string, src any) error { + fset := token.NewFileSet() + node, err := goParser.ParseFile(fset, fname, src, goParser.SkipObjectResolution) + if err != nil { + return LocatedError{ + Location: fname, + Kind: "Go parser", + Err: err, + } + } + + ast.Inspect(node, func(n ast.Node) bool { + // search for function calls of the form `anything.Tr(any-string-lit)` + + call, ok := n.(*ast.CallExpr) + if !ok || len(call.Args) != 1 { + return true + } + + funSel, ok := call.Fun.(*ast.SelectorExpr) + if (!ok) || !isLocaleTrFunction(funSel.Sel.Name) { + return true + } + + argLit, ok := call.Args[0].(*ast.BasicLit) + if (!ok) || argLit.Kind != token.STRING { + return true + } + + // extract string content + arg, err := strconv.Unquote(argLit.Value) + if err != nil { + return true + } + + // found interesting string + omh(fset, argLit.ValuePos, arg) + + return true + }) + + return nil +} + +// derived from source: modules/templates/scopedtmpl/scopedtmpl.go, L169-L213 +func (omh OnMsgidHandler) handleTemplateNode(fset *token.FileSet, node tmplParser.Node) { + switch node.Type() { + case tmplParser.NodeAction: + omh.handleTemplatePipeNode(fset, node.(*tmplParser.ActionNode).Pipe) + case tmplParser.NodeList: + nodeList := node.(*tmplParser.ListNode) + omh.handleTemplateFileNodes(fset, nodeList.Nodes) + case tmplParser.NodePipe: + omh.handleTemplatePipeNode(fset, node.(*tmplParser.PipeNode)) + case tmplParser.NodeTemplate: + omh.handleTemplatePipeNode(fset, node.(*tmplParser.TemplateNode).Pipe) + case tmplParser.NodeIf: + nodeIf := node.(*tmplParser.IfNode) + omh.handleTemplateBranchNode(fset, nodeIf.BranchNode) + case tmplParser.NodeRange: + nodeRange := node.(*tmplParser.RangeNode) + omh.handleTemplateBranchNode(fset, nodeRange.BranchNode) + case tmplParser.NodeWith: + nodeWith := node.(*tmplParser.WithNode) + omh.handleTemplateBranchNode(fset, nodeWith.BranchNode) + + case tmplParser.NodeCommand: + nodeCommand := node.(*tmplParser.CommandNode) + + omh.handleTemplateFileNodes(fset, nodeCommand.Args) + + if len(nodeCommand.Args) != 2 { + return + } + + nodeChain, ok := nodeCommand.Args[0].(*tmplParser.ChainNode) + if !ok { + return + } + + nodeString, ok := nodeCommand.Args[1].(*tmplParser.StringNode) + if !ok { + return + } + + nodeIdent, ok := nodeChain.Node.(*tmplParser.IdentifierNode) + if !ok || nodeIdent.Ident != "ctx" { + return + } + + if len(nodeChain.Field) != 2 || nodeChain.Field[0] != "Locale" || !isLocaleTrFunction(nodeChain.Field[1]) { + return + } + + // found interesting string + // the column numbers are a bit "off", but much better than nothing + omh(fset, token.Pos(nodeString.Pos), nodeString.Text) + + default: + } +} + +func (omh OnMsgidHandler) handleTemplatePipeNode(fset *token.FileSet, pipeNode *tmplParser.PipeNode) { + if pipeNode == nil { + return + } + + // NOTE: we can't pass `pipeNode.Cmds` to handleTemplateFileNodes due to incompatible argument types + for _, node := range pipeNode.Cmds { + omh.handleTemplateNode(fset, node) + } +} + +func (omh OnMsgidHandler) handleTemplateBranchNode(fset *token.FileSet, branchNode tmplParser.BranchNode) { + omh.handleTemplatePipeNode(fset, branchNode.Pipe) + omh.handleTemplateFileNodes(fset, branchNode.List.Nodes) + if branchNode.ElseList != nil { + omh.handleTemplateFileNodes(fset, branchNode.ElseList.Nodes) + } +} + +func (omh OnMsgidHandler) handleTemplateFileNodes(fset *token.FileSet, nodes []tmplParser.Node) { + for _, node := range nodes { + omh.handleTemplateNode(fset, node) + } +} + +func (omh OnMsgidHandler) HandleTemplateFile(fname string, src any) error { + var tmplContent []byte + switch src2 := src.(type) { + case nil: + var err error + tmplContent, err = os.ReadFile(fname) + if err != nil { + return LocatedError{ + Location: fname, + Kind: "ReadFile", + Err: err, + } + } + case []byte: + tmplContent = src2 + case string: + // SAFETY: we do not modify tmplContent below + tmplContent = util.UnsafeStringToBytes(src2) + default: + panic("invalid type for 'src'") + } + + fset := token.NewFileSet() + fset.AddFile(fname, 1, len(tmplContent)).SetLinesForContent(tmplContent) + // SAFETY: we do not modify tmplContent2 below + tmplContent2 := util.UnsafeBytesToString(tmplContent) + + tmpl := template.New(fname) + tmpl.Funcs(fjTemplates.NewFuncMap()) + tmplParsed, err := tmpl.Parse(tmplContent2) + if err != nil { + return LocatedError{ + Location: fname, + Kind: "Template parser", + Err: err, + } + } + omh.handleTemplateFileNodes(fset, tmplParsed.Tree.Root.Nodes) + return nil +} + +// This command assumes that we get started from the project root directory +// +// Possible command line flags: +// +// --allow-missing-msgids don't return an error code if missing message IDs are found +// +// EXIT CODES: +// +// 0 success, no issues found +// 1 unable to walk directory tree +// 2 unable to parse locale ini/json files +// 3 unable to parse go or text/template files +// 4 found missing message IDs +// +//nolint:forbidigo +func main() { + allowMissingMsgids := false + for _, arg := range os.Args[1:] { + if arg == "--allow-missing-msgids" { + allowMissingMsgids = true + } + } + + onError := func(err error) { + if err == nil { + return + } + fmt.Println(err.Error()) + os.Exit(3) + } + + 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) + if err != nil { + fmt.Printf("%s:\tERROR: %s\n", localeFile, err.Error()) + os.Exit(2) + } + + if err = locale.IterateMessagesContent(localeContent, onMsgid); err != nil { + fmt.Printf("%s:\tERROR: %s\n", localeFile, err.Error()) + os.Exit(2) + } + + localeFile = filepath.Join(filepath.Join("options", "locale_next"), "locale_en-US.json") + localeContent, err = os.ReadFile(localeFile) + if err != nil { + fmt.Printf("%s:\tERROR: %s\n", localeFile, err.Error()) + os.Exit(2) + } + + if err := locale.IterateMessagesNextContent(localeContent, onMsgid); err != nil { + fmt.Printf("%s:\tERROR: %s\n", localeFile, err.Error()) + os.Exit(2) + } + + gotAnyMsgidError := false + + omh := OnMsgidHandler(func(fset *token.FileSet, pos token.Pos, msgid string) { + if !msgids.Contains(msgid) { + gotAnyMsgidError = true + fmt.Printf("%s:\tmissing msgid: %s\n", fset.Position(pos).String(), msgid) + } + }) + + if err := filepath.WalkDir(".", func(fpath string, d fs.DirEntry, err error) error { + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + name := d.Name() + if d.IsDir() { + if name == "docker" || name == ".git" || name == "node_modules" { + return fs.SkipDir + } + } else if name == "bindata.go" { + // skip false positives + } else if strings.HasSuffix(name, ".go") { + onError(omh.HandleGoFile(fpath, nil)) + } else if strings.HasSuffix(name, ".tmpl") { + if strings.HasPrefix(fpath, "tests") && strings.HasSuffix(name, ".ini.tmpl") { + // skip false positives + } else { + onError(omh.HandleTemplateFile(fpath, nil)) + } + } + return nil + }); err != nil { + fmt.Printf("walkdir ERROR: %s\n", err.Error()) + os.Exit(1) + } + + if !allowMissingMsgids && gotAnyMsgidError { + os.Exit(4) + } +} diff --git a/build/lint-locale-usage/lint-locale-usage_test.go b/build/lint-locale-usage/lint-locale-usage_test.go new file mode 100644 index 0000000000..3b3b746053 --- /dev/null +++ b/build/lint-locale-usage/lint-locale-usage_test.go @@ -0,0 +1,44 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package main + +import ( + "go/token" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func HandleGoFileWrapped(t *testing.T, fname, src string) []string { + var ret []string + omh := OnMsgidHandler(func(fset *token.FileSet, pos token.Pos, msgid string) { + ret = append(ret, msgid) + }) + require.NoError(t, omh.HandleGoFile(fname, src)) + return ret +} + +func HandleTemplateFileWrapped(t *testing.T, fname, src string) []string { + var ret []string + omh := OnMsgidHandler(func(fset *token.FileSet, pos token.Pos, msgid string) { + ret = append(ret, msgid) + }) + require.NoError(t, omh.HandleTemplateFile(fname, src)) + return ret +} + +func TestUsagesParser(t *testing.T) { + t.Run("go, simple", func(t *testing.T) { + assert.EqualValues(t, + []string{"what.an.example"}, + HandleGoFileWrapped(t, "", "package main\nfunc Render(ctx *context.Context) string { return ctx.Tr(\"what.an.example\"); }\n")) + }) + + t.Run("template, simple", func(t *testing.T) { + assert.EqualValues(t, + []string{"what.an.example"}, + HandleTemplateFileWrapped(t, "", "{{ ctx.Locale.Tr \"what.an.example\" }}\n")) + }) +} diff --git a/build/lint-locale.go b/build/lint-locale/lint-locale.go similarity index 79% rename from build/lint-locale.go rename to build/lint-locale/lint-locale.go index 0fb500222f..a738fbd684 100644 --- a/build/lint-locale.go +++ b/build/lint-locale/lint-locale.go @@ -5,7 +5,6 @@ package main import ( - "encoding/json" //nolint:depguard "fmt" "html" "io/fs" @@ -15,9 +14,10 @@ import ( "slices" "strings" + "code.gitea.io/gitea/modules/locale" + "github.com/microcosm-cc/bluemonday" "github.com/sergi/go-diff/diffmatchpatch" - "gopkg.in/ini.v1" //nolint:depguard ) var ( @@ -98,49 +98,28 @@ func checkValue(trKey, value string) []string { } func checkLocaleContent(localeContent []byte) []string { - // Same configuration as Forgejo uses. - cfg := ini.Empty(ini.LoadOptions{ - IgnoreContinuation: true, - }) - cfg.NameMapper = ini.SnackCase + errors := []string{} - if err := cfg.Append(localeContent); err != nil { + if err := locale.IterateMessagesContent(localeContent, func(trKey, trValue string) error { + errors = append(errors, checkValue(trKey, trValue)...) + return nil + }); err != nil { panic(err) } - errors := []string{} - for _, section := range cfg.Sections() { - for _, key := range section.Keys() { - var trKey string - if section.Name() == "" || section.Name() == "DEFAULT" || section.Name() == "common" { - trKey = key.Name() - } else { - trKey = section.Name() + "." + key.Name() - } - - errors = append(errors, checkValue(trKey, key.Value())...) - } - } return errors } -func checkLocaleNextContent(data map[string]any, trKey ...string) []string { +func checkLocaleNextContent(localeContent []byte) []string { errors := []string{} - for key, value := range data { - currentKey := key - if len(trKey) == 1 { - currentKey = trKey[0] + "." + key - } - switch value := value.(type) { - case string: - errors = append(errors, checkValue(currentKey, value)...) - case map[string]any: - errors = append(errors, checkLocaleNextContent(value, currentKey)...) - default: - panic(fmt.Sprintf("Unexpected type during linting locale next: %s - %T", currentKey, value)) - } + if err := locale.IterateMessagesNextContent(localeContent, func(trKey, trValue string) error { + errors = append(errors, checkValue(trKey, trValue)...) + return nil + }); err != nil { + panic(err) } + return errors } @@ -168,6 +147,7 @@ func main() { localeContent, err := os.ReadFile(filepath.Join(localeDir, localeFile.Name())) if err != nil { + fmt.Println(localeFile.Name()) panic(err) } @@ -195,15 +175,11 @@ func main() { for _, localeFile := range localeFiles { localeContent, err := os.ReadFile(filepath.Join(localeDir, localeFile.Name())) if err != nil { + fmt.Println(localeFile.Name()) panic(err) } - var localeData map[string]any - if err := json.Unmarshal(localeContent, &localeData); err != nil { - panic(err) - } - - if err := checkLocaleNextContent(localeData); len(err) > 0 { + if err := checkLocaleNextContent(localeContent); len(err) > 0 { fmt.Println(localeFile.Name()) fmt.Println(strings.Join(err, "\n")) fmt.Println() diff --git a/build/lint-locale_test.go b/build/lint-locale/lint-locale_test.go similarity index 88% rename from build/lint-locale_test.go rename to build/lint-locale/lint-locale_test.go index 408897d781..791f5278bc 100644 --- a/build/lint-locale_test.go +++ b/build/lint-locale/lint-locale_test.go @@ -69,26 +69,26 @@ func TestNextLocalizationPolicy(t *testing.T) { initRemoveTags() t.Run("Nested locales", func(t *testing.T) { - assert.Empty(t, checkLocaleNextContent(map[string]any{ - "settings": map[string]any{ - "hidden_comment_types_description": `Comment types checked here will not be shown inside issue pages. Checking "Label" for example removes all " added/removed