awalterschulze / goderive

Derives and generates mundane golang functions that you do not want to maintain yourself
Apache License 2.0
1.23k stars 44 forks source link

fix for issue #58 #59

Closed deosjr closed 3 years ago

deosjr commented 3 years ago

Fixes #58

deriveKeys now calls .Underlying() on the maptype when checking if applicable

deosjr commented 3 years ago

One thing I noticed while working on this: when deriving sort(keys(map)) we get a warning but succeed anyways. Has nothing to do with type aliases; is this expected behaviour?

$ goderive ./...
could not yet generate: deriveSorted(deriveKeys(y))
awalterschulze commented 3 years ago

I think we do need to add a test https://github.com/awalterschulze/goderive/blob/master/test/normal/sortedkeys_test.go to make sure we do not regress.

awalterschulze commented 3 years ago

On the warning: Yes I think the idea was that the compiler would give you an error, if the function still does not exist after generation. Maybe I was too afraid to commit to giving an error on this. Struggling to remember my reasoning. I really should write more design decision documents.

deosjr commented 3 years ago

The thing is though, the function does exist after generation... I think the warning is given when deriveKeys is not generated yet, but it backtracks upon completion of that derive and then succesfully derives deriveSorted anyways. Just adds some confusion, but not really harmful

awalterschulze commented 3 years ago

Wait is this happening for the test you are trying to write!? and then it doesn't generate the deriveKeys function? which map are you trying to alias? Are you doing it in that sortedkeys_test.go file? Are there other maps it could be conflicting with?

deosjr commented 3 years ago

It happened when trying to recreate the alias issue before I started fixing it. I added a test now in this PR, which succeeds but does log the warning. Here is how you can reproduce the 'issue' locally:

package main

import (
    "fmt"
)

type X struct {}

type Y map[string]*X

func test(y Y) {
    for _, k := range deriveSorted(deriveKeys(y)) {
        fmt.Println(y[k])
    }
}

func main() {
    y := map[string]*X{}
    test(y)
    fmt.Println("Succes")
}

Put that in a go file somewhere and run goderive ./.... You will see the warning

could not yet generate: deriveSorted(deriveKeys(y))

but afterwards you can inspect derived.gen.go and it will have derived both functions just fine.

awalterschulze commented 3 years ago

Aha I looked at the code and maybe this warning should be silenced or rather read could not yet generate, trying again

https://github.com/awalterschulze/goderive/blob/544f7cf4181e572572bf50080219b1b63bf9d4ba/derive/generate.go#L296

Because there is clearly later a point, where the error is returned if it really could not generate

https://github.com/awalterschulze/goderive/blob/544f7cf4181e572572bf50080219b1b63bf9d4ba/derive/generate.go#L334

So this just looks like debugging info that could be deleted. Good catch :)