chore: enable and fixes rules from gocritic

.golangci.yml

@@ -3,6 +3,7 @@ version: "2"
 linters:
   enable:
     - errorlint
+    - gocritic
     - unconvert
     - unparam
   exclusions:
@@ -11,6 +12,15 @@ linters:
       - comments
       - std-error-handling
   settings:
+    gocritic:
+      disabled-checks:
+        - appendAssign
+        - hugeParam
+        - importShadow
+        - rangeValCopy
+        - sloppyReassign
+        - unnamedResult
+      enable-all: true
     staticcheck:
       # Enable all options, with some exceptions.
       # For defaults, see https://golangci-lint.run/usage/linters/#staticcheck
@@ -24,3 +34,7 @@ formatters:
     - gofumpt
   exclusions:
     generated: disable
+  settings:
+    gofumpt:
+      extra:
+        group-params: true

capability/capability_linux.go

@@ -73,15 +73,16 @@ func mkStringCap(c Capabilities, which CapType) (ret string) {
 	return
 }
 
-func mkString(c Capabilities, max CapType) (ret string) {
+func mkString(c Capabilities, maxCapability CapType) (ret string) {
 	ret = "{"
-	for i := CapType(1); i <= max; i <<= 1 {
+	for i := CapType(1); i <= maxCapability; i <<= 1 {
 		ret += " " + i.String() + "=\""
-		if c.Empty(i) {
+		switch {
+		case c.Empty(i):
 			ret += "empty"
-		} else if c.Full(i) {
+		case c.Full(i):
 			ret += "full"
-		} else {
+		default:
 			ret += c.StringCap(i)
 		}
 		ret += "\""

capability/capability_test.go

@@ -133,7 +133,7 @@ func TestNewPid2Load(t *testing.T) {
 	// Assuming that at least bounding set is not empty.
 	bset := c.StringCap(BOUNDING)
 	t.Logf("Bounding set: %s", bset)
-	if len(bset) == 0 {
+	if bset == "" {
 		t.Fatal("loaded bounding set: want non-empty, got empty")
 	}
 }

mount/mount_unix_test.go

@@ -201,15 +201,7 @@ func TestRecursiveUnmountTooGreedy(t *testing.T) {
 	// a mount point, or we'll hit the fast path in RecursiveUnmount.
 	dirs := []string{"dir-other", "dir/subdir1", "dir/subdir1/subsub", "dir/subdir2/subsub"}
 	for _, d := range dirs {
-		dir := path.Join(tmp, d)
-		if err := os.MkdirAll(dir, 0o700); err != nil {
-			t.Fatal(err)
-		}
-		if err := Mount("tmpfs", dir, "tmpfs", ""); err != nil {
-			t.Fatal(err)
-		}
-		//nolint:errcheck
-		defer Unmount(dir)
+		createAndMountTmpfs(t, tmp, d)
 	}
 	// sanity check
 	mounted, err := mountinfo.Mounted(path.Join(tmp, "dir-other"))
@@ -232,6 +224,18 @@ func TestRecursiveUnmountTooGreedy(t *testing.T) {
 	}
 }
 
+func createAndMountTmpfs(t *testing.T, tmp, d string) {
+	dir := path.Join(tmp, d)
+	if err := os.MkdirAll(dir, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := Mount("tmpfs", dir, "tmpfs", ""); err != nil {
+		t.Fatal(err)
+	}
+	//nolint:errcheck // Ignore error in tests
+	Unmount(dir)
+}
+
 func TestRecursiveUnmount_SubMountFailsToUnmount(t *testing.T) {
 	if os.Getuid() != 0 {
 		t.Skip("root required")
@@ -255,11 +259,7 @@ func TestRecursiveUnmount_SubMountFailsToUnmount(t *testing.T) {
 	// are listed in mountinfo, but since they are unreachable, unmount will fail.
 	toMount := []string{grandChild, child, parent}
 	for _, dir := range toMount {
-		dir := dir
-		if err := Mount("tmpfs", dir, "tmpfs", ""); err != nil {
-			t.Fatal(err)
-		}
-		defer Unmount(dir) //nolint:errcheck
+		mountTemporaryDirectory(t, dir)
 	}
 
 	// unmount shadowed mounts
@@ -273,3 +273,10 @@ func TestRecursiveUnmount_SubMountFailsToUnmount(t *testing.T) {
 		})
 	}
 }
+
+func mountTemporaryDirectory(t *testing.T, dir string) {
+	if err := Mount("tmpfs", dir, "tmpfs", ""); err != nil {
+		t.Fatal(err)
+	}
+	Unmount(dir) //nolint:errcheck // Ignore error in tests
+}

mount/mounter_linux_test.go

@@ -106,7 +106,7 @@ func ensureUnmount(t *testing.T, mnt string) {
 }
 
 // validateMount checks that mnt has the given options
-func validateMount(t *testing.T, mnt string, opts, optional, vfs string) {
+func validateMount(t *testing.T, mnt, opts, optional, vfs string) {
 	info, err := mountinfo.GetMounts(nil)
 	if err != nil {
 		t.Fatal(err)

mount/sharedsubtree_linux_test.go

@@ -313,9 +313,7 @@ func TestSubtreeUnbindable(t *testing.T) {
 	} else if err == nil {
 		t.Fatalf("%q should not have been bindable", sourceDir)
 	}
-	defer func() {
-		if err := Unmount(targetDir); err != nil {
-			t.Fatal(err)
-		}
-	}()
+	if err := Unmount(targetDir); err != nil {
+		t.Fatal(err)
+	}
 }

mountinfo/mountinfo_linux_test.go

@@ -506,7 +506,7 @@ func TestParseMountInfo(t *testing.T) {
 
 func TestParseFedoraMountinfoFilterFields(t *testing.T) {
 	r := bytes.NewBuffer([]byte(fedoraMountinfo))
-	_, err := GetMountsFromReader(r, func(info *Info) (skip bool, stop bool) {
+	_, err := GetMountsFromReader(r, func(info *Info) (skip, stop bool) {
 		mi := Info{
 			ID:         15,
 			Parent:     35,

user/idtools_unix.go

@@ -142,7 +142,7 @@ func lookupSubRangesFile(path string, usr User) ([]IDMap, error) {
 			ParentID: idrange.SubID,
 			Count:    idrange.Count,
 		})
-		containerID = containerID + idrange.Count
+		containerID += idrange.Count
 	}
 	return idMap, nil
 }

user/user_fuzzer.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 )
 
-func IsDivisbleBy(n int, divisibleby int) bool {
+func IsDivisbleBy(n, divisibleby int) bool {
 	return (n % divisibleby) == 0
 }