Merge pull request #104 from vimeo/flags_skip_intermediate

flatten mangler: propagate an allow-list of tags

Author: dfinkel

sources/env/env.go

@@ -31,6 +31,7 @@ var _ dials.Source = (*Source)(nil)
 func (e *Source) Value(_ context.Context, t *dials.Type) (reflect.Value, error) {
 	// flatten the nested fields
 	flattenMangler := transform.NewFlattenMangler(common.DialsTagName, caseconversion.EncodeUpperCamelCase, caseconversion.EncodeUpperCamelCase)
+	flattenMangler.AddPropogateTags(common.DialsEnvTagName)
 	// reformat the tags so they are SCREAMING_SNAKE_CASE
 	reformatTagMangler := tagformat.NewTagReformattingMangler(common.DialsTagName, caseconversion.DecodeGoTags, caseconversion.EncodeUpperSnakeCase)
 	// copy tags from "dials" to "dialsenv" tag

sources/flag/flag.go

@@ -167,7 +167,7 @@ func Must(s *Set, err error) *Set {
 
 const (
 	// HelpTextTag is the name of the struct tags for flag descriptions
-	HelpTextTag = "dialsdesc"
+	HelpTextTag = common.DialsHelpTextTag
 	// DefaultFlagHelpText is the default help-text for fields with an
 	// unset dialsdesc tag.
 	DefaultFlagHelpText = "unset description (`" + HelpTextTag + "` struct tag)"
@@ -202,6 +202,7 @@ func (s *Set) parse() error {
 
 func (s *Set) registerFlags(tmpl reflect.Value, ptyp reflect.Type) error {
 	fm := transform.NewFlattenMangler(common.DialsTagName, s.NameCfg.FieldNameEncodeCasing, s.NameCfg.TagEncodeCasing)
+	fm.AddPropogateTags(common.DialsFlagTagName, HelpTextTag)
 	tfmr := transform.NewTransformer(ptyp, transform.NewAliasMangler(common.DialsTagName, common.DialsFlagTagName), fm)
 	val, TrnslErr := tfmr.Translate()
 	if TrnslErr != nil {

sources/flag/flag_test.go

@@ -39,6 +39,8 @@ func TestDirectBasic(t *testing.T) {
 
 	d, err := dials.Config(ctx, &Config{Hello: "nothing"}, src)
 	if err != nil {
+		t.Log(buf.String())
+		fs.PrintDefaults()
 		t.Fatal(err)
 	}
 	src.Flags.Usage()

sources/pflag/pflag.go

@@ -210,6 +210,7 @@ func (s *Set) parse() error {
 
 func (s *Set) registerFlags(tmpl reflect.Value, ptyp reflect.Type) error {
 	fm := transform.NewFlattenMangler(common.DialsTagName, s.NameCfg.FieldNameEncodeCasing, s.NameCfg.TagEncodeCasing)
+	fm.AddPropogateTags(common.DialsPFlagTag, common.DialsPFlagShortTag, common.DialsHelpTextTag)
 	tfmr := transform.NewTransformer(ptyp, transform.NewAliasMangler(common.DialsTagName, common.DialsPFlagTag, common.DialsPFlagShortTag), fm)
 	val, TrnslErr := tfmr.Translate()
 	if TrnslErr != nil {

transform/flatten_mangler.go

@@ -3,6 +3,7 @@ package transform
 import (
 	"encoding"
 	"fmt"
+	"maps"
 	"reflect"
 	"strings"
 
@@ -26,24 +27,36 @@ type FlattenMangler struct {
 	tag              string
 	nameEncodeCasing caseconversion.EncodeCasingFunc
 	tagEncodeCasing  caseconversion.EncodeCasingFunc
+
+	propagateChildTags map[string]struct{}
+}
+
+// AddPropogateTags inserts an additional set of tags to propagate down to the
+// final leaf fields from intermediate-level struct-fields.
+func (f *FlattenMangler) AddPropogateTags(tags ...string) {
+	for _, tag := range tags {
+		f.propagateChildTags[tag] = struct{}{}
+	}
 }
 
 // DefaultFlattenMangler returns a FlattenMangler with preset values for tag,
 // nameEncodeCasing, and tagEncodeCasing
 func DefaultFlattenMangler() *FlattenMangler {
 	return &FlattenMangler{
-		tag:              common.DialsTagName,
-		nameEncodeCasing: caseconversion.EncodeUpperCamelCase,
-		tagEncodeCasing:  caseconversion.EncodeCasePreservingSnakeCase,
+		tag:                common.DialsTagName,
+		nameEncodeCasing:   caseconversion.EncodeUpperCamelCase,
+		tagEncodeCasing:    caseconversion.EncodeCasePreservingSnakeCase,
+		propagateChildTags: map[string]struct{}{},
 	}
 }
 
 // NewFlattenMangler is the constructor for FlattenMangler
 func NewFlattenMangler(tag string, nameEnc, tagEnc caseconversion.EncodeCasingFunc) *FlattenMangler {
 	return &FlattenMangler{
-		tag:              tag,
-		nameEncodeCasing: nameEnc,
-		tagEncodeCasing:  tagEnc,
+		tag:                tag,
+		nameEncodeCasing:   nameEnc,
+		tagEncodeCasing:    tagEnc,
+		propagateChildTags: map[string]struct{}{},
 	}
 }
 
@@ -64,7 +77,9 @@ func (f *FlattenMangler) Mangle(sf reflect.StructField) ([]reflect.StructField,
 	out := []reflect.StructField{}
 	fieldPath := []string{sf.Name}
 
-	tag, prefixTag, tagErr := f.getTag(&sf, nil, fieldPath)
+	propagatedTags := map[string]string{}
+
+	tag, prefixTag, tagErr := f.getTag(&sf, nil, fieldPath, propagatedTags)
 	if tagErr != nil {
 		return out, tagErr
 	}
@@ -79,7 +94,7 @@ func (f *FlattenMangler) Mangle(sf reflect.StructField) ([]reflect.StructField,
 		if !sf.Anonymous {
 			fieldPrefix = append(fieldPrefix, sf.Name)
 		}
-		return f.flattenStruct(fieldPrefix, prefixTag, fieldPath, sf)
+		return f.flattenStruct(fieldPrefix, prefixTag, fieldPath, maps.Clone(propagatedTags), sf)
 	default:
 	}
 
@@ -99,7 +114,7 @@ func (f *FlattenMangler) Mangle(sf reflect.StructField) ([]reflect.StructField,
 
 // flattenStruct takes a struct and flattens all the fields and makes a recursive
 // call if the field is a struct too
-func (f *FlattenMangler) flattenStruct(fieldPrefix, tagPrefix, fieldPath []string, sf reflect.StructField) ([]reflect.StructField, error) {
+func (f *FlattenMangler) flattenStruct(fieldPrefix, tagPrefix, fieldPath []string, propagatedTags map[string]string, sf reflect.StructField) ([]reflect.StructField, error) {
 
 	// get underlying type after removing pointers. Ignoring the kind
 	_, ft := getUnderlyingKindType(sf.Type)
@@ -122,8 +137,10 @@ func (f *FlattenMangler) flattenStruct(fieldPrefix, tagPrefix, fieldPath []strin
 		// embedded fields to the slice so we can iterate through and get the original field
 		flattenedPath := append(fieldPath[:len(fieldPath):len(fieldPath)], nestedsf.Name)
 
+		propagatedFieldTags := maps.Clone(propagatedTags)
+
 		// add the tag of the current field to the list of flattened tags
-		tag, flattenedTags, tagErr := f.getTag(&nestedsf, tagPrefix, flattenedPath)
+		tag, flattenedTags, tagErr := f.getTag(&nestedsf, tagPrefix, flattenedPath, propagatedFieldTags)
 		if tagErr != nil {
 			return out, tagErr
 		}
@@ -137,7 +154,7 @@ func (f *FlattenMangler) flattenStruct(fieldPrefix, tagPrefix, fieldPath []strin
 			if nestedT.Implements(textMReflectType) || reflect.PointerTo(nestedT).Implements(textMReflectType) {
 				break
 			}
-			flattened, err := f.flattenStruct(flattenedNames, flattenedTags, flattenedPath, nestedsf)
+			flattened, err := f.flattenStruct(flattenedNames, flattenedTags, flattenedPath, propagatedFieldTags, nestedsf)
 			if err != nil {
 				return out, err
 			}
@@ -162,7 +179,7 @@ func (f *FlattenMangler) flattenStruct(fieldPrefix, tagPrefix, fieldPath []strin
 // getTag uses the tag if one already exists or creates one based on the
 // configured EncodingCasing function and fieldName. It returns the new parsed
 // StructTag, the updated slice of tags, and any error encountered
-func (f *FlattenMangler) getTag(sf *reflect.StructField, tags, flattenedPath []string) (reflect.StructTag, []string, error) {
+func (f *FlattenMangler) getTag(sf *reflect.StructField, tags, flattenedPath []string, propagatedTags map[string]string) (reflect.StructTag, []string, error) {
 	tag, ok := sf.Tag.Lookup(f.tag)
 
 	// tag already exists so use the existing tag and append to prefix tags
@@ -199,6 +216,19 @@ func (f *FlattenMangler) getTag(sf *reflect.StructField, tags, flattenedPath []s
 		Name: strings.Join(flattenedPath, ","),
 	})
 
+	for presTagName := range f.propagateChildTags {
+		if v, ok := sf.Tag.Lookup(presTagName); ok {
+			propagatedTags[presTagName] = v
+			continue
+		}
+		if val, ok := propagatedTags[presTagName]; ok {
+			parsedTag.Set(&structtag.Tag{
+				Key:  presTagName,
+				Name: val,
+			})
+		}
+	}
+
 	return reflect.StructTag(parsedTag.String()), tags, nil
 }
 

transform/flatten_mangler_test.go

@@ -25,7 +25,7 @@ func (u tu) UnmarshalText(data []byte) error {
 
 func TestFlattenMangler(t *testing.T) {
 	type Foo struct {
-		Location    string `dials:"Location"`
+		Location    string `dials:"Location" littlebiddle:"boop"`
 		Coordinates int    `dials:"Coordinates"`
 		SomeTime    time.Duration
 	}
@@ -48,6 +48,11 @@ func TestFlattenMangler(t *testing.T) {
 		AnotherField int `dials:"AnotherField"`
 	}
 
+	type intermediateLevel struct {
+		Name      string `dials:"Name"`
+		FizzleBit bar    `dials:"Bit" ooble:"ooops"`
+	}
+
 	b := bar{
 		Name: "test",
 		Foobar: &Foo{
@@ -78,8 +83,9 @@ func TestFlattenMangler(t *testing.T) {
 	}
 
 	testCases := []struct {
-		name       string
-		testStruct any
+		name           string
+		testStruct     any
+		additionalTags []string
 		// modify will fill the flatten struct value after Mangling
 		modify    func(t testing.TB, val reflect.Value)
 		assertion func(t testing.TB, i any)
@@ -135,6 +141,69 @@ func TestFlattenMangler(t *testing.T) {
 				assert.EqualValues(t, curTime, *i.(*time.Time))
 			},
 		},
+		{
+			name: "intermediate level struct with an auxillary tag",
+			testStruct: intermediateLevel{
+				Name:      "foobar",
+				FizzleBit: b,
+			},
+			additionalTags: []string{"ooble"},
+			modify: func(t testing.TB, val reflect.Value) {
+				vt := val.Type()
+				for _, field := range []string{"ConfigFieldFizzleBitName", "ConfigFieldFizzleBitAnotherField",
+					"ConfigFieldFizzleBitFoobarLocation", "ConfigFieldFizzleBitFoobarCoordinates",
+					"ConfigFieldFizzleBitFoobarSomeTime"} {
+					if oobleField, ok := vt.FieldByName(field); ok {
+						if tagVal, tagOK := oobleField.Tag.Lookup("ooble"); tagOK {
+							if tagVal != "ooops" {
+								t.Errorf("unexpected tag value for field %q; got %q; want %q", field, tagVal, "ooops")
+							}
+						} else {
+							t.Errorf("missing ooble tag on %q; only %q", field, oobleField.Tag)
+						}
+					} else {
+						t.Errorf("missing field %q type %s", field, vt)
+					}
+				}
+			},
+			assertion: func(t testing.TB, i any) {},
+		},
+		{
+			name: "leaf auxilliary tag",
+			testStruct: Foo{
+				Location:    "boop",
+				Coordinates: 3333,
+			},
+			additionalTags: []string{"littlebiddle"},
+			modify: func(t testing.TB, val reflect.Value) {
+				vt := val.Type()
+				// check the fields we expect to see the littlebiddle tag on
+				for _, field := range []string{"ConfigFieldLocation"} {
+					if littlebiddleField, ok := vt.FieldByName(field); ok {
+						if tagVal, tagOK := littlebiddleField.Tag.Lookup("littlebiddle"); tagOK {
+							if tagVal != "boop" {
+								t.Errorf("unexpected tag value for field %q; got %q; want %q", field, tagVal, "ooops")
+							}
+						} else {
+							t.Errorf("missing littlebiddle tag on %q; only %q", field, littlebiddleField.Tag)
+						}
+					} else {
+						t.Errorf("missing field %q type %s", field, vt)
+					}
+				}
+				// check the ones that we don't expect to see the littlebiddle tag on
+				for _, field := range []string{"ConfigFieldCoordinates", "ConfigFieldSomeTime"} {
+					if oobleField, ok := vt.FieldByName(field); ok {
+						if tagVal, tagOK := oobleField.Tag.Lookup("littlebiddle"); tagOK {
+							t.Errorf("unexpectedly present littlebiddle tag on %q; only %q; with value %q", field, oobleField.Tag, tagVal)
+						}
+					} else {
+						t.Errorf("missing field %q type %s", field, vt)
+					}
+				}
+			},
+			assertion: func(t testing.TB, i any) {},
+		},
 		{
 			name: "one level nested struct unexposed fields",
 			testStruct: struct {
@@ -671,6 +740,9 @@ func TestFlattenMangler(t *testing.T) {
 
 			ptrifiedConfigType := ptrify.Pointerify(configStructType, reflect.New(configStructType).Elem())
 			f := DefaultFlattenMangler()
+			if tc.additionalTags != nil {
+				f.AddPropogateTags(tc.additionalTags...)
+			}
 			tfmr := NewTransformer(ptrifiedConfigType, f)
 			val, err := tfmr.Translate()
 			require.NoError(t, err)