Fix decoding of slices

[Gilthoniel]

Without this fix, it could happen that a structure with an already filled slice will keep the values alongside with the decoded ones.

[ineiti]

There is one corner-case that is not caught in this PR. Given a protobuf-message like:

message Nasty {
  repeated int32 first = 1;
  repeated int32 second = 2;
}

Now if the protobuf-message contains the following elements:

  first[0]
  second[0]
  first[1]
  second[1]

your fix will still fail.

Also, you will not reset fields where there was no element in the protobuf-message at all. Go's protobuf-implementation does the following:

https://godoc.org/github.com/golang/protobuf/proto#Unmarshal

The Unmarshal always resets the whole structure, while the UnmarshalMerge does what the protobuf.Decode prior to your fix does.

I propose that you fully reset the structure in protobuf.Decode, setting all slice-lengths to 0, and putting all pointers to nil. Then you don't need the prevFieldi variable anymore.

[Gilthoniel]

Correct, that's a much better approach.

[Gilthoniel]

So I was hoping to clear a struct at the top level but quickly found issues with interfaces as they can't be instantiate if we clear them so I went for a different where any struct detected in the decode input is cleared field by field except the interfaces.

[jeffallen]

I had written this: "This fix seems right but fragile, because what if the "root" item that they ask to decode into is a slice of non-struct items?" Then I wrote some test code and convinced myself that passing in pointers to slices never worked before anyway. For example, this code fails with reflect: NumField of non-struct type on the Encode() because the argument to encode is not a pointer to a struct, but a pointer to a slice of structs. (And the same thing happens, of course, if you replace []T with []int).

func TestSlice(t *testing.T) {
    type T struct {
        IntList []int
    }
    t1 := &[]T{{IntList: []int{1, 2, 3}}}
    t2 := &[]T{{IntList: []int{4, 5, 6}}}

    buf, err := Encode(t1)
    require.NoError(t, err)

    err = Decode(buf, t2)
    require.NoError(t, err)

    require.Equal(t, t1, t2)
}

So, ship it! (And thanks for making me understand this package more, even years after starting to hack on it.)

[ineiti]

And thanks for making me understand this package more, even years after starting to hack on it

+1 here ;)