Pointers decoding inconsistency (and discussion)

When decoding a struct containing a string pointer and a struct pointer into a map, the resulting map has a pointer to the string but not a pointer to the struct.

func TestDecode_structToMap_PointersConsistency(t *testing.T) {
    type SourceChild struct {
        String string `mapstructure:"string"`
    }

    type SourceParent struct {
        ChildPtr  *SourceChild `mapstructure:"child-ptr"`
        StringPtr *string      `mapstructure:"string-ptr"`
    }

    var target map[string]interface{}

    var value = "goodbye"

    source := SourceParent{
        ChildPtr: &SourceChild{
            String: "hello",
        },
        StringPtr: &value,
    }

    if err := Decode(source, &target); err != nil {
        t.Fatalf("got error: %s", err)
    }

    expected := map[string]interface{}{
        "child-ptr": map[string]interface{}{
            "string": "hello",
        },
        "string-ptr": "goodbye",
    }

    expectedPointers := map[string]interface{}{
        "child-ptr": &map[string]interface{}{
            "string": "hello",
        },
        "string-ptr": stringPtr("goodbye"),
    }

    if !reflect.DeepEqual(target, expected) && !reflect.DeepEqual(target, expectedPointers) {
        t.Fatalf("bad: \nexpected        : %#v\nexpectedPointers: %#v\nresult          : %#v", expected, expectedPointers, target)
    }
}

Produces:

=== RUN   TestDecode_structToMap_PointersConsistency
    mapstructure_test.go:2941: bad: 
        expected        : map[string]interface {}{"child-ptr":map[string]interface {}{"string":"hello"}, "string-ptr":"goodbye"}
        expectedPointers: map[string]interface {}{"child-ptr":(*map[string]interface {})(0xc0000a8058), "string-ptr":(*string)(0xc00008eec0)}
        result          : map[string]interface {}{"child-ptr":map[string]interface {}{"string":"hello"}, "string-ptr":(*string)(0xc00008ede0)}
--- FAIL: TestDecode_structToMap_PointersConsistency (0.00s)

mapstructure version https://github.com/mitchellh/mapstructure/commit/bf980b35cac4dfd34e05254ee5aba086504c3f96 go version go1.18.7 linux/amd64

I can't see a reason why structures and base types should behave differently. I would expect to either keep the pointers or remove them. Can you please explain to me if this is a design choice or not? My guess is that struct pointers where not supported before PR #271 which made it dereference struct pointers systematically.

Discussion

Should all pointers be represented in the target map or none?

From the perspective of my use case, the pointer representation in the map is not necessary.

Here are the benefits of the pointers usage in this context:

Data representation : allows the representation of "no information". E.g. a nul pointer is different from an empty string. -> nul pointer can be represented as an unset key
Memory and speed optimization : redundent data structures can be referenced and large data structure can be referenced so that they are not copied on return -> this can be usefull when the decoded structure contains big data that should not be copied

For these reasons, I would suggest to:

Handle pointers of any type in the same way (e.g. struct pointers and string pointers)
Create a global option tho choose whether to keep or not the pointer representation in the target map (in the other direction, it's the type of the struct data that defines if it's a pointer or not). Defaults to "remove pointers". Name suggestion : "PreserveReferences"(true/false)
For retro-compatibility, add an option to keep the pointer representation on non-struct types. Eventually set this by default until next major. Name suggestion : "LegacyReferencesHandling" (true/false)
Add a property tag option to keep pointer representation and not copy data but only the pointer value (used for optimiazation in case of a large data structure, for example a string, that should be referenced in the map instead of copied). Name suggestion: "nocopy"

As an alternative, the options could be merge in a "ReferencesHandling" option with values legacy/preserve/discard.

Please let me know what you think and if there are any thechnical constraints that would not allow this. I would be happy to contribute.

mitchellh / mapstructure

Pointers decoding inconsistency (and discussion) #312

Discussion