From 17a0a6461c8155ff65abb56468acb23f1168c7be Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Tue, 30 Apr 2024 21:57:21 +0800 Subject: [PATCH] chore: Remove redundant loop captures (#62264) 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. --- BUILD.bazel | 1 - cmd/frontend/graphqlbackend/codeintel.go | 3 +-- dev/linters/exportloopref/BUILD.bazel | 12 ------------ dev/linters/exportloopref/exportloopref.go | 8 -------- dev/linters/go.mod | 1 - dev/linters/go.sum | 2 -- dev/sg/internal/generate/golang/golang.go | 3 --- internal/codeintel/policies/matcher.go | 9 ++------- internal/database/migration/definition/definition.go | 3 +-- internal/embeddings/similarity_search.go | 2 -- internal/updatecheck/handler.go | 8 +------- lib/batches/env/var.go | 2 -- linter_deps.bzl | 7 ------- nogo_config.json | 9 --------- 14 files changed, 5 insertions(+), 65 deletions(-) delete mode 100644 dev/linters/exportloopref/BUILD.bazel delete mode 100644 dev/linters/exportloopref/exportloopref.go diff --git a/BUILD.bazel b/BUILD.bazel index 8fafe9b5870..17a23b4e561 100644 --- 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 index a06ce47ea62..970219ae1e6 100644 --- 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 deleted file mode 100644 index a49fce59702..00000000000 --- a/dev/linters/exportloopref/BUILD.bazel +++ /dev/null @@ -1,12 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") - -go_library( - name = "exportloopref", - srcs = ["exportloopref.go"], - importpath = "github.com/sourcegraph/sourcegraph/dev/linters/exportloopref", - visibility = ["//visibility:public"], - deps = [ - "//dev/linters/nolint", - "@com_github_kyoh86_exportloopref//:go_default_library", - ], -) diff --git a/dev/linters/exportloopref/exportloopref.go b/dev/linters/exportloopref/exportloopref.go deleted file mode 100644 index ba570a545b1..00000000000 --- a/dev/linters/exportloopref/exportloopref.go +++ /dev/null @@ -1,8 +0,0 @@ -package exportloopref - -import ( - "github.com/kyoh86/exportloopref" - "github.com/sourcegraph/sourcegraph/dev/linters/nolint" -) - -var Analyzer = nolint.Wrap(exportloopref.Analyzer) diff --git a/dev/linters/go.mod b/dev/linters/go.mod index 7ef526798cf..e858f5fec51 100644 --- 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 index 252a65ed211..fddb31e842f 100644 --- 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 index 5137fde5ce1..1e2d952f5fe 100644 --- 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 index 224ff451563..715ecd479cb 100644 --- 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 index 4bf30eaa111..a62b7abf6b9 100644 --- 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 index d7f4d83b639..1909efc9780 100644 --- 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 index 6d87b49166b..ef0e5a49714 100644 --- 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 index c6d4cde11bf..1e63c006627 100644 --- 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 index 0c7a8f0186b..0a649650890 100644 --- 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 index d9433f39fde..46187ee3190 100644 --- 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" - } } }