Go: proposal: cmd/vet: vet should warn when time.Time type (or types embed it) is used as map keys.

Created on 4 Dec 2017  ·  3Comments  ·  Source: golang/go

What version of Go are you using (go version)?

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

yes

What did you do?

package main

import "time"

type T struct {
    time.Time
}

var a map[T]string

func main() {
}

$ go vet a.go
$ [nothing reported]

What did you expect to see?

$ go vet a.go
$ time.Time is not a good map key type.

Like time.Time, there may be some other types, which are not capable to be used as map key types.

What did you see instead?

nothing reported

FrozenDueToAge Proposal

Most helpful comment

I don't think this meets the bar for a vet check.

It's perfectly valid to use a time.Time as a map key, if you know what you're doing. I think it would be too noisy for valid code.

But somebody could do some analysis on GitHub to see what percentage of time.Time-keyed maps are valid.

All 3 comments

I don't think this meets the bar for a vet check.

It's perfectly valid to use a time.Time as a map key, if you know what you're doing. I think it would be too noisy for valid code.

But somebody could do some analysis on GitHub to see what percentage of time.Time-keyed maps are valid.

I wrote a linter that detects this type of issue if you're interested: https://github.com/m3db/build-tools/blob/master/linters/badtime/README.md

I'm trying to get it integrated with gometalinter right now.

Now that vet is part of go test, the bar is even higher for new checks. Because it _is_ valid sometimes to have a time.Time as a map key, rejecting it in vet is not OK.

You might talk to @dominikh about adding this to megacheck or one of the other linters.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

OneOfOne picture OneOfOne  ·  3Comments

longzhizhi picture longzhizhi  ·  3Comments

Miserlou picture Miserlou  ·  3Comments

myitcv picture myitcv  ·  3Comments

ianlancetaylor picture ianlancetaylor  ·  3Comments