Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to go 1.22 #3573

Closed
jesseduffield opened this issue May 19, 2024 · 4 comments · Fixed by #3586
Closed

Upgrade to go 1.22 #3573

jesseduffield opened this issue May 19, 2024 · 4 comments · Fixed by #3586
Assignees
Labels
enhancement New feature or request

Comments

@jesseduffield
Copy link
Owner

Is your feature request related to a problem? Please describe.
go 1.22 is out and notably it updates the way that loops are handled:

Previously, the variables declared by a “for” loop were created once and updated by each iteration. In Go 1.22, each iteration of the loop creates new variables, to avoid accidental sharing bugs. The transition support tooling described in the proposal continues to work in the same way it did in Go 1.21.

This is a very good improvement: it means we don't need to say x := x on the first line of any loop in which x will be closed over by a function. The reason I'm raising this issue is because just now I had a bug because I forgot to add x := x myself!

Describe the solution you'd like
Let's upgrade to go 1.22 and go find all the places we're doing x := x in loops and delete those lines.

@jesseduffield jesseduffield added the enhancement New feature or request label May 19, 2024
@jesseduffield jesseduffield self-assigned this May 19, 2024
@kyu08
Copy link
Contributor

kyu08 commented May 22, 2024

As you may already know, by using a linter called copyloopvar, you can detect unnecessary assignments. Once the update to Go 1.22 is complete, enabling this linter would reduce the effort of having to point out these issues during PR reviews.

To enable copyloopvar:

diff --git a/.golangci.yml b/.golangci.yml
index 85a95650..5b7c5e32 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -14,6 +14,7 @@ linters:
     - exhaustive
     - makezero
     - nakedret
+    - copyloopvar
     # - goconst # TODO: enable and fix issues
   fast: false

For example, if there was unnecessary assignment, output is like this:

$ golangci-lint run -c ./.golangci.yml  ./...
pkg/app/app.go:161:3: The copy of the 'for' variable "repoDir" can be deleted (Go 1.22+) (copyloopvar)
                repoDir := repoDir
                ^

@jesseduffield
Copy link
Owner Author

Ah nice! Would you be up to raising a PR for that @kyu08 ?

@kyu08
Copy link
Contributor

kyu08 commented May 22, 2024

Sure! I'd be happy to do that. 👍

@jesseduffield
Copy link
Owner Author

Awesome, thanks

stefanhaller added a commit that referenced this issue May 24, 2024
Resolves #3573 
It looks like the update to go1.22 is done by
#3574. So it should be OK
to close #3573 when this PR is merged.

## PR Description
Add [`copyloopvar`](https://github.com/karamaru-alpha/copyloopvar) to
enabled linters to detect unnecessary assignments in `for` loops.

## Context
@jesseduffield and I were talking about necessity of this linter here.
#3573 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants