cmd/vet: time.Since should not be used in defer statement

[devoxel]

Background

func main() {
	start := time.Now()
	defer fmt.Println(time.Since(start))
	time.Sleep(1 * time.Second)
}

People unfamiliar to the intricacies of the defer statement might expect that this code will print 1s, but it prints 0s. (See on go playground).

A correct version is:

func main() {
	start := time.Now()
	defer func() { fmt.Println(time.Since(start)) }()
	time.Sleep(1 * time.Second)
}

This is expected behaviour, but seems to be a somewhat common gotcha. I've made the mistake on a few occasions. I did a search with github codesearch and I found several instances where this is happening in public repositories, some examples:

• gravitational/teleport
• GoogleCloudPlatform/prometheus
• huawei-cloudnative/kubernetes

Summary

I can't think of many use-cases where one calls defer f(time.Since(t)) and intends the behaviour to work as it does. I propose that a vet check is added to disallow this case.

I have implemented an analyser that does this already BTW but the guidelines indicate I should open an issue first for discussion.

[adonovan]

A quick search for defer\ [^{]*time.Since in Google's corpus turned up about 70 hits nearly all of which look wrong. Typically something like:

  defer recordLatency(time.Since(start))

[rsc]

I looked into this in the public corpus and found tons of hits for time.Since called as an argument to a deferred call; all of them seemed wrong. Are there other such functions that are worth checking and commonly misused?

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/499875 mentions this issue: go/analysis/passes/defermistake: add analyser for defer mistake

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/502975 mentions this issue: gopls/internal/lsp/source: enable new defers analyzer

[devoxel]

Hey this is has been in gopls for a month now, @alandonovan should I open a change adding this to vet?

[adonovan]

Hey this is has been in gopls for a month now, @alandonovan should I open a change adding this to vet?

Yes, that would be great. Thanks!

[gopherbot]

Change https://go.dev/cl/527095 mentions this issue: cmd/vet: add defers analysis pass