Skip to content

Commit a4f9060

Browse files
pohlypohly
committed
fix golangci-lint issues
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 { ^
1 parent ff82b97 commit a4f9060

28 files changed

+126
-105
lines changed

‎examples/benchmarks/benchmarks_test.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"go.uber.org/zap"
2828
"go.uber.org/zap/zapcore"
2929

30+
"k8s.io/klog/examples/util/require"
3031
"k8s.io/klog/v2"
3132
)
3233

@@ -38,11 +39,11 @@ func init() {
3839
// klog gets configured so that it writes to a single output file that
3940
// will be set during tests with SetOutput.
4041
klog.InitFlags(nil)
41-
flag.Set("v", fmt.Sprintf("%d", verbosityThreshold))
42-
flag.Set("log_file", "/dev/null")
43-
flag.Set("logtostderr", "false")
44-
flag.Set("alsologtostderr", "false")
45-
flag.Set("stderrthreshold", "10")
42+
require.NoError(flag.Set("v", fmt.Sprintf("%d", verbosityThreshold)))
43+
require.NoError(flag.Set("log_file", "/dev/null"))
44+
require.NoError(flag.Set("logtostderr", "false"))
45+
require.NoError(flag.Set("alsologtostderr", "false"))
46+
require.NoError(flag.Set("stderrthreshold", "10"))
4647
}
4748

4849
type testcase struct {

‎examples/coexist_glog/coexist_glog.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ import (
44
"flag"
55

66
"github.com/golang/glog"
7+
"k8s.io/klog/examples/util/require"
78
"k8s.io/klog/v2"
89
)
910

1011
func main() {
11-
flag.Set("alsologtostderr", "true")
12+
require.NoError(flag.Set("alsologtostderr", "true"))
1213
flag.Parse()
1314

1415
klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
@@ -19,7 +20,7 @@ func main() {
1920
f2 := klogFlags.Lookup(f1.Name)
2021
if f2 != nil {
2122
value := f1.Value.String()
22-
f2.Value.Set(value)
23+
require.NoError(f2.Value.Set(value))
2324
}
2425
})
2526

‎examples/flushing/flushing_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"testing"
66

77
"go.uber.org/goleak"
8+
9+
"k8s.io/klog/examples/util/require"
810
"k8s.io/klog/v2"
911
)
1012

