Go-tools: simple: flag unnecessary assignment to the blank identifier

[dominikh]

While debugging code, one (at least I do) often temporarily stops using a variable and assigns it to the blank identifier to shut up the compiler. For example:

x1, x2 := fn()
_ = x1
// use(x1)
use(x2)

As debugging continues and code is uncommented, this may result in

x1, x2 := fn()
_ = x1
use(x1)
use(x2)

with a leftover, unnecessary assignment to _. It might be desirable to flag this.

[dmitshur]

One thing that immediately comes to mind is that you’ll want to be careful about expressions that may panic. Removing them would change program behavior (potentially significantly, if there’d be no panic otherwise).

There are some false positives to avoid, such as “early bound checks”, e.g.:

https://gotools.org/encoding/binary#binary.go-L68

Otherwise, this seems like a nice simple check. (That said, personally, I don’t often find myself leaving such unnecessary assignments to blank identifier after finishing debugging or development.)

[dominikh]

We'll limit it to identifiers that are local variables; no function calls or indexing or the like. That should get rid of any false positives.

[mvdan]

I have forgotten about these a couple of times myself too. If the decision is about "is this useful", it has my thumbs up.

Perhaps print and println should be in the same boat somehow, as in general they should never be used in production code and tend to be used for debugging. But those tend to be harder to miss, as you are likely to see the debug output.