Regression (?) on SA4017 in benchmark tests

[fishy]

We could have a benchmark test to run a function repetitively just to get the benchmark data, for example:

package main

import (
	"testing"
	"time"
)

func BenchmarkParse(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			time.Parse(time.RFC3339, "2006-01-02T15:04:05Z07:00")
		}
	})
}

but staticcheck will complain with SA4017:

$ staticcheck .
go_test.go:11:4: Parse doesn't have side effects and its return value is ignored (SA4017)

My environment is:

$ staticcheck -debug.version
staticcheck 2023.1 (v0.4.0)

Compiled with Go version: go1.20
Main module:
        honnef.co/go/tools@v0.4.0 (sum: h1:lyXVV1c8wUBJRKqI8JgIpT8TW1VDagfYYaxbKa/HoL8=)
Dependencies:
        github.com/BurntSushi/toml@v1.2.1 (sum: h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=)
        golang.org/x/exp/typeparams@v0.0.0-20221208152030-732eee02a75a (sum: h1:Jw5wfR+h9mnIYH+OtGT2im5wV1YGGDora5vTv/aa5bE=)
        golang.org/x/mod@v0.7.0 (sum: h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=)
        golang.org/x/sys@v0.3.0 (sum: h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=)
        golang.org/x/tools@v0.4.1-0.20221208213631-3f74d914ae6d (sum: h1:9ZNWAi4CYhNv60mXGgAncgq7SGc5qa7C8VZV8Tg7Ggs=)
$ go version
go version go1.20 linux/amd64

this seems to be a regression from either staticcheck 2023.1, or a regression triggered by go1.20, as I don't see it complain about similar code before.

https://staticcheck.dev/docs/checks/#SA4017 says this check was there since 2017.1, but searching for github issues with SA4017 I only saw issues seemly unrelated, is this check id reused somehow?

[fishy]

Upgraded to 2023.1.1 and still got the same result:

$ staticcheck -debug.version
staticcheck 2023.1.1 (v0.4.1)

Compiled with Go version: go1.20
Main module:
        honnef.co/go/tools@v0.4.1 (sum: h1:HPeloSr0mLOEMOkhT9Au5aeki44kvP6ka3v1xIsM6Zo=)
Dependencies:
        github.com/BurntSushi/toml@v1.2.1 (sum: h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=)
        golang.org/x/exp/typeparams@v0.0.0-20221208152030-732eee02a75a (sum: h1:Jw5wfR+h9mnIYH+OtGT2im5wV1YGGDora5vTv/aa5bE=)
        golang.org/x/mod@v0.7.0 (sum: h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=)
        golang.org/x/sys@v0.3.0 (sum: h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=)
        golang.org/x/tools@v0.4.1-0.20221208213631-3f74d914ae6d (sum: h1:9ZNWAi4CYhNv60mXGgAncgq7SGc5qa7C8VZV8Tg7Ggs=)

[dominikh]

This isn't a regression, because it isn't a bug. The 2023.1 release improved SA4017 and it detects more pure functions now, including most functions in the time package.

Flagging these even in benchmarks makes sense, as a clever enough compiler could elide the call, rendering the benchmark useless. See golang/go#27400 for some discussion on that problem. The usual solution to this is to store the result of the function call in a package variable.