Add support for the new behaviour of for loops in go 1.22.

[dominiquelefevre]

Go 1.22 has changed the behaviour of for loops. Every iteration makes new loop variables. It is now safe to take their addresses because they are guaranteed to be unique. Similarly, it is now safe to capture loop variables in functions.

[dominiquelefevre]

Hi, is anybody out here to take a look at the patch? @denisvmedia, @chavacava?

[chavacava]

Hi @dominiquelefevre, thanks for the PR ! I' ll try to review before the weekend.

[chavacava]

Thanks @dominiquelefevre !

[dominiquelefevre]

@chavacava could you please tag a new release so that I can make a PR to golangci-lint to use the up-to-date revive?

[ccoVeille]

Let me step out the data race fix for a moment and go back to the initial needs described in the ticket

How different is the feature brought by this PR from https://github.com/karamaru-alpha/copyloopvar created by @karamaru-alpha

And added to golangci-lint via https://github.com/golangci/golangci-lint/pull/4382

I'm glad so see it added to revive if it's the same feature but it could have consequence for copyloopvar or golangci projects maybe

[dominiquelefevre]

Before go 1.22, all iterations of a loop shared their loop variables. If you created a function inside the loop, all those functions would capture the same one variable. Hence, if you iterated over a collection and wanted to create a function for every item in a collection, you needed to make copies of loop variables manually.

Starting with 1.22, the go compiler does that for you. Thus,

1. capturing loop variables directly no longer introduces aliasing.
2. making copies manually is no longer needed,

This PR addresses 1 and removes false reports about errors introduced by aliasing. Copyloopvar addresses 2 and warns about extraneous copies.

[ccoVeille]

Thank you @dominiquelefevre it's way clearer now.

[ldez]

@chavacava This PR will create a performance regression because go list is expensive and will be called on each package (even when range-val-address is not used).

"Funny thing", the regression impacts all the rules except range-val-address because this rule is skipped with go1.22.

It will be a performance regression for revive and for golangci-lint. For golangci-lint, a way to define the Go version externally to bypass the go list is a solution. I don't know enough about the revive design to propose a solution for revive.

We encountered the same problem with gosec https://github.com/golangci/golangci-lint/issues/4735

log the call to detectGoVersion on revive's code

```console $ cd revive $ ./revive ./... detectGoVersion detectGoVersion detectGoVersion detectGoVersion detectGoVersion detectGoVersion detectGoVersion detectGoVersion detectGoVersion detectGoVersion detectGoVersion rule/datarace.go:83:3: var getIds should be getIDs lint/file.go:191:22: parameter 'filename' seems to be unused, consider removing or renaming it as _ ```

benchmark on kubernetes

```toml # sample.toml ignoreGeneratedHeader = false severity = "warning" confidence = 0.8 errorCode = 0 warningCode = 0 #[rule.range-val-address] [rule.context-as-argument] ``` ```console $ cd kubernetes $ rm go.work go.work.sum $ hyperfine './revive-bbe5eb7 -config sample.toml ./...' './revive-4242f24 -config sample.toml ./...' Benchmark 1: ./revive-bbe5eb7 -config sample.toml ./... Time (mean ± σ): 1.989 s ± 0.125 s [User: 11.202 s, System: 1.519 s] Range (min … max): 1.796 s … 2.220 s 10 runs Benchmark 2: ./revive-4242f24 -config sample.toml ./... Time (mean ± σ): 3.880 s ± 0.171 s [User: 22.235 s, System: 14.092 s] Range (min … max): 3.633 s … 4.193 s 10 runs Summary ./revive-bbe5eb7 -config sample.toml ./... ran 1.95 ± 0.15 times faster than ./revive-4242f24 -config sample.toml ./... ``` ```console $ cd revive $ git lg * 4242f24 - Add support for the new implementation of for loop variables in go 1.22. (#993) * bbe5eb7 - fix(deps): update module github.com/burntsushi/toml to v1.4.0 (#992) ... ``` With the commit, the run of `context-as-argument` (just an example) is about 2x slower than the previous commit (and about 10x slower on the system).

[ldez]

Also, the implementation has a bug with Go workspaces:

can't parse the output of go list: invalid character '{' after top-level value

Inside a Go workspace, go list always returns all the modules, not just the current module.

As a reference, a working implementation: https://github.com/golangci/modinfo/blob/main/module.go

```console $ cd kubernetes $ go list -m -json { "Path": "k8s.io/kubernetes", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes", "GoMod": "/home/ldez/sources/experimental/kubernetes/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/api", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/api", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/api/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/apiextensions-apiserver", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiextensions-apiserver", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiextensions-apiserver/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/apimachinery", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apimachinery", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apimachinery/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/apiserver", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiserver", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiserver/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/cli-runtime", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cli-runtime", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cli-runtime/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/client-go", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/client-go", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/client-go/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/cloud-provider", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cloud-provider", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cloud-provider/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/cluster-bootstrap", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cluster-bootstrap", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cluster-bootstrap/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/code-generator", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/code-generator", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/code-generator/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/component-base", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-base", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-base/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/component-helpers", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-helpers", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-helpers/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/controller-manager", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/controller-manager", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/controller-manager/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/cri-api", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-api", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-api/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/cri-client", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-client", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-client/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/csi-translation-lib", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/csi-translation-lib", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/csi-translation-lib/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/dynamic-resource-allocation", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/dynamic-resource-allocation", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/dynamic-resource-allocation/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/endpointslice", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/endpointslice", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/endpointslice/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/kms", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kms", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kms/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/kube-aggregator", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-aggregator", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-aggregator/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/kube-controller-manager", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-controller-manager", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-controller-manager/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/kube-proxy", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-proxy", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-proxy/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/kube-scheduler", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-scheduler", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-scheduler/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/kubectl", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubectl", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubectl/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/kubelet", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubelet", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubelet/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/metrics", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/metrics", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/metrics/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/mount-utils", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/mount-utils", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/mount-utils/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/pod-security-admission", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/pod-security-admission", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/pod-security-admission/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/sample-apiserver", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-apiserver", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-apiserver/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/sample-cli-plugin", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-cli-plugin", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-cli-plugin/go.mod", "GoVersion": "1.22.0" } { "Path": "k8s.io/sample-controller", "Main": true, "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-controller", "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-controller/go.mod", "GoVersion": "1.22.0" } ```

FYI, the issue comment (https://github.com/golang/go/issues/44753#issuecomment-790089020), used as a reference inside the PR, is outdated since Go workspace (go1.18).

[ldez]

I will open a dedicated issue. -> https://github.com/mgechev/revive/issues/995