Merge pull request #2765 from lavalamp/conv

conversion refactoring

pkg/conversion/converter.go

@@ -103,29 +103,63 @@ type scope struct {
 	converter *Converter
 	meta      *Meta
 	flags     FieldMatchingFlags
-	srcTagStack  []reflect.StructTag
+
-	destTagStack []reflect.StructTag
+	// srcStack & destStack are separate because they may not have a 1:1
+	// relationship.
+	srcStack  scopeStack
+	destStack scopeStack
 }
 
-// push adds a level to the src/dest tag stacks.
+type scopeStackElem struct {
-func (s *scope) push() {
+	tag   reflect.StructTag
-	s.srcTagStack = append(s.srcTagStack, "")
+	value reflect.Value
-	s.destTagStack = append(s.destTagStack, "")
+	key   string
 }
 
-// pop removes a level to the src/dest tag stacks.
+type scopeStack []scopeStackElem
-func (s *scope) pop() {
+
-	n := len(s.srcTagStack)
+func (s *scopeStack) pop() {
-	s.srcTagStack = s.srcTagStack[:n-1]
+	n := len(*s)
-	s.destTagStack = s.destTagStack[:n-1]
+	*s = (*s)[:n-1]
 }
 
-func (s *scope) setSrcTag(tag reflect.StructTag) {
+func (s *scopeStack) push(e scopeStackElem) {
-	s.srcTagStack[len(s.srcTagStack)-1] = tag
+	*s = append(*s, e)
 }
 
-func (s *scope) setDestTag(tag reflect.StructTag) {
+func (s *scopeStack) top() *scopeStackElem {
-	s.destTagStack[len(s.destTagStack)-1] = tag
+	return &(*s)[len(*s)-1]
+}
+
+func (s scopeStack) describe() string {
+	desc := ""
+	if len(s) > 1 {
+		desc = "(" + s[1].value.Type().String() + ")"
+	}
+	for i, v := range s {
+		if i < 2 {
+			// First layer on stack is not real; second is handled specially above.
+			continue
+		}
+		if v.key == "" {
+			desc += fmt.Sprintf(".%v", v.value.Type())
+		} else {
+			desc += fmt.Sprintf(".%v", v.key)
+		}
+	}
+	return desc
+}
+
+// Formats src & dest as indices for printing.
+func (s *scope) setIndices(src, dest int) {
+	s.srcStack.top().key = fmt.Sprintf("[%v]", src)
+	s.destStack.top().key = fmt.Sprintf("[%v]", dest)
+}
+
+// Formats src & dest as map keys for printing.
+func (s *scope) setKeys(src, dest interface{}) {
+	s.srcStack.top().key = fmt.Sprintf(`["%v"]`, src)
+	s.destStack.top().key = fmt.Sprintf(`["%v"]`, dest)
 }
 
 // Convert continues a conversion.
@@ -135,12 +169,12 @@ func (s *scope) Convert(src, dest interface{}, flags FieldMatchingFlags) error {
 
 // SrcTag returns the tag of the struct containing the current source item, if any.
 func (s *scope) SrcTag() reflect.StructTag {
-	return s.srcTagStack[len(s.srcTagStack)-1]
+	return s.srcStack.top().tag
 }
 
 // DestTag returns the tag of the struct containing the current dest item, if any.
 func (s *scope) DestTag() reflect.StructTag {
-	return s.destTagStack[len(s.destTagStack)-1]
+	return s.destStack.top().tag
 }
 
 // Flags returns the flags with which the current conversion was started.
@@ -153,6 +187,19 @@ func (s *scope) Meta() *Meta {
 	return s.meta
 }
 
+// describe prints the path to get to the current (source, dest) values.
+func (s *scope) describe() (src, dest string) {
+	return s.srcStack.describe(), s.destStack.describe()
+}
+
+// error makes an error that includes information about where we were in the objects
+// we were asked to convert.
+func (s *scope) error(message string, args ...interface{}) error {
+	srcPath, destPath := s.describe()
+	where := fmt.Sprintf("converting %v to %v: ", srcPath, destPath)
+	return fmt.Errorf(where+message, args...)
+}
+
 // Register registers a conversion func with the Converter. conversionFunc must take
 // three parameters: a pointer to the input type, a pointer to the output type, and
 // a conversion.Scope (which should be used if recursive conversion calls are desired).
@@ -265,7 +312,9 @@ func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags, met
 		flags:     flags,
 		meta:      meta,
 	}
-	s.push() // Easy way to make SrcTag and DestTag never fail
+	// Leave something on the stack, so that calls to struct tag getters never fail.
+	s.srcStack.push(scopeStackElem{})
+	s.destStack.push(scopeStackElem{})
 	return c.convert(sv, dv, s)
 }
 
@@ -288,7 +337,7 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error {
 	}
 
 	if !scope.flags.IsSet(AllowDifferentFieldTypeNames) && c.NameFunc(dt) != c.NameFunc(st) {
-		return fmt.Errorf("can't convert %v to %v because type names don't match (%v, %v).", st, dt, c.NameFunc(st), c.NameFunc(dt))
+		return scope.error("type names don't match (%v, %v)", c.NameFunc(st), c.NameFunc(dt))
 	}
 
 	// This should handle all simple types.
@@ -305,12 +354,14 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error {
 		c.Debug.Logf("Trying to convert '%v' to '%v'", st, dt)
 	}
 
-	scope.push()
+	scope.srcStack.push(scopeStackElem{value: sv})
-	defer scope.pop()
+	scope.destStack.push(scopeStackElem{value: dv})
+	defer scope.srcStack.pop()
+	defer scope.destStack.pop()
 
 	switch dv.Kind() {
 	case reflect.Struct:
-		return c.convertStruct(sv, dv, scope)
+		return c.convertKV(toKVValue(sv), toKVValue(dv), scope)
 	case reflect.Slice:
 		if sv.IsNil() {
 			// Don't make a zero-length slice.
@@ -319,6 +370,7 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error {
 		}
 		dv.Set(reflect.MakeSlice(dt, sv.Len(), sv.Cap()))
 		for i := 0; i < sv.Len(); i++ {
+			scope.setIndices(i, i)
 			if err := c.convert(sv.Index(i), dv.Index(i), scope); err != nil {
 				return err
 			}
@@ -344,55 +396,111 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error {
 				return err
 			}
 			dkv := reflect.New(dt.Elem()).Elem()
+			scope.setKeys(sk.Interface(), dk.Interface())
 			if err := c.convert(sv.MapIndex(sk), dkv, scope); err != nil {
 				return err
 			}
 			dv.SetMapIndex(dk, dkv)
 		}
 	default:
-		return fmt.Errorf("couldn't copy '%v' into '%v'", st, dt)
+		return scope.error("couldn't copy '%v' into '%v'; didn't understand types", st, dt)
 	}
 	return nil
 }
 
-func (c *Converter) convertStruct(sv, dv reflect.Value, scope *scope) error {
+func toKVValue(v reflect.Value) kvValue {
-	dt, st := dv.Type(), sv.Type()
+	switch v.Kind() {
-
+	case reflect.Struct:
-	listType := dt
+		return structAdaptor(v)
-	if scope.flags.IsSet(SourceToDest) {
-		listType = st
 	}
-	for i := 0; i < listType.NumField(); i++ {
+	return nil
-		f := listType.Field(i)
+}
-		if found, err := c.checkStructField(f.Name, sv, dv, scope); found {
+
+// kvValue lets us write the same conversion logic to work with both maps
+// and structs. Only maps with string keys make sense for this.
+type kvValue interface {
+	// returns all keys, as a []string.
+	keys() []string
+	// Will just return "" for maps.
+	tagOf(key string) reflect.StructTag
+	// Will return the zero Value if the key doesn't exist.
+	value(key string) reflect.Value
+	// Maps require explict setting-- will do nothing for structs.
+	// Returns false on failure.
+	confirmSet(key string, v reflect.Value) bool
+}
+
+type structAdaptor reflect.Value
+
+func (sa structAdaptor) len() int {
+	v := reflect.Value(sa)
+	return v.Type().NumField()
+}
+
+func (sa structAdaptor) keys() []string {
+	v := reflect.Value(sa)
+	t := v.Type()
+	keys := make([]string, t.NumField())
+	for i := range keys {
+		keys[i] = t.Field(i).Name
+	}
+	return keys
+}
+
+func (sa structAdaptor) tagOf(key string) reflect.StructTag {
+	v := reflect.Value(sa)
+	field, ok := v.Type().FieldByName(key)
+	if ok {
+		return field.Tag
+	}
+	return ""
+}
+
+func (sa structAdaptor) value(key string) reflect.Value {
+	v := reflect.Value(sa)
+	return v.FieldByName(key)
+}
+
+func (sa structAdaptor) confirmSet(key string, v reflect.Value) bool {
+	return true
+}
+
+// convertKV can convert things that consist of key/value pairs, like structs
+// and some maps.
+func (c *Converter) convertKV(skv, dkv kvValue, scope *scope) error {
+	if skv == nil || dkv == nil {
+		// TODO: add keys to stack to support really understandable error messages.
+		return fmt.Errorf("Unable to convert %#v to %#v", skv, dkv)
+	}
+
+	lister := dkv
+	if scope.flags.IsSet(SourceToDest) {
+		lister = skv
+	}
+	for _, key := range lister.keys() {
+		if found, err := c.checkField(key, skv, dkv, scope); found {
 			if err != nil {
 				return err
 			}
 			continue
 		}
-		df := dv.FieldByName(f.Name)
+		df := dkv.value(key)
-		sf := sv.FieldByName(f.Name)
+		sf := skv.value(key)
-		if sf.IsValid() {
-			// No need to check error, since we know it's valid.
-			field, _ := st.FieldByName(f.Name)
-			scope.setSrcTag(field.Tag)
-		}
-		if df.IsValid() {
-			field, _ := dt.FieldByName(f.Name)
-			scope.setDestTag(field.Tag)
-		}
-		// TODO: set top level of scope.src/destTagStack with these field tags here.
 		if !df.IsValid() || !sf.IsValid() {
 			switch {
 			case scope.flags.IsSet(IgnoreMissingFields):
 				// No error.
 			case scope.flags.IsSet(SourceToDest):
-				return fmt.Errorf("%v not present in dest (%v to %v)", f.Name, st, dt)
+				return scope.error("%v not present in dest", key)
 			default:
-				return fmt.Errorf("%v not present in src (%v to %v)", f.Name, st, dt)
+				return scope.error("%v not present in src", key)
 			}
 			continue
 		}
+		scope.srcStack.top().key = key
+		scope.srcStack.top().tag = skv.tagOf(key)
+		scope.destStack.top().key = key
+		scope.destStack.top().tag = dkv.tagOf(key)
 		if err := c.convert(sf, df, scope); err != nil {
 			return err
 		}
@@ -400,12 +508,12 @@ func (c *Converter) convertStruct(sv, dv reflect.Value, scope *scope) error {
 	return nil
 }
 
-// checkStructField returns true if the field name matches any of the struct
+// checkField returns true if the field name matches any of the struct
 // field copying rules. The error should be ignored if it returns false.
-func (c *Converter) checkStructField(fieldName string, sv, dv reflect.Value, scope *scope) (bool, error) {
+func (c *Converter) checkField(fieldName string, skv, dkv kvValue, scope *scope) (bool, error) {
 	replacementMade := false
 	if scope.flags.IsSet(DestFromSource) {
-		df := dv.FieldByName(fieldName)
+		df := dkv.value(fieldName)
 		if !df.IsValid() {
 			return false, nil
 		}
@@ -413,22 +521,25 @@ func (c *Converter) checkStructField(fieldName string, sv, dv reflect.Value, sco
 		// Check each of the potential source (type, name) pairs to see if they're
 		// present in sv.
 		for _, potentialSourceKey := range c.structFieldSources[destKey] {
-			sf := sv.FieldByName(potentialSourceKey.fieldName)
+			sf := skv.value(potentialSourceKey.fieldName)
 			if !sf.IsValid() {
 				continue
 			}
 			if sf.Type() == potentialSourceKey.fieldType {
 				// Both the source's name and type matched, so copy.
+				scope.srcStack.top().key = potentialSourceKey.fieldName
+				scope.destStack.top().key = fieldName
 				if err := c.convert(sf, df, scope); err != nil {
 					return true, err
 				}
+				dkv.confirmSet(fieldName, df)
 				replacementMade = true
 			}
 		}
 		return replacementMade, nil
 	}
 
-	sf := sv.FieldByName(fieldName)
+	sf := skv.value(fieldName)
 	if !sf.IsValid() {
 		return false, nil
 	}
@@ -436,15 +547,18 @@ func (c *Converter) checkStructField(fieldName string, sv, dv reflect.Value, sco
 	// Check each of the potential dest (type, name) pairs to see if they're
 	// present in dv.
 	for _, potentialDestKey := range c.structFieldDests[srcKey] {
-		df := dv.FieldByName(potentialDestKey.fieldName)
+		df := dkv.value(potentialDestKey.fieldName)
 		if !df.IsValid() {
 			continue
 		}
 		if df.Type() == potentialDestKey.fieldType {
 			// Both the dest's name and type matched, so copy.
+			scope.srcStack.top().key = fieldName
+			scope.destStack.top().key = potentialDestKey.fieldName
 			if err := c.convert(sf, df, scope); err != nil {
 				return true, err
 			}
+			dkv.confirmSet(potentialDestKey.fieldName, df)
 			replacementMade = true
 		}
 	}