panic when trying to patch struct containing pointer to array of strings with nil

[jdmeyer3]

I'm having issues when I have a struct that contains a pointer to an array of string. An example of this problem

package main

import (
    "github.com/r3labs/diff/v2"
)

type MyStruct struct{
    MyArr  *[]string  `diff:"myarr"`
}

func saptr(s []string) *[]string {
	return &s
}

func main() {
    orig := MyStruct{
        MyArr: saptr([]string{"foobar"}),
    } 
    new := MyStruct{
        MyArr: nil,
    }
    
    changelog, err := diff.Diff(orig, new)
    if err != nil {
        panic(err)
    }

    _ = diff.Patch(changelog, &orig)
}

produces the panic error

panic: reflect: call of reflect.Value.Set on zero Value [recovered]
        panic: interface conversion: interface {} is *reflect.ValueError, not string

Please let know know if I'm misusing the library or I've implemented this incorrectly.

[purehyperbole]

Hi @jdmeyer3

Thanks for raising the issue. There was a bug with the way patch was setting nil values on structs. This should be fixed in v2.13.5. Please could you give it a go?

[jdmeyer3]

Thanks for the quick reply. With the example I provided it appears as though it fixed it, however there is another edge case of this that fails. When I try to patch the changes into a struct that hasn't initialized any of the values it still fails, this updated test case would look like

package main

import (
    "fmt"
    "github.com/r3labs/diff/v2"
)

type MyStruct struct{
    MyArr  *[]string  `diff:"myarr"`
}

func saptr(s []string) *[]string {
	return &s
}

func main() {
    orig := MyStruct{
        MyArr: saptr([]string{"foobar"}),
    } 
    new := MyStruct{
        MyArr: nil,
    }

    delta := MyStruct{}

    changelog, err := diff.Diff(orig, new)
    if err != nil {
        panic(err)
    }

    _ = diff.Patch(changelog, &delta)
}

I apologize for failing to recognize this test case. I did find a workaround, both cases I provided appear to work when the change_value.go:129 is changed from

t := c.target.Elem()
t.Set(reflect.Zero(t.Type()))

to

c.target.Set(reflect.Zero(c.target.Type()))

However, I can recognize that patching an empty struct with the changelog rather than the original diff'ed struct may not be within the scope of this library. Let me know whether that fix could work or if this would be out of the scope of the project.

[purehyperbole]

Hey @jdmeyer3

Thanks for providing the additional test case! Unfortunately while that fix does not cause a panic, if you check it with your original example you will find that it does not correctly update the string slice to be empty.

The issue with the second test case is that we were not handling the difference between a value that has nil assigned to a pointer vs not assigning anything, which when it comes to reflection are two completely different things apparently.

I've pushed a fix v2.13.6, please could you give it a go?