Repository: mitchellh/mapstructure (go)

Issue:
1.4.0: decode hooks receive the wrong type for the `to` argument
I attempted to upgrade from mapstructure 1.3.3 to 1.4.0 in Consul and a few of the test failed.

Debugging one of the tests it seems to be that the decode hook (`reflect.Type` hook) is receiving `map[string]interface {}` as the type of the  `to` argument, where as previously it received `time.Time` as the type of `to`.

Test failures can be seen here: https://app.circleci.com/pipelines/github/hashicorp/consul/15588/workflows/76c7848b-f780-4049-aa5c-f091b737207b/jobs/302146

I suspect the change which added a new `reflect.Value` version of hooks may have broken this. I'll keep looking to see if I can find a fix.

More details:
1. squash is not being used
2. the decode is struct->struct
3. this particular hook is checking the type of both `from` and `to`
3. previously the decode hook would be called twice, now the decode hook is being called 4 times
4. previously it was the second call which receives a from and to of the correct types, now the second call receives the right type for the `from`, and the forth call receives the correct type for `to`. In both cases the other type is `map[string]interface {}`, even though neither the input or output contain a map.

I suspect it is not actually the change to hooks that broke this. It seems like it was one of the changes to `decodeStructFromMap`.

Pull request:
decodeMapFromStruct: Only deref element if we're squashing the struct
Fixes #231

This also reverts the test _change_ that was introduced in 231 back to
the original test. This retains the _new tests_ added in 231 to verify
the behavior of 231.

cc @dnephin 

Patch:
diff --git a/mapstructure.go b/mapstructure.go
index 6cb703c..3643901 100644
--- a/mapstructure.go
+++ b/mapstructure.go
@@ -894,10 +894,6 @@ func (d *Decoder) decodeMapFromStruct(name string, dataVal reflect.Value, val re
 		// Next get the actual value of this field and verify it is assignable
 		// to the map value.
 		v := dataVal.Field(i)
-		if v.Kind() == reflect.Ptr && v.Elem().Kind() == reflect.Struct {
-			// Handle embedded struct pointers as embedded structs.
-			v = v.Elem()
-		}
 		if !v.Type().AssignableTo(valMap.Type().Elem()) {
 			return fmt.Errorf("cannot assign type '%s' to map value field of type '%s'", v.Type(), valMap.Type().Elem())
 		}
@@ -907,6 +903,7 @@ func (d *Decoder) decodeMapFromStruct(name string, dataVal reflect.Value, val re
 
 		// If Squash is set in the config, we squash the field down.
 		squash := d.config.Squash && v.Kind() == reflect.Struct && f.Anonymous
+
 		// Determine the name of the key in the map
 		if index := strings.Index(tagValue, ","); index != -1 {
 			if tagValue[:index] == "-" {
@@ -919,8 +916,16 @@ func (d *Decoder) decodeMapFromStruct(name string, dataVal reflect.Value, val re
 
 			// If "squash" is specified in the tag, we squash the field down.
 			squash = !squash && strings.Index(tagValue[index+1:], "squash") != -1
-			if squash && v.Kind() != reflect.Struct {
-				return fmt.Errorf("cannot squash non-struct type '%s'", v.Type())
+			if squash {
+				// When squashing, the embedded type can be a pointer to a struct.
+				if v.Kind() == reflect.Ptr && v.Elem().Kind() == reflect.Struct {
+					v = v.Elem()
+				}
+
+				// The final type must be a struct
+				if v.Kind() != reflect.Struct {
+					return fmt.Errorf("cannot squash non-struct type '%s'", v.Type())
+				}
 			}
 			keyName = tagValue[:index]
 		} else if len(tagValue) > 0 {


Test patch:
diff --git a/mapstructure_bugs_test.go b/mapstructure_bugs_test.go
index 579d179..bcfc505 100644
--- a/mapstructure_bugs_test.go
+++ b/mapstructure_bugs_test.go
@@ -567,3 +567,37 @@ func TestDecode_weakEmptyStringToInt(t *testing.T) {
 		t.Errorf("expected \n%#v, got: \n%#v", expectedResultWeak, resultWeak)
 	}
 }
+
+// GH-228: Squash cause *time.Time set to zero
+func TestMapSquash(t *testing.T) {
+	type AA struct {
+		T *time.Time
+	}
+	type A struct {
+		AA
+	}
+
+	v := time.Now()
+	in := &AA{
+		T: &v,
+	}
+	out := &A{}
+	d, err := NewDecoder(&DecoderConfig{
+		Squash: true,
+		Result: out,
+	})
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if err := d.Decode(in); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// these failed
+	if !v.Equal(*out.T) {
+		t.Fatal("expected equal")
+	}
+	if out.T.IsZero() {
+		t.Fatal("expected false")
+	}
+}
diff --git a/mapstructure_test.go b/mapstructure_test.go
index ccd474c..53b2d0a 100644
--- a/mapstructure_test.go
+++ b/mapstructure_test.go
@@ -2390,26 +2390,7 @@ func TestDecode_StructTaggedWithOmitempty_KeepNonEmptyValues(t *testing.T) {
 		"visible-map":      emptyMap,
 		"omittable-map":    map[string]interface{}{"k": "v"},
 		"visible-nested":   emptyNested,
-		"omittable-nested": map[string]interface{}{
-			"Vbar": map[string]interface{}{
-				"Vbool":       false,
-				"Vdata":       interface{}(nil),
-				"Vextra":      "",
-				"Vfloat":      float64(0),
-				"Vint":        0,
-				"Vint16":      int16(0),
-				"Vint32":      int32(0),
-				"Vint64":      int64(0),
-				"Vint8":       int8(0),
-				"VjsonFloat":  float64(0),
-				"VjsonInt":    0,
-				"VjsonNumber": json.Number(""),
-				"VjsonUint":   uint(0),
-				"Vstring":     "",
-				"Vuint":       uint(0),
-			},
-			"Vfoo": "",
-		},
+		"omittable-nested": &Nested{},
 	}
 
 	actual := &map[string]interface{}{}