From 06d1acd667d52507597bfb0f90dfad9195ddeaff Mon Sep 17 00:00:00 2001 From: Florian Vogt Date: Thu, 16 Jun 2022 00:02:23 +0200 Subject: [PATCH] Support for inline functions --- commands/root.go | 6 +- extract/etype/token.go | 5 +- extract/extractors/context.go | 35 +++++--- extract/runner.go | 1 + goextractors/definition.go | 104 ++++++++++++++--------- goextractors/funccall.go | 4 +- goextractors/funccall_test.go | 3 +- goextractors/funcreturn.go | 3 +- goextractors/globalassign.go | 3 + goextractors/mapsdef.go | 2 + goextractors/slicedef.go | 2 + goextractors/structdef.go | 1 + goextractors/testlib_test.go | 5 +- goextractors/utils.go | 52 ++++++++++++ goextractors/varassign.go | 2 + result/processors/comment_cleaner.go | 8 +- result/processors/unprintable_checker.go | 46 ++++++++++ testdata/project/funccall.go | 4 + 18 files changed, 221 insertions(+), 65 deletions(-) create mode 100644 result/processors/unprintable_checker.go diff --git a/commands/root.go b/commands/root.go index 70d0c6c..ad2a8a3 100644 --- a/commands/root.go +++ b/commands/root.go @@ -56,8 +56,8 @@ func init() { fs.BoolVarP(&extractCfg.ExtractErrors, "extract-errors", "e", def.ExtractErrors, "Strings from errors.New(STRING) are extracted") fs.StringVar(&extractCfg.ErrorContext, "errors-context", def.ErrorContext, "Context which is automatically assigned to extracted errors") - fs.StringArrayVarP(&extractCfg.TemplatePatterns, "template-directory", "t", def.TemplatePatterns, "Set a list of paths to which the template files contain. Regular expressions can be used.") - fs.String("template-prefix", ".T", "Sets a prefix for the translation functions, which is used within the templates") + fs.StringArrayVarP(&extractCfg.TemplatePatterns, "template-directory", "t", []string{}, "Set a list of paths to which the template files contain. Regular expressions can be used.") + fs.String("template-prefix", "", "Sets a prefix for the translation functions, which is used within the templates") fs.BoolVar(&extractCfg.TmplIsMonolingual, "template-use-kv", false, "Determines whether the strings from templates should be handled as key-value") fs.StringArrayP("template-keyword", "k", []string{}, "Sets a keyword that is used within templates to identify translation functions") @@ -77,7 +77,7 @@ func validateExtractConfig(cmd *cobra.Command) { fs := cmd.Flags() if keywordPrefix, errP := fs.GetString("template-prefix"); errP != nil { log.WithError(errP).Fatal("Args could not be parsed") - } else { + } else if keywordPrefix != "" { extractCfg.Keywords = tmpl.DefaultKeywords(keywordPrefix, extractCfg.TmplIsMonolingual) } diff --git a/extract/etype/token.go b/extract/etype/token.go index 2a34666..8751a76 100644 --- a/extract/etype/token.go +++ b/extract/etype/token.go @@ -5,12 +5,11 @@ type Token string const ( None Token = "" Singular Token = "Singular" + Key Token = "Key" + PluralKey Token = "PluralKey" Plural Token = "Plural" Domain Token = "Domain" Context Token = "Context" - Message Token = "Message" - Key Token = "Key" - PluralKey Token = "PluralKey" ) func IsMessageID(tok Token) bool { diff --git a/extract/extractors/context.go b/extract/extractors/context.go index e718ee0..34bac67 100644 --- a/extract/extractors/context.go +++ b/extract/extractors/context.go @@ -199,11 +199,13 @@ type SearchResult struct { func (c *Context) SearchStrings(startExpr ast.Expr) []*SearchResult { results := make([]*SearchResult, 0) + found := make(map[ast.Node]struct{}) // String was created at the current position extracted, originNode := ExtractStringLiteral(startExpr) if extracted != "" { results = append(results, &SearchResult{Raw: extracted, Node: originNode}) + found[originNode] = struct{}{} } // Backtracking the string @@ -211,7 +213,7 @@ func (c *Context) SearchStrings(startExpr ast.Expr) []*SearchResult { if !ok || startIdent.Obj == nil { return results } - mainMessage := extracted + c.Inspector.WithStack([]ast.Node{&ast.AssignStmt{}}, func(raw ast.Node, push bool, stack []ast.Node) (proceed bool) { proceed = false @@ -219,6 +221,7 @@ func (c *Context) SearchStrings(startExpr ast.Expr) []*SearchResult { if len(node.Lhs) != len(node.Rhs) || len(node.Lhs) == 0 { return } + for i, left := range node.Lhs { leftIdent, isIdent := left.(*ast.Ident) if !isIdent { @@ -227,9 +230,15 @@ func (c *Context) SearchStrings(startExpr ast.Expr) []*SearchResult { if leftIdent.Obj != startIdent.Obj { continue } + + if _, ok := found[node.Rhs[i]]; ok { + continue + } + extracted, originNode = ExtractStringLiteral(node.Rhs[i]) - if extracted != "" && extracted != mainMessage { - results = append(results, &SearchResult{Raw: extracted, Node: node}) + if extracted != "" { + found[node.Rhs[i]] = struct{}{} + results = append(results, &SearchResult{Raw: extracted, Node: originNode}) } } @@ -242,8 +251,19 @@ func (c *Context) SearchStrings(startExpr ast.Expr) []*SearchResult { // GetComments extracts the Go comments for a list of nodes. func (c *Context) GetComments(pkg *packages.Package, nodes ...ast.Node) []string { var comments []string + + if _, hasPkg := c.CommentMaps[pkg.PkgPath]; !hasPkg { + return comments + } + found := make(map[*ast.CommentGroup]bool) + for _, node := range nodes { + pos := c.GetPosition(node.Pos()) + if _, hasFile := c.CommentMaps[pkg.PkgPath][pos.Filename]; !hasFile { + break + } + c.Inspector.WithStack([]ast.Node{node}, func(n ast.Node, push bool, stack []ast.Node) (proceed bool) { proceed = false // Search stack for our node @@ -251,16 +271,7 @@ func (c *Context) GetComments(pkg *packages.Package, nodes ...ast.Node) []string return } - if _, hasPkg := c.CommentMaps[pkg.PkgPath]; !hasPkg { - return - } - // Find the first node of the line - pos := c.GetPosition(node.Pos()) - if _, hasFile := c.CommentMaps[pkg.PkgPath][pos.Filename]; !hasFile { - return - } - var topNode = node for i := len(stack) - 1; i >= 0; i-- { entry := stack[i] diff --git a/extract/runner.go b/extract/runner.go index debdd2e..67aca60 100644 --- a/extract/runner.go +++ b/extract/runner.go @@ -32,6 +32,7 @@ func NewRunner(cfg *config.Config, _ map[string]*packages.Package) (*Runner, err p = append(p, processors.NewCommentCleaner(cfg), processors.NewSkipIgnore(), + processors.NewUnprintableCheck(), processors.NewPrepareKey(), ) diff --git a/goextractors/definition.go b/goextractors/definition.go index 9dd229c..e862f73 100644 --- a/goextractors/definition.go +++ b/goextractors/definition.go @@ -48,6 +48,8 @@ func (de *definitionExtractorRunner) searchDefinitions(n ast.Node, push bool) bo switch v := n.(type) { case *ast.FuncDecl: de.extractFunc(v) + case *ast.AssignStmt: + de.extractInlineFunc(v) case *ast.GenDecl: switch v.Tok { case token.VAR: @@ -178,58 +180,78 @@ func (de *definitionExtractorRunner) extractStruct(decl *ast.GenDecl) { // func translate(msgid localize.Singular, plural localize.Plural) // func getTranslation() (localize.Singular, localize.Plural). func (de *definitionExtractorRunner) extractFunc(decl *ast.FuncDecl) { - pck, obj := de.extractCtx.GetType(decl.Name) - if pck == nil { + if decl.Type == nil || decl.Type.Params == nil { return } - if decl.Type == nil { + de.extractFunctionsParams(decl.Name, decl.Type) +} + +func (de *definitionExtractorRunner) extractInlineFunc(assign *ast.AssignStmt) { + if len(assign.Lhs) == 0 || len(assign.Lhs) != len(assign.Rhs) { return } - // function call - if decl.Type.Params != nil { - for i, param := range decl.Type.Params.List { - tok, _ := de.extractCtx.SearchIdentAndToken(param) - if tok == etype.None || tok == etype.Message { - continue - } + ident, ok := assign.Lhs[0].(*ast.Ident) + if !ok { + return + } - if len(param.Names) == 0 { - def := &extractors.Definition{ - Type: extractors.FunctionParam, - Token: tok, - Pck: pck, - Ident: decl.Name, - Path: obj.Pkg().Path(), - ID: util.ObjToKey(obj), - Obj: obj, - FieldIdent: nil, - FieldName: strconv.Itoa(i), + funcLit, ok := assign.Rhs[0].(*ast.FuncLit) + if !ok || funcLit.Type == nil || funcLit.Type.Params == nil { + return + } - FieldPos: i, - IsVariadic: isEllipsis(param.Type), - } - de.addDefinition(def) - } + de.extractFunctionsParams(ident, funcLit.Type) +} - for ii, name := range param.Names { - def := &extractors.Definition{ - Type: extractors.FunctionParam, - Token: tok, - Pck: pck, - Ident: decl.Name, - Path: obj.Pkg().Path(), - ID: util.ObjToKey(obj), - Obj: obj, - FieldIdent: name, - FieldName: name.Name, - IsVariadic: isEllipsis(param.Type), +func (de *definitionExtractorRunner) extractFunctionsParams(ident *ast.Ident, t *ast.FuncType) { + pck, obj := de.extractCtx.GetType(ident) + if pck == nil { + return + } - FieldPos: calculatePosIdx(i, ii), - } - de.addDefinition(def) + // function call + for i, param := range t.Params.List { + tok, _ := de.extractCtx.SearchIdentAndToken(param) + if tok == etype.None { + continue + } + + if len(param.Names) == 0 { + def := &extractors.Definition{ + Type: extractors.FunctionParam, + Token: tok, + Pck: pck, + Ident: ident, + Path: obj.Pkg().Path(), + ID: util.ObjToKey(obj), + Obj: obj, + FieldIdent: nil, + FieldName: strconv.Itoa(i), + + FieldPos: i, + IsVariadic: isEllipsis(param.Type), + } + de.addDefinition(def) + } + + for ii, name := range param.Names { + def := &extractors.Definition{ + Type: extractors.FunctionParam, + Token: tok, + Pck: pck, + Ident: ident, + Path: obj.Pkg().Path(), + ID: util.ObjToKey(obj), + Obj: obj, + FieldIdent: name, + FieldName: name.Name, + IsVariadic: isEllipsis(param.Type), + + FieldPos: calculatePosIdx(i, ii), } + de.addDefinition(def) } } } diff --git a/goextractors/funccall.go b/goextractors/funccall.go index 4c8cbb6..a80f916 100644 --- a/goextractors/funccall.go +++ b/goextractors/funccall.go @@ -70,7 +70,8 @@ func (v funcCallExtractor) Run(_ context.Context, extractCtx *extractors.Context issues = append(issues, issue) } - + } else if tok != etype.None { + writeMissingMessageID(extractCtx.GetPosition(ident.Pos()), tok, "") } funcParameterDefs := extractCtx.Definitions.GetFields(util.ObjToKey(obj)) @@ -105,6 +106,7 @@ func (v funcCallExtractor) Run(_ context.Context, extractCtx *extractors.Context } } + collector.CheckMissingMessageID(extractCtx) for i, singularResult := range collector.Singulars { issue := result.Issue{ FromExtractor: v.Name(), diff --git a/goextractors/funccall_test.go b/goextractors/funccall_test.go index e8c0245..ab81961 100644 --- a/goextractors/funccall_test.go +++ b/goextractors/funccall_test.go @@ -11,7 +11,7 @@ func TestFuncCallExtractor(t *testing.T) { assert.NotEmpty(t, issues) want := []string{ - // "f-msgid", "f-plural", "f-context", "f-domain", + "f-msgid", "f-plural", "f-context", "f-domain", "init", "localizer func call", "noop-msgid", "noop-plural", "noop-context", "noop-domain", "msgid", @@ -28,6 +28,7 @@ func TestFuncCallExtractor(t *testing.T) { "no-param-msgid", "no-param-plural", "multi-names-a", "multi-names-b", "init backtrace", "assign backtrace", + "inline function", } got := collectIssueStrings(issues) assert.ElementsMatch(t, want, got) diff --git a/goextractors/funcreturn.go b/goextractors/funcreturn.go index 0ddc20d..5b8cde9 100644 --- a/goextractors/funcreturn.go +++ b/goextractors/funcreturn.go @@ -36,7 +36,7 @@ func (e *funcReturnExtractor) Run(_ context.Context, extractCtx *extractors.Cont extractedResults := make([]etype.Token, len(node.Type.Results.List)) var foundType bool for i, res := range node.Type.Results.List { - if tok, _ := extractCtx.SearchIdentAndToken(res); tok == etype.None || tok == etype.Message { + if tok, _ := extractCtx.SearchIdentAndToken(res); tok == etype.None { extractedResults[i] = etype.None continue } else { @@ -86,6 +86,7 @@ func (e *funcReturnExtractor) Run(_ context.Context, extractCtx *extractors.Cont } } + collector.CheckMissingMessageID(extractCtx) for i, singularResult := range collector.Singulars { issue := result.Issue{ FromExtractor: e.Name(), diff --git a/goextractors/globalassign.go b/goextractors/globalassign.go index 300e690..52e399c 100644 --- a/goextractors/globalassign.go +++ b/goextractors/globalassign.go @@ -35,6 +35,9 @@ func (v globalAssignExtractor) Run(_ context.Context, extractCtx *extractors.Con tok := extractCtx.GetLocalizeTypeToken(selector) if tok != etype.Singular { + if tok != etype.None { + writeMissingMessageID(extractCtx.GetPosition(selector.Pos()), tok, "") + } return } diff --git a/goextractors/mapsdef.go b/goextractors/mapsdef.go index 3c35b6f..b154dbb 100644 --- a/goextractors/mapsdef.go +++ b/goextractors/mapsdef.go @@ -80,6 +80,8 @@ func (v mapsDefExtractor) Run(_ context.Context, extractCtx *extractors.Context) } continue + } else if token != etype.None { + writeMissingMessageID(extractCtx.GetPosition(ident.Pos()), token, "") } // Array of structs diff --git a/goextractors/slicedef.go b/goextractors/slicedef.go index 5826531..4d4a541 100644 --- a/goextractors/slicedef.go +++ b/goextractors/slicedef.go @@ -97,6 +97,8 @@ func (v sliceDefExtractor) Run(_ context.Context, extractCtx *extractors.Context } return + } else if token != etype.None { + writeMissingMessageID(extractCtx.GetPosition(node.Pos()), token, "") } structAttr := extractCtx.Definitions.GetFields(util.ObjToKey(obj)) diff --git a/goextractors/structdef.go b/goextractors/structdef.go index b4b5dc6..4f643bf 100644 --- a/goextractors/structdef.go +++ b/goextractors/structdef.go @@ -134,6 +134,7 @@ func extractStruct(extractCtx *extractors.Context, node *ast.CompositeLit, obj t } } + collector.CheckMissingMessageID(extractCtx) for i, singularResult := range collector.Singulars { issue := result.Issue{ FromExtractor: "extract_struct", diff --git a/goextractors/testlib_test.go b/goextractors/testlib_test.go index a819710..22fbf4f 100644 --- a/goextractors/testlib_test.go +++ b/goextractors/testlib_test.go @@ -2,6 +2,7 @@ package goextractors import ( "context" + "go/ast" "go/parser" "go/token" "path/filepath" @@ -21,12 +22,12 @@ const ( func TestPrintAst(t *testing.T) { fset := token.NewFileSet() // positions are relative to fset - _, err := parser.ParseFile(fset, filepath.Join(testdataDir, "slices.go"), nil, 0) + f, err := parser.ParseFile(fset, filepath.Join(testdataDir, "funccall.go"), nil, 0) if err != nil { panic(err) } - // ast.Print(fset, f) + ast.Print(fset, f) } func runExtraction(t *testing.T, dir string, testExtractors ...extractors.Extractor) []result.Issue { diff --git a/goextractors/utils.go b/goextractors/utils.go index f066763..82b96f1 100644 --- a/goextractors/utils.go +++ b/goextractors/utils.go @@ -2,11 +2,18 @@ package goextractors import ( "go/ast" + "go/token" + "os" + "path/filepath" + + log "github.com/sirupsen/logrus" "github.com/vorlif/xspreak/extract/etype" "github.com/vorlif/xspreak/extract/extractors" ) +var workingDir, _ = os.Getwd() + func searchSelector(expr interface{}) *ast.SelectorExpr { switch v := expr.(type) { case *ast.SelectorExpr: @@ -43,6 +50,31 @@ type searchCollector struct { ExtraNodes []ast.Node } +func writeMissingMessageID(position token.Position, token etype.Token, text string) { + var typeName string + switch token { + case etype.Plural: + typeName = "plural" + case etype.Context: + typeName = "context" + case etype.Domain: + typeName = "domain" + default: + typeName = "unknown" + } + + filename := position.Filename + if relPath, err := filepath.Rel(workingDir, filename); err == nil { + filename = relPath + } + + if text != "" { + log.Warnf("%s:%d usage of %s without MessageID is not supported", filename, position.Line, typeName) + } else { + log.Warnf("%s:%d usage of %s without MessageID is not supported: %q", filename, position.Line, typeName, text) + } +} + func newSearchCollector() *searchCollector { return &searchCollector{ Singulars: make([]*extractors.SearchResult, 0), @@ -100,3 +132,23 @@ func (sc *searchCollector) GetNodes() []ast.Node { return nodes } + +func (sc *searchCollector) CheckMissingMessageID(extractCtx *extractors.Context) { + for _, sing := range sc.Singulars { + if sing.Raw != "" { + return + } + } + + for _, plural := range sc.Plurals { + writeMissingMessageID(extractCtx.GetPosition(plural.Node.Pos()), etype.Plural, plural.Raw) + } + + for _, ctx := range sc.Contexts { + writeMissingMessageID(extractCtx.GetPosition(ctx.Node.Pos()), etype.Plural, ctx.Raw) + } + + for _, domain := range sc.Domains { + writeMissingMessageID(extractCtx.GetPosition(domain.Node.Pos()), etype.Plural, domain.Raw) + } +} diff --git a/goextractors/varassign.go b/goextractors/varassign.go index 21a5fd1..8251c54 100644 --- a/goextractors/varassign.go +++ b/goextractors/varassign.go @@ -55,6 +55,8 @@ func (v varAssignExtractor) Run(_ context.Context, extractCtx *extractors.Contex issues = append(issues, issue) } + } else if token != etype.None { + writeMissingMessageID(extractCtx.GetPosition(node.Pos()), token, "") } return diff --git a/result/processors/comment_cleaner.go b/result/processors/comment_cleaner.go index 93b769d..6e11721 100644 --- a/result/processors/comment_cleaner.go +++ b/result/processors/comment_cleaner.go @@ -48,6 +48,8 @@ func (s commentCleaner) Process(inIssues []result.Issue) ([]result.Issue, error) // filter text for _, lines := range commentLines { isTranslatorComment := false + + var cleanedLines []string for _, line := range lines { line = strings.TrimSpace(line) if s.hasTranslatorPrefix(line) { @@ -62,9 +64,13 @@ func (s commentCleaner) Process(inIssues []result.Issue) ([]result.Issue, error) } if isTranslatorComment { - cleanedComments = append(cleanedComments, line) + cleanedLines = append(cleanedLines, line) } } + + if len(cleanedLines) > 0 { + cleanedComments = append(cleanedComments, strings.Join(cleanedLines, " ")) + } } iss.Comments = cleanedComments diff --git a/result/processors/unprintable_checker.go b/result/processors/unprintable_checker.go new file mode 100644 index 0000000..c8fd226 --- /dev/null +++ b/result/processors/unprintable_checker.go @@ -0,0 +1,46 @@ +package processors + +import ( + "go/token" + "os" + "path/filepath" + "strconv" + "unicode" + + log "github.com/sirupsen/logrus" + + "github.com/vorlif/xspreak/result" +) + +var workingDir, _ = os.Getwd() + +type unprintableChecker struct{} + +func NewUnprintableCheck() Processor { + return &unprintableChecker{} +} + +func (u unprintableChecker) Process(issues []result.Issue) ([]result.Issue, error) { + + for _, iss := range issues { + checkForUnprintableChars(iss.MsgID, iss.Pos) + checkForUnprintableChars(iss.PluralID, iss.Pos) + } + + return issues, nil +} + +func (u unprintableChecker) Name() string { return "unprintable check" } + +func checkForUnprintableChars(s string, pos token.Position) { + for _, r := range s { + if !(unicode.IsPrint(r) || unicode.IsSpace(r)) { + filename := pos.Filename + if relPath, err := filepath.Rel(workingDir, filename); err == nil { + filename = relPath + } + + log.Warnf("%s:%d internationalized messages should not contain the %s character", filename, pos.Line, strconv.QuoteRune(r)) + } + } +} diff --git a/testdata/project/funccall.go b/testdata/project/funccall.go index 77162e4..97297e1 100644 --- a/testdata/project/funccall.go +++ b/testdata/project/funccall.go @@ -58,6 +58,10 @@ func builtInFunctions() { panic(err) } + inlineFunc := func(inlineParam alias.Singular) {} + + inlineFunc("inline function") + t := spreak.NewLocalizer(bundle, "en") // TRANSLATORS: Test // multiline