From ef25537ae5d41941682b835688b2e46fd2ec4c1f Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 1 Jun 2023 18:33:26 +0200 Subject: [PATCH] fix revive issues 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 } ^ --- examples/output_test/output_test.go | 4 +- internal/clock/testing/fake_clock.go | 10 ++-- klog_test.go | 70 ++++++++++++++-------------- klogr/klogr_test.go | 32 ++++++------- ktesting/testinglogger.go | 4 +- textlogger/textlogger.go | 16 +++---- 6 files changed, 69 insertions(+), 67 deletions(-) diff --git a/examples/output_test/output_test.go b/examples/output_test/output_test.go index 872612f7..e9a79d4c 100644 --- a/examples/output_test/output_test.go +++ b/examples/output_test/output_test.go @@ -34,7 +34,9 @@ import ( "k8s.io/klog/v2/textlogger" ) -func newLogger(out io.Writer, v int, vmodule string) logr.Logger { +// newLogger is a test.OutputConfig.NewLogger callback which creates a zapr +// logger. The vmodule parameter is ignored because zapr does not support that. +func newLogger(out io.Writer, v int, _ string) logr.Logger { return newZaprLogger(out, v) } diff --git a/internal/clock/testing/fake_clock.go b/internal/clock/testing/fake_clock.go index 392f81e0..61d26a5c 100644 --- a/internal/clock/testing/fake_clock.go +++ b/internal/clock/testing/fake_clock.go @@ -258,30 +258,30 @@ func (i *IntervalClock) Since(ts time.Time) time.Duration { // After is unimplemented, will panic. // TODO: make interval clock use FakeClock so this can be implemented. -func (*IntervalClock) After(d time.Duration) <-chan time.Time { +func (*IntervalClock) After(time.Duration) <-chan time.Time { panic("IntervalClock doesn't implement After") } // NewTimer is unimplemented, will panic. // TODO: make interval clock use FakeClock so this can be implemented. -func (*IntervalClock) NewTimer(d time.Duration) clock.Timer { +func (*IntervalClock) NewTimer(time.Duration) clock.Timer { panic("IntervalClock doesn't implement NewTimer") } // AfterFunc is unimplemented, will panic. // TODO: make interval clock use FakeClock so this can be implemented. -func (*IntervalClock) AfterFunc(d time.Duration, f func()) clock.Timer { +func (*IntervalClock) AfterFunc(time.Duration, func()) clock.Timer { panic("IntervalClock doesn't implement AfterFunc") } // NewTicker has no implementation yet and is omitted. // TODO: make interval clock use FakeClock so this can be implemented. -func (*IntervalClock) NewTicker(d time.Duration) clock.Ticker { +func (*IntervalClock) NewTicker(time.Duration) clock.Ticker { panic("IntervalClock doesn't implement NewTicker") } // Sleep is unimplemented, will panic. -func (*IntervalClock) Sleep(d time.Duration) { +func (*IntervalClock) Sleep(time.Duration) { panic("IntervalClock doesn't implement Sleep") } diff --git a/klog_test.go b/klog_test.go index a9ddc5a7..4528bc2a 100644 --- a/klog_test.go +++ b/klog_test.go @@ -92,7 +92,7 @@ func contents(s severity.Severity) string { } // contains reports whether the string is contained in the log. -func contains(s severity.Severity, str string, t *testing.T) bool { +func contains(s severity.Severity, str string) bool { return strings.Contains(contents(s), str) } @@ -108,10 +108,10 @@ func TestInfo(t *testing.T) { setFlags() defer logging.swap(logging.newBuffers()) Info("test") - if !contains(severity.InfoLog, "I", t) { + if !contains(severity.InfoLog, "I") { t.Errorf("Info has wrong character: %q", contents(severity.InfoLog)) } - if !contains(severity.InfoLog, "test", t) { + if !contains(severity.InfoLog, "test") { t.Error("Info failed") } } @@ -181,10 +181,10 @@ func TestStandardLog(t *testing.T) { setFlags() defer logging.swap(logging.newBuffers()) stdLog.Print("test") - if !contains(severity.InfoLog, "I", t) { + if !contains(severity.InfoLog, "I") { t.Errorf("Info has wrong character: %q", contents(severity.InfoLog)) } - if !contains(severity.InfoLog, "test", t) { + if !contains(severity.InfoLog, "test") { t.Error("Info failed") } } @@ -239,17 +239,17 @@ func TestError(t *testing.T) { setFlags() defer logging.swap(logging.newBuffers()) Error("test") - if !contains(severity.ErrorLog, "E", t) { + if !contains(severity.ErrorLog, "E") { t.Errorf("Error has wrong character: %q", contents(severity.ErrorLog)) } - if !contains(severity.ErrorLog, "test", t) { + if !contains(severity.ErrorLog, "test") { t.Error("Error failed") } str := contents(severity.ErrorLog) - if !contains(severity.WarningLog, str, t) { + if !contains(severity.WarningLog, str) { t.Error("Warning failed") } - if !contains(severity.InfoLog, str, t) { + if !contains(severity.InfoLog, str) { t.Error("Info failed") } } @@ -263,17 +263,17 @@ func TestErrorWithOneOutput(t *testing.T) { logging.oneOutput = true defer logging.swap(logging.newBuffers()) Error("test") - if !contains(severity.ErrorLog, "E", t) { + if !contains(severity.ErrorLog, "E") { t.Errorf("Error has wrong character: %q", contents(severity.ErrorLog)) } - if !contains(severity.ErrorLog, "test", t) { + if !contains(severity.ErrorLog, "test") { t.Error("Error failed") } str := contents(severity.ErrorLog) - if contains(severity.WarningLog, str, t) { + if contains(severity.WarningLog, str) { t.Error("Warning failed") } - if contains(severity.InfoLog, str, t) { + if contains(severity.InfoLog, str) { t.Error("Info failed") } } @@ -286,14 +286,14 @@ func TestWarning(t *testing.T) { setFlags() defer logging.swap(logging.newBuffers()) Warning("test") - if !contains(severity.WarningLog, "W", t) { + if !contains(severity.WarningLog, "W") { t.Errorf("Warning has wrong character: %q", contents(severity.WarningLog)) } - if !contains(severity.WarningLog, "test", t) { + if !contains(severity.WarningLog, "test") { t.Error("Warning failed") } str := contents(severity.WarningLog) - if !contains(severity.InfoLog, str, t) { + if !contains(severity.InfoLog, str) { t.Error("Info failed") } } @@ -307,14 +307,14 @@ func TestWarningWithOneOutput(t *testing.T) { logging.oneOutput = true defer logging.swap(logging.newBuffers()) Warning("test") - if !contains(severity.WarningLog, "W", t) { + if !contains(severity.WarningLog, "W") { t.Errorf("Warning has wrong character: %q", contents(severity.WarningLog)) } - if !contains(severity.WarningLog, "test", t) { + if !contains(severity.WarningLog, "test") { t.Error("Warning failed") } str := contents(severity.WarningLog) - if contains(severity.InfoLog, str, t) { + if contains(severity.InfoLog, str) { t.Error("Info failed") } } @@ -326,10 +326,10 @@ func TestV(t *testing.T) { defer logging.swap(logging.newBuffers()) require.NoError(t, logging.verbosity.Set("2")) V(2).Info("test") - if !contains(severity.InfoLog, "I", t) { + if !contains(severity.InfoLog, "I") { t.Errorf("Info has wrong character: %q", contents(severity.InfoLog)) } - if !contains(severity.InfoLog, "test", t) { + if !contains(severity.InfoLog, "test") { t.Error("Info failed") } } @@ -350,10 +350,10 @@ func TestVmoduleOn(t *testing.T) { t.Error("V enabled for 3") } V(2).Info("test") - if !contains(severity.InfoLog, "I", t) { + if !contains(severity.InfoLog, "I") { t.Errorf("Info has wrong character: %q", contents(severity.InfoLog)) } - if !contains(severity.InfoLog, "test", t) { + if !contains(severity.InfoLog, "test") { t.Error("Info failed") } } @@ -375,7 +375,7 @@ func TestVmoduleOff(t *testing.T) { } } -func TestSetOutputDataRace(t *testing.T) { +func TestSetOutputDataRace(*testing.T) { defer CaptureState().Restore() setFlags() defer logging.swap(logging.newBuffers()) @@ -965,7 +965,7 @@ func TestInfoObjectRef(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { Info(tt.ref) - if !contains(severity.InfoLog, tt.want, t) { + if !contains(severity.InfoLog, tt.want) { t.Errorf("expected %v, got %v", tt.want, contents(severity.InfoLog)) } }) @@ -1465,7 +1465,7 @@ func TestLogFilter(t *testing.T) { for _, tc := range testcases { logging.newBuffers() f.logFunc(tc.args...) - got := contains(f.severity, "[FILTERED]", t) + got := contains(f.severity, "[FILTERED]") if got != tc.expectFiltered { t.Errorf("%s filter application failed, got %v, want %v", f.name, got, tc.expectFiltered) } @@ -1804,7 +1804,7 @@ func (l *testLogr) reset() { l.entries = []testLogrEntry{} } -func (l *testLogr) Info(level int, msg string, keysAndValues ...interface{}) { +func (l *testLogr) Info(_ int, msg string, keysAndValues ...interface{}) { l.mutex.Lock() defer l.mutex.Unlock() l.entries = append(l.entries, testLogrEntry{ @@ -1825,12 +1825,12 @@ func (l *testLogr) Error(err error, msg string, keysAndValues ...interface{}) { }) } -func (l *testLogr) Init(info logr.RuntimeInfo) {} -func (l *testLogr) Enabled(level int) bool { return true } +func (l *testLogr) Init(logr.RuntimeInfo) {} +func (l *testLogr) Enabled(int) bool { return true } func (l *testLogr) V(int) logr.Logger { panic("not implemented") } func (l *testLogr) WithName(string) logr.LogSink { panic("not implemented") } func (l *testLogr) WithValues(...interface{}) logr.LogSink { panic("not implemented") } -func (l *testLogr) WithCallDepth(depth int) logr.LogSink { return l } +func (l *testLogr) WithCallDepth(int) logr.LogSink { return l } var _ logr.LogSink = &testLogr{} var _ logr.CallDepthLogSink = &testLogr{} @@ -1856,7 +1856,7 @@ func (l *callDepthTestLogr) WithCallDepth(depth int) logr.LogSink { return l } -func (l *callDepthTestLogr) Info(level int, msg string, keysAndValues ...interface{}) { +func (l *callDepthTestLogr) Info(_ int, msg string, keysAndValues ...interface{}) { l.mutex.Lock() defer l.mutex.Unlock() // Add 2 to depth for the wrapper function caller and for invocation in @@ -2201,12 +2201,12 @@ func TestSettingsDeepCopy(t *testing.T) { }, }, } - copy := settings.deepCopy() - if !reflect.DeepEqual(settings, copy) { - t.Fatalf("Copy not identical to original settings. Original:\n %+v\nCopy: %+v", settings, copy) + clone := settings.deepCopy() + if !reflect.DeepEqual(settings, clone) { + t.Fatalf("Copy not identical to original settings. Original:\n %+v\nCopy: %+v", settings, clone) } settings.vmodule.filter[1].pattern = "x" - if copy.vmodule.filter[1].pattern == settings.vmodule.filter[1].pattern { + if clone.vmodule.filter[1].pattern == settings.vmodule.filter[1].pattern { t.Fatal("Copy should not have shared vmodule.filter.") } } diff --git a/klogr/klogr_test.go b/klogr/klogr_test.go index 621017d0..0aeec030 100644 --- a/klogr/klogr_test.go +++ b/klogr/klogr_test.go @@ -20,7 +20,7 @@ const ( ) func testOutput(t *testing.T, format string) { - new := func() logr.Logger { + createLogger := func() logr.Logger { switch format { case formatNew: return New() @@ -39,7 +39,7 @@ func testOutput(t *testing.T, format string) { expectedKlogOutput string }{ "should log with values passed to keysAndValues": { - klogr: new().V(0), + klogr: createLogger().V(0), text: "test", keysAndValues: []interface{}{"akey", "avalue"}, expectedOutput: ` "msg"="test" "akey"="avalue" @@ -48,7 +48,7 @@ func testOutput(t *testing.T, format string) { `, }, "should log with name and values passed to keysAndValues": { - klogr: new().V(0).WithName("me"), + klogr: createLogger().V(0).WithName("me"), text: "test", keysAndValues: []interface{}{"akey", "avalue"}, expectedOutput: `me "msg"="test" "akey"="avalue" @@ -57,7 +57,7 @@ func testOutput(t *testing.T, format string) { `, }, "should log with multiple names and values passed to keysAndValues": { - klogr: new().V(0).WithName("hello").WithName("world"), + klogr: createLogger().V(0).WithName("hello").WithName("world"), text: "test", keysAndValues: []interface{}{"akey", "avalue"}, expectedOutput: `hello/world "msg"="test" "akey"="avalue" @@ -66,7 +66,7 @@ func testOutput(t *testing.T, format string) { `, }, "may print duplicate keys with the same value": { - klogr: new().V(0), + klogr: createLogger().V(0), text: "test", keysAndValues: []interface{}{"akey", "avalue", "akey", "avalue"}, expectedOutput: ` "msg"="test" "akey"="avalue" @@ -75,7 +75,7 @@ func testOutput(t *testing.T, format string) { `, }, "may print duplicate keys when the values are passed to Info": { - klogr: new().V(0), + klogr: createLogger().V(0), text: "test", keysAndValues: []interface{}{"akey", "avalue", "akey", "avalue2"}, expectedOutput: ` "msg"="test" "akey"="avalue2" @@ -84,7 +84,7 @@ func testOutput(t *testing.T, format string) { `, }, "should only print the duplicate key that is passed to Info if one was passed to the logger": { - klogr: new().WithValues("akey", "avalue"), + klogr: createLogger().WithValues("akey", "avalue"), text: "test", keysAndValues: []interface{}{"akey", "avalue"}, expectedOutput: ` "msg"="test" "akey"="avalue" @@ -93,7 +93,7 @@ func testOutput(t *testing.T, format string) { `, }, "should sort within logger and parameter key/value pairs in the default format and dump the logger pairs first": { - klogr: new().WithValues("akey9", "avalue9", "akey8", "avalue8", "akey1", "avalue1"), + klogr: createLogger().WithValues("akey9", "avalue9", "akey8", "avalue8", "akey1", "avalue1"), text: "test", keysAndValues: []interface{}{"akey5", "avalue5", "akey4", "avalue4"}, expectedOutput: ` "msg"="test" "akey1"="avalue1" "akey4"="avalue4" "akey5"="avalue5" "akey8"="avalue8" "akey9"="avalue9" @@ -102,7 +102,7 @@ func testOutput(t *testing.T, format string) { `, }, "should only print the key passed to Info when one is already set on the logger": { - klogr: new().WithValues("akey", "avalue"), + klogr: createLogger().WithValues("akey", "avalue"), text: "test", keysAndValues: []interface{}{"akey", "avalue2"}, expectedOutput: ` "msg"="test" "akey"="avalue2" @@ -111,7 +111,7 @@ func testOutput(t *testing.T, format string) { `, }, "should correctly handle odd-numbers of KVs": { - klogr: new(), + klogr: createLogger(), text: "test", keysAndValues: []interface{}{"akey", "avalue", "akey2"}, expectedOutput: ` "msg"="test" "akey"="avalue" "akey2"="(MISSING)" @@ -120,7 +120,7 @@ func testOutput(t *testing.T, format string) { `, }, "should correctly handle odd-numbers of KVs in WithValue": { - klogr: new().WithValues("keyWithoutValue"), + klogr: createLogger().WithValues("keyWithoutValue"), text: "test", keysAndValues: []interface{}{"akey", "avalue", "akey2"}, // klogr format sorts all key/value pairs. @@ -130,7 +130,7 @@ func testOutput(t *testing.T, format string) { `, }, "should correctly html characters": { - klogr: new(), + klogr: createLogger(), text: "test", keysAndValues: []interface{}{"akey", "<&>"}, expectedOutput: ` "msg"="test" "akey"="<&>" @@ -139,7 +139,7 @@ func testOutput(t *testing.T, format string) { `, }, "should correctly handle odd-numbers of KVs in both log values and Info args": { - klogr: new().WithValues("basekey1", "basevar1", "basekey2"), + klogr: createLogger().WithValues("basekey1", "basevar1", "basekey2"), text: "test", keysAndValues: []interface{}{"akey", "avalue", "akey2"}, // klogr format sorts all key/value pairs. @@ -149,7 +149,7 @@ func testOutput(t *testing.T, format string) { `, }, "should correctly print regular error types": { - klogr: new().V(0), + klogr: createLogger().V(0), text: "test", keysAndValues: []interface{}{"err", errors.New("whoops")}, expectedOutput: ` "msg"="test" "err"="whoops" @@ -158,7 +158,7 @@ func testOutput(t *testing.T, format string) { `, }, "should use MarshalJSON in the default format if an error type implements it": { - klogr: new().V(0), + klogr: createLogger().V(0), text: "test", keysAndValues: []interface{}{"err", &customErrorJSON{"whoops"}}, expectedOutput: ` "msg"="test" "err"="WHOOPS" @@ -167,7 +167,7 @@ func testOutput(t *testing.T, format string) { `, }, "should correctly print regular error types when using logr.Error": { - klogr: new().V(0), + klogr: createLogger().V(0), text: "test", err: errors.New("whoops"), expectedOutput: ` "msg"="test" "error"="whoops" diff --git a/ktesting/testinglogger.go b/ktesting/testinglogger.go index 14a22bf4..ae30f058 100644 --- a/ktesting/testinglogger.go +++ b/ktesting/testinglogger.go @@ -77,8 +77,8 @@ type TL interface { // later release. type NopTL struct{} -func (n NopTL) Helper() {} -func (n NopTL) Log(args ...interface{}) {} +func (n NopTL) Helper() {} +func (n NopTL) Log(...interface{}) {} var _ TL = NopTL{} diff --git a/textlogger/textlogger.go b/textlogger/textlogger.go index 857c7692..c0f916f8 100644 --- a/textlogger/textlogger.go +++ b/textlogger/textlogger.go @@ -88,7 +88,7 @@ func (l *tlogger) Enabled(level int) bool { return l.config.vstate.Enabled(verbosity.Level(level), 2+l.callDepth) } -func (l *tlogger) Info(level int, msg string, kvList ...interface{}) { +func (l *tlogger) Info(_ int, msg string, kvList ...interface{}) { l.print(nil, severity.InfoLog, msg, kvList) } @@ -145,18 +145,18 @@ func (l *tlogger) WriteKlogBuffer(data []byte) { // uses '/' characters to separate name elements. Callers should not pass '/' // in the provided name string, but this library does not actually enforce that. func (l *tlogger) WithName(name string) logr.LogSink { - new := *l + clone := *l if len(l.prefix) > 0 { - new.prefix = l.prefix + "/" + clone.prefix = l.prefix + "/" } - new.prefix += name - return &new + clone.prefix += name + return &clone } func (l *tlogger) WithValues(kvList ...interface{}) logr.LogSink { - new := *l - new.values = serialize.WithValues(l.values, kvList) - return &new + clone := *l + clone.values = serialize.WithValues(l.values, kvList) + return &clone } // KlogBufferWriter is implemented by the textlogger LogSink.