go: proposal: cmd/vet: time.Time should not compared

background

Time is hard and time.Time is really great in most all cases, but between the two issues (#45958 #45960) I came across recently, it seems like anything that does a runtime equality check is probably a bug. Please let me know if there are cases where comparing time is good, safe, and necessary.

summary

As such, it seems reasonable to add a vet check to disallow using time.Time like so:

if time.Time{} == time.Time{} {}
map[time.Time]struct{}{{}: {}}

alternatives

I support enforcing this via the complier, but I get the impression that is a non starter:

package time

type Time struct {
	_    [0]func()
	wall uint64
	ext  int64
	loc  *Location
}

We could also add a new symbol that is intentionally comparable, say time.Comparable, but it would be somewhat confusing when to use which one, and that would probably involve a go vet rule too.

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 9
  • Comments: 19 (13 by maintainers)

Most upvoted comments

if we are not pursuing this avenue, what is the suggested resolution to the meta issue “time.Time is hard to use”? I am happy to start an out of band discussion or look into alternatives, but it suggesting people rtfm does not appear to be working.

if we feel this check is too false positive prone, I think moving it to StaticCheck is reasonable

Note that Staticcheck isn’t the right home for checks with false positives, either.

The next major release of Staticcheck will, when running inside gopls, emit a suggestion to use time.Time.Equal instead of ==, but it will be a suggestion, not a warning or error, and it will not be emitted when running Staticcheck standalone. It is merely a suggestion to look at the code carefully. image

The docs make clear that it is perfectly fine to use time.Time with == and in map keys provided you do it correctly. I don’t see how a checker can tell whether you are doing it correctly, and if it can’t, then it will have too many false positives (or no reports).