Check for reassignment before use.

[kisielk]

It would be nice to catch code such as:

var (
    x, y int
    err error
)
x, err = foo()
y, err = a(x)
return x, y

This is not an error according to the compiler: http://play.golang.org/p/3DJj2e3-8L
The upcoming ssa package would likely be useful here.

[robfig]

Thanks for the great package! We run errcheck as part of our continuous build at work.

I'm interested in getting this case covered, but it seems like it may be tough. It does seem like the best way would be to use SSA (btw, check out this ssa viewer: http://golang-ssaview.herokuapp.com/), but that seems like it would require basically starting from scratch.

Did you have any subsequent thoughts on the matter (I see this was opened long ago)? Do you think it would be preferable to implement all of errcheck using SSA anyway?

(As an aside, I wonder if it would be faster that way -- it already takes 40+ seconds to check ~20k lines of code.)

[kisielk]

Glad to hear it's working well for you :)

I have definitely thought about this some more, but rewriting the whole program using SSA just for this is a lot of work for what is likely very little gain. I'm not sure how much of a performance gain it would yield either.

40+ seconds for 20k lines seems like a long time, the Go standard library takes under 4 seconds to check on my machine.

[robfig]

The 40+ seconds happens on both Darwin & Linux, laptop and workstation, so I don't think it's a quirk of my setup.

FWIW, the Go std library checks quickly for me too. Maybe errcheck has to scan all dependencies as well, which would include standard library, plus the ~50 third-party packages that we use.

[kisielk]

Yes, I'm pretty sure the type analysis done by go.types needs to include all dependencies as well. I don't think using SSA would eliminate this since we would still need type analysis to determine which return values are errors.

[robfig]

That's too bad. In theory, it should be sufficient to look only at the signatures of functions that are directly called (the first degree dependencies). That should be much faster than constructing the AST for my entire GOPATH

[kisielk]

hm yes, that's true. I'll have to look in to it some more when I have time, I'm pretty sure when I originally wrote errcheck that wasn't possible to do.

[kisielk]

but the APIs of all the type-related packages have evolved a lot since then.

[dmage]

Also it would be nice to catch unhandled error in this example:

package main

import "net/http"

func main() {
    resp1, err := http.Get("1")
    resp1 = resp1

    resp2, err := http.Get("2")
    if err != nil {
        panic(err)
    }
    resp2 = resp2
}

[kisielk]

I agree. Will have to learn about using the SSA package to figure out how
to do this. If you have any ideas or examples any help would be welcome.
On Sun, Jul 12, 2015 at 05:41 Oleg Bulatov [email protected] wrote:

Also it would be nice to catch unhandled error in this example:

package main

import "net/http"

func main() {
resp1, err := http.Get("1")
resp1 = resp1

resp2, err := http.Get("2")
if err != nil {
    panic(err)
}
resp2 = resp2

}

—
Reply to this email directly or view it on GitHub
#7 (comment).

[mhamrle]

this tool does something similar https://github.com/gordonklaus/ineffassign . I can catch example from first comment.

[tamird]

https://github.com/dominikh/go-staticcheck catches this sort of thing, FWIW.

[echlebek]

@kisielk should we close this, given that staticcheck has it covered?

[kisielk]

I think it still falls in the scope of this package, maybe it will get added at some point

[prestonvanloon]

Here's a test case for potential reassignment: 8320fd2f