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]

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 #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.