[Feature idea] Detect inneffectual iteration variable pointer assignment

[soypat]

Issue is explained with examples here: https://github.com/golang/go/issues/43565.

Basically: if you are iterating using a variable and assigning its pointer to another thing then the values assigned al point to the last value v took in the iteration! Very tricky bug!

for _, v := range x {
    y = append(y, &v) // y will be a slice of the same pointer! Very likely not what you want
}

[gordonklaus]

Thanks for the suggestion. This sort of thing has tripped up many a Gopher and it would be great if we could detect it. Unfortunately, it's outside the scope of this tool.

This tool detects assignments that are guaranteed to be always ineffectual. In your example, however, the final assignment to v is always effectual. In particular, if len(x) is 0 or 1, there are no ineffectual assignments. So we would have to statically know len(x) in order to be able to avoid raising a false positive, and that kind of analysis is definitely beyond the scope of this tool.

Additionally, there are legitimate uses that we don't want the tool to flag. It can be convenient to assign the current element to a variable that is not used until after the loop, just to access the last element that was encountered by the loop. This makes the most sense in the context of iterating over a map because the iteration order is random.

(Also, strictly speaking, in your example all bets are off because you take the address of the loop variable. When this tool encounters &x (or x.f, which can implicitly take the address of x, if f is a method with a pointer receiver) it considers x to have escaped and gives up tracking it beyond that point. The reason being that, once a value escapes it can be used from another part of the program, which can be arbitrarily difficult to determine. A full pointer analysis is needed to determine all possible flows of data, which is very expensive to perform.)

I would be surprised if someone hasn't made a linter for this. I don't see anything in https://staticcheck.io/ but that would be a good place for it to exist. Something as simple as checking for taking the address of a loop variable would capture some cases; but there's the question of whether it would raise too many false positives.

[soypat]

Alright, thanks for the detailed answer! I'll check staticcheck out.