From b477589fa45cf1f02aa5b68e2934cf311c790491 Mon Sep 17 00:00:00 2001 From: Nick Elliot Date: Wed, 30 Oct 2024 13:55:19 -0700 Subject: [PATCH] Updating template check for go templates (#12097) --- .github/workflows/mmv1-check-templates.yml | 4 +- .../ruby.go => gotemplate/gotemplate.go} | 16 +++-- .../gotemplate/gotemplate_test.go | 47 +++++++++++++++ tools/template-check/main.go | 4 +- tools/template-check/ruby/ruby_test.go | 59 ------------------- 5 files changed, 61 insertions(+), 69 deletions(-) rename tools/template-check/{ruby/ruby.go => gotemplate/gotemplate.go} (81%) create mode 100644 tools/template-check/gotemplate/gotemplate_test.go delete mode 100644 tools/template-check/ruby/ruby_test.go diff --git a/.github/workflows/mmv1-check-templates.yml b/.github/workflows/mmv1-check-templates.yml index 0c4a9c1f3101..f8b1f4c052ce 100644 --- a/.github/workflows/mmv1-check-templates.yml +++ b/.github/workflows/mmv1-check-templates.yml @@ -5,7 +5,7 @@ permissions: read-all on: pull_request: paths: - - 'mmv1/**/*.erb' + - 'mmv1/**/*.tmpl' jobs: version-guard-check: @@ -27,4 +27,4 @@ jobs: - name: Check for invalid version guards run: | cd repo/tools/template-check - git diff --name-only --diff-filter=d origin/${{ github.base_ref }} ../../*.erb | sed 's=^=../../=g' | go run main.go + git diff --name-only --diff-filter=d origin/${{ github.base_ref }} ../../*.tmpl | sed 's=^=../../=g' | go run main.go diff --git a/tools/template-check/ruby/ruby.go b/tools/template-check/gotemplate/gotemplate.go similarity index 81% rename from tools/template-check/ruby/ruby.go rename to tools/template-check/gotemplate/gotemplate.go index c04da738c980..ef39eafa2228 100644 --- a/tools/template-check/ruby/ruby.go +++ b/tools/template-check/gotemplate/gotemplate.go @@ -1,4 +1,4 @@ -package ruby +package gotemplate import ( "bufio" @@ -13,10 +13,14 @@ import ( // need to add more options to this list in the future, like `private`. The main thing we want to // prevent is targeting `beta` in version guards, because it mishandles either `ga` or `private`. var allowedGuards = []string{ - "<% unless version == 'ga' -%>", - "<% if version == 'ga' -%>", - "<% unless version == \"ga\" -%>", - "<% if version == \"ga\" -%>", + `{{- if ne $.TargetVersionName "ga" }}`, + `{{ if ne $.TargetVersionName "ga" }}`, + `{{ if ne $.TargetVersionName "ga" -}}`, + `{{- if ne $.TargetVersionName "ga" -}}`, + `{{- if eq $.TargetVersionName "ga" }}`, + `{{ if eq $.TargetVersionName "ga" }}`, + `{{ if eq $.TargetVersionName "ga" -}}`, + `{{- if eq $.TargetVersionName "ga" -}}`, } // Note: this does not account for _every_ possible use of a version guard (for example, those @@ -24,7 +28,7 @@ var allowedGuards = []string{ // the goal is to capture (and validate) all "standard" version guards that would be added for new // resources/fields. func isVersionGuard(line string) bool { - re := regexp.MustCompile("<% [a-z]+ version ") + re := regexp.MustCompile(`{{-? if (eq|ne) \$\.TargetVersionName`) return re.MatchString(line) } diff --git a/tools/template-check/gotemplate/gotemplate_test.go b/tools/template-check/gotemplate/gotemplate_test.go new file mode 100644 index 000000000000..b82d6c08d1b5 --- /dev/null +++ b/tools/template-check/gotemplate/gotemplate_test.go @@ -0,0 +1,47 @@ +package gotemplate + +import ( + "strings" + "testing" +) + +func TestCheckVersionGuards(t *testing.T) { + cases := map[string]struct { + fileText string + expectedResults []string + }{ + "allow standard format targeting ga": { + fileText: "some text\n{{- if ne $.TargetVersionName \"ga\" }}\nmore text", + expectedResults: nil, + }, + "disallow targeting beta": { + fileText: "some text\n{{- if ne $.TargetVersionName \"beta\" }}\nmore text", + expectedResults: []string{`2: {{- if ne $.TargetVersionName "beta" }}`}, + }, + "one valid, one invalid": { + fileText: "some text\n{{- if ne $.TargetVersionName \"beta\" }}\nmore text\n{{- if ne $.TargetVersionName \"ga\" }}", + expectedResults: []string{`2: {{- if ne $.TargetVersionName "beta" }}`}, + }, + "multiple invalid": { + fileText: "some text\n{{- if ne $.TargetVersionName \"beta\" }}\nmore text\n\n\n{{- if eq $.TargetVersionName \"beta\" }}", + expectedResults: []string{`2: {{- if ne $.TargetVersionName "beta" }}`, `6: {{- if eq $.TargetVersionName "beta" }}`}, + }, + } + + for tn, tc := range cases { + tc := tc + t.Run(tn, func(t *testing.T) { + t.Parallel() + results := CheckVersionGuards(strings.NewReader(tc.fileText)) + if len(results) != len(tc.expectedResults) { + t.Errorf("Expected length %d, got %d", len(tc.expectedResults), len(results)) + return + } + for i, result := range results { + if result != tc.expectedResults[i] { + t.Errorf("Expected %v, got %v", tc.expectedResults[i], result) + } + } + }) + } +} diff --git a/tools/template-check/main.go b/tools/template-check/main.go index 3d0ff20c9405..ac44d3b3a30a 100644 --- a/tools/template-check/main.go +++ b/tools/template-check/main.go @@ -6,11 +6,11 @@ import ( "fmt" "os" - "github.com/GoogleCloudPlatform/magic-modules/tools/template-check/ruby" + "github.com/GoogleCloudPlatform/magic-modules/tools/template-check/gotemplate" ) func isValidTemplate(filename string) (bool, error) { - results, err := ruby.CheckVersionGuardsForFile(filename) + results, err := gotemplate.CheckVersionGuardsForFile(filename) if err != nil { return false, err } diff --git a/tools/template-check/ruby/ruby_test.go b/tools/template-check/ruby/ruby_test.go deleted file mode 100644 index e25d9be59af0..000000000000 --- a/tools/template-check/ruby/ruby_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package ruby - -import ( - "strings" - "testing" -) - -func TestCheckVersionGuards(t *testing.T) { - cases := map[string]struct { - fileText string - expectedResults []string - }{ - "allow standard format targeting ga": { - fileText: "some text\n<% unless version == 'ga' -%>\nmore text", - expectedResults: nil, - }, - "disallow targeting beta": { - fileText: "some text\n<% unless version == 'beta' -%>\nmore text", - expectedResults: []string{"2: <% unless version == 'beta' -%>"}, - }, - "disallow 'if not'": { - fileText: "some text\n<% if version != 'ga' -%>\nmore text", - expectedResults: []string{"2: <% if version != 'ga' -%>"}, - }, - "disallow single '='": { - fileText: "some text\n<% unless version = 'ga' -%>\nmore text", - expectedResults: []string{"2: <% unless version = 'ga' -%>"}, - }, - "disallow leaving trailing line break": { - fileText: "some text\n<% unless version == 'ga' %>\nmore text", - expectedResults: []string{"2: <% unless version == 'ga' %>"}, - }, - "one valid, one invalid": { - fileText: "some text\n<% unless version == 'beta' -%>\nmore text\n<% unless version == 'ga' -%>", - expectedResults: []string{"2: <% unless version == 'beta' -%>"}, - }, - "multiple invalid": { - fileText: "some text\n<% unless version == 'beta' -%>\nmore text\n\n\n<% if version == \"beta\" -%>", - expectedResults: []string{"2: <% unless version == 'beta' -%>", "6: <% if version == \"beta\" -%>"}, - }, - } - - for tn, tc := range cases { - tc := tc - t.Run(tn, func(t *testing.T) { - t.Parallel() - results := CheckVersionGuards(strings.NewReader(tc.fileText)) - if len(results) != len(tc.expectedResults) { - t.Errorf("Expected length %d, got %d", len(tc.expectedResults), len(results)) - return - } - for i, result := range results { - if result != tc.expectedResults[i] { - t.Errorf("Expected %v, got %v", tc.expectedResults[i], result) - } - } - }) - } -}