bodyclose icon indicating copy to clipboard operation
bodyclose copied to clipboard

false positive on first class function

Open carnott-snap opened this issue 6 years ago • 1 comments

summary

Similar to #18, when the http.Response.Body.Close method is extracted and called, bodyclose misses the call and incorrectly warns:

resp, err := http.Get("http://example.com/")
if err != nil {
        // handle error
}
f := resp.Body.Close
defer f()
body, err = ioutil.ReadAll(resp.Body)

background

While this example is so simple as to be meaningless, the core problem comes up when trying to handle the potential error that io.Closer.Close can return:

func pick(one *error, two func() error) {
        err := two()
        if one != nil && *one == nil {
                *one = err
        }
}

func call() (err error) {
        resp, err := http.Get("http://example.com/")
        if err != nil {
                // handle error
        }
        defer pick(&err, resp.Body.Close)
        _, err = ioutil.ReadAll(resp.Body)
        if err != nil {
                // handle error
        }
        return nil
}

This is resolved if you inline/duplicate the pick method, defer func() {if close := resp.Body.Close(); err == nil { err = close } }(), since now there is a direct call to resp.Body.Close(), but the polymorphism and convenience of making a function for this is paramount.

It is also worth noting that defer func(f func() error) { f() }(resp.Body.Close) also fails.

carnott-snap avatar Aug 14 '19 20:08 carnott-snap

It looks like passing the io.Closer into pick works around this issue:

func pick(one *error, two io.Closer) {
        if err := two.Close(); one != nil && *one == nil {
                *one = err
        }
}

carnott-snap avatar Feb 18 '21 02:02 carnott-snap