chore: Remove redundant loop captures (#62264)

varungandhi-src · web-flow · commit 17a0a6461c81 · 2024-04-30T07:57:21.000-06:00
Go 1.22 changes loop variables to have more sensible
semantics where the variable is not reused across iterations
by the codegen, so we can simplify a bunch of code
working around that counterintuitive behavior.
diff --git a/BUILD.bazel b/BUILD.bazel
@@ -293,7 +293,6 @@ nogo(
         "//conditions:default": [
             "//dev/linters/bodyclose",
             "//dev/linters/depguard",
-            "//dev/linters/exportloopref",
             "//dev/linters/forbidigo",
             "//dev/linters/gocheckcompilerdirectives",
             "//dev/linters/gocritic",
diff --git a/cmd/frontend/graphqlbackend/codeintel.go b/cmd/frontend/graphqlbackend/codeintel.go
@@ -24,8 +24,7 @@ func NewCodeIntelResolver(resolver *resolverstubs.Resolver) *Resolver {
 
 func (r *Resolver) NodeResolvers() map[string]NodeByIDFunc {
 	m := map[string]NodeByIDFunc{}
-	for name, f := range r.Resolver.NodeResolvers() {
-		resolverFunc := f // do not capture loop variable
+	for name, resolverFunc := range r.Resolver.NodeResolvers() {
 		m[name] = func(ctx context.Context, id graphql.ID) (Node, error) {
 			return resolverFunc(ctx, id)
 		}
diff --git a/dev/linters/exportloopref/BUILD.bazel b/dev/linters/exportloopref/BUILD.bazel
diff --git a/dev/linters/exportloopref/exportloopref.go b/dev/linters/exportloopref/exportloopref.go
diff --git a/dev/linters/go.mod b/dev/linters/go.mod
@@ -10,7 +10,6 @@ require (
 	github.com/gobwas/glob v0.2.3
 	github.com/gordonklaus/ineffassign v0.0.0-20230107090616-13ace0543b28
 	github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db
-	github.com/kyoh86/exportloopref v0.1.11
 	github.com/sourcegraph/sourcegraph/lib v0.0.0-20231122233253-1f857814717c
 	github.com/timakin/bodyclose v0.0.0-20221125081123-e39cf3fc478e
 	golang.org/x/tools v0.15.0
diff --git a/dev/linters/go.sum b/dev/linters/go.sum
@@ -105,8 +105,6 @@ github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
-github.com/kyoh86/exportloopref v0.1.11 h1:1Z0bcmTypkL3Q4k+IDHMWTcnCliEZcaPiIe0/ymEyhQ=
-github.com/kyoh86/exportloopref v0.1.11/go.mod h1:qkV4UF1zGl6EkF1ox8L5t9SwyeBAZ3qLMd6up458uqA=
 github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/lib/pq v1.1.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
diff --git a/dev/sg/internal/generate/golang/golang.go b/dev/sg/internal/generate/golang/golang.go
@@ -220,9 +220,6 @@ func runGoGenerateOnPaths(ctx context.Context, pkgPaths []string, progressBar bo
 	)
 
 	for _, pkgPath := range pkgPaths {
-		// Do not capture loop variable in goroutine below
-		pkgPath := pkgPath
-
 		p.Go(func(ctx context.Context) error {
 			file := filepath.Base(pkgPath) // *.go
 			directory := filepath.Dir(pkgPath)
diff --git a/internal/codeintel/policies/matcher.go b/internal/codeintel/policies/matcher.go
@@ -243,13 +243,9 @@ func (m *Matcher) matchCommitsOnBranch(ctx context.Context, context matcherConte
 					continue policyLoop
 				}
 
-				// Don't capture loop variable pointers
-				localPolicyID := policyID
-				commitDate := commitDate
-
 				context.commitMap[commit] = append(context.commitMap[commit], PolicyMatch{
 					Name:           branchName,
-					PolicyID:       &localPolicyID,
+					PolicyID:       &policyID,
 					PolicyDuration: policyDuration,
 					CommittedAt:    commitDate,
 				})
@@ -279,10 +275,9 @@ func (m *Matcher) matchCommitPolicies(ctx context.Context, context matcherContex
 				continue
 			}
 
-			id := policy.ID // avoid a reference to the loop variable
 			context.commitMap[policy.Pattern] = append(context.commitMap[policy.Pattern], PolicyMatch{
 				Name:           string(commit.ID),
-				PolicyID:       &id,
+				PolicyID:       &policy.ID,
 				PolicyDuration: policyDuration,
 				CommittedAt:    commit.Committer.Date,
 			})
diff --git a/internal/database/migration/definition/definition.go b/internal/database/migration/definition/definition.go
@@ -321,8 +321,7 @@ func (ds *Definitions) traverse(targetIDs []int, next func(definition Definition
 			}
 
 			for _, id := range next(definition) {
-				nodeID := n.id // avoid referencing the loop variable
-				newFrontier = append(newFrontier, node{id, &nodeID})
+				newFrontier = append(newFrontier, node{id, &n.id})
 			}
 		}
 
diff --git a/internal/embeddings/similarity_search.go b/internal/embeddings/similarity_search.go
@@ -107,8 +107,6 @@ func (index *EmbeddingIndex) SimilaritySearch(
 	if len(rowsPerWorker) > 1 {
 		var wg conc.WaitGroup
 		for workerIdx := range len(rowsPerWorker) {
-			// Capture the loop variable value so we can use it in the closure below.
-			workerIdx := workerIdx
 			wg.Go(func() {
 				heaps[workerIdx] = index.partialSimilaritySearch(query, numResults, rowsPerWorker[workerIdx], opts)
 			})
diff --git a/internal/updatecheck/handler.go b/internal/updatecheck/handler.go
@@ -515,10 +515,7 @@ func reserializeNewCodeIntelUsage(payload json.RawMessage) (json.RawMessage, err
 	}
 
 	countsByLanguage := make([]jsonCodeIntelRepositoryCountsByLanguage, 0, len(codeIntelUsage.CountsByLanguage))
-	for language, counts := range codeIntelUsage.CountsByLanguage {
-		// note: do not capture loop var by ref
-		languageID := language
-
+	for languageID, counts := range codeIntelUsage.CountsByLanguage {
 		countsByLanguage = append(countsByLanguage, jsonCodeIntelRepositoryCountsByLanguage{
 			LanguageID:                            &languageID,
 			NumRepositoriesWithUploadRecords:      counts.NumRepositoriesWithUploadRecords,
@@ -545,9 +542,6 @@ func reserializeNewCodeIntelUsage(payload json.RawMessage) (json.RawMessage, err
 
 	languageRequests := make([]jsonLanguageRequest, 0, len(codeIntelUsage.LanguageRequests))
 	for _, request := range codeIntelUsage.LanguageRequests {
-		// note: do not capture loop var by ref
-		request := request
-
 		languageRequests = append(languageRequests, jsonLanguageRequest{
 			LanguageID:  &request.LanguageID,
 			NumRequests: &request.NumRequests,
diff --git a/lib/batches/env/var.go b/lib/batches/env/var.go
@@ -51,7 +51,6 @@ func (v *variable) UnmarshalJSON(data []byte) error {
 
 	for k, value := range kv {
 		v.name = k
-		//nolint:exportloopref // There should only be one iteration, so the value of `value` should not change
 		v.value = &value
 	}
 
@@ -78,7 +77,6 @@ func (v *variable) UnmarshalYAML(unmarshal func(any) error) error {
 
 	for k, value := range kv {
 		v.name = k
-		//nolint:exportloopref // There should only be one iteration, so the value of `value` should not change
 		v.value = &value
 	}
 
diff --git a/linter_deps.bzl b/linter_deps.bzl
@@ -148,13 +148,6 @@ def linter_dependencies():
         sum = "h1:6WHiuFL9FNjg8RljAaT7FNUuKDbvMqS1i5cr2OE2sLQ=",
     )
 
-    go_repository(
-        name = "com_github_kyoh86_exportloopref",
-        importpath = "github.com/kyoh86/exportloopref",
-        version = "v0.1.11",
-        sum = "h1:1Z0bcmTypkL3Q4k+IDHMWTcnCliEZcaPiIe0/ymEyhQ=",
-    )
-
     go_repository(
         name = "co_honnef_go_tools",
         importpath = "honnef.co/go/tools",
diff --git a/nogo_config.json b/nogo_config.json
@@ -46,14 +46,5 @@
       ".*_pb\\.go$": "ignore protobuff generated code",
       "dev/buildchecker/slack\\.go$": "ignore false positive"
     }
-  },
-  "exportloopref": {
-    "exclude_files": {
-      "lib/batches/env/var\\.go$": "false positive",
-      "rules_go.*/.*": "ignore rules_go working directory",
-      "external/.*": "no need to vet third party code",
-      ".*_generated\\.go$": "ignore generated code",
-      ".*_pb\\.go$": "ignore protobuff generated code"
-    }
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -24,8 +24,7 @@ func NewCodeIntelResolver(resolver resolverstubs.Resolver) Resolver {`
`24`	`24`
`25`	`25`	`func (r *Resolver) NodeResolvers() map[string]NodeByIDFunc {`
`26`	`26`	`m := map[string]NodeByIDFunc{}`
`27`		`- for name, f := range r.Resolver.NodeResolvers() {`
`28`		`- resolverFunc := f // do not capture loop variable`
	`27`	`+ for name, resolverFunc := range r.Resolver.NodeResolvers() {`
`29`	`28`	`m[name] = func(ctx context.Context, id graphql.ID) (Node, error) {`
`30`	`29`	`return resolverFunc(ctx, id)`
`31`	`30`	`}`
Original file line number	Diff line number	Diff line change
`@@ -220,9 +220,6 @@ func runGoGenerateOnPaths(ctx context.Context, pkgPaths []string, progressBar bo`
`220`	`220`	`)`
`221`	`221`
`222`	`222`	`for _, pkgPath := range pkgPaths {`
`223`		`- // Do not capture loop variable in goroutine below`
`224`		`- pkgPath := pkgPath`
`225`		`-`
`226`	`223`	`p.Go(func(ctx context.Context) error {`
`227`	`224`	`file := filepath.Base(pkgPath) // *.go`
`228`	`225`	`directory := filepath.Dir(pkgPath)`
Original file line number	Diff line number	Diff line change
`@@ -321,8 +321,7 @@ func (ds *Definitions) traverse(targetIDs []int, next func(definition Definition`
`321`	`321`	`}`
`322`	`322`
`323`	`323`	`for _, id := range next(definition) {`
`324`		`- nodeID := n.id // avoid referencing the loop variable`
`325`		`- newFrontier = append(newFrontier, node{id, &nodeID})`
	`324`	`+ newFrontier = append(newFrontier, node{id, &n.id})`
`326`	`325`	`}`
`327`	`326`	`}`
`328`	`327`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,6 @@ func (v *variable) UnmarshalJSON(data []byte) error {`
`51`	`51`
`52`	`52`	`for k, value := range kv {`
`53`	`53`	`v.name = k`
`54`		- //nolint:exportloopref // There should only be one iteration, so the value of `value` should not change
`55`	`54`	`v.value = &value`
`56`	`55`	`}`
`57`	`56`
`@@ -78,7 +77,6 @@ func (v *variable) UnmarshalYAML(unmarshal func(any) error) error {`
`78`	`77`
`79`	`78`	`for k, value := range kv {`
`80`	`79`	`v.name = k`
`81`		- //nolint:exportloopref // There should only be one iteration, so the value of `value` should not change
`82`	`80`	`v.value = &value`
`83`	`81`	`}`
`84`	`82`
Original file line number	Diff line number	Diff line change
`@@ -46,14 +46,5 @@`
`46`	`46`	`".*_pb\\.go$": "ignore protobuff generated code",`
`47`	`47`	`"dev/buildchecker/slack\\.go$": "ignore false positive"`
`48`	`48`	`}`
`49`		`- },`
`50`		`- "exportloopref": {`
`51`		`- "exclude_files": {`
`52`		`- "lib/batches/env/var\\.go$": "false positive",`
`53`		`- "rules_go./.": "ignore rules_go working directory",`
`54`		`- "external/.*": "no need to vet third party code",`
`55`		`- ".*_generated\\.go$": "ignore generated code",`
`56`		`- ".*_pb\\.go$": "ignore protobuff generated code"`
`57`		`- }`
`58`	`49`	`}`
`59`	`50`	`}`