Forbid captured loop variables

[max-melentyev]

Hey, what do you think about the mode that forbid capturing loop variables to prevent errors like this:

package main

import (
	"fmt"
)

func main() {
	x := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 0}
	fs := []func(){}
	for _, y := range x {
		// y := y
		fs = append(fs, func() { fmt.Println(y) })
	}
	for _, f := range fs {
		f()
	}
}

Though it won't be necessary when go 1.22 is out: https://go.dev/blog/loopvar-preview

[kyoh86]

I have not found a realistic analysis to detect only the cases that are definitely problematic from the kind of code you have presented. The ASTs require a very deep search to determine in what context loop variables are handled in a function.

Therefore, I created exportloopref as a linter that can detect only those cases that are definitely problematic in a somewhat lightweight way. We also created looppointer as a linter that cannot reliably determine if there is a problem, but can detect all suspicious cases.

Both of these linters are designed to detect the same problem of forgetting to capture loop variables, but they have different detection policies. looppointer will also report the loop variables that should be captured for the code you have presented here. (I haven't checked, so maybe I'm wrong) However, as mentioned above, looppointer also warns to capture "suspicious but not inherently problematic" cases, so there may be situations where you should ignore them by using nolint or other methods. Each developer is free to choose which to use.

I'm hoping that with Go 1.22, this problem will no longer occur. I hope either looppointer or exportloopref will be useful to you in the short time before Go 1.22 is released.

[max-melentyev]

Thanks for details. I've tried looppointer and it didn't detect this kind of issues. From its readme I understand that it also only warns about referenced loop variables. For my code I'd prefer to have linter error for every loop var captured by closure. I understand that it may give false positives when function is synchronous and invoked immediately, but I think it's a reasonable tradeoff for avoiding bugs.

Anyway, I appreciate your work on existing implementation, and I don't know how much effort it'll require to add support for this case. Especially considering that the issue may be fixed in go in 4 months. Please feel free to close the issue as a won't-fix.

PS. I've updated example and remove async code.

[kyoh86]

In Go 1.22, it is not a problem