lint: enable gocritic

Signed-off-by: Tonis Tiigi <[email protected]>

.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

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) {

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)

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")

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
 				}
 			}
 		}

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)
 }

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()

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)

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()

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...))

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)

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"

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()

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

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
 }

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, "[") &&

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)
 		})

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)
 	}

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) {

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
 	}

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
 }
 

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", "/")
 }

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

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
 					}
 				}
 			}

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)