Skip to content

Commit

Permalink
chore: add test to ensure config struct tags match across marshal for…
Browse files Browse the repository at this point in the history
…mats

- add tags to interpreter so it passes
  • Loading branch information
donaldguy authored and twpayne committed Jun 12, 2024
1 parent 75e4680 commit e102de5
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 2 deletions.
4 changes: 2 additions & 2 deletions internal/chezmoi/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
96 changes: 96 additions & 0 deletions internal/cmd/config_tags_test.go
Original file line number Diff line number Diff line change
@@ -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()
}
40 changes: 40 additions & 0 deletions internal/cmd/config_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package cmd

import (
"fmt"
"io"
"io/fs"
"path/filepath"
"reflect"
"runtime"
"strconv"
"strings"
"testing"

"github.com/alecthomas/assert/v2"
Expand All @@ -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)
Expand Down

0 comments on commit e102de5

Please sign in to comment.