lint: enable gocritic

tonistiigi · tonistiigi · commit 265d18f2e635 · 2024-05-15T16:44:38.000-07:00
Signed-off-by: Tonis Tiigi &lt;tonistiigi@gmail.com&gt;
diff --git a/.golangci.yml b/.golangci.yml
@@ -9,6 +9,7 @@ linters:
     - depguard
     - errname
     - forbidigo
+    - gocritic
     - gofmt
     - goimports
     - gosec
@@ -28,6 +29,12 @@ linters:
   disable-all: true
 
 linters-settings:
+  gocritic:
+    disabled-checks:
+      - "ifElseChain"
+      - "assignOp"
+      - "appendAssign"
+      - "singleCaseSwitch"
   govet:
     enable:
       - nilness
diff --git a/cache/contenthash/checksum.go b/cache/contenthash/checksum.go
@@ -1176,11 +1176,11 @@ func poolsCopy(dst io.Writer, src io.Reader) (written int64, err error) {
 }
 
 func convertPathToKey(p string) []byte {
-	return bytes.Replace([]byte(p), []byte("/"), []byte{0}, -1)
+	return bytes.ReplaceAll([]byte(p), []byte("/"), []byte{0})
 }
 
 func convertKeyToPath(p []byte) string {
-	return string(bytes.Replace(p, []byte{0}, []byte("/"), -1))
+	return string(bytes.ReplaceAll(p, []byte{0}, []byte("/")))
 }
 
 func splitKey(k []byte) ([]byte, []byte) {
diff --git a/client/connhelper/npipe/npipe_windows.go b/client/connhelper/npipe/npipe_windows.go
@@ -19,7 +19,7 @@ func Helper(u *url.URL) (*connhelper.ConnectionHelper, error) {
 	if len(addrParts) != 2 {
 		return nil, errors.Errorf("invalid address %s", u)
 	}
-	address := strings.Replace(addrParts[1], "/", "\\", -1)
+	address := strings.ReplaceAll(addrParts[1], "/", "\\")
 	return &connhelper.ConnectionHelper{
 		ContextDialer: func(ctx context.Context, addr string) (net.Conn, error) {
 			return winio.DialPipeContext(ctx, address)
diff --git a/cmd/buildctl/build.go b/cmd/buildctl/build.go
@@ -289,10 +289,8 @@ func buildAction(clicontext *cli.Context) error {
 		if len(def.Def) == 0 {
 			return errors.Errorf("empty definition sent to build. Specify --frontend instead?")
 		}
-	} else {
-		if clicontext.Bool("no-cache") {
-			solveOpt.FrontendAttrs["no-cache"] = ""
-		}
+	} else if clicontext.Bool("no-cache") {
+		solveOpt.FrontendAttrs["no-cache"] = ""
 	}
 
 	refFile := clicontext.String("ref-file")
diff --git a/executor/oci/resolvconf.go b/executor/oci/resolvconf.go
@@ -53,10 +53,8 @@ func GetResolvConf(ctx context.Context, stateDir string, idmap *idtools.Identity
 						generate = true
 						lastNotEmpty = false
 					}
-				} else {
-					if fi.ModTime().Before(fiMain.ModTime()) {
-						generate = true
-					}
+				} else if fi.ModTime().Before(fiMain.ModTime()) {
+					generate = true
 				}
 			}
 		}
diff --git a/executor/resources/pids_test.go b/executor/resources/pids_test.go
@@ -19,7 +19,7 @@ func TestParsePidsStat(t *testing.T) {
 	expectedPidsStat := &resourcestypes.PIDsStat{
 		Current: uint64Ptr(123),
 	}
-	stats, err := getCgroupPIDsStat(filepath.Join(testDir))
+	stats, err := getCgroupPIDsStat(filepath.Clean(testDir))
 	require.NoError(t, err)
 	assert.Equal(t, expectedPidsStat, stats)
 }
diff --git a/exporter/local/export.go b/exporter/local/export.go
@@ -142,7 +142,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
 				if e.opts.PlatformSplit {
 					st := fstypes.Stat{
 						Mode: uint32(os.ModeDir | 0755),
-						Path: strings.Replace(k, "/", "_", -1),
+						Path: strings.ReplaceAll(k, "/", "_"),
 					}
 					if e.opts.Epoch != nil {
 						st.ModTime = e.opts.Epoch.UnixNano()
diff --git a/exporter/local/fs.go b/exporter/local/fs.go
@@ -189,7 +189,7 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab
 			if !opt.PlatformSplit {
 				nameExt := path.Ext(name)
 				namBase := strings.TrimSuffix(name, nameExt)
-				name = fmt.Sprintf("%s.%s%s", namBase, strings.Replace(k, "/", "_", -1), nameExt)
+				name = fmt.Sprintf("%s.%s%s", namBase, strings.ReplaceAll(k, "/", "_"), nameExt)
 			}
 			if _, ok := names[name]; ok {
 				return nil, nil, errors.Errorf("duplicate attestation path name %s", name)
diff --git a/exporter/tar/export.go b/exporter/tar/export.go
@@ -96,7 +96,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
 
 		st := fstypes.Stat{
 			Mode: uint32(os.ModeDir | 0755),
-			Path: strings.Replace(k, "/", "_", -1),
+			Path: strings.ReplaceAll(k, "/", "_"),
 		}
 		if e.opts.Epoch != nil {
 			st.ModTime = e.opts.Epoch.UnixNano()
diff --git a/frontend/dockerfile/dockerfile2llb/convert_runmount.go b/frontend/dockerfile/dockerfile2llb/convert_runmount.go
@@ -130,11 +130,9 @@ func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []*
 		}
 		if src := path.Join("/", mount.Source); src != "/" {
 			mountOpts = append(mountOpts, llb.SourcePath(src))
-		} else {
-			if mount.UID != nil || mount.GID != nil || mount.Mode != nil {
-				st = setCacheUIDGID(mount, st)
-				mountOpts = append(mountOpts, llb.SourcePath("/cache"))
-			}
+		} else if mount.UID != nil || mount.GID != nil || mount.Mode != nil {
+			st = setCacheUIDGID(mount, st)
+			mountOpts = append(mountOpts, llb.SourcePath("/cache"))
 		}
 
 		out = append(out, llb.AddMount(target, st, mountOpts...))
diff --git a/frontend/dockerfile/dockerfile_addgit_test.go b/frontend/dockerfile/dockerfile_addgit_test.go
@@ -52,7 +52,7 @@ func testAddGit(t *testing.T, sb integration.Sandbox) {
 	err = runShell(gitDir, gitCommands...)
 	require.NoError(t, err)
 
-	server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(gitDir))))
+	server := httptest.NewServer(http.FileServer(http.Dir(filepath.Clean(gitDir))))
 	defer server.Close()
 	serverURL := server.URL
 	t.Logf("serverURL=%q", serverURL)
diff --git a/frontend/dockerfile/dockerfile_provenance_test.go b/frontend/dockerfile/dockerfile_provenance_test.go
@@ -276,7 +276,7 @@ COPY myapp.Dockerfile /
 	expectedGitSHA, err := cmd.Output()
 	require.NoError(t, err)
 
-	server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(dir.Name))))
+	server := httptest.NewServer(http.FileServer(http.Dir(filepath.Clean(dir.Name))))
 	defer server.Close()
 
 	target := registry + "/buildkit/testwithprovenance:git"
diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go
@@ -3548,7 +3548,7 @@ COPY --from=build foo bar2
 	)
 	require.NoError(t, err)
 
-	server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(gitDir))))
+	server := httptest.NewServer(http.FileServer(http.Dir(filepath.Clean(gitDir))))
 	defer server.Close()
 
 	destDir := t.TempDir()
diff --git a/frontend/dockerfile/exclude_patterns_test.go b/frontend/dockerfile/exclude_patterns_test.go
@@ -203,7 +203,7 @@ COPY --from=builder6 / /builder6
 		err = runShell(srcDir.Name, gitCommands...)
 		require.NoError(t, err)
 
-		server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(srcDir.Name))))
+		server := httptest.NewServer(http.FileServer(http.Dir(filepath.Clean(srcDir.Name))))
 
 		defer server.Close()
 		serverURL := server.URL
diff --git a/frontend/dockerfile/instructions/commands.go b/frontend/dockerfile/instructions/commands.go
@@ -154,7 +154,7 @@ type MaintainerCommand struct {
 }
 
 // NewLabelCommand creates a new 'LABEL' command
-func NewLabelCommand(k string, v string, NoExp bool) *LabelCommand {
+func NewLabelCommand(k string, v string, noExp bool) *LabelCommand {
 	kvp := KeyValuePair{Key: k, Value: v}
 	c := "LABEL "
 	c += kvp.String()
@@ -164,7 +164,7 @@ func NewLabelCommand(k string, v string, NoExp bool) *LabelCommand {
 		Labels: KeyValuePairs{
 			kvp,
 		},
-		noExpand: NoExp,
+		noExpand: noExp,
 	}
 	return cmd
 }
diff --git a/frontend/dockerfile/instructions/errors_windows.go b/frontend/dockerfile/instructions/errors_windows.go
@@ -18,7 +18,7 @@ func errNotJSON(command, original string) error {
 	// double backslash and a [] pair. No, this is not perfect, but it doesn't
 	// have to be. It's simply a hint to make life a little easier.
 	extra := ""
-	original = filepath.FromSlash(strings.ToLower(strings.Replace(strings.ToLower(original), strings.ToLower(command)+" ", "", -1)))
+	original = filepath.FromSlash(strings.ToLower(strings.ReplaceAll(strings.ToLower(original), strings.ToLower(command)+" ", "")))
 	if len(regexp.MustCompile(`"[a-z]:\\.*`).FindStringSubmatch(original)) > 0 &&
 		!strings.Contains(original, `\\`) &&
 		strings.Contains(original, "[") &&
diff --git a/frontend/dockerfile/parser/parser_test.go b/frontend/dockerfile/parser/parser_test.go
@@ -60,7 +60,7 @@ func TestParseCases(t *testing.T) {
 
 			if runtime.GOOS == "windows" {
 				// CRLF --> CR to match Unix behavior
-				content = bytes.Replace(content, []byte{'\x0d', '\x0a'}, []byte{'\x0a'}, -1)
+				content = bytes.ReplaceAll(content, []byte{'\x0d', '\x0a'}, []byte{'\x0a'})
 			}
 			require.Equal(t, string(content), result.AST.Dump()+"\n", dockerfile)
 		})
diff --git a/frontend/subrequests/targets/targets.go b/frontend/subrequests/targets/targets.go
@@ -72,10 +72,8 @@ func PrintTargets(dt []byte, w io.Writer) error {
 		name := t.Name
 		if name == "" && t.Default {
 			name = "(default)"
-		} else {
-			if t.Default {
-				name = fmt.Sprintf("%s (default)", name)
-			}
+		} else if t.Default {
+			name = fmt.Sprintf("%s (default)", name)
 		}
 		fmt.Fprintf(tw, "%s\t%s\n", name, t.Description)
 	}
diff --git a/snapshot/snapshotter_test.go b/snapshot/snapshotter_test.go
@@ -637,9 +637,7 @@ func pathCallback[T any](ctx context.Context, t *testing.T, sn *mergeSnapshotter
 
 func tryStatPath(ctx context.Context, t *testing.T, sn *mergeSnapshotter, key, path string) *syscall.Stat_t {
 	t.Helper()
-	return pathCallback(ctx, t, sn, key, path, func(t *testing.T, path string) *syscall.Stat_t {
-		return trySyscallStat(t, path)
-	})
+	return pathCallback(ctx, t, sn, key, path, trySyscallStat)
 }
 
 func statPath(ctx context.Context, t *testing.T, sn *mergeSnapshotter, key, path string) (st *syscall.Stat_t) {
diff --git a/util/network/cniprovider/cni_linux.go b/util/network/cniprovider/cni_linux.go
@@ -62,7 +62,7 @@ func readFileAt(dirfd int, filename string, buf []byte) (int64, error) {
 	}
 	defer syscall.Close(fd)
 
-	n, err := syscall.Read(fd, buf[:])
+	n, err := syscall.Read(fd, buf)
 	if err != nil {
 		return 0, err
 	}
diff --git a/util/progress/progressui/display.go b/util/progress/progressui/display.go
@@ -822,7 +822,7 @@ func (t *trace) displayInfo() (d displayInfo) {
 		}
 		var jobs []*job
 		j := &job{
-			name:        strings.Replace(v.Name, "\t", " ", -1),
+			name:        strings.ReplaceAll(v.Name, "\t", " "),
 			vertex:      v,
 			isCompleted: true,
 		}
@@ -912,7 +912,7 @@ func addTime(tm *time.Time, d time.Duration) *time.Time {
 	if tm == nil {
 		return nil
 	}
-	t := (*tm).Add(d)
+	t := tm.Add(d)
 	return &t
 }
 
diff --git a/util/staticfs/static.go b/util/staticfs/static.go
@@ -71,9 +71,9 @@ func (fs *FS) Open(p string) (io.ReadCloser, error) {
 }
 
 func convertPathToKey(p string) string {
-	return strings.Replace(p, "/", "\x00", -1)
+	return strings.ReplaceAll(p, "/", "\x00")
 }
 
 func convertKeyToPath(p string) string {
-	return strings.Replace(p, "\x00", "/", -1)
+	return strings.ReplaceAll(p, "\x00", "/")
 }
diff --git a/util/system/path.go b/util/system/path.go
@@ -89,15 +89,15 @@ func ToSlash(inputPath, inputOS string) string {
 	if inputOS != "windows" {
 		return inputPath
 	}
-	return strings.Replace(inputPath, "\\", "/", -1)
+	return strings.ReplaceAll(inputPath, "\\", "/")
 }
 
 func FromSlash(inputPath, inputOS string) string {
 	separator := "/"
 	if inputOS == "windows" {
 		separator = "\\"
 	}
-	return strings.Replace(inputPath, "/", separator, -1)
+	return strings.ReplaceAll(inputPath, "/", separator)
 }
 
 // NormalizeWorkdir will return a normalized version of the new workdir, given
diff --git a/util/winlayers/applier.go b/util/winlayers/applier.go
@@ -148,12 +148,10 @@ func filter(in io.Reader, f func(*tar.Header) bool) (io.Reader, func(error)) {
 							return err
 						}
 					}
-				} else {
-					if h.Size > 0 {
-						//nolint:gosec // never read into memory
-						if _, err := io.Copy(io.Discard, tarReader); err != nil {
-							return err
-						}
+				} else if h.Size > 0 {
+					//nolint:gosec // never read into memory
+					if _, err := io.Copy(io.Discard, tarReader); err != nil {
+						return err
 					}
 				}
 			}
diff --git a/worker/runc/runc_test.go b/worker/runc/runc_test.go
@@ -138,7 +138,7 @@ func TestRuncWorker(t *testing.T) {
 	target, err = lm.Mount()
 	require.NoError(t, err)
 
-	//Verifies fix for issue https://github.com/moby/buildkit/issues/429
+	// verifies fix for issue https://github.com/moby/buildkit/issues/429
 	dt, err := os.ReadFile(filepath.Join(target, "run", "bar"))
 
 	require.NoError(t, err)

Original file line number	Diff line number	Diff line change
`@@ -1176,11 +1176,11 @@ func poolsCopy(dst io.Writer, src io.Reader) (written int64, err error) {`
`1176`	`1176`	`}`
`1177`	`1177`
`1178`	`1178`	`func convertPathToKey(p string) []byte {`
`1179`		`- return bytes.Replace([]byte(p), []byte("/"), []byte{0}, -1)`
	`1179`	`+ return bytes.ReplaceAll([]byte(p), []byte("/"), []byte{0})`
`1180`	`1180`	`}`
`1181`	`1181`
`1182`	`1182`	`func convertKeyToPath(p []byte) string {`
`1183`		`- return string(bytes.Replace(p, []byte{0}, []byte("/"), -1))`
	`1183`	`+ return string(bytes.ReplaceAll(p, []byte{0}, []byte("/")))`
`1184`	`1184`	`}`
`1185`	`1185`
`1186`	`1186`	`func splitKey(k []byte) ([]byte, []byte) {`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ func Helper(u url.URL) (connhelper.ConnectionHelper, error) {`
`19`	`19`	`if len(addrParts) != 2 {`
`20`	`20`	`return nil, errors.Errorf("invalid address %s", u)`
`21`	`21`	`}`
`22`		`- address := strings.Replace(addrParts[1], "/", "\\", -1)`
	`22`	`+ address := strings.ReplaceAll(addrParts[1], "/", "\\")`
`23`	`23`	`return &connhelper.ConnectionHelper{`
`24`	`24`	`ContextDialer: func(ctx context.Context, addr string) (net.Conn, error) {`
`25`	`25`	`return winio.DialPipeContext(ctx, address)`
Original file line number	Diff line number	Diff line change
`@@ -289,10 +289,8 @@ func buildAction(clicontext *cli.Context) error {`
`289`	`289`	`if len(def.Def) == 0 {`
`290`	`290`	`return errors.Errorf("empty definition sent to build. Specify --frontend instead?")`
`291`	`291`	`}`
`292`		`- } else {`
`293`		`- if clicontext.Bool("no-cache") {`
`294`		`- solveOpt.FrontendAttrs["no-cache"] = ""`
`295`		`- }`
	`292`	`+ } else if clicontext.Bool("no-cache") {`
	`293`	`+ solveOpt.FrontendAttrs["no-cache"] = ""`
`296`	`294`	`}`
`297`	`295`
`298`	`296`	`refFile := clicontext.String("ref-file")`
Original file line number	Diff line number	Diff line change
`@@ -53,10 +53,8 @@ func GetResolvConf(ctx context.Context, stateDir string, idmap *idtools.Identity`
`53`	`53`	`generate = true`
`54`	`54`	`lastNotEmpty = false`
`55`	`55`	`}`
`56`		`- } else {`
`57`		`- if fi.ModTime().Before(fiMain.ModTime()) {`
`58`		`- generate = true`
`59`		`- }`
	`56`	`+ } else if fi.ModTime().Before(fiMain.ModTime()) {`
	`57`	`+ generate = true`
`60`	`58`	`}`
`61`	`59`	`}`
`62`	`60`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ func TestParsePidsStat(t *testing.T) {`
`19`	`19`	`expectedPidsStat := &resourcestypes.PIDsStat{`
`20`	`20`	`Current: uint64Ptr(123),`
`21`	`21`	`}`
`22`		`- stats, err := getCgroupPIDsStat(filepath.Join(testDir))`
	`22`	`+ stats, err := getCgroupPIDsStat(filepath.Clean(testDir))`
`23`	`23`	`require.NoError(t, err)`
`24`	`24`	`assert.Equal(t, expectedPidsStat, stats)`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ func (e localExporterInstance) Export(ctx context.Context, inp exporter.Source`
`142`	`142`	`if e.opts.PlatformSplit {`
`143`	`143`	`st := fstypes.Stat{`
`144`	`144`	`Mode: uint32(os.ModeDir \| 0755),`
`145`		`- Path: strings.Replace(k, "/", "_", -1),`
	`145`	`+ Path: strings.ReplaceAll(k, "/", "_"),`
`146`	`146`	`}`
`147`	`147`	`if e.opts.Epoch != nil {`
`148`	`148`	`st.ModTime = e.opts.Epoch.UnixNano()`
Original file line number	Diff line number	Diff line change
`@@ -189,7 +189,7 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab`
`189`	`189`	`if !opt.PlatformSplit {`
`190`	`190`	`nameExt := path.Ext(name)`
`191`	`191`	`namBase := strings.TrimSuffix(name, nameExt)`
`192`		`- name = fmt.Sprintf("%s.%s%s", namBase, strings.Replace(k, "/", "_", -1), nameExt)`
	`192`	`+ name = fmt.Sprintf("%s.%s%s", namBase, strings.ReplaceAll(k, "/", "_"), nameExt)`
`193`	`193`	`}`
`194`	`194`	`if _, ok := names[name]; ok {`
`195`	`195`	`return nil, nil, errors.Errorf("duplicate attestation path name %s", name)`
Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@ func (e localExporterInstance) Export(ctx context.Context, inp exporter.Source`
`96`	`96`
`97`	`97`	`st := fstypes.Stat{`
`98`	`98`	`Mode: uint32(os.ModeDir \| 0755),`
`99`		`- Path: strings.Replace(k, "/", "_", -1),`
	`99`	`+ Path: strings.ReplaceAll(k, "/", "_"),`
`100`	`100`	`}`
`101`	`101`	`if e.opts.Epoch != nil {`
`102`	`102`	`st.ModTime = e.opts.Epoch.UnixNano()`
Original file line number	Diff line number	Diff line change
`@@ -130,11 +130,9 @@ func dispatchRunMounts(d dispatchState, c instructions.RunCommand, sources []*`
`130`	`130`	`}`
`131`	`131`	`if src := path.Join("/", mount.Source); src != "/" {`
`132`	`132`	`mountOpts = append(mountOpts, llb.SourcePath(src))`
`133`		`- } else {`
`134`		`- if mount.UID != nil \|\| mount.GID != nil \|\| mount.Mode != nil {`
`135`		`- st = setCacheUIDGID(mount, st)`
`136`		`- mountOpts = append(mountOpts, llb.SourcePath("/cache"))`
`137`		`- }`
	`133`	`+ } else if mount.UID != nil \|\| mount.GID != nil \|\| mount.Mode != nil {`
	`134`	`+ st = setCacheUIDGID(mount, st)`
	`135`	`+ mountOpts = append(mountOpts, llb.SourcePath("/cache"))`
`138`	`136`	`}`
`139`	`137`
`140`	`138`	`out = append(out, llb.AddMount(target, st, mountOpts...))`