From f71d3c126fec55a0dd29697c1d6abeedc4fdbf71 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Fri, 7 Aug 2026 13:02:42 +0200 Subject: [PATCH] enhance linters configuration and improve code readability Signed-off-by: Matthieu MOREL --- .golangci.yml | 29 +++++++++++++++++++++++++++++ capability/capability_linux.go | 7 ++++--- capability/capability_test.go | 2 +- devices/device_unix_test.go | 6 +++--- mount/mounter_linux_test.go | 2 +- mount/sharedsubtree_linux_test.go | 8 +++----- mountinfo/mounted_linux_test.go | 6 ++---- reexec/reexec_test.go | 2 +- signal/signal.go | 4 ++-- user/idtools_unix.go | 8 ++++---- 10 files changed, 50 insertions(+), 24 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index efb6567e..26b1786b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,6 +3,9 @@ version: "2" linters: enable: - errorlint + - gocritic + - modernize + - revive - unconvert - unparam exclusions: @@ -11,6 +14,28 @@ linters: - comments - std-error-handling settings: + gocritic: + disabled-checks: + - appendAssign + - builtinShadow + - deferInLoop + - hugeParam + - unnamedResult + - whyNoLint + enable-all: true + revive: + enable-all-rules: false + enable-default-rules: true + max-open-files: 2048 + rules: + - name: dot-imports + disabled: true + - name: package-comments + disabled: true + - name: redefines-builtin-id + disabled: true + - name: var-naming + disabled: true staticcheck: # Enable all options, with some exceptions. # For defaults, see https://golangci-lint.run/usage/linters/#staticcheck @@ -24,3 +49,7 @@ formatters: - gofumpt exclusions: generated: disable + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/capability/capability_linux.go b/capability/capability_linux.go index 234b1efb..eac2ce49 100644 --- a/capability/capability_linux.go +++ b/capability/capability_linux.go @@ -77,11 +77,12 @@ func mkString(c Capabilities, max CapType) (ret string) { ret = "{" for i := CapType(1); i <= max; 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 += "\"" diff --git a/capability/capability_test.go b/capability/capability_test.go index c9a24f90..30748120 100644 --- a/capability/capability_test.go +++ b/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") } } diff --git a/devices/device_unix_test.go b/devices/device_unix_test.go index 262671c0..668fd63f 100644 --- a/devices/device_unix_test.go +++ b/devices/device_unix_test.go @@ -42,7 +42,7 @@ func TestDeviceFromPathLstatFailure(t *testing.T) { testError := errors.New("test error") // Override unix.Lstat to inject error. - unixLstat = func(path string, stat *unix.Stat_t) error { + unixLstat = func(_ string, _ *unix.Stat_t) error { return testError } defer cleanupTest() @@ -57,7 +57,7 @@ func TestHostDevicesIoutilReadDirFailure(t *testing.T) { testError := errors.New("test error") // Override os.ReadDir to inject error. - osReadDir = func(dirname string) ([]fs.DirEntry, error) { + osReadDir = func(_ string) ([]fs.DirEntry, error) { return nil, testError } defer cleanupTest() @@ -73,7 +73,7 @@ func TestHostDevicesIoutilReadDirDeepFailure(t *testing.T) { called := false // Override os.ReadDir to inject error after the first call. - osReadDir = func(dirname string) ([]fs.DirEntry, error) { + osReadDir = func(_ string) ([]fs.DirEntry, error) { if called { return nil, testError } diff --git a/mount/mounter_linux_test.go b/mount/mounter_linux_test.go index 461deb49..32220fe3 100644 --- a/mount/mounter_linux_test.go +++ b/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) diff --git a/mount/sharedsubtree_linux_test.go b/mount/sharedsubtree_linux_test.go index f3bed20f..ce5ac56e 100644 --- a/mount/sharedsubtree_linux_test.go +++ b/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) + } } diff --git a/mountinfo/mounted_linux_test.go b/mountinfo/mounted_linux_test.go index 50667d8a..89d65db1 100644 --- a/mountinfo/mounted_linux_test.go +++ b/mountinfo/mounted_linux_test.go @@ -60,7 +60,7 @@ var testMounts = []testMount{ { desc: "non-existent path", isNotExist: true, - prepare: func(t *testing.T) string { + prepare: func(_ *testing.T) string { return "/non/existent/path" }, }, @@ -385,9 +385,7 @@ func TestMountedBy(t *testing.T) { t.Errorf("%s: expected false on error", name) } } else if mounted != tc.isMount { - if tc.isBind && strings.HasSuffix(name, "mountedByStat") { - // mountedByStat can not detect bind mounts. - } else { + if !tc.isBind || !strings.HasSuffix(name, "mountedByStat") { t.Errorf("%s: expected %v, got %v", name, tc.isMount, mounted) } } diff --git a/reexec/reexec_test.go b/reexec/reexec_test.go index b637eb62..702b6781 100644 --- a/reexec/reexec_test.go +++ b/reexec/reexec_test.go @@ -212,7 +212,7 @@ func TestDispatch(t *testing.T) { }{ { name: "not-registered", - ctx: func(t *testing.T) context.Context { + ctx: func(_ *testing.T) context.Context { return context.Background() }, check: func(t *testing.T, ok bool, err error) { diff --git a/signal/signal.go b/signal/signal.go index 74b74d37..6bf7b836 100644 --- a/signal/signal.go +++ b/signal/signal.go @@ -41,11 +41,11 @@ func ParseSignal(rawSignal string) (syscall.Signal, error) { } return syscall.Signal(s), nil } - signal, ok := SignalMap[strings.TrimPrefix(strings.ToUpper(rawSignal), "SIG")] + sig, ok := SignalMap[strings.TrimPrefix(strings.ToUpper(rawSignal), "SIG")] if !ok { return -1, fmt.Errorf("invalid signal: %s", rawSignal) } - return signal, nil + return sig, nil } // ValidSignalForPlatform returns true if a signal is valid on the platform diff --git a/user/idtools_unix.go b/user/idtools_unix.go index 4e39d244..e535f8c1 100644 --- a/user/idtools_unix.go +++ b/user/idtools_unix.go @@ -51,16 +51,16 @@ func mkdirAs(path string, mode os.FileMode, uid, gid int, mkAll, onlyNew bool) e paths = append(paths, dirPath) } } - if err = os.MkdirAll(path, mode); err != nil { + if err := os.MkdirAll(path, mode); err != nil { return err } - } else if err = os.Mkdir(path, mode); err != nil { + } else if err := os.Mkdir(path, mode); err != nil { return err } // even if it existed, we will chown the requested path + any subpaths that // didn't exist when we called MkdirAll for _, pathComponent := range paths { - if err = setPermissions(pathComponent, mode, uid, gid, nil); err != nil { + if err := setPermissions(pathComponent, mode, uid, gid, nil); err != nil { return err } } @@ -137,7 +137,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 }