@@ -13,8 +15,8 @@ func main() {
1315

1416
// By default klog writes to stderr. Setting logtostderr to false makes klog
1517
// write to a log file.
16-
flag.Set("logtostderr", "false")
17-
flag.Set("log_file", "myfile.log")
18+
require.NoError(flag.Set("logtostderr", "false"))
19+
require.NoError(flag.Set("log_file", "myfile.log"))
1820
flag.Parse()
1921

2022
// Info writes the first log message. When the first log file is created,

‎examples/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
module example
1+
module k8s.io/klog/examples
22

33
go 1.13
44

‎examples/klogr/main.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"flag"
55

6+
"k8s.io/klog/examples/util/require"
67
"k8s.io/klog/v2"
78
"k8s.io/klog/v2/klogr"
89
)
@@ -17,7 +18,7 @@ func (e myError) Error() string {
1718

1819
func main() {
1920
klog.InitFlags(nil)
20-
flag.Set("v", "3")
21+
require.NoError(flag.Set("v", "3"))
2122
flag.Parse()
2223
log := klogr.New().WithName("MyName").WithValues("user", "you")
2324
log.Info("hello", "val1", 1, "val2", map[string]int{"k": 1})

‎examples/log_file/usage_log_file.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@ package main
33
import (
44
"flag"
55

6+
"k8s.io/klog/examples/util/require"
67
"k8s.io/klog/v2"
78
)
89

910
func main() {
1011
klog.InitFlags(nil)
1112
// By default klog writes to stderr. Setting logtostderr to false makes klog
1213
// write to a log file.
13-
flag.Set("logtostderr", "false")
14-
flag.Set("log_file", "myfile.log")
14+
require.NoError(flag.Set("logtostderr", "false"))
15+
require.NoError(flag.Set("log_file", "myfile.log"))
1516
flag.Parse()
1617
klog.Info("nice to meet you")
1718
klog.Flush()

‎examples/set_output/usage_set_output.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import (
44
"bytes"
55
"flag"
66
"fmt"
7+
8+
"k8s.io/klog/examples/util/require"
79
"k8s.io/klog/v2"
810
)
911

1012
func main() {
1113
klog.InitFlags(nil)
12-
flag.Set("logtostderr", "false")
13-
flag.Set("alsologtostderr", "false")
14+
require.NoError(flag.Set("logtostderr", "false"))
15+
require.NoError(flag.Set("alsologtostderr", "false"))
1416
flag.Parse()
1517

1618
buf := new(bytes.Buffer)

‎examples/structured_logging/structured_logging.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ func main() {
2525
flag.Parse()
2626

2727
someData := MyStruct{
28-
Name: "hello",
29-
Data: "world",
28+
Name: "hello",
29+
Data: "world",
30+
internal: 42,
3031
}
3132

3233
longData := MyStruct{

‎examples/util/require/require.go

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package require
2+
3+
func NoError(err error) {
4+
if err != nil {
5+
panic(err)
6+
}
7+
}

‎exit_test.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,15 @@ func ExampleFlushAndExit() {
2727

2828
var fs flag.FlagSet
2929
klog.InitFlags(&fs)
30-
fs.Set("skip_headers", "true")
31-
defer flag.Set("skip_headers", "false")
32-
fs.Set("logtostderr", "false")
33-
defer fs.Set("logtostderr", "true")
30+
state := klog.CaptureState()
31+
defer state.Restore()
32+
if err := fs.Set("skip_headers", "true"); err != nil {
33+
panic(err)
34+
}
35+
if err := fs.Set("logtostderr", "false"); err != nil {
36+
panic(err)
37+
}
3438
klog.SetOutput(os.Stdout)
35-
defer klog.SetOutput(nil)
3639
klog.OsExit = func(exitCode int) {
3740
fmt.Printf("os.Exit(%d)\n", exitCode)
3841
}

‎format_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func TestFormat(t *testing.T) {
4343
`, klog.Format(obj).(fmt.Stringer).String(), "Format(config).String()")
4444
// fmt.Sprintf would call String if it was available.
4545
str := fmt.Sprintf("%s", klog.Format(obj).(logr.Marshaler).MarshalLog())
46-
if strings.Index(str, "kind is config") >= 0 {
46+
if strings.Contains(str, "kind is config") {
4747
t.Errorf("fmt.Sprintf called TypeMeta.String for klog.Format(obj).MarshalLog():\n%s", str)
4848
}
4949

‎internal/buffer/buffer.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ var (
3636
// use. It also provides some helper methods for output formatting.
3737
type Buffer struct {
3838
bytes.Buffer
39-
Tmp [64]byte // temporary byte array for creating headers.
40-
next *Buffer
39+
Tmp [64]byte // temporary byte array for creating headers.
4140
}
4241

4342
var buffers = sync.Pool{

‎internal/clock/clock.go

+2-19
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,6 @@ type Clock interface {
3939
// Sleep sleeps for the provided duration d.
4040
// Consider making the sleep interruptible by using 'select' on a context channel and a timer channel.
4141
Sleep(d time.Duration)
42-
// Tick returns the channel of a new Ticker.
43-
// This method does not allow to free/GC the backing ticker. Use
44-
// NewTicker from WithTicker instead.
45-
Tick(d time.Duration) <-chan time.Time
46-
}
47-
48-
// WithTicker allows for injecting fake or real clocks into code that
49-
// needs to do arbitrary things based on time.
50-
type WithTicker interface {
51-
Clock
5242
// NewTicker returns a new Ticker.
5343
NewTicker(time.Duration) Ticker
5444
}
@@ -66,7 +56,7 @@ type WithDelayedExecution interface {
6656
// WithTickerAndDelayedExecution allows for injecting fake or real clocks
6757
// into code that needs Ticker and AfterFunc functionality
6858
type WithTickerAndDelayedExecution interface {
69-
WithTicker
59+
Clock
7060
// AfterFunc executes f in its own goroutine after waiting
7161
// for d duration and returns a Timer whose channel can be
7262
// closed by calling Stop() on the Timer.
@@ -79,7 +69,7 @@ type Ticker interface {
7969
Stop()
8070
}
8171

82-
var _ = WithTicker(RealClock{})
72+
var _ Clock = RealClock{}
8373

8474
// RealClock really calls time.Now()
8575
type RealClock struct{}
@@ -115,13 +105,6 @@ func (RealClock) AfterFunc(d time.Duration, f func()) Timer {
115105
}
116106
}
117107

118-
// Tick is the same as time.Tick(d)
119-
// This method does not allow to free/GC the backing ticker. Use
120-
// NewTicker instead.
121-
func (RealClock) Tick(d time.Duration) <-chan time.Time {
122-
return time.Tick(d)
123-
}
124-
125108
// NewTicker returns a new Ticker.
126109
func (RealClock) NewTicker(d time.Duration) Ticker {
127110
return &realTicker{

‎internal/clock/testing/fake_clock.go

-7
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525

2626
var (
2727
_ = clock.PassiveClock(&FakePassiveClock{})
28-
_ = clock.WithTicker(&FakeClock{})
2928
_ = clock.Clock(&IntervalClock{})
3029
)
3130

@@ -275,12 +274,6 @@ func (*IntervalClock) AfterFunc(d time.Duration, f func()) clock.Timer {
275274
panic("IntervalClock doesn't implement AfterFunc")
276275
}
277276

278-
// Tick is unimplemented, will panic.
279-
// TODO: make interval clock use FakeClock so this can be implemented.
280-
func (*IntervalClock) Tick(d time.Duration) <-chan time.Time {
281-
panic("IntervalClock doesn't implement Tick")
282-
}
283-
284277
// NewTicker has no implementation yet and is omitted.
285278
// TODO: make interval clock use FakeClock so this can be implemented.
286279
func (*IntervalClock) NewTicker(d time.Duration) clock.Ticker {

‎internal/test/require/require.go

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
Copyright 2023 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package require
18+
19+
import "testing"
20+
21+
func NoError(tb testing.TB, err error) {
22+
if err != nil {
23+
tb.Helper()
24+
tb.Fatalf("Unexpected error: %v", err)
25+
}
26+
}

‎internal/verbosity/verbosity.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,7 @@ func (vs *VState) setV(pc uintptr) Level {
288288
fn := runtime.FuncForPC(pc)
289289
file, _ := fn.FileLine(pc)
290290
// The file is something like /a/b/c/d.go. We want just the d.
291-
if strings.HasSuffix(file, ".go") {
292-
file = file[:len(file)-3]
293-
}
291+
file = strings.TrimSuffix(file, ".go")
294292
if slash := strings.LastIndex(file, "/"); slash >= 0 {
295293
file = file[slash+1:]
296294
}

‎internal/verbosity/verbosity_test.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ package verbosity
1919

2020
import (
2121
"testing"
22+
23+
"k8s.io/klog/v2/internal/test/require"
2224
)
2325

2426
func TestV(t *testing.T) {
2527
vs := New()
26-
vs.verbosity.Set("2")
28+
require.NoError(t, vs.verbosity.Set("2"))
2729
depth := 0
2830
if !vs.Enabled(1, depth) {
2931
t.Error("not enabled for 1")
@@ -38,7 +40,7 @@ func TestV(t *testing.T) {
3840

3941
func TestVmoduleOn(t *testing.T) {
4042
vs := New()
41-
vs.vmodule.Set("verbosity_test=2")
43+
require.NoError(t, vs.vmodule.Set("verbosity_test=2"))
4244
depth := 0
4345
if !vs.Enabled(1, depth) {
4446
t.Error("not enabled for 1")
@@ -81,7 +83,7 @@ var vGlobs = map[string]bool{
8183
// Test that vmodule globbing works as advertised.
8284
func testVmoduleGlob(pat string, match bool, t *testing.T) {
8385
vs := New()
84-
vs.vmodule.Set(pat)
86+
require.NoError(t, vs.vmodule.Set(pat))
8587
depth := 0
8688
actual := vs.Enabled(2, depth)
8789
if actual != match {

0 commit comments

Comments
 (0)