Add looppointer to Linters

[scorpionknifes]

Is your feature request related to a problem? Please describe.

scopelint is said to be obsoleted and should use looppointer and exportloopref instead.

exportloopref is already a Linter in golangci-lint but not looppointer.

Describe the solution you'd like

Add looppointer to Linters. Keep scopelint (for backward compatibility and warn the user if used)

[boring-cyborg[bot]]

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

[sayboras]

Seems related to the previous issue https://github.com/golangci/golangci-lint/issues/1041, in which the discussion leaned to exportloopref instead of looppointer.

[winmillwill]

As a user who just got bitten by a bug caused by a pointer to the loop variable and has reviewed the discussion in #1041, I prefer using looppointer. The drawback of false positives seems to be tiny:

• The additional assignment line would not cause a relevant system resource cost, so just do it.
• The additional assignment would cause a relevant system resource cost, so add the nolint:looppointer comment.

Conversely, the drawback of a false negative seems large: hard to find bugs, made all the more confusing because there's a linter that should catch the bug, and you kind of have to read the list of linters and their caveats and all the recent commits to even get to this level of confusion. To me this is a difference between small and bounded human effort to appease the linter and prevent a bug in production in the case of a false positive vs unbounded human effort in the case of a false negative, while the bug is in production causing whatever negative experience. This decision doesn't seem at all close to me, but I can appreciate that my argument may not be convincing to everyone. Similarly, I would not likely find counter-arguments convincing, so I'd just say "why not support both?".

[james-johnston-thumbtack]

I'm disappointed to find out that the pull request #1924 was apparently rejected. As an end-user, I'd rather use looppointer for all the reasons that @winmillwill gave. Taking the address of a loop variable is a code smell, and I'd rather just not allow it - period - rather than allow users to write code that takes extensive analysis to determine if it's actually safe or not. When code takes more work for a person to read to determine if it's correct, code readability suffers. Yes, "doing A action is safe because of convoluted reasons B and C and D and E" might be "correct," but code that says "doing A action is safe because it does action B" will usually be more readable IMHO.

The reasoning for rejecting pull request #1924 doesn't make sense to me. As I understand it, rejection is because the tool may actually reject some code that "runs correctly," so to speak... I hesitate to call it a "false positive" like others have, because from a code style perspective, I think it's a 100% accurate linter. I recently enabled gosec on my project, and it certainly had its own share of "false positives" as well on code that "isn't actually a security risk". So I'm not sure why a different bar for entry is being applied here.

At the moment I am staring at a deprecation notice about scopelint from golangci-lint without a good path forward (I do not agree with using exportloopref as a replacement in my case). At the end of the day, as an end user, I want the option of running a linter that might flag some "correct" code... This is something that golangci-lint generally excels at letting me do; at the moment, my alternative seems to be download/install the tool separately (along with other tools in the future that may emit false positives if this is a new rule applied more broadly...??). Linters have opinions about how code should look, and ultimately I want my tooling to have some flexibility to choose which of those opinions I agree with (something that golint lacked).

[ashanbrown]

As the author of another golangci-lint integrated linter designed simply to prevent likely, but not necessarily certain, problems (https://github.com/ashanbrown/makezero), I definitely favor the option to have "conservative" linter. That said, if exportloopref and looppointer were similar enough that they could be provided by a single linter, and this more conservative behavior were available as an option, that feels like it would be ideal to me.

[invidian]

I really agree with @james-johnston-thumbtack here. In https://github.com/kunwardeep/paralleltest/issues/8#issuecomment-872071521, I realized that I disabled scopelint because it has been deprecated and the documentation says it has a replacement, but the mentioned replaced misses some code smells. Would be nice to get this functionality back.

[sonatard]

I agree with @james-johnston-thumbtack .

If looppointer doesn't add golangci-lint, I can't stop using scopelint. because it means 'if I add a bug, I won't find it.'

I can't raise the risk of my product.

[kyoh86]

The lints detected by Exportloopref and Looppointer are very similar, but they are detected in very different ways. There are challenges to provide more precise detection in the future, so I do not plan to unify them.

In the first place, users can specify "what kind of lints they want detected" by the type of Analyzer they specify for go vet or the type of linter they specify for golangci-lint. Specifying more options for linters will only complicate the configurations and will not do any good. Linters can only be integrated if they can be processed efficiently.

[Antonboom]

Not actual after Go 1.22 (or Go 1.21 + GOEXPERIMENT=loopvar)?

https://github.com/golang/go/wiki/LoopvarExperiment

(cc) @ldez

[ldez]

@Antonboom is right, Go 1.22 will definitively remove this behavior.