Merge pull request #60 from Aerex/fix/ignore-regexp-checkup-variable

README.md

@@ -5,7 +5,7 @@ i18n Tooling for the Go Language [![Build Status](https://travis-ci.org/maximili
 
 This is a general purpose internationalization (i18n) tooling for Go language (Golang) programs. It allows you to prepare Go language code for internationalization and localization (l10n). You can also use it to help maintain the resulting i18n-enabled Golang code so that it remains internationalized. This tool was extracted while we worked on enabling the [Cloud Foundry CLI](https://github.com/cloudfoundry/cli) with i18n support.
 
-This tool is licensed under the [Apache 2.0 OSS license](https://github.com/maximilien/i18n4go/blob/master/LICENSE). We'd love to hear from you if you are using, attempting to use, or planning to use this tool. 
+This tool is licensed under the [Apache 2.0 OSS license](https://github.com/maximilien/i18n4go/blob/master/LICENSE). We'd love to hear from you if you are using, attempting to use, or planning to use this tool.
 
 Two additional ways, besides Gitter or Slack chat above, to contact us:
 
@@ -453,6 +453,8 @@ The general usage for `-c checkup` command is:
 
   -q                    the qualifier to use when calling the T(...), defaults to empty but can be used to set to something like i18n for example, such that, i18n.T(...) is used for T(...) function
 
+  --ignore-regexp       [optional] a perl-style regular expression for files to ignore, e.g., ".*test.*"
+
 ```
 
 The `checkup` command ensures that the strings in code match strings in resource files and vice versa.

cmds/checkup.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -19,12 +20,14 @@ type Checkup struct {
 	options common.Options
 
 	I18nStringInfos []common.I18nStringInfo
+	IgnoreRegexp    *regexp.Regexp
 }
 
 func NewCheckup(options common.Options) Checkup {
 	return Checkup{
 		options:         options,
 		I18nStringInfos: []common.I18nStringInfo{},
+		IgnoreRegexp:    common.GetIgnoreRegexp(options.IgnoreRegexpFlag),
 	}
 }
 
@@ -57,7 +60,7 @@ func (cu *Checkup) Run() error {
 		return err
 	}
 
-	locales := findTranslationFiles(".")
+	locales := findTranslationFiles(".", cu.IgnoreRegexp, false)
 
 	englishFiles := locales["en_US"]
 	if englishFiles == nil {
@@ -114,7 +117,91 @@ func getGoFiles(dir string) (files []string) {
 	return
 }
 
+func (cu *Checkup) inspectAssignStmt(stmtMap map[string][]ast.AssignStmt, node *ast.AssignStmt) {
+	// use a hashmap for defined variables to a list of reassigned variables sharing the same var name
+	if assignStmt, okIdent := node.Lhs[0].(*ast.Ident); okIdent {
+		varName := assignStmt.Name
+		if node.Tok == token.DEFINE {
+			stmtMap[varName] = []ast.AssignStmt{}
+		} else if node.Tok == token.ASSIGN {
+			if _, exists := stmtMap[varName]; exists {
+				stmtMap[varName] = append(stmtMap[varName], *node)
+			}
+		}
+	}
+}
+
+func (cu *Checkup) inspectStmt(translatedStrings []string, stmtMap map[string][]ast.AssignStmt, node ast.AssignStmt) []string {
+	if strStmtArg, ok := node.Rhs[0].(*ast.BasicLit); ok {
+		varName := node.Lhs[0].(*ast.Ident).Name
+		translatedString, err := strconv.Unquote(strStmtArg.Value)
+		if err != nil {
+			panic(err.Error())
+		}
+		translatedStrings = append(translatedStrings, translatedString)
+		// apply all translation ids from reassigned variables
+		if _, exists := stmtMap[varName]; exists {
+			for _, assignStmt := range stmtMap[varName] {
+				strVarVal := assignStmt.Rhs[0].(*ast.BasicLit).Value
+				translatedString, err := strconv.Unquote(strVarVal)
+				if err != nil {
+					panic(err.Error())
+				}
+				translatedStrings = append(translatedStrings, translatedString)
+
+			}
+		}
+	}
+
+	return translatedStrings
+}
+
+func (cu *Checkup) inspectTFunc(translatedStrings []string, stmtMap map[string][]ast.AssignStmt, node ast.CallExpr) []string {
+	if stringArg, ok := node.Args[0].(*ast.BasicLit); ok {
+		translatedString, err := strconv.Unquote(stringArg.Value)
+		if err != nil {
+			panic(err.Error())
+		}
+		translatedStrings = append(translatedStrings, translatedString)
+	}
+	if idt, okIdt := node.Args[0].(*ast.Ident); okIdt {
+		if obj := idt.Obj; obj != nil {
+			if stmtArg, okStmt := obj.Decl.(*ast.AssignStmt); okStmt {
+				translatedStrings = cu.inspectStmt(translatedStrings, stmtMap, *stmtArg)
+			}
+		}
+	}
+
+	return translatedStrings
+}
+
+func (cu *Checkup) inspectCallExpr(translatedStrings []string, stmtMap map[string][]ast.AssignStmt, node *ast.CallExpr) []string {
+	switch node.Fun.(type) {
+	case *ast.Ident:
+		funName := node.Fun.(*ast.Ident).Name
+		// inspect any T() or t() method calls
+		if funName == "T" || funName == "t" {
+			translatedStrings = cu.inspectTFunc(translatedStrings, stmtMap, *node)
+		}
+
+	case *ast.SelectorExpr:
+		expr := node.Fun.(*ast.SelectorExpr)
+		if ident, ok := expr.X.(*ast.Ident); ok {
+			funName := expr.Sel.Name
+			// inspect any <MODULE>.T() or <MODULE>.t() method calls (eg. i18n.T())
+			if ident.Name == cu.options.QualifierFlag && (funName == "T" || funName == "t") {
+				translatedStrings = cu.inspectTFunc(translatedStrings, stmtMap, *node)
+			}
+		}
+	default:
+		//Skip!
+	}
+
+	return translatedStrings
+}
+
 func (cu *Checkup) inspectFile(file string) (translatedStrings []string, err error) {
+	defineAssignStmtMap := make(map[string][]ast.AssignStmt)
 	fset := token.NewFileSet()
 	astFile, err := parser.ParseFile(fset, file, nil, parser.AllErrors)
 	if err != nil {
@@ -124,37 +211,18 @@ func (cu *Checkup) inspectFile(file string) (translatedStrings []string, err err
 
 	ast.Inspect(astFile, func(n ast.Node) bool {
 		switch x := n.(type) {
+		case *ast.AssignStmt:
+			// inspect any potential translation string in defined / assigned statement nodes
+			// add node to map if variable contains a translation string
+			// eg: translation := "Hello {{.FirstName}}"
+			//     T(translation)
+			//     translation = "Hello {{.LastName}}"
+			//     T(translation)
+			cu.inspectAssignStmt(defineAssignStmtMap, x)
 		case *ast.CallExpr:
-			switch x.Fun.(type) {
-			case *ast.Ident:
-				funName := x.Fun.(*ast.Ident).Name
-
-				if funName == "T" || funName == "t" {
-					if stringArg, ok := x.Args[0].(*ast.BasicLit); ok {
-						translatedString, err := strconv.Unquote(stringArg.Value)
-						if err != nil {
-							panic(err.Error())
-						}
-						translatedStrings = append(translatedStrings, translatedString)
-					}
-				}
-			case *ast.SelectorExpr:
-				expr := x.Fun.(*ast.SelectorExpr)
-				if ident, ok := expr.X.(*ast.Ident); ok {
-					funName := expr.Sel.Name
-					if ident.Name == cu.options.QualifierFlag && (funName == "T" || funName == "t") {
-						if stringArg, ok := x.Args[0].(*ast.BasicLit); ok {
-							translatedString, err := strconv.Unquote(stringArg.Value)
-							if err != nil {
-								panic(err.Error())
-							}
-							translatedStrings = append(translatedStrings, translatedString)
-						}
-					}
-				}
-			default:
-				//Skip!
-			}
+			// inspect any T()/t() or <MODULE>.T()/<MODULE>.t() (eg. i18n.T()) method calls using map
+			/// then retrieve a list of translation strings that were passed into method
+			translatedStrings = cu.inspectCallExpr(translatedStrings, defineAssignStmtMap, x)
 		}
 		return true
 	})
@@ -209,7 +277,7 @@ func getI18nFile(locale, dir string) (filePath string) {
 	return
 }
 
-func findTranslationFiles(dir string) (locales map[string][]string) {
+func findTranslationFiles(dir string, ignoreRegexp *regexp.Regexp, verbose bool) (locales map[string][]string) {
 	locales = make(map[string][]string)
 	contents, _ := ioutil.ReadDir(dir)
 
@@ -222,19 +290,30 @@ func findTranslationFiles(dir string) (locales map[string][]string) {
 				var locale string
 
 				for _, part := range parts {
-					if !strings.Contains(part, "json") && !strings.Contains(part, "all") {
+					invalidLangRegexp, _ := regexp.Compile("excluded|json|all")
+					if !invalidLangRegexp.MatchString(part) {
 						locale = part
 					}
 				}
 
+				// No locale found so skipping
+				if locale == "" {
+					continue
+				}
+
 				if locales[locale] == nil {
 					locales[locale] = []string{}
 				}
 
 				locales[locale] = append(locales[locale], filepath.Join(dir, fileInfo.Name()))
 			}
 		} else {
-			for locale, files := range findTranslationFiles(filepath.Join(dir, fileInfo.Name())) {
+			if ignoreRegexp != nil {
+				if ignoreRegexp.MatchString(filepath.Join(dir, fileInfo.Name())) {
+					continue
+				}
+			}
+			for locale, files := range findTranslationFiles(filepath.Join(dir, fileInfo.Name()), ignoreRegexp, verbose) {
 				if locales[locale] == nil {
 					locales[locale] = []string{}
 				}

cmds/extract_strings.go

@@ -44,14 +44,6 @@ type extractStrings struct {
 }
 
 func NewExtractStrings(options common.Options) extractStrings {
-	var compiledRegexp *regexp.Regexp
-	if options.IgnoreRegexpFlag != "" {
-		compiledReg, err := regexp.Compile(options.IgnoreRegexpFlag)
-		if err != nil {
-			fmt.Println("WARNING compiling ignore-regexp:", err)
-		}
-		compiledRegexp = compiledReg
-	}
 
 	return extractStrings{options: options,
 		Filename:         "extracted_strings.json",
@@ -63,7 +55,7 @@ func NewExtractStrings(options common.Options) extractStrings {
 		TotalStringsDir:  0,
 		TotalStrings:     0,
 		TotalFiles:       0,
-		IgnoreRegexp:     compiledRegexp,
+		IgnoreRegexp:     common.GetIgnoreRegexp(options.IgnoreRegexpFlag),
 	}
 }
 

cmds/fixup.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -24,12 +25,14 @@ type Fixup struct {
 	English         []common.I18nStringInfo
 	Source          map[string]int
 	Locales         map[string]map[string]string
+	IgnoreRegexp    *regexp.Regexp
 }
 
 func NewFixup(options common.Options) Fixup {
 	return Fixup{
 		options:         options,
 		I18nStringInfos: []common.I18nStringInfo{},
+		IgnoreRegexp:    common.GetIgnoreRegexp(options.IgnoreRegexpFlag),
 	}
 }
 
@@ -63,7 +66,7 @@ func (fix *Fixup) Run() error {
 		return err
 	}
 
-	locales := findTranslationFiles(".")
+	locales := findTranslationFiles(".", fix.IgnoreRegexp, fix.options.VerboseFlag)
 	englishFiles, ok := locales["en_US"]
 	if !ok {
 		fmt.Println("Unable to find english translation files")

common/common.go

@@ -360,3 +360,14 @@ func getInterpolatedStringRegexp() (*regexp.Regexp, error) {
 
 	return interpolatedStringRegexp, err
 }
+
+func GetIgnoreRegexp(ignoreRegexp string) (compiledRegexp *regexp.Regexp) {
+	if ignoreRegexp != "" {
+		reg, err := regexp.Compile(ignoreRegexp)
+		if err != nil {
+			fmt.Println("WARNING: fail to compile ignore-regexp:", err)
+		}
+		compiledRegexp = reg
+	}
+	return
+}

integration/checkup/checkup_test.go

@@ -118,4 +118,22 @@ var _ = Describe("checkup", func() {
 			Ω(session.ExitCode()).Should(Equal(1))
 		})
 	})
+
+	Context("When translation IDs are (re)assigned to variables", func() {
+		BeforeEach(func() {
+			fixturesPath = filepath.Join("..", "..", "test_fixtures", "checkup", "variable")
+			err = os.Chdir(fixturesPath)
+			Ω(err).ToNot(HaveOccurred(), "Could not change to fixtures directory")
+
+			session = Runi18n("-c", "checkup", "-v")
+		})
+
+		It("returns 0", func() {
+			Ω(session.ExitCode()).Should(Equal(0))
+		})
+
+		It("prints a reassuring message", func() {
+			Ω(session).Should(Say("OK"))
+		})
+	})
 })

test_fixtures/checkup/variable/src/code/main.go

@@ -0,0 +1,12 @@
+package code
+
+import "fmt"
+
+func main() {
+	locale := "Translated hello world!"
+	fmt.Println(T(locale))
+	locale = "For you and for me"
+	fmt.Println(T(locale))
+	locale = "I like bananas"
+	fmt.Println(T(locale))
+}

test_fixtures/checkup/variable/translations/en_US.all.json

@@ -0,0 +1,14 @@
+[
+  {
+    "id": "Translated hello world!",
+    "translation": "Translated hello world!"
+  },
+  {
+    "id": "For you and for me",
+    "translation": "For you and for me"
+  },
+  {
+    "id": "I like bananas",
+    "translation": "I like bananas"
+  }
+]

test_fixtures/checkup/variable/translations/zh_CN.all.json

@@ -0,0 +1,14 @@
+[
+  {
+    "id": "Translated hello world!",
+    "translation": "你好世界!"
+  },
+   {
+      "id": "For you and for me",
+      "translation": "为你，为我"
+   },
+  {
+    "id": "I like bananas",
+    "translation": "我喜欢吃香蕉"
+  }
+]