cmd/vet: flag atomic.Value usages with interface types

[dsnet]

Consider the following:

var v atomic.Value
var err error

err = &http.ProtocolError{}
v.Store(err)
err = io.EOF
v.Store(err)

The intention to have a atomic value store for errors. However, running this code panics:

panic: sync/atomic: store of inconsistently typed value into Value

This is because atomic.Value requires that the underlying concrete type be the same (which is a reasonable expectation for its implementation). When going through the atomic.Value.Store method call, the fact that both these are of the error interface is lost.

Perhaps we should add a vet check that flags usages of atomic.Value where the argument passed in is an interface type?

Vet criterions:

• frequency: not sure, I haven't done an analysis through all Go corpus.
• correctness: this is almost always wrong. Any "correct" usages should type assert to the concrete value first.
• accuracy: if the type information available can conclusively show an argument is an interface type, then very accurate.

\cc @robpike \cc @dominikh for staticcheck

[robpike]

Deleted an incorrect reference from a CL to help others avoid the confusion it caused me.

I suspect the frequency is very low, and since the runtime catches the error anyway, and is likely to do so the first time the program is run, some convincing is required.

[agnivade]

/cc @mvdan @alandonovan

[alandonovan]

Seems very obscure. atomic.Value is not an everyday or (everyyear) thing.

[mvdan]

Here's a self-contained reproducer: https://play.golang.org/p/mM_n7IQFJ0p

I am also unsure that this passes the frequency requirement. I've only used atomic.Value a handful of times, and I've never encountered this bug.

I've also quickly put together a gogrep query to find these, and to verify that std cmd golang.org/x/tools/... has zero occurrences. I realise tha the query is a bit silly; I need to refine the tool.

$ gogrep -x '$v.Store($x)' -x '$v' -a 'type(atomic.Value)' -p 2 -x '$x' -a 'is(interface)' -p 2 f.go
/home/mvdan/f.go:17:2: v.Store(err)
/home/mvdan/f.go:19:2: v.Store(err)

Perhaps the query can be used on a large corpus of Go code, to check if any matches would be found.