diff --git a/Makefile b/Makefile index 530dd33950..c3a90ab2c7 100644 --- a/Makefile +++ b/Makefile @@ -51,6 +51,10 @@ test: $(MAKE) -C ./k6 go-test go test ./cmd/... ./pkg/... -timeout 1m30s +.PHONY: lint-translation-keys +lint-translation-keys: + go run ./devtools/gotemplatelinter --ignore-rule indentation --ignore-rule eol-at-eof ./resources/authgear/templates/en/web/authflowv2 + .PHONY: lint lint: golangci-lint run ./cmd/... ./pkg/... --timeout 7m @@ -66,7 +70,7 @@ lint: -go run ./devtools/importlinter worker api lib util >> .make-lint-expect 2>&1 -go run ./devtools/bandimportlinter ./pkg ./cmd >> .make-lint-expect 2>&1 git diff --exit-code .make-lint-expect > /dev/null 2>&1 - go run ./devtools/gotemplatelinter ./resources/authgear/templates/en/web/authflowv2 + go run ./devtools/gotemplatelinter --ignore-rule translation-key ./resources/authgear/templates/en/web/authflowv2 .PHONY: fmt fmt: diff --git a/devtools/gotemplatelinter/args_flags.go b/devtools/gotemplatelinter/args_flags.go new file mode 100644 index 0000000000..4f87a29013 --- /dev/null +++ b/devtools/gotemplatelinter/args_flags.go @@ -0,0 +1,39 @@ +package main + +import ( + "flag" + "strings" +) + +type ArgsFlags struct { + Paths []string + RulesToIgnore []string +} + +func ParseArgsFlags() ArgsFlags { + var rulesToIgnore RulesToIgnoreFlags + flag.Var(&rulesToIgnore, "ignore-rule", "rules to ignore, repeat this flag to ignore more rules") + + flag.Parse() + + paths := flag.Args() + + return ArgsFlags{ + Paths: paths, + RulesToIgnore: rulesToIgnore, + } +} + +type RulesToIgnoreFlags []string + +func (rs *RulesToIgnoreFlags) String() string { + if rs == nil { + return "" + } + return strings.Join(*rs, ",") +} + +func (rs *RulesToIgnoreFlags) Set(value string) error { + *rs = append(*rs, value) + return nil +} diff --git a/devtools/gotemplatelinter/final_newline_rule.go b/devtools/gotemplatelinter/final_newline_rule.go index c8e6012caf..3be71173e8 100644 --- a/devtools/gotemplatelinter/final_newline_rule.go +++ b/devtools/gotemplatelinter/final_newline_rule.go @@ -4,9 +4,11 @@ import ( "strings" ) -type FinalNewlineRule struct{} +type EOLAtEOFRule struct{} -func (r FinalNewlineRule) Check(content string, path string) LintViolations { +var _ Rule = EOLAtEOFRule{} + +func (r EOLAtEOFRule) Check(content string, path string) LintViolations { var violations LintViolations lines := strings.Split(content, "\n") if len(content) > 0 && !strings.HasSuffix(content, "\n") { @@ -21,3 +23,7 @@ func (r FinalNewlineRule) Check(content string, path string) LintViolations { } return violations } + +func (r EOLAtEOFRule) Key() string { + return "eol-at-eof" +} diff --git a/devtools/gotemplatelinter/final_newline_rule_test.go b/devtools/gotemplatelinter/final_newline_rule_test.go index c0ac10c30c..008c8b539f 100644 --- a/devtools/gotemplatelinter/final_newline_rule_test.go +++ b/devtools/gotemplatelinter/final_newline_rule_test.go @@ -6,10 +6,10 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -func TestFinalNewlineRule(t *testing.T) { +func TestEOLAtEOFRule(t *testing.T) { Convey("Given a empty file", t, func() { html := `` - violations := FinalNewlineRule{}.Check(html, "test.html") + violations := EOLAtEOFRule{}.Check(html, "test.html") Convey("Then I expect to see a lint violation", func() { So(len(violations), ShouldEqual, 0) }) @@ -17,7 +17,7 @@ func TestFinalNewlineRule(t *testing.T) { Convey("Given a file with a single line", t, func() { html := `Test

Test

` - violations := FinalNewlineRule{}.Check(html, "test.html") + violations := EOLAtEOFRule{}.Check(html, "test.html") Convey("Then I expect to see a lint violation", func() { So(len(violations), ShouldEqual, 1) So(violations[0].Path, ShouldEqual, "test.html") @@ -30,7 +30,7 @@ func TestFinalNewlineRule(t *testing.T) { Convey("Given a file with a single line and a newline", t, func() { html := `Test

Test

` - violations := FinalNewlineRule{}.Check(html, "test.html") + violations := EOLAtEOFRule{}.Check(html, "test.html") Convey("Then I expect to see no violation", func() { So(len(violations), ShouldEqual, 0) }) @@ -39,7 +39,7 @@ func TestFinalNewlineRule(t *testing.T) { Convey("Given a file with a single character and a newline", t, func() { html := `A ` - violations := FinalNewlineRule{}.Check(html, "test.html") + violations := EOLAtEOFRule{}.Check(html, "test.html") Convey("Then I expect to see no violation", func() { So(len(violations), ShouldEqual, 0) }) @@ -55,7 +55,7 @@ func TestFinalNewlineRule(t *testing.T) {

Test

` - violations := FinalNewlineRule{}.Check(html, "test.html") + violations := EOLAtEOFRule{}.Check(html, "test.html") Convey("Then I expect to see a lint violation", func() { So(len(violations), ShouldEqual, 1) So(violations[0].Path, ShouldEqual, "test.html") diff --git a/devtools/gotemplatelinter/indent_rule.go b/devtools/gotemplatelinter/indent_rule.go index 582008bffb..2271604889 100644 --- a/devtools/gotemplatelinter/indent_rule.go +++ b/devtools/gotemplatelinter/indent_rule.go @@ -7,6 +7,8 @@ import ( type IndentationRule struct{} +var _ Rule = IndentationRule{} + func (r IndentationRule) Check(content string, path string) LintViolations { var violations LintViolations lines := strings.Split(content, "\n") @@ -24,3 +26,7 @@ func (r IndentationRule) Check(content string, path string) LintViolations { } return violations } + +func (r IndentationRule) Key() string { + return "indentation" +} diff --git a/devtools/gotemplatelinter/main.go b/devtools/gotemplatelinter/main.go index fce8811a6f..f7e092c7cb 100644 --- a/devtools/gotemplatelinter/main.go +++ b/devtools/gotemplatelinter/main.go @@ -1,15 +1,20 @@ package main import ( + "cmp" "fmt" "io" "os" "path/filepath" + "slices" "strings" + + "github.com/authgear/authgear-server/pkg/util/slice" ) type Rule interface { Check(content string, path string) LintViolations + Key() string } type LintViolation struct { @@ -23,17 +28,9 @@ type LintViolations []LintViolation func (violations LintViolations) Error() string { var buf strings.Builder - violationsByPath := make(map[string]LintViolations) for _, v := range violations { - violationsByPath[v.Path] = append(violationsByPath[v.Path], v) + fmt.Fprintf(&buf, "%s:%d:%d: %s\n", v.Path, v.Line, v.Column, v.Message) } - - for _, violations := range violationsByPath { - for _, v := range violations { - fmt.Fprintf(&buf, "%s:%d:%d: %s\n", v.Path, v.Line, v.Column, v.Message) - } - } - return buf.String() } @@ -98,27 +95,68 @@ func (l *Linter) LintFile(path string, info os.FileInfo) (violations LintViolati return } +func constructRules(rulesToIgnore []string) []Rule { + indentationRule := IndentationRule{} + EOLAtEOFRule := EOLAtEOFRule{} + translationKeyRule := TranslationKeyRule{} + rules := []Rule{ + indentationRule, + EOLAtEOFRule, + translationKeyRule, + } + ignoreRuleFn := func(rule Rule) { + rules = slice.Filter[Rule](rules, func(r Rule) bool { + return r != rule + }) + } + for _, ruleToIgnore := range rulesToIgnore { + switch ruleToIgnore { + case indentationRule.Key(): + ignoreRuleFn(indentationRule) + case EOLAtEOFRule.Key(): + ignoreRuleFn(EOLAtEOFRule) + case translationKeyRule.Key(): + ignoreRuleFn(translationKeyRule) + } + } + + return rules +} + func doMain() (violations LintViolations, err error) { if len(os.Args) < 2 { - err = fmt.Errorf("usage: gotemplatelinter ") + err = fmt.Errorf("usage: gotemplatelinter --path --ignore-rule rule1ToIgnore --ignore-rule rule2ToIgnore") return } - path := os.Args[1] - linter := Linter{ - IgnorePatterns: []string{ - "__generated_asset.html", - }, - Rules: []Rule{ - IndentationRule{}, - FinalNewlineRule{}, - }, - Path: path, + argsFlags := ParseArgsFlags() + rules := constructRules(argsFlags.RulesToIgnore) + ignorePatterns := []string{ + "__generated_asset.html", } - violations, err = linter.Lint() - if err != nil { - return + linters := slice.Map(argsFlags.Paths, func(path string) Linter { + return Linter{ + IgnorePatterns: ignorePatterns, + Rules: rules, + Path: path, + } + }) + for _, linter := range linters { + newViolations, err := linter.Lint() + if err != nil { + return violations, err + } + violations = append(violations, newViolations...) } + slices.SortStableFunc(violations, func(a, b LintViolation) int { + return cmp.Or( + cmp.Compare(a.Path, b.Path), + cmp.Compare(a.Line, b.Line), + cmp.Compare(a.Column, b.Column), + cmp.Compare(a.Message, b.Message), + ) + }) + return } diff --git a/devtools/gotemplatelinter/transation_key_rule_check_include.go b/devtools/gotemplatelinter/transation_key_rule_check_include.go new file mode 100644 index 0000000000..360f60ac82 --- /dev/null +++ b/devtools/gotemplatelinter/transation_key_rule_check_include.go @@ -0,0 +1,37 @@ +package main + +import ( + "strings" + "text/template/parse" +) + +// validate `include` command +// +// e.g. (include "v2-error-screen-title" nil) +// e.g. (include (printf "v2-oauth-branding-%s" .provider_type) nil) +// e.g. (include $description_key nil) +func CheckCommandInclude(includeNode *parse.CommandNode) (err error) { + // 2nd arg should be translation key + for idx, arg := range includeNode.Args { + if idx == 1 { + if isHTMLInclude(arg) { // false positive + break + } + err = CheckTranslationKeyNode(arg) + if err != nil { + return err + } + } + + } + return +} + +func isHTMLInclude(arg parse.Node) bool { + if str, ok := arg.(*parse.StringNode); ok { + if strings.Contains(str.Text, ".html") { + return true + } + } + return false +} diff --git a/devtools/gotemplatelinter/transation_key_rule_check_template.go b/devtools/gotemplatelinter/transation_key_rule_check_template.go new file mode 100644 index 0000000000..7d80925f15 --- /dev/null +++ b/devtools/gotemplatelinter/transation_key_rule_check_template.go @@ -0,0 +1,27 @@ +package main + +import ( + "strings" + "text/template/parse" +) + +// validate `{{ template }}` nodes +// +// e.g. {{template "setup-totp-get-google-authenticator-description"}} +// e.g. {{template "setup-totp-raw-secret" (dict "secret" $.Secret)}} +// e.g. {{template "settings-totp-item-description" (dict "time" .CreatedAt "rfc3339" (rfc3339 .CreatedAt))}} +func CheckTemplate(templateNode *parse.TemplateNode) (err error) { + if isHTMLTemplate(templateNode) { // false positive + return + } + translationKey := templateNode.Name + err = CheckTranslationKeyPattern(translationKey) + if err != nil { + return err + } + return +} + +func isHTMLTemplate(templateNode *parse.TemplateNode) bool { + return strings.Contains(templateNode.Name, ".html") +} diff --git a/devtools/gotemplatelinter/transation_key_rule_check_translate.go b/devtools/gotemplatelinter/transation_key_rule_check_translate.go new file mode 100644 index 0000000000..6cc3149561 --- /dev/null +++ b/devtools/gotemplatelinter/transation_key_rule_check_translate.go @@ -0,0 +1,22 @@ +package main + +import ( + "text/template/parse" +) + +// validate `translate` command +// +// e.g. (translate "app.name" nil) +func CheckCommandTranslate(translateNode *parse.CommandNode) (err error) { + // 2nd arg should be translation key + for idx, arg := range translateNode.Args { + if idx == 1 { + err = CheckTranslationKeyNode(arg) + if err != nil { + return err + } + } + + } + return +} diff --git a/devtools/gotemplatelinter/transation_key_rule_check_translations_render_text.go b/devtools/gotemplatelinter/transation_key_rule_check_translations_render_text.go new file mode 100644 index 0000000000..810c3c3cd8 --- /dev/null +++ b/devtools/gotemplatelinter/transation_key_rule_check_translations_render_text.go @@ -0,0 +1,23 @@ +package main + +import ( + "text/template/parse" +) + +// validate commands with variable `$.Translations.RenderText` or field `.Translations.RenderText` +// +// example: `($.Translations.RenderText "customer-support-link" nil)` +// example: (.Translations.RenderText "terms-of-service-link" nil) +func CheckCommandTranslationsRenderText(node *parse.CommandNode) (err error) { + // 2nd arg should be translation key + for idx, arg := range node.Args { + if idx == 1 { + err = CheckTranslationKeyNode(arg) + if err != nil { + return err + } + } + + } + return +} diff --git a/devtools/gotemplatelinter/translation_key_rule.go b/devtools/gotemplatelinter/translation_key_rule.go new file mode 100644 index 0000000000..b5c38850b8 --- /dev/null +++ b/devtools/gotemplatelinter/translation_key_rule.go @@ -0,0 +1,104 @@ +package main + +import ( + htmltemplate "html/template" + "text/template/parse" + + authgearutiltemplate "github.com/authgear/authgear-server/pkg/util/template" +) + +type TranslationKeyRule struct{} + +var _ Rule = TranslationKeyRule{} + +func (r TranslationKeyRule) Check(content string, path string) LintViolations { + return r.check(content, path) +} + +func (r TranslationKeyRule) Key() string { + return "translation-key" +} + +func (r TranslationKeyRule) check(content string, path string) LintViolations { + t := r.makeTemplate(content) + + violations := r.validateHTMLTemplate(t, path) + return violations +} + +func (r TranslationKeyRule) makeTemplate(content string) *htmltemplate.Template { + t := htmltemplate.New("") + funcMap := authgearutiltemplate.MakeTemplateFuncMap(t) + t.Funcs(funcMap) + parsed := htmltemplate.Must(t.Parse(content)) + return parsed +} + +func (r TranslationKeyRule) validateHTMLTemplate(template *htmltemplate.Template, path string) LintViolations { + tpls := template.Templates() + + var violations LintViolations + + for _, tpl := range tpls { + if tpl.Tree == nil { + continue + } + if tplViolations := validateTree(tpl.Tree, path); len(tplViolations) != 0 { + violations = append(violations, tplViolations...) + } + } + return violations +} + +func validateTree(tree *parse.Tree, path string) LintViolations { + var violations LintViolations + var err error + + handleNodeErrFn := func(n parse.Node, _err error) { + if _err == nil { + return + } + line, col, _ := TreeErrorContext(tree, n) + violations = append(violations, LintViolation{ + Line: line, + Column: col, + Path: path, + Message: _err.Error(), + }) + } + + validateFn := func(n parse.Node, depth int) (cont bool) { + switch n := n.(type) { + case *parse.CommandNode: + for _, arg := range n.Args { + if variable, ok := arg.(*parse.VariableNode); ok && variable.String() == "$.Translations.RenderText" { + err = CheckCommandTranslationsRenderText(n) + handleNodeErrFn(n, err) + } + if field, ok := arg.(*parse.FieldNode); ok && field.String() == ".Translations.RenderText" { + err = CheckCommandTranslationsRenderText(n) + handleNodeErrFn(n, err) + } + if ident, ok := arg.(*parse.IdentifierNode); ok { + switch ident.String() { + case "include": + err = CheckCommandInclude(n) + handleNodeErrFn(n, err) + case "translate": + err = CheckCommandTranslate(n) + handleNodeErrFn(n, err) + } + } + } + case *parse.TemplateNode: + err = CheckTemplate(n) + handleNodeErrFn(n, err) + } + + // always continue to traverse + return true + } + + TraverseTree(tree, validateFn) + return violations +} diff --git a/devtools/gotemplatelinter/translation_key_rule_check_translation_key.go b/devtools/gotemplatelinter/translation_key_rule_check_translation_key.go new file mode 100644 index 0000000000..ef05c5a073 --- /dev/null +++ b/devtools/gotemplatelinter/translation_key_rule_check_translation_key.go @@ -0,0 +1,91 @@ +package main + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "regexp" + "strings" + "text/template/parse" +) + +// v2.page... +// v2.component... +const TranslationKeyPattern = `^(v2)\.(page|component)\.([-a-z]+)\.([-a-z]+)\.([-a-z]+)$` +const ErrTranslationKeyPattern = `^(v2)\.(error)\.([-a-z]+)$` +const enTranslationJSONPath = "resources/authgear/templates/en/translation.json" + +var validKey *regexp.Regexp +var validErrKey *regexp.Regexp +var translationKeys []string + +func init() { + validKey = regexp.MustCompile(TranslationKeyPattern) + validErrKey = regexp.MustCompile(ErrTranslationKeyPattern) + translationKeys = getEnJSONTranslationKeys() +} + +func CheckTranslationKeyNode(translationKeyNode parse.Node) (err error) { + switch translationKeyNode.Type() { + case parse.NodeString: + return CheckTranslationKeyPattern(translationKeyNode.(*parse.StringNode).Text) + case parse.NodeVariable: + // FIXME: support variable, like $label, $label_key, $variant_label_key + fallthrough + case parse.NodePipe: + // FIXME: support pipe, like (printf "territory-%s" $.AddressCountry) + fallthrough + default: + return fmt.Errorf("invalid translation key: \"%v\"", translationKeyNode.String()) + } +} + +func CheckTranslationKeyPattern(translationKey string) (err error) { + key := strings.Trim(translationKey, "\"") + + if key == "" { + return fmt.Errorf("translation key is empty") + } + + if !isTranslationKeyDefined(key) { + return fmt.Errorf("translation key not defined: \"%v\"", key) + } + + if !validKey.MatchString(key) && !validErrKey.MatchString(key) { + return fmt.Errorf("invalid translation key: \"%v\"", key) + } + + return +} + +func isTranslationKeyDefined(targetKey string) bool { + for _, key := range translationKeys { + if key == targetKey { + return true + } + } + return false +} + +func getEnJSONTranslationKeys() []string { + bytes, err := ioutil.ReadFile(enTranslationJSONPath) + if err != nil { + panic(fmt.Errorf("failed to read %v: %w", enTranslationJSONPath, err)) + } + + var translationData map[string]string + + err = json.Unmarshal(bytes, &translationData) + if err != nil { + panic(fmt.Errorf("failed to unmarshal %v: %w", enTranslationJSONPath, err)) + } + + keys := make([]string, len(translationData)) + i := 0 + for key := range translationData { + keys[i] = key + i++ + } + return keys + +} diff --git a/devtools/gotemplatelinter/translation_key_rule_traverse_tree.go b/devtools/gotemplatelinter/translation_key_rule_traverse_tree.go new file mode 100644 index 0000000000..c483ec5fcc --- /dev/null +++ b/devtools/gotemplatelinter/translation_key_rule_traverse_tree.go @@ -0,0 +1,91 @@ +// This file is mostly copied from pkg/util/template/validation.go +package main + +import ( + "strconv" + "strings" + "text/template/parse" +) + +func TraverseTree(tree *parse.Tree, fn func(n parse.Node, depth int) (cont bool)) { + traverseTreeVisit(tree.Root, 0, fn) +} + +func traverseTreeVisitBranch(n *parse.BranchNode, depth int, fn func(n parse.Node, depth int) (cont bool)) (cont bool) { + if cont = traverseTreeVisit(n.Pipe, depth, fn); !cont { + return + } + if cont = traverseTreeVisit(n.List, depth, fn); !cont { + return + } + if n.ElseList != nil { + if cont = traverseTreeVisit(n.ElseList, depth, fn); !cont { + return false + } + } + return +} + +func traverseTreeVisit(n parse.Node, depth int, fn func(n parse.Node, depth int) (cont bool)) (cont bool) { + cont = fn(n, depth) + if !cont { + return + } + + switch n := n.(type) { + case *parse.PipeNode: + for _, cmd := range n.Cmds { + if cont = traverseTreeVisit(cmd, depth, fn); !cont { + break + } + } + case *parse.CommandNode: + for _, arg := range n.Args { + if pipe, ok := arg.(*parse.PipeNode); ok { + if cont = traverseTreeVisit(pipe, depth+1, fn); !cont { + break + } + } + } + case *parse.ActionNode: + cont = traverseTreeVisit(n.Pipe, depth, fn) + case *parse.TemplateNode: + if n.Pipe != nil { + if cont = traverseTreeVisit(n.Pipe, depth+1, fn); !cont { + break + } + } + case *parse.TextNode: + break + case *parse.IfNode: + cont = traverseTreeVisitBranch(&n.BranchNode, depth, fn) + case *parse.RangeNode: + cont = traverseTreeVisitBranch(&n.BranchNode, depth, fn) + case *parse.WithNode: + cont = traverseTreeVisitBranch(&n.BranchNode, depth, fn) + case *parse.ListNode: + for _, n := range n.Nodes { + if cont = traverseTreeVisit(n, depth+1, fn); !cont { + break + } + } + } + + return +} + +func TreeErrorContext(tree *parse.Tree, n parse.Node) (line int, col int, ctx string) { + location, ctx := tree.ErrorContext(n) + locationInfo := strings.Split(location, ":") // location is of %s:%d:%d + + line, err := strconv.Atoi(locationInfo[1]) + if err != nil { + line = 0 + } + col, err = strconv.Atoi(locationInfo[2]) + if err != nil { + col = 0 + } + + return +}