Skip to content

enable all gocritic options - #252

Draft
thaJeztah wants to merge 7 commits into
moby:mainfrom
thaJeztah:pr_246_followups
Draft

enable all gocritic options#252
thaJeztah wants to merge 7 commits into
moby:mainfrom
thaJeztah:pr_246_followups

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Sep 4, 2026

Copy link
Copy Markdown
Member

These were the failures;

mountinfo/mountinfo_bsd.go:21:2: rangeValCopy: each iteration copies 2168 bytes (consider pointers or indexing) (gocritic)
	for _, entry := range entries {
	^
mount/flags_unix.go:118:1: unnamedResult: consider giving a name to these results (gocritic)
func parseOptions(options string) (int, string) {
^
signal/signal.go:44:2: importShadow: shadow of imported package 'signal' (gocritic)
	signal, ok := SignalMap[strings.TrimPrefix(strings.ToUpper(rawSignal), "SIG")]
	^
user/idtools.go:52:1: unnamedResult: consider giving a name to these results (gocritic)
func getRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) {
^
user/idtools.go:106:1: unnamedResult: consider giving a name to these results (gocritic)
func (i IdentityMapping) RootPair() (int, int) {
^
user/idtools.go:113:1: unnamedResult: consider giving a name to these results (gocritic)
func (i IdentityMapping) ToHost(uid, gid int) (int, int, error) {
^
user/idtools_unix.go:59:6: sloppyReassign: re-assignment to `err` can be replaced with `err := os.MkdirAll(path, mode)` (gocritic)
		if err = os.MkdirAll(path, mode); err != nil {
		   ^
user/idtools_unix.go:62:12: sloppyReassign: re-assignment to `err` can be replaced with `err := os.Mkdir(path, mode)` (gocritic)
	} else if err = os.Mkdir(path, mode); err != nil {
	          ^
user/idtools_unix.go:68:6: sloppyReassign: re-assignment to `err` can be replaced with `err := setPermissions(pathComponent, mode, uid, gid, nil)` (gocritic)
		if err = setPermissions(pathComponent, mode, uid, gid, nil); err != nil {
		   ^
user/idtools_unix.go:124:39: hugeParam: usr is heavy (96 bytes); consider passing it by pointer (gocritic)
func lookupSubRangesFile(path string, usr User) ([]IDMap, error) {
                                      ^
user/user_test.go:142:12: appendAssign: append result not assigned to the same slice (gocritic)
			data := append(pad, beyond...)
			        ^
user/user_utils.go:90:28: hugeParam: u is heavy (96 bytes); consider passing it by pointer (gocritic)
func (ua *userArg) matches(u User) bool {
                           ^

@thaJeztah

Copy link
Copy Markdown
Member Author

Ah, it found some more; wondering if it can be configured to not require an explanation for errcheck 🤔

Error: mount/mount_unix_test.go:211:3: whyNoLint: include an explanation for nolint directive (gocritic)
		//nolint:errcheck
		^
Error: mount/mount_unix_test.go:212:3: deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop (gocritic)
		defer Unmount(dir)
		^
Error: mount/mount_unix_test.go:262:3: deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop (gocritic)
		defer Unmount(dir) //nolint:errcheck
		^
Error: mount/mounter_linux_test.go:109:1: paramTypeCombine: func(t *testing.T, mnt string, opts, optional, vfs string) could be replaced with func(t *testing.T, mnt, opts, optional, vfs string) (gocritic)

thaJeztah and others added 7 commits September 4, 2026 10:50
The ensureUnmount utility was defined in a Linux-only file, but
referenced from the Unix-tests, which wouldn't compile on non-Linux
because of that.

Move it to the Unix test file, and mark it as helper while updating.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants