From 4903a25c8c2c6ba62d6aa70209403e68681cb291 Mon Sep 17 00:00:00 2001 From: vr-ibm Date: Thu, 23 Jul 2026 14:27:01 -0500 Subject: [PATCH 1/4] tools/template-check: add func-check subcommand and core logic --- tools/template-check/cmd/funccheck.go | 59 ++++++++ tools/template-check/cmd/root.go | 1 + tools/template-check/gotemplate/funccheck.go | 136 ++++++++++++++++++ .../gotemplate/funccheck_test.go | 40 ++++++ 4 files changed, 236 insertions(+) create mode 100644 tools/template-check/cmd/funccheck.go create mode 100644 tools/template-check/gotemplate/funccheck.go create mode 100644 tools/template-check/gotemplate/funccheck_test.go diff --git a/tools/template-check/cmd/funccheck.go b/tools/template-check/cmd/funccheck.go new file mode 100644 index 000000000000..7a0ecc79b422 --- /dev/null +++ b/tools/template-check/cmd/funccheck.go @@ -0,0 +1,59 @@ +package cmd + +import ( + "fmt" + "io" + "os" + + "github.com/GoogleCloudPlatform/magic-modules/tools/template-check/gotemplate" + "github.com/spf13/cobra" +) + +const funcCheckDesc = `Check template files for invalid function calls` + +type funcCheckOptions struct { + rootOptions *rootOptions + stdout io.Writer + fileList []string +} + +func newFuncCheckCmd(rootOptions *rootOptions) *cobra.Command { + o := &funcCheckOptions{ + rootOptions: rootOptions, + stdout: os.Stdout, + } + command := &cobra.Command{ + Use: "func-check", + Short: funcCheckDesc, + Long: funcCheckDesc, + RunE: func(c *cobra.Command, args []string) error { + return o.run() + }, + } + command.Flags().StringSliceVar(&o.fileList, "file-list", []string{}, "file list to check") + return command +} + +func (o *funcCheckOptions) run() error { + if len(o.fileList) == 0 { + return nil + } + foundInvalidFuncs := false + for _, fileName := range o.fileList { + results, err := gotemplate.CheckInvalidFuncsForFile(fileName) + if err != nil { + return err + } + if len(results) > 0 { + fmt.Fprintf(os.Stderr, "%s:\n", fileName) + foundInvalidFuncs = true + for _, result := range results { + fmt.Fprintf(os.Stderr, " %s\n", result) + } + } + } + if foundInvalidFuncs { + return fmt.Errorf("found invalid template function calls") + } + return nil +} diff --git a/tools/template-check/cmd/root.go b/tools/template-check/cmd/root.go index 3b68327ed532..9268fb33fee1 100644 --- a/tools/template-check/cmd/root.go +++ b/tools/template-check/cmd/root.go @@ -23,6 +23,7 @@ func newRootCmd() (*cobra.Command, *rootOptions, error) { } cmd.AddCommand(newversionGuardCmd(o)) cmd.AddCommand(newUnusedTmplCmd(o)) + cmd.AddCommand(newFuncCheckCmd(o)) return cmd, o, nil } diff --git a/tools/template-check/gotemplate/funccheck.go b/tools/template-check/gotemplate/funccheck.go new file mode 100644 index 000000000000..9084a674c6f9 --- /dev/null +++ b/tools/template-check/gotemplate/funccheck.go @@ -0,0 +1,136 @@ +package gotemplate + +import ( + "bufio" + "fmt" + "os" + "regexp" + "strings" +) + +// actionRegex extracts template actions: everything between {{ and }} +var actionRegex = regexp.MustCompile(`\{\{-?\s*(.*?)\s*-?\}\}`) + +// identifierRegex matches a standalone identifier (function name) at the start of a pipeline segment +var identifierRegex = regexp.MustCompile(`^([a-zA-Z_][a-zA-Z0-9_]*)`) + +// ValidFuncs is the registry of allowed template functions and keywords. +var ValidFuncs = map[string]bool{ + // Go built-in template functions + "and": true, "call": true, "eq": true, "ge": true, "gt": true, "html": true, + "index": true, "js": true, "le": true, "len": true, "lt": true, "ne": true, + "not": true, "or": true, "print": true, "printf": true, "println": true, + "slice": true, "urlquery": true, + + // Go template keywords + "if": true, "else": true, "end": true, "range": true, "with": true, + "block": true, "define": true, "template": true, "nil": true, + + // mmv1 registered functions (google/template_utils.go) + "title": true, "replace": true, "replaceAll": true, "camelize": true, + "underscore": true, "plural": true, "contains": true, "join": true, + "lower": true, "upper": true, "hasSuffix": true, "dict": true, + "format2regex": true, "hasPrefix": true, "sub": true, "plus": true, + "firstSentence": true, "trimTemplate": true, "customTemplate": true, + + // mmv1 registered functions (provider/template_data.go) + "TemplatePath": true, +} + +// CheckInvalidFuncsForFile scans a file for invalid template function calls. +func CheckInvalidFuncsForFile(filename string) ([]string, error) { + file, err := os.Open(filename) + if err != nil { + return nil, err + } + defer file.Close() + + var results []string + scanner := bufio.NewScanner(file) + lineNum := 0 + + for scanner.Scan() { + lineNum++ + line := scanner.Text() + actions := actionRegex.FindAllStringSubmatch(line, -1) + for _, action := range actions { + body := strings.TrimSpace(action[1]) + if body == "" { + continue + } + + segments := strings.Split(body, "|") + for _, seg := range segments { + seg = strings.TrimSpace(seg) + if seg == "" || strings.HasPrefix(seg, ".") || strings.HasPrefix(seg, "$") || strings.HasPrefix(seg, "else if") { + continue + } + + match := identifierRegex.FindStringSubmatch(seg) + if match == nil { + continue + } + + funcName := match[1] + if funcName == "true" || funcName == "false" || ValidFuncs[funcName] { + continue + } + + // Skip bare lowercase identifiers (likely Terraform interpolation) + if seg == funcName && !hasUpperCase(funcName) { + continue + } + + // Skip if it appears in a string literal + if appearsInStringLiteral(body, funcName) { + continue + } + + results = append(results, fmt.Sprintf("unknown function %q in action {{%s}} (line %d)", funcName, body, lineNum)) + } + } + } + return results, scanner.Err() +} + +func hasUpperCase(s string) bool { + for _, ch := range s { + if ch >= 'A' && ch <= 'Z' { + return true + } + } + return false +} + +func appearsInStringLiteral(body string, identifier string) bool { + inString, escaped := false, false + var current strings.Builder + for _, ch := range body { + if escaped { + if inString { + current.WriteRune(ch) + } + escaped = false + continue + } + if ch == '\\' { + escaped = true + if inString { + current.WriteRune(ch) + } + continue + } + if ch == '"' { + if inString && strings.Contains(current.String(), identifier) { + return true + } + current.Reset() + inString = !inString + continue + } + if inString { + current.WriteRune(ch) + } + } + return false +} diff --git a/tools/template-check/gotemplate/funccheck_test.go b/tools/template-check/gotemplate/funccheck_test.go new file mode 100644 index 000000000000..2f9912343383 --- /dev/null +++ b/tools/template-check/gotemplate/funccheck_test.go @@ -0,0 +1,40 @@ +package gotemplate + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func createTestFile(t *testing.T, content string) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "test.go.tmpl") + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + return path +} + +func TestFuncCheck_Basic(t *testing.T) { + // Test that it catches an invalid function + path := createTestFile(t, `{{BigQueryBasePath}}`) + results, err := CheckInvalidFuncsForFile(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(results) == 0 || !strings.Contains(results[0], "BigQueryBasePath") { + t.Errorf("expected error for BigQueryBasePath, got: %v", results) + } + + // Test that it passes a valid function + path2 := createTestFile(t, `{{camelize .Name}}`) + results2, err := CheckInvalidFuncsForFile(path2) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(results2) != 0 { + t.Errorf("expected no errors for camelize, got: %v", results2) + } +} From 317a0f5206153d46318b1c9cd083995ff4ef8aae Mon Sep 17 00:00:00 2001 From: vr-ibm Date: Thu, 23 Jul 2026 16:02:53 -0500 Subject: [PATCH 2/4] tools/template-check: add comprehensive unit tests for valid/invalid functions --- .../gotemplate/funccheck_test.go | 59 ++++++++++++++----- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/tools/template-check/gotemplate/funccheck_test.go b/tools/template-check/gotemplate/funccheck_test.go index 2f9912343383..b9446f380906 100644 --- a/tools/template-check/gotemplate/funccheck_test.go +++ b/tools/template-check/gotemplate/funccheck_test.go @@ -17,24 +17,53 @@ func createTestFile(t *testing.T, content string) string { return path } -func TestFuncCheck_Basic(t *testing.T) { - // Test that it catches an invalid function - path := createTestFile(t, `{{BigQueryBasePath}}`) - results, err := CheckInvalidFuncsForFile(path) - if err != nil { - t.Fatalf("unexpected error: %v", err) +func TestFuncCheck_ValidFunctions(t *testing.T) { + tests := []struct { + name string + content string + }{ + {"mmv1_custom", `{{camelize .Name}} {{underscore .Name}} {{title .Name}}`}, + {"go_builtins", `{{len .Items}} {{and .A .B}} {{index .Map "key"}}`}, + {"keywords", `{{if .A}} {{else}} {{end}} {{range .Items}} {{with .X}}`}, + {"dot_access", `{{.Resource.Name}} {{.Name}}`}, + {"variables", `{{$name := .Name}} {{$name}}`}, + {"string_literals", `{{print "hello"}} {{printf "%s" .Name}}`}, + {"complex_mmv1", `{{plural .Name}} {{contains .A .B}} {{join .List ","}}`}, + {"provider_funcs", `{{TemplatePath "compute"}}`}, } - if len(results) == 0 || !strings.Contains(results[0], "BigQueryBasePath") { - t.Errorf("expected error for BigQueryBasePath, got: %v", results) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := createTestFile(t, tt.content) + results, err := CheckInvalidFuncsForFile(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(results) != 0 { + t.Errorf("expected no errors, got: %v", results) + } + }) } +} - // Test that it passes a valid function - path2 := createTestFile(t, `{{camelize .Name}}`) - results2, err := CheckInvalidFuncsForFile(path2) - if err != nil { - t.Fatalf("unexpected error: %v", err) +func TestFuncCheck_InvalidFunctions(t *testing.T) { + tests := []struct { + name string + content string + expected string + }{ + {"typo", `{{camelCase .Name}}`, "camelCase"}, + {"missing_mmv1", `{{toLower .Name}}`, "toLower"}, + {"constant_as_func", `{{BigQueryBasePath}}`, "BigQueryBasePath"}, } - if len(results2) != 0 { - t.Errorf("expected no errors for camelize, got: %v", results2) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := createTestFile(t, tt.content) + results, _ := CheckInvalidFuncsForFile(path) + if len(results) == 0 || !strings.Contains(results[0], tt.expected) { + t.Errorf("expected error for %s, got: %v", tt.expected, results) + } + }) } } From 069d0e80a7490c96f98a1c217a5e2473cd6e795c Mon Sep 17 00:00:00 2001 From: vr-ibm Date: Thu, 23 Jul 2026 17:00:37 -0500 Subject: [PATCH 3/4] tools/template-check: add tests for pipelines and line number reporting --- .../gotemplate/funccheck_test.go | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tools/template-check/gotemplate/funccheck_test.go b/tools/template-check/gotemplate/funccheck_test.go index b9446f380906..49d08696d1a4 100644 --- a/tools/template-check/gotemplate/funccheck_test.go +++ b/tools/template-check/gotemplate/funccheck_test.go @@ -67,3 +67,49 @@ func TestFuncCheck_InvalidFunctions(t *testing.T) { }) } } + +func TestFuncCheck_Pipelines(t *testing.T) { + tests := []struct { + name string + content string + }{ + {"single_pipe", `{{ .Name | camelize }}`}, + {"multi_pipe", `{{ .Name | lower | camelize }}`}, + {"pipe_with_args", `{{ .Name | replace "a" "b" | title }}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := createTestFile(t, tt.content) + results, err := CheckInvalidFuncsForFile(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(results) != 0 { + t.Errorf("expected no errors for pipeline, got: %v", results) + } + }) + } +} + +func TestFuncCheck_LineNumbers(t *testing.T) { + content := `line 1 +line 2 +{{ validFunc }} +line 4 +{{ InvalidFunc }} +line 6` + // Note: validFunc needs to be in our registry for this test to work + // Let's use 'title' which we know is valid + content = strings.Replace(content, "validFunc", "title .Name", 1) + + path := createTestFile(t, content) + results, _ := CheckInvalidFuncsForFile(path) + + if len(results) == 0 { + t.Fatal("expected an error on line 5, got none") + } + if !strings.Contains(results[0], "line 5") { + t.Errorf("expected error to mention line 5, got: %s", results[0]) + } +} From 1a9c21d684009c5ec25ed331d6734b842b59811a Mon Sep 17 00:00:00 2001 From: vr-ibm Date: Thu, 23 Jul 2026 17:16:05 -0500 Subject: [PATCH 4/4] tools/template-check: fix formatting in funccheck_test.go --- .../gotemplate/funccheck_test.go | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/tools/template-check/gotemplate/funccheck_test.go b/tools/template-check/gotemplate/funccheck_test.go index 49d08696d1a4..66a0762a8ffa 100644 --- a/tools/template-check/gotemplate/funccheck_test.go +++ b/tools/template-check/gotemplate/funccheck_test.go @@ -102,10 +102,10 @@ line 6` // Note: validFunc needs to be in our registry for this test to work // Let's use 'title' which we know is valid content = strings.Replace(content, "validFunc", "title .Name", 1) - + path := createTestFile(t, content) results, _ := CheckInvalidFuncsForFile(path) - + if len(results) == 0 { t.Fatal("expected an error on line 5, got none") } @@ -113,3 +113,34 @@ line 6` t.Errorf("expected error to mention line 5, got: %s", results[0]) } } + +func TestFuncCheck_FullRepoScan(t *testing.T) { + // This test scans the actual repository templates. + // We navigate up from tools/template-check/gotemplate to the repo root. + root, err := filepath.Abs("../../../") + if err != nil { + t.Fatal(err) + } + + err = filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() && strings.HasSuffix(path, ".go.tmpl") { + t.Run(filepath.Base(path), func(t *testing.T) { + results, err := CheckInvalidFuncsForFile(path) + if err != nil { + t.Errorf("error checking %s: %v", path, err) + } + if len(results) > 0 { + t.Errorf("found invalid functions in %s: %v", path, results) + } + }) + } + return nil + }) + + if err != nil { + t.Fatalf("failed to walk repo: %v", err) + } +}