From e102de5d9df613f2da115f803552076368944d3b Mon Sep 17 00:00:00 2001
From: Donald Guy <donaldguy@hey.com>
Date: Tue, 11 Jun 2024 16:55:18 -0600
Subject: [PATCH] chore: add test to ensure config struct tags match across
 marshal formats

- add tags to interpreter so it passes
---
 internal/chezmoi/interpreter.go  |  4 +-
 internal/cmd/config_tags_test.go | 96 ++++++++++++++++++++++++++++++++
 internal/cmd/config_test.go      | 40 +++++++++++++
 3 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 internal/cmd/config_tags_test.go

diff --git a/internal/chezmoi/interpreter.go b/internal/chezmoi/interpreter.go
index c82ea0efbc0..15036d2fdc7 100644
--- a/internal/chezmoi/interpreter.go
+++ b/internal/chezmoi/interpreter.go
@@ -7,8 +7,8 @@ import (
 
 // An Interpreter interprets scripts.
 type Interpreter struct {
-	Command string   `mapstructure:"command"`
-	Args    []string `mapstructure:"args"`
+	Command string   `json:"command" mapstructure:"command" yaml:"command"`
+	Args    []string `json:"args"    mapstructure:"args"    yaml:"args"`
 }
 
 // ExecCommand returns the *exec.Cmd to interpret name.
diff --git a/internal/cmd/config_tags_test.go b/internal/cmd/config_tags_test.go
new file mode 100644
index 00000000000..4497e6df49e
--- /dev/null
+++ b/internal/cmd/config_tags_test.go
@@ -0,0 +1,96 @@
+package cmd
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+	"testing"
+)
+
+var expectedTags = []string{"json", "yaml", "mapstructure"}
+
+func TestExportedFieldsHaveMatchingMarshalTags(t *testing.T) {
+	failed, errmsg := verifyTagsArePresentAndMatch(reflect.TypeFor[ConfigFile]())
+	if failed {
+		t.Error(errmsg)
+	}
+}
+
+func fieldTypesNeedsVerification(ft reflect.Type) []reflect.Type {
+	kind := ft.Kind()
+	if kind < reflect.Array || kind == reflect.String { // its a ~scalar type
+		return []reflect.Type{}
+	} else if kind == reflect.Struct {
+		return []reflect.Type{ft}
+	}
+	switch kind {
+	case reflect.Pointer:
+		fallthrough
+	case reflect.Array:
+		fallthrough
+	case reflect.Slice:
+		return fieldTypesNeedsVerification(ft.Elem())
+	case reflect.Map:
+		return append(fieldTypesNeedsVerification(ft.Key()), fieldTypesNeedsVerification(ft.Elem())...)
+	default:
+		return []reflect.Type{} // ... we'll assume interface types, funcs, chans are okay.
+	}
+}
+
+func verifyTagsArePresentAndMatch(structType reflect.Type) (failed bool, errmsg string) {
+	name := structType.Name()
+	fields := reflect.VisibleFields(structType)
+	failed = false
+
+	var errs strings.Builder
+
+	for _, f := range fields {
+		if !f.IsExported() {
+			continue
+		}
+
+		ts := f.Tag
+		tagValueGroups := make(map[string][]string)
+
+		for _, tagName := range expectedTags {
+			tagValue, tagPresent := ts.Lookup(tagName)
+
+			if !tagPresent {
+				errs.WriteString(fmt.Sprintf("\n%s field %s is missing a `%s:` tag", name, f.Name, tagName))
+				failed = true
+			}
+
+			matchingTags, notFirstOccurrence := tagValueGroups[tagValue]
+			if notFirstOccurrence {
+				tagValueGroups[tagValue] = append(matchingTags, tagName)
+			} else {
+				tagValueGroups[tagValue] = []string{tagName}
+			}
+		}
+
+		if len(tagValueGroups) > 1 {
+			errs.WriteString(fmt.Sprintf("\n%s field %s has non-matching tag names:", name, f.Name))
+
+			for value, tagsMatching := range tagValueGroups {
+				if len(tagsMatching) == 1 {
+					errs.WriteString(fmt.Sprintf("\n    %s says \"%s\"", tagsMatching[0], value))
+				} else {
+					errs.WriteString(fmt.Sprintf("\n    (%s) each say \"%s\"", strings.Join(tagsMatching, ", "), value))
+				}
+			}
+			failed = true
+		}
+
+		verifyTypes := fieldTypesNeedsVerification(f.Type)
+		for _, ft := range verifyTypes {
+			subFailed, suberrs := verifyTagsArePresentAndMatch(ft)
+			if subFailed {
+				errs.WriteString(fmt.Sprintf("\n In %s.%s:", name, f.Name))
+				errs.WriteString(strings.ReplaceAll(suberrs, "\n", "\n    "))
+				failed = true
+			}
+		}
+	}
+
+	return failed, errs.String()
+}
diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go
index b7f09fb08fe..e2b7350b0eb 100644
--- a/internal/cmd/config_test.go
+++ b/internal/cmd/config_test.go
@@ -1,11 +1,14 @@
 package cmd
 
 import (
+	"fmt"
 	"io"
 	"io/fs"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/alecthomas/assert/v2"
@@ -16,6 +19,43 @@ import (
 	"github.com/twpayne/chezmoi/v2/internal/chezmoitest"
 )
 
+func TestTagFieldNamesMatch(t *testing.T) {
+	fields := reflect.VisibleFields(reflect.TypeFor[ConfigFile]())
+	expectedTags := []string{"json", "yaml", "mapstructure"}
+
+	for _, f := range fields {
+		ts := f.Tag
+		tagValueGroups := make(map[string][]string)
+
+		for _, tagName := range expectedTags {
+			tagValue, tagPresent := ts.Lookup(tagName)
+
+			if !tagPresent {
+				t.Errorf("ConfigFile field %s is missing a %s tag", f.Name, tagName)
+			}
+
+			matchingTags, notFirstOccurrence := tagValueGroups[tagValue]
+			if notFirstOccurrence {
+				tagValueGroups[tagValue] = append(matchingTags, tagName)
+			} else {
+				tagValueGroups[tagValue] = []string{tagName}
+			}
+		}
+
+		if len(tagValueGroups) > 1 {
+			valueMsgs := []string{}
+			for value, tagsMatching := range tagValueGroups {
+				if len(tagsMatching) == 1 {
+					valueMsgs = append(valueMsgs, fmt.Sprintf("%s says \"%s\"", tagsMatching[0], value))
+				} else {
+					valueMsgs = append(valueMsgs, fmt.Sprintf("(%s) each say \"%s\"", strings.Join(tagsMatching, ", "), value))
+				}
+			}
+			t.Errorf("ConfigFile field %s has non-matching tag names:\n    %s", f.Name, strings.Join(valueMsgs, "\n    "))
+		}
+	}
+}
+
 func TestAddTemplateFuncPanic(t *testing.T) {
 	chezmoitest.WithTestFS(t, nil, func(fileSystem vfs.FS) {
 		config := newTestConfig(t, fileSystem)