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

Zero value for bool is false #48

Closed jeffbski closed 5 years ago

jeffbski commented 5 years ago

Zero value for bool should be false

https://tour.golang.org/basics/12

I had an error from a generated deriveComposeFooBar because the final function returned (bool, error) and the derived fn was returning 0, err.

Here is example code that will fail without the fix.

func foo() (string, error) {
    return "", fmt.Errorf("somethings wrong")
}

func bar(string) (bool, error) {
    return true, nil
}

cat, err := deriveComposeFooBar(
    foo,
    bar,
)()

it generates:

func deriveComposeFooBar(f0 func() (string, error), f1 func(string) (bool, error)) func() (bool, error) {
    return func() (bool, error) {
        v_1_0, err0 := f0()
        if err0 != nil {
            return 0, err0
        }
        v_2_0, err1 := f1(v_1_0)
        if err1 != nil {
            return 0, err1
        }
        return v_2_0, nil
    }
}

when what we really should have is:

func deriveComposeFooBar(f0 func() (string, error), f1 func(string) (bool, error)) func() (bool, error) {
    return func() (bool, error) {
        v_1_0, err0 := f0()
        if err0 != nil {
            return false, err0
        }
        v_2_0, err1 := f1(v_1_0)
        if err1 != nil {
            return false, err1
        }
        return v_2_0, nil
    }
}

I couldn't get the project to build so I wasn't sure how to add a test, so hopefully you can go from what I have here to complete the fix.

PS. I really like the project!

jeffbski commented 5 years ago

oops I had wrong src pushed, it is corrected now.

awalterschulze commented 5 years ago

Thanks this looks great. Don't know how I missed that :)

May I ask what error you got when you tried to build? Did you run make?

Also if you are using it in a project, it would be great if you could add the project to the users list.

awalterschulze commented 5 years ago

You should be able to add a test here

https://github.com/awalterschulze/goderive/blob/master/test/normal/compose_test.go

To be fair I should update this document to explain how to build and how to contribute a fix. https://github.com/awalterschulze/goderive/blob/master/Contributing.md

jeffbski commented 5 years ago

The error I got when trying to build was this. I am using the latest Go v 1.11

go install . make -C test test cd normal && make test rm gostring_gen_test.go || true rm: gostring_gen_test.go: No such file or directory goderive ./... gofmt -d . derived.gen.go:11938:43: missing ',' in parameter list make[2]: [gofmt] Error 2 make[1]: [test] Error 2 make: *** [test] Error 2

awalterschulze commented 5 years ago

Could you provide the function that was generated at derived.gen.go:11938:43 ?

awalterschulze commented 5 years ago

I just installed go v1.11 and it all works for me

Can you also run which gofmt ?

jeffbski commented 5 years ago

I think I am missing vendortest.AVendoredObject, how do I install that?

// deriveEqual_86 returns whether this and that are equal.

-func deriveEqual_86(this, that []*vendortest.AVendoredObject) bool {
+func deriveEqual_86(this, that []*invalid type) bool {

Error occurs because it generated invalid type instead of vendortest.AVendoredObject

jeffbski commented 5 years ago

I can do the make using your directory but as soon as I try to do it from mine then it fails to find vendortest, even though the files are all there.

I was able to add a test and cp the files to your directory to perform the test, yuck, but it worked.

I added a PR for the test #49

awalterschulze commented 5 years ago

I can do the make using your directory but as soon as I try to do it from mine then it fails to find vendortest, even though the files are all there.

Ah yes, I am quite bad at making make work in folders where make is not located.

It totally worked, thank you.