golangci-lint action #380

pohly · 2023-06-01T16:39:02Z

What this PR does / why we need it:

This fixes several linting issues as found by golangci-lint v1.52.2 with default linters and revive enabled, then changes the golangci-lint invocation so that it runs with more linters, covers the examples, and posts issues as annotations.

Special notes for your reviewer:

See individual commit messages for the issues as reported by golangci-lint.

Release note:

NONE

golangci-lint v1.52.2 in its default configuration pointed out several issues that are worth fixing: internal/verbosity/verbosity_test.go:26:18: Error return value of `vs.verbosity.Set` is not checked (errcheck) vs.verbosity.Set("2") ^ internal/verbosity/verbosity_test.go:41:16: Error return value of `vs.vmodule.Set` is not checked (errcheck) vs.vmodule.Set("verbosity_test=2") ^ internal/verbosity/verbosity_test.go:84:16: Error return value of `vs.vmodule.Set` is not checked (errcheck) vs.vmodule.Set(pat) ^ internal/verbosity/verbosity.go:291:2: S1017: should replace this if statement with an unconditional strings.TrimSuffix (gosimple) if strings.HasSuffix(file, ".go") { ^ textlogger/options.go:140:19: Error return value of `(flag.Value).Set` is not checked (errcheck) c.Verbosity().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) ^ textlogger/textlogger.go:137:26: Error return value of `l.config.co.output.Write` is not checked (errcheck) l.config.co.output.Write(b.Bytes()) ^ textlogger/textlogger.go:141:26: Error return value of `l.config.co.output.Write` is not checked (errcheck) l.config.co.output.Write(data) ^ ktesting/options.go:163:18: Error return value of `(flag.Value).Set` is not checked (errcheck) c.vstate.V().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) ^ klogr/calldepth-test/call_depth_main_test.go:21:22: Error return value of `flag.CommandLine.Set` is not checked (errcheck) flag.CommandLine.Set("v", "10") ^ klogr/calldepth-test/call_depth_main_test.go:22:22: Error return value of `flag.CommandLine.Set` is not checked (errcheck) flag.CommandLine.Set("skip_headers", "false") ^ klogr/calldepth-test/call_depth_main_test.go:23:22: Error return value of `flag.CommandLine.Set` is not checked (errcheck) flag.CommandLine.Set("logtostderr", "false") ^ klog.go:903:34: Error return value of `(io.Writer).Write` is not checked (errcheck) l.file[severity.InfoLog].Write(data) ^ klog.go:913:20: Error return value of `(io.Writer).Write` is not checked (errcheck) l.file[s].Write(data) ^ klog.go:917:37: Error return value of `(io.Writer).Write` is not checked (errcheck) l.file[severity.FatalLog].Write(data) ^ klog.go:952:12: Error return value of `f.Write` is not checked (errcheck) f.Write(trace) ^ klog.go:1208:13: Error return value of `file.Sync` is not checked (errcheck) file.Sync() // ignore error ^ klog_file.go:113:14: Error return value of `os.Symlink` is not checked (errcheck) os.Symlink(name, symlink) // ignore err ^ klog_test.go:328:23: Error return value of `logging.verbosity.Set` is not checked (errcheck) logging.verbosity.Set("2") ^ klog_test.go:343:21: Error return value of `logging.vmodule.Set` is not checked (errcheck) logging.vmodule.Set("klog_test=2") ^ klog_test.go:367:21: Error return value of `logging.vmodule.Set` is not checked (errcheck) logging.vmodule.Set("notthisfile=2") ^ klog_test.go:459:21: Error return value of `logging.vmodule.Set` is not checked (errcheck) logging.vmodule.Set(pat) ^ klog_test.go:807:23: Error return value of `logging.verbosity.Set` is not checked (errcheck) logging.verbosity.Set("0") ^ klog_test.go:878:9: Error return value of `fs1.Set` is not checked (errcheck) fs1.Set("log_dir", "/test1") ^ klog_test.go:879:9: Error return value of `fs1.Set` is not checked (errcheck) fs1.Set("log_file_max_size", "1") ^ klog_test.go:885:9: Error return value of `fs2.Set` is not checked (errcheck) fs2.Set("log_file_max_size", "2048") ^ klog_test.go:1173:23: Error return value of `logging.verbosity.Set` is not checked (errcheck) logging.verbosity.Set("2") ^ klogr.go:35:2: field `level` is unused (unused) level int ^ klog.go:1287:2: S1017: should replace this if statement with an unconditional strings.TrimSuffix (gosimple) if strings.HasSuffix(file, ".go") { ^ klog.go:521:2: S1001: should use copy(to, from) instead of a loop (gosimple) for i := range s.vmodule.filter { ^ klog_test.go:79:2: S1001: should copy arrays using assignment instead of using a loop (gosimple) for i, w := range writers { ^ klogr/klogr.go:118:16: Error return value of `encoder.Encode` is not checked (errcheck) encoder.Encode(value) ^ klogr/klogr_test.go:208:8: Error return value of `fs.Set` is not checked (errcheck) fs.Set("skip_headers", "true") ^ klogr/klogr.go:119:27: S1030: should use buffer.String() instead of string(buffer.Bytes()) (gosimple) return strings.TrimSpace(string(buffer.Bytes())) ^ ktesting/testinglogger_test.go:176:8: Error return value of `fs.Set` is not checked (errcheck) fs.Set("alsologtostderr", "false") ^ ktesting/testinglogger_test.go:177:8: Error return value of `fs.Set` is not checked (errcheck) fs.Set("logtostderr", "false") ^ internal/buffer/buffer.go:40:2: field `next` is unused (unused) next *Buffer ^ internal/clock/clock.go:122:9: SA1015: using time.Tick leaks the underlying ticker, consider using it only in endless functions, tests and the main package, and use time.NewTicker here (staticcheck) return time.Tick(d) ^ exit_test.go:31:16: Error return value of `flag.Set` is not checked (errcheck) defer flag.Set("skip_headers", "false") ^ format_test.go:46:5: S1003: should use strings.Contains(str, "kind is config") instead (gosimple) if strings.Index(str, "kind is config") >= 0 { ^

examples/output_test/output_test.go

logicalhan · 2023-06-15T16:53:22Z

/triage accepted

examples/output_test/output_test.go

exit_test.go

In golangci-lint and upstream, revive has replaced golint because golint is no longer maintained. revive finds some more issues that need to be fixed before it can be used: ktesting/testinglogger.go:81:20: unused-parameter: parameter 'args' seems to be unused, consider removing or renaming it as _ (revive) func (n NopTL) Log(args ...interface{}) {} ^ klogr/klogr_test.go:23:2: redefines-builtin-id: redefinition of the built-in function new (revive) new := func() logr.Logger { switch format { case formatNew: return New() case formatDefault: return NewWithOptions() default: return NewWithOptions(WithFormat(Format(format))) } } internal/clock/testing/fake_clock.go:261:29: unused-parameter: parameter 'd' seems to be unused, consider removing or renaming it as _ (revive) func (*IntervalClock) After(d time.Duration) <-chan time.Time { ^ internal/clock/testing/fake_clock.go:267:32: unused-parameter: parameter 'd' seems to be unused, consider removing or renaming it as _ (revive) func (*IntervalClock) NewTimer(d time.Duration) clock.Timer { ^ internal/clock/testing/fake_clock.go:273:33: unused-parameter: parameter 'd' seems to be unused, consider removing or renaming it as _ (revive) func (*IntervalClock) AfterFunc(d time.Duration, f func()) clock.Timer { ^ textlogger/textlogger.go:148:2: redefines-builtin-id: redefinition of the built-in function new (revive) new := *l ^ textlogger/textlogger.go:157:2: redefines-builtin-id: redefinition of the built-in function new (revive) new := *l ^ textlogger/textlogger.go:91:24: unused-parameter: parameter 'level' seems to be unused, consider removing or renaming it as _ (revive) func (l *tlogger) Info(level int, msg string, kvList ...interface{}) { ^ klog_test.go:2204:2: redefines-builtin-id: redefinition of the built-in function copy (revive) copy := settings.deepCopy() ^ klog_test.go:95:48: unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _ (revive) func contains(s severity.Severity, str string, t *testing.T) bool { ^ klog_test.go:378:28: unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _ (revive) func TestSetOutputDataRace(t *testing.T) { ^ klog_test.go:1807:25: unused-parameter: parameter 'level' seems to be unused, consider removing or renaming it as _ (revive) func (l *testLogr) Info(level int, msg string, keysAndValues ...interface{}) { ^ klog_test.go:1828:25: unused-parameter: parameter 'info' seems to be unused, consider removing or renaming it as _ (revive) func (l *testLogr) Init(info logr.RuntimeInfo) {} ^ klog_test.go:1829:28: unused-parameter: parameter 'level' seems to be unused, consider removing or renaming it as _ (revive) func (l *testLogr) Enabled(level int) bool { return true } ^ klog_test.go:1833:34: unused-parameter: parameter 'depth' seems to be unused, consider removing or renaming it as _ (revive) func (l *testLogr) WithCallDepth(depth int) logr.LogSink { return l } ^

The main advantage is that issues get posted as annotations, which makes them easier to find when looking at a diff for a PR. While at it, golangci-lint gets invoked so that it runs in its default configuration plus the linters that were enabled explicitly before (misspell, gofmt, revive as replacement for golint). It also gets applied to the examples package.

harshanarayana · 2023-10-26T12:08:55Z

/lgtm

/cc @dims

dims · 2023-10-26T14:27:44Z

/approve

k8s-ci-robot · 2023-10-26T14:27:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, harshanarayana, pohly

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [dims]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [k8s.io/klog/v2](https://togithub.com/kubernetes/klog) | require | minor | `v2.100.1` -> `v2.110.1` | --- ### Release Notes <details> <summary>kubernetes/klog (k8s.io/klog/v2)</summary> ### [`v2.110.1`](https://togithub.com/kubernetes/klog/releases/tag/v2.110.1): Prepare klog release for Kubernetes v1.29 (Take 1) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.100.1...v2.110.1) #### What's Changed - fix: SetLogger via klog.SetLogger will output an unexpected newline by [@aimuz](https://togithub.com/aimuz) in [https://github.com/kubernetes/klog/pull/378](https://togithub.com/kubernetes/klog/pull/378) - resolve comments warning by [@lowang-bh](https://togithub.com/lowang-bh) in [https://github.com/kubernetes/klog/pull/379](https://togithub.com/kubernetes/klog/pull/379) - stderrthreshold: fix flag comment by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/376](https://togithub.com/kubernetes/klog/pull/376) - enable "go vet" checks for parameters by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/390](https://togithub.com/kubernetes/klog/pull/390) - promote experimental code to stable by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/392](https://togithub.com/kubernetes/klog/pull/392) - golangci-lint action by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/380](https://togithub.com/kubernetes/klog/pull/380) - output: handle WithName like zapr does by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/391](https://togithub.com/kubernetes/klog/pull/391) - slog support + logr 1.3.0 update by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/384](https://togithub.com/kubernetes/klog/pull/384) #### New Contributors - [@aimuz](https://togithub.com/aimuz) made their first contribution in [https://github.com/kubernetes/klog/pull/378](https://togithub.com/kubernetes/klog/pull/378) - [@lowang-bh](https://togithub.com/lowang-bh) made their first contribution in [https://github.com/kubernetes/klog/pull/379](https://togithub.com/kubernetes/klog/pull/379) **Full Changelog**: kubernetes/klog@v2.100.1...v2.110.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/hetznercloud/hcloud-cloud-controller-manager).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 1, 2023

k8s-ci-robot requested review from dims and harshanarayana June 1, 2023 16:39

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 1, 2023

pohly commented Jun 1, 2023

View reviewed changes

examples/output_test/output_test.go Show resolved Hide resolved

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 15, 2023

harshanarayana reviewed Oct 26, 2023

View reviewed changes

examples/output_test/output_test.go Show resolved Hide resolved

exit_test.go Show resolved Hide resolved

pohly added 2 commits October 26, 2023 13:47

pohly force-pushed the golangci-lint-action branch from 2a4075d to 1a0dfc5 Compare October 26, 2023 11:47

k8s-ci-robot assigned harshanarayana Oct 26, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2023

k8s-ci-robot merged commit b588475 into kubernetes:main Oct 26, 2023
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

golangci-lint action #380

golangci-lint action #380

pohly commented Jun 1, 2023

logicalhan commented Jun 15, 2023

harshanarayana commented Oct 26, 2023

dims commented Oct 26, 2023

k8s-ci-robot commented Oct 26, 2023

golangci-lint action #380

golangci-lint action #380

Conversation

pohly commented Jun 1, 2023

logicalhan commented Jun 15, 2023

harshanarayana commented Oct 26, 2023

dims commented Oct 26, 2023

k8s-ci-robot commented Oct 26, 2023