We recently added golang-ci-lint, which runs on pull requests and on master.
Currently, looks like the linter on PR's only checks the changed lines, but found various issues in the existing code on master (see #2618 (comment)); the linter picked up the problem in https://github.com/opencontainers/runc/runs/1190924966
Looks like that output is a good list of things to work on;
Error: `userFromOS` is unused (deadcode)
Error: `groupFromOS` is unused (deadcode)
Error: `loadConfig` is unused (deadcode)
Error: `removePath` is unused (deadcode)
Error: `wildcard` is unused (deadcode)
Error: Error return value of `os.Mkdir` is not checked (errcheck)
Error: Error return value of `os.Symlink` is not checked (errcheck)
Error: Error return value of `console.ClearONLCR` is not checked (errcheck)
Error: Error return value of `io.Copy` is not checked (errcheck)
Error: Error return value of `io.Copy` is not checked (errcheck)
Error: Error return value of `io.Copy` is not checked (errcheck)
Error: Error return value of `cmd.Process.Kill` is not checked (errcheck)
Error: Error return value of `cmd.Wait` is not checked (errcheck)
Error: Error return value of `fHook.Run` is not checked (errcheck)
Error: Error return value of `join` is not checked (errcheck)
Error: Error return value of `os.Symlink` is not checked (errcheck)
Error: Error return value of `dbusConnection.ResetFailedUnit` is not checked (errcheck)
Error: Error return value of `dbusConnection.ResetFailedUnit` is not checked (errcheck)
Error: Error return value of `unix.Unmount` is not checked (errcheck)
Error: Error return value of `criuClientCon.CloseWrite` is not checked (errcheck)
Error: Error return value of `Cgroupfs` is not checked (errcheck)
Error: Error return value of `unix.Unmount` is not checked (errcheck)
Error: Error return value of `m.Freeze` is not checked (errcheck)
Error: Error return value of `p.wait` is not checked (errcheck)
Error: Error return value of `p.cmd.Wait` is not checked (errcheck)
Error: Error return value of `p.cmd.Wait` is not checked (errcheck)
Error: Error return value of `p.cmd.Wait` is not checked (errcheck)
Error: Error return value of `p.manager.Destroy` is not checked (errcheck)
Error: Error return value of `p.intelRdtManager.Destroy` is not checked (errcheck)
Error: Error return value of `p.wait` is not checked (errcheck)
Error: Error return value of `signalAllProcesses` is not checked (errcheck)
Error: Error return value of `unix.Unmount` is not checked (errcheck)
Error: Error return value of `cleanupTmp` is not checked (errcheck)
Error: Error return value of `selinux.SetKeyLabel` is not checked (errcheck)
Error: Error return value of `selinux.SetExecLabel` is not checked (errcheck)
Error: Error return value of `selinux.SetKeyLabel` is not checked (errcheck)
Error: Error return value of `selinux.SetExecLabel` is not checked (errcheck)
Error: Error return value of `i.c.initProcess.signal` is not checked (errcheck)
Error: Error return value of `p.Wait` is not checked (errcheck)
Error: Error return value of `container.Destroy` is not checked (errcheck)
Error: Error return value of `showFile` is not checked (errcheck)
Error: Error return value of `showFile` is not checked (errcheck)
Error: Error return value of `showFile` is not checked (errcheck)
Error: Error return value of `container.Destroy` is not checked (errcheck)
Error: Error return value of `container.Destroy` is not checked (errcheck)
Error: Error return value of `unmountOp` is not checked (errcheck)
Error: Error return value of `unmountOp` is not checked (errcheck)
Error: Error return value of `unmountOp` is not checked (errcheck)
Error: Error return value of `container1.Destroy` is not checked (errcheck)
Error: Error return value of `container2.Destroy` is not checked (errcheck)
Error: Error return value of `container1.Destroy` is not checked (errcheck)
Error: Error return value of `container2.Destroy` is not checked (errcheck)
Error: Error return value of `console.ClearONLCR` is not checked (errcheck)
Error: Error return value of `h.Write` is not checked (errcheck)
Error: Error return value of `client.Write` is not checked (errcheck)
Error: ineffectual assignment to `paths` (ineffassign)
Error: `si_code` is unused (structcheck)
Error: `pad` is unused (structcheck)
Error: `si_signo` is unused (structcheck)
Error: `si_errno` is unused (structcheck)
Error: S1002: should omit comparison to bool constant, can be simplified to `ok` (gosimple)
Error: S1002: should omit comparison to bool constant, can be simplified to `ok` (gosimple)
Error: S1002: should omit comparison to bool constant, can be simplified to `ok` (gosimple)
Error: S1002: should omit comparison to bool constant, can be simplified to `!symLink` (gosimple)
Error: S1032: should use sort.Ints(...) instead of sort.Sort(sort.IntSlice(...)) (gosimple)
Error: S1032: should use sort.Ints(...) instead of sort.Sort(sort.IntSlice(...)) (gosimple)
Error: S1023: redundant `return` statement (gosimple)
Error: S1023: redundant `return` statement (gosimple)
Error: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
Error: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
Error: S1006: should use for {} instead of for true {} (gosimple)
Error: S1008: should use 'return err == nil' instead of 'if err != nil { return false }; return true' (gosimple)
Error: S1021: should merge variable declaration with assignment on next line (gosimple)
Error: S1021: should merge variable declaration with assignment on next line (gosimple)
Error: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
Error: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
Error: S1030: should use stdout2.String() instead of string(stdout2.Bytes()) (gosimple)
Error: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
Error: S1030: should use stdout2.String() instead of string(stdout2.Bytes()) (gosimple)
Error: SA4006: this value of `process` is never used (staticcheck)
Error: SA4000: identical expressions on the left and right side of the '-' operator (staticcheck)
Error: SA5001: should check returned error before deferring file.Close() (staticcheck)
Error: SA9002: file mode '777' evaluates to 01411; did you mean '0777'? (staticcheck)
Error: SA1019: prog.Attach is deprecated: use link.RawAttachProgram instead. (staticcheck)
Error: SA1019: prog.Detach is deprecated: use link.RawDetachProgram instead. (staticcheck)
Error: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead. (staticcheck)
Error: func `(*linuxContainer).deleteState` is unused (unused)
Some notes there;
- I'm not sure it the logs output all errors, or a limited list; usually, golang-ci-lint is configured to limit some issues if they occur too many times
- Depending on the above, may consider disabling
errcheck, or configure a rule/regex for error-handling we think is acceptable to ignore
There's also some false positives #2625 (comment)
Error: userFromOS is unused (deadcode)
Error: groupFromOS is unused (deadcode)
I think those functions are actually used for windows environment. Doesn't the current CI script recognize windows environment?
w.r.t Windows: it may be only checking for the platform it runs on. IIRC, there's also an option to specify which build tags to use when linting. (so multiple combinations of build-tags could be needed to lint everything).
#2625 (comment)
These should also be ignored likely (but could potentially be candidates for inclusion in golang.org/x/sys);
Error: `si_code` is unused (structcheck)
Error: `pad` is unused (structcheck)
Error: `si_signo` is unused (structcheck)
Error: `si_errno` is unused (structcheck)
Action items:
- write some instructions how to run the linter locally, and how to run it to show all errors (not omitting repeated errors)
- review the linter configuration (decide on what linters can be ignored/disabled, perhaps what linters should be added)
- split the list up to a checkbox-style list, so that things can be worked by contributors in parallel
- look for code that could be moved elsewhere (e.g. if there's low-level system things that are generic enough for inclusion in golang.org/x/sys)
We recently added golang-ci-lint, which runs on pull requests and on master.
Currently, looks like the linter on PR's only checks the changed lines, but found various issues in the existing code on master (see #2618 (comment)); the linter picked up the problem in https://github.com/opencontainers/runc/runs/1190924966
Looks like that output is a good list of things to work on;
Some notes there;
errcheck, or configure a rule/regex for error-handling we think is acceptable to ignoreThere's also some false positives #2625 (comment)
w.r.t Windows: it may be only checking for the platform it runs on. IIRC, there's also an option to specify which build tags to use when linting. (so multiple combinations of build-tags could be needed to lint everything).
#2625 (comment)
Action items: