Enable linter 'copyloopvar' and fix the issues (#2160)

## Changes - Remove all unnecessary copies of the loop variable, it is not necessary since Go 1.22 https://go.dev/blog/loopvar-preview - Enable the linter that catches this issue https://github.com/karamaru-alpha/copyloopvar ## Tests Existing tests.
databricks · Jan 16, 2025 · b273dc5 · b273dc5
1 parent a002a24
commit b273dc5
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 139 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -42,6 +42,8 @@ linters-settings:
     disable:
       # good check, but we have too many assert.(No)?Errorf? so excluding for now
       - require-error
+  copyloopvar:
+    check-alias: true
 issues:
   exclude-dirs-use-default: false  # recommended by docs https://golangci-lint.run/usage/false-positives/
   max-issues-per-linter: 1000

diff --git a/bundle/config/validate/validate_sync_patterns.go b/bundle/config/validate/validate_sync_patterns.go
@@ -47,15 +47,13 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di
 	var errs errgroup.Group
 	var diags diag.Diagnostics
 
-	for i, pattern := range patterns {
-		index := i
-		fullPattern := pattern
+	for index, pattern := range patterns {
 		// If the pattern is negated, strip the negation prefix
 		// and check if the pattern matches any files.
 		// Negation in gitignore syntax means "don't look at this path'
 		// So if p matches nothing it's useless negation, but if there are matches,
 		// it means: do not include these files into result set
-		p := strings.TrimPrefix(fullPattern, "!")
+		p := strings.TrimPrefix(pattern, "!")
 		errs.Go(func() error {
 			fs, err := fileset.NewGlobSet(rb.BundleRoot(), []string{p})
 			if err != nil {
@@ -72,7 +70,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di
 				mu.Lock()
 				diags = diags.Append(diag.Diagnostic{
 					Severity:  diag.Warning,
-					Summary:   fmt.Sprintf("Pattern %s does not match any files", fullPattern),
+					Summary:   fmt.Sprintf("Pattern %s does not match any files", pattern),
 					Locations: []dyn.Location{loc.Location()},
 					Paths:     []dyn.Path{loc.Path()},
 				})

diff --git a/cmd/bundle/generate/utils.go b/cmd/bundle/generate/utils.go
@@ -138,9 +138,7 @@ func (n *downloader) FlushToDisk(ctx context.Context, force bool) error {
 	}
 
 	errs, errCtx := errgroup.WithContext(ctx)
-	for k, v := range n.files {
-		targetPath := k
-		filePath := v
+	for targetPath, filePath := range n.files {
 		errs.Go(func() error {
 			reader, err := n.w.Workspace.Download(errCtx, filePath)
 			if err != nil {

diff --git a/integration/cmd/fs/cat_test.go b/integration/cmd/fs/cat_test.go
@@ -18,13 +18,11 @@ func TestFsCat(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			f, tmpDir := tc.setupFiler(t)
+			f, tmpDir := testCase.setupFiler(t)
 
 			err := f.Write(context.Background(), "hello.txt", strings.NewReader("abcd"), filer.CreateParentDirectories)
 			require.NoError(t, err)
@@ -40,13 +38,11 @@ func TestFsCatOnADir(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			f, tmpDir := tc.setupFiler(t)
+			f, tmpDir := testCase.setupFiler(t)
 
 			err := f.Mkdir(context.Background(), "dir1")
 			require.NoError(t, err)
@@ -61,13 +57,11 @@ func TestFsCatOnNonExistentFile(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			_, tmpDir := tc.setupFiler(t)
+			_, tmpDir := testCase.setupFiler(t)
 
 			_, _, err := testcli.RequireErrorRun(t, ctx, "fs", "cat", path.Join(tmpDir, "non-existent-file"))
 			assert.ErrorIs(t, err, fs.ErrNotExist)

diff --git a/integration/cmd/fs/cp_test.go b/integration/cmd/fs/cp_test.go
@@ -126,14 +126,12 @@ func TestFsCpDir(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			testcli.RequireSuccessfulRun(t, ctx, "fs", "cp", sourceDir, targetDir, "--recursive")
@@ -147,14 +145,12 @@ func TestFsCpFileToFile(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceFile(t, context.Background(), sourceFiler)
 
 			testcli.RequireSuccessfulRun(t, ctx, "fs", "cp", path.Join(sourceDir, "foo.txt"), path.Join(targetDir, "bar.txt"))
@@ -168,14 +164,12 @@ func TestFsCpFileToDir(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceFile(t, context.Background(), sourceFiler)
 
 			testcli.RequireSuccessfulRun(t, ctx, "fs", "cp", path.Join(sourceDir, "foo.txt"), targetDir)
@@ -205,14 +199,12 @@ func TestFsCpDirToDirFileNotOverwritten(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			// Write a conflicting file to target
@@ -231,14 +223,12 @@ func TestFsCpFileToDirFileNotOverwritten(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			// Write a conflicting file to target
@@ -255,14 +245,12 @@ func TestFsCpFileToFileFileNotOverwritten(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			// Write a conflicting file to target
@@ -279,14 +267,12 @@ func TestFsCpDirToDirWithOverwriteFlag(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			// Write a conflicting file to target
@@ -303,14 +289,12 @@ func TestFsCpFileToFileWithOverwriteFlag(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			// Write a conflicting file to target
@@ -327,14 +311,12 @@ func TestFsCpFileToDirWithOverwriteFlag(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			// Write a conflicting file to target
@@ -351,13 +333,11 @@ func TestFsCpErrorsWhenSourceIsDirWithoutRecursiveFlag(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			_, tmpDir := tc.setupFiler(t)
+			_, tmpDir := testCase.setupFiler(t)
 
 			_, _, err := testcli.RequireErrorRun(t, ctx, "fs", "cp", path.Join(tmpDir), path.Join(tmpDir, "foobar"))
 			r := regexp.MustCompile("source path .* is a directory. Please specify the --recursive flag")
@@ -376,14 +356,12 @@ func TestFsCpSourceIsDirectoryButTargetIsFile(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range copyTests() {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			sourceFiler, sourceDir := tc.setupSource(t)
-			targetFiler, targetDir := tc.setupTarget(t)
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
 			setupSourceDir(t, context.Background(), sourceFiler)
 
 			// Write a conflicting file to target

diff --git a/integration/cmd/fs/ls_test.go b/integration/cmd/fs/ls_test.go
@@ -43,13 +43,11 @@ func TestFsLs(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			f, tmpDir := tc.setupFiler(t)
+			f, tmpDir := testCase.setupFiler(t)
 			setupLsFiles(t, f)
 
 			stdout, stderr := testcli.RequireSuccessfulRun(t, ctx, "fs", "ls", tmpDir, "--output=json")
@@ -77,13 +75,11 @@ func TestFsLsWithAbsolutePaths(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			f, tmpDir := tc.setupFiler(t)
+			f, tmpDir := testCase.setupFiler(t)
 			setupLsFiles(t, f)
 
 			stdout, stderr := testcli.RequireSuccessfulRun(t, ctx, "fs", "ls", tmpDir, "--output=json", "--absolute")
@@ -111,13 +107,11 @@ func TestFsLsOnFile(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			f, tmpDir := tc.setupFiler(t)
+			f, tmpDir := testCase.setupFiler(t)
 			setupLsFiles(t, f)
 
 			_, _, err := testcli.RequireErrorRun(t, ctx, "fs", "ls", path.Join(tmpDir, "a", "hello.txt"), "--output=json")
@@ -131,13 +125,11 @@ func TestFsLsOnEmptyDir(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			_, tmpDir := tc.setupFiler(t)
+			_, tmpDir := testCase.setupFiler(t)
 
 			stdout, stderr := testcli.RequireSuccessfulRun(t, ctx, "fs", "ls", tmpDir, "--output=json")
 			assert.Equal(t, "", stderr.String())
@@ -155,13 +147,11 @@ func TestFsLsForNonexistingDir(t *testing.T) {
 	t.Parallel()
 
 	for _, testCase := range fsTests {
-		tc := testCase
-
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
 
 			ctx := context.Background()
-			_, tmpDir := tc.setupFiler(t)
+			_, tmpDir := testCase.setupFiler(t)
 
 			_, _, err := testcli.RequireErrorRun(t, ctx, "fs", "ls", path.Join(tmpDir, "nonexistent"), "--output=json")
 			assert.ErrorIs(t, err, fs.ErrNotExist)