*: replace exportloopref with copyloopvar (#56503)

close #56509
pingcap · Oct 9, 2024 · c43d058 · c43d058
1 parent a5a64f7
commit c43d058
Show file tree

Hide file tree

Showing 48 changed files with 23 additions and 173 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -17,7 +17,7 @@ linters:
     - gosec
     - asciicheck
     - bodyclose
-    - exportloopref
+    - copyloopvar
     - rowserrcheck
     - makezero
     - durationcheck

diff --git a/DEPS.bzl b/DEPS.bzl
@@ -1850,19 +1850,6 @@ def go_deps():
             "https://storage.googleapis.com/pingcapmirror/gomod/github.com/facette/natsort/com_github_facette_natsort-v0.0.0-20181210072756-2cd4dd1e2dcb.zip",
         ],
     )
-    go_repository(
-        name = "com_github_fatanugraha_noloopclosure",
-        build_file_proto_mode = "disable_global",
-        importpath = "github.com/fatanugraha/noloopclosure",
-        sha256 = "2fdc7dfcdee917b4e224c18f743e856a631a0dfac763f4f21c9a109f7411dc1e",
-        strip_prefix = "github.com/fatanugraha/noloopclosure@v0.1.1",
-        urls = [
-            "http://bazel-cache.pingcap.net:8080/gomod/github.com/fatanugraha/noloopclosure/com_github_fatanugraha_noloopclosure-v0.1.1.zip",
-            "http://ats.apps.svc/gomod/github.com/fatanugraha/noloopclosure/com_github_fatanugraha_noloopclosure-v0.1.1.zip",
-            "https://cache.hawkingrei.com/gomod/github.com/fatanugraha/noloopclosure/com_github_fatanugraha_noloopclosure-v0.1.1.zip",
-            "https://storage.googleapis.com/pingcapmirror/gomod/github.com/fatanugraha/noloopclosure/com_github_fatanugraha_noloopclosure-v0.1.1.zip",
-        ],
-    )
     go_repository(
         name = "com_github_fatih_color",
         build_file_proto_mode = "disable_global",

diff --git a/br/pkg/backup/prepare_snap/prepare.go b/br/pkg/backup/prepare_snap/prepare.go
@@ -184,8 +184,6 @@ func (p *Preparer) DriveLoopAndWaitPrepare(ctx context.Context) error {
 func (p *Preparer) Finalize(ctx context.Context) error {
 	eg := new(errgroup.Group)
 	for id, cli := range p.clients {
-		cli := cli
-		id := id
 		eg.Go(func() error {
 			if err := cli.Finalize(ctx); err != nil {
 				return errors.Annotatef(err, "failed to finalize the prepare stream for %d", id)

diff --git a/br/pkg/metautil/metafile.go b/br/pkg/metautil/metafile.go
@@ -118,8 +118,7 @@ func walkLeafMetaFile(
 	}
 	eg, ectx := errgroup.WithContext(ctx)
 	workers := tidbutil.NewWorkerPool(8, "download files workers")
-	for _, node_ := range file.MetaFiles {
-		node := node_
+	for _, node := range file.MetaFiles {
 		workers.ApplyOnErrorGroup(eg, func() error {
 			content, err := storage.ReadFile(ectx, node.Name)
 			if err != nil {

diff --git a/br/pkg/restore/data/data.go b/br/pkg/restore/data/data.go
@@ -231,7 +231,6 @@ func (recovery *Recovery) ReadRegionMeta(ctx context.Context) error {
 	defer close(metaChan)
 
 	for i := 0; i < totalStores; i++ {
-		i := i
 		storeId := recovery.allStores[i].GetId()
 		storeAddr := recovery.allStores[i].GetAddress()
 
@@ -343,9 +342,6 @@ func (recovery *Recovery) RecoverRegions(ctx context.Context) (err error) {
 		if err := ectx.Err(); err != nil {
 			break
 		}
-		storeId := storeId
-		plan := plan
-
 		workers.ApplyOnErrorGroup(eg, func() error {
 			return recovery.RecoverRegionOfStore(ectx, storeId, plan)
 		})

diff --git a/br/pkg/restore/split/client.go b/br/pkg/restore/split/client.go
@@ -581,8 +581,6 @@ func (c *pdClient) SplitKeysAndScatter(ctx context.Context, sortedSplitKeys [][]
 		workerPool := tidbutil.NewWorkerPool(uint(c.splitConcurrency), "split keys")
 		eg, eCtx := errgroup.WithContext(ctx)
 		for region, splitKeys := range splitKeyMap {
-			region := region
-			splitKeys := splitKeys
 			workerPool.ApplyOnErrorGroup(eg, func() error {
 				// TODO(lance6716): add error handling to retry from scan or retry from split
 				newRegions, err2 := c.SplitWaitAndScatter(eCtx, region, splitKeys)

diff --git a/br/pkg/storage/gcs.go b/br/pkg/storage/gcs.go
@@ -426,7 +426,6 @@ func (s *GCSStorage) Reset(ctx context.Context) error {
 	s.clients = make([]*storage.Client, gcsClientCnt)
 	eg, egCtx := util.NewErrorGroupWithRecoverWithCtx(ctx)
 	for i := range s.clients {
-		i := i
 		eg.Go(func() error {
 			client, err := storage.NewClient(egCtx, s.clientOps...)
 			if err != nil {

diff --git a/br/pkg/stream/stream_metas.go b/br/pkg/stream/stream_metas.go
@@ -172,11 +172,9 @@ func (ms *StreamMetadataSet) RemoveDataFilesAndUpdateMetadataInBatch(
 	worker := util.NewWorkerPool(ms.MetadataDownloadBatchSize, "delete files")
 	eg, cx := errgroup.WithContext(ctx)
 	for path, metaInfo := range ms.metadataInfos {
-		path := path
-		minTS := metaInfo.MinTS
 		// It's safety to remove the item within a range loop
 		delete(ms.metadataInfos, path)
-		if minTS >= from {
+		if metaInfo.MinTS >= from {
 			// That means all the datafiles wouldn't be removed,
 			// so that the metadata is skipped.
 			continue

diff --git a/br/pkg/streamhelper/advancer.go b/br/pkg/streamhelper/advancer.go
@@ -236,7 +236,6 @@ func (c *CheckpointAdvancer) tryAdvance(ctx context.Context, length int,
 	})
 	clampedRanges := utils.IntersectAll(ranges, utils.CloneSlice(c.taskRange))
 	for _, r := range clampedRanges {
-		r := r
 		workers.ApplyOnErrorGroup(eg, func() (e error) {
 			defer c.recordTimeCost("get regions in range")()
 			defer utils.PanicToErr(&e)

diff --git a/br/pkg/utils/iter/combinator_types.go b/br/pkg/utils/iter/combinator_types.go
@@ -30,7 +30,6 @@ func (m *chunkMapping[T, R]) fillChunk(ctx context.Context) IterResult[fromSlice
 	}
 	r := make([]R, len(s.Item))
 	for i := 0; i < len(s.Item); i++ {
-		i := i
 		m.quota.ApplyOnErrorGroup(eg, func() error {
 			var err error
 			r[i], err = m.mapper(cx, s.Item[i])

diff --git a/br/pkg/utils/retry_test.go b/br/pkg/utils/retry_test.go
@@ -29,7 +29,6 @@ func TestRetryAdapter(t *testing.T) {
 	wg := new(sync.WaitGroup)
 	wg.Add(len(requestedBackOff))
 	for _, bms := range requestedBackOff {
-		bms := bms
 		go func() {
 			bo.RequestBackOff(bms)
 			wg.Done()

diff --git a/build/BUILD.bazel b/build/BUILD.bazel
@@ -125,7 +125,6 @@ nogo(
                "@org_golang_x_tools//go/analysis/passes/httpresponse:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/ifaceassert:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library",
-               "@org_golang_x_tools//go/analysis/passes/loopclosure:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/lostcancel:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/nilfunc:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/nilness:go_default_library",
@@ -148,10 +147,10 @@ nogo(
                "//build/linter/bodyclose",
                "//build/linter/bootstrap",
                "//build/linter/constructor",
+               "//build/linter/copyloopvar",
                "//build/linter/deferrecover",
                "//build/linter/durationcheck",
                "//build/linter/etcdconfig",
-               "//build/linter/exportloopref",
                "//build/linter/forcetypeassert",
                "//build/linter/gofmt",
                "//build/linter/gci",
@@ -160,7 +159,6 @@ nogo(
                "//build/linter/makezero",
                "//build/linter/mirror",
                "//build/linter/misspell",
-               "//build/linter/noloopclosure",
                "//build/linter/prealloc",
                "//build/linter/predeclared",
                "//build/linter/printexpression",

diff --git a/build/linter/allrevive/analyzer.go b/build/linter/allrevive/analyzer.go
@@ -92,8 +92,8 @@ var allRules = append([]lint.Rule{
 	&rule.BlankImportsRule{},
 	//&rule.FunctionResultsLimitRule{},
 	//&rule.MaxPublicStructsRule{},
-	&rule.RangeValInClosureRule{},
-	&rule.RangeValAddress{},
+	//&rule.RangeValInClosureRule{},
+	//&rule.RangeValAddress{},
 	&rule.WaitGroupByValueRule{},
 	&rule.AtomicRule{},
 	&rule.EmptyLinesRule{},

diff --git a/build/linter/exportloopref/BUILD.bazel → build/linter/copyloopvar/BUILD.bazel b/build/linter/exportloopref/BUILD.bazel → build/linter/copyloopvar/BUILD.bazel
@@ -1,12 +1,12 @@
 load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
-    name = "exportloopref",
+    name = "copyloopvar",
     srcs = ["analyzer.go"],
-    importpath = "github.com/pingcap/tidb/build/linter/exportloopref",
+    importpath = "github.com/pingcap/tidb/build/linter/copyloopvar",
     visibility = ["//visibility:public"],
     deps = [
         "//build/linter/util",
-        "@com_github_kyoh86_exportloopref//:exportloopref",
+        "@com_github_karamaru_alpha_copyloopvar//:copyloopvar",
     ],
 )
diff --git a/build/linter/exportloopref/analyzer.go → build/linter/copyloopvar/analyzer.go b/build/linter/exportloopref/analyzer.go → build/linter/copyloopvar/analyzer.go
@@ -12,15 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package exportloopref
+package copyloopvar
 
 import (
-	"github.com/kyoh86/exportloopref"
+	"github.com/karamaru-alpha/copyloopvar"
 	"github.com/pingcap/tidb/build/linter/util"
 )
 
-// Analyzer is the analyzer struct of exportloopref.
-var Analyzer = exportloopref.Analyzer
+// Analyzer is the analyzer struct of copyloopvar.
+var Analyzer = copyloopvar.NewAnalyzer()
 
 func init() {
 	util.SkipAnalyzerByConfig(Analyzer)

diff --git a/build/linter/gosec/analysis.go b/build/linter/gosec/analysis.go
@@ -61,7 +61,6 @@ func run(pass *analysis.Pass) (any, error) {
 	createdPkgs = append(createdPkgs, util.MakeFakeLoaderPackageInfo(pass))
 	allPkgs := make(map[*types.Package]*loader.PackageInfo)
 	for _, pkg := range createdPkgs {
-		pkg := pkg
 		allPkgs[pkg.Pkg] = pkg
 	}
 	prog := &loader.Program{

diff --git a/build/linter/noloopclosure/BUILD.bazel b/build/linter/noloopclosure/BUILD.bazel
diff --git a/build/linter/noloopclosure/analysis.go b/build/linter/noloopclosure/analysis.go
diff --git a/build/nogo_config.json b/build/nogo_config.json
@@ -113,6 +113,13 @@
       "/cgo/": "ignore cgo code"
     }
   },
+  "copyloopvar": {
+    "exclude_files": {
+      "pkg/parser/parser.go": "parser/parser.go code",
+      "external/": "no need to vet third party code",
+      ".*_generated\\.go$": "ignore generated code"
+    }
+  },
   "ctrlflow": {
     "exclude_files": {
       "pkg/parser/parser.go": "parser/parser.go code",
@@ -165,13 +172,6 @@
       ".*_/testmain\\.go$": "ignore code"
     }
   },
-  "exportloopref": {
-    "exclude_files": {
-      "pkg/parser/parser.go": "parser/parser.go code",
-      "external/": "no need to vet third party code",
-      ".*_generated\\.go$": "ignore generated code"
-    }
-  },
   "filepermission": {
     "exclude_files": {
       "pkg/parser/parser.go": "parser/parser.go code",
@@ -346,13 +346,6 @@
       ".*_generated\\.go$": "ignore generated code"
     }
   },
-  "loopclosure": {
-    "exclude_files": {
-      "pkg/parser/parser.go": "parser/parser.go code",
-      "external/": "no need to vet third party code",
-      ".*_generated\\.go$": "ignore generated code"
-    }
-  },
   "lostcancel": {
     "exclude_files": {
       "pkg/parser/parser.go": "parser/parser.go code",
@@ -494,22 +487,6 @@
       "/cgo/": "ignore cgo"
     }
   },
-  "noloopclosure": {
-    "exclude_files": {
-      "pkg/parser/parser.go": "parser/parser.go code",
-      "external/": "no need to vet third party code",
-      ".*_generated\\.go$": "ignore generated code"
-    },
-    "only_files": {
-      "pkg/lightning/mydump/": "pkg/lightning/mydump/",
-      "lightning/pkg/importer/opts": "lightning/pkg/importer/opts",
-      "pkg/kv/": "kv code",
-      "pkg/util/memory": "util/memory",
-      "pkg/ddl/": "ddl",
-      "pkg/planner/": "planner",
-      "pkg/extension/": "extension code"
-    }
-  },
   "pkgfact": {
     "exclude_files": {
       "pkg/parser/parser.go": "parser/parser.go code",

diff --git a/go.mod b/go.mod
@@ -38,7 +38,6 @@ require (
 	github.com/docker/go-units v0.5.0
 	github.com/dolthub/swiss v0.2.1
 	github.com/emirpasic/gods v1.18.1
-	github.com/fatanugraha/noloopclosure v0.1.1
 	github.com/fatih/color v1.17.0
 	github.com/felixge/fgprof v0.9.3
 	github.com/fsouza/fake-gcs-server v1.44.0
@@ -69,10 +68,10 @@ require (
 	github.com/jingyugao/rowserrcheck v1.1.1
 	github.com/johannesboyne/gofakes3 v0.0.0-20230506070712-04da935ef877
 	github.com/joho/sqltocsv v0.0.0-20210428211105-a6d6801d59df
+	github.com/karamaru-alpha/copyloopvar v1.1.0
 	github.com/kisielk/errcheck v1.7.0
 	github.com/klauspost/compress v1.17.9
 	github.com/ks3sdklib/aws-sdk-go v1.2.9
-	github.com/kyoh86/exportloopref v0.1.11
 	github.com/lestrrat-go/jwx/v2 v2.0.21
 	github.com/mgechev/revive v1.3.10-0.20240809190117-a638ed6e2499
 	github.com/ngaut/pools v0.0.0-20180318154953-b7bc8c42aac7

diff --git a/go.sum b/go.sum
@@ -239,8 +239,6 @@ github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1m
 github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk=
 github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE=
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
-github.com/fatanugraha/noloopclosure v0.1.1 h1:AhepjAikNpk50qTZoipHZqeZtnyKT/C2Tk5dGn7nC+A=
-github.com/fatanugraha/noloopclosure v0.1.1/go.mod h1:Mi9CiG5QvEgvPLtZLsTzjYwjIDnWAbo10r0BG7JpJII=
 github.com/fatih/color v1.10.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM=
 github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4=
 github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI=
@@ -523,6 +521,8 @@ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1
 github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk=
 github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
 github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
+github.com/karamaru-alpha/copyloopvar v1.1.0 h1:x7gNyKcC2vRBO1H2Mks5u1VxQtYvFiym7fCjIP8RPos=
+github.com/karamaru-alpha/copyloopvar v1.1.0/go.mod h1:u7CIfztblY0jZLOQZgH3oYsJzpC2A7S6u/lfgSXHy0k=
 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
@@ -556,8 +556,6 @@ github.com/ks3sdklib/aws-sdk-go v1.2.9 h1:Eg0fM56r4Gjp9PiK1Bg9agJUxCAWCk236qq9DI
 github.com/ks3sdklib/aws-sdk-go v1.2.9/go.mod h1:xBNbOrxSnd36AQpZ8o99mGGu+blblUd9rI0MKGmeufo=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
-github.com/kyoh86/exportloopref v0.1.11 h1:1Z0bcmTypkL3Q4k+IDHMWTcnCliEZcaPiIe0/ymEyhQ=
-github.com/kyoh86/exportloopref v0.1.11/go.mod h1:qkV4UF1zGl6EkF1ox8L5t9SwyeBAZ3qLMd6up458uqA=
 github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N+AkAr5k=
 github.com/lestrrat-go/blackmagic v1.0.2/go.mod h1:UrEqBzIR2U6CnzVyUtfM6oZNMt/7O7Vohk2J0OGSAtU=
 github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE=

diff --git a/lightning/pkg/importer/dup_detect.go b/lightning/pkg/importer/dup_detect.go
@@ -181,7 +181,6 @@ func (d *dupDetector) addKeys(ctx context.Context, detector *duplicate.Detector)
 			continue
 		}
 		for _, chunk := range ecp.Chunks {
-			chunk := chunk
 			g.Go(func() error {
 				adder, err := detector.KeyAdder(ctx)
 				if err != nil {

diff --git a/lightning/pkg/importer/precheck_impl.go b/lightning/pkg/importer/precheck_impl.go
@@ -274,7 +274,6 @@ func (ci *emptyRegionCheckItem) Check(ctx context.Context) (*precheck.CheckResul
 		}
 	}
 	for _, store := range storeInfo.Stores {
-		store := store
 		stores[store.Store.ID] = &store
 	}
 	tableCount := 0
@@ -356,7 +355,6 @@ func (ci *regionDistributionCheckItem) Check(ctx context.Context) (*precheck.Che
 	}
 	stores := make([]*pdhttp.StoreInfo, 0, len(storesInfo.Stores))
 	for _, store := range storesInfo.Stores {
-		store := store
 		if metapb.StoreState(metapb.StoreState_value[store.Store.StateName]) != metapb.StoreState_Up {
 			continue
 		}

diff --git a/pkg/distsql/request_builder_test.go b/pkg/distsql/request_builder_test.go
@@ -593,8 +593,6 @@ func TestRequestBuilder7(t *testing.T) {
 		{kv.ReplicaReadFollower, "Follower"},
 		{kv.ReplicaReadMixed, "Mixed"},
 	} {
-		// copy iterator variable into a new variable, see issue #27779
-		replicaRead := replicaRead
 		t.Run(replicaRead.src, func(t *testing.T) {
 			dctx := NewDistSQLContextForTest()
 			dctx.ReplicaReadType = replicaRead.replicaReadType
@@ -735,8 +733,6 @@ func TestScanLimitConcurrency(t *testing.T) {
 		{tipb.ExecType_TypeTableScan, 1000000, dctx.DistSQLConcurrency, "TblScan_SessionVars"},
 		{tipb.ExecType_TypeIndexScan, 1000000, dctx.DistSQLConcurrency, "IdxScan_SessionVars"},
 	} {
-		// copy iterator variable into a new variable, see issue #27779
-		tt := tt
 		t.Run(tt.src, func(t *testing.T) {
 			firstExec := &tipb.Executor{Tp: tt.tp}
 			switch tt.tp {