Skip to content

Commit

Permalink
fix golangci-lint issues
Browse files Browse the repository at this point in the history
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 {
    	   ^
  • Loading branch information
pohly committed Jun 1, 2023
1 parent ff82b97 commit a4f9060
Show file tree
Hide file tree
Showing 28 changed files with 126 additions and 105 deletions.
11 changes: 6 additions & 5 deletions examples/benchmarks/benchmarks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"go.uber.org/zap"
"go.uber.org/zap/zapcore"

"k8s.io/klog/examples/util/require"
"k8s.io/klog/v2"
)

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

type testcase struct {
Expand Down
5 changes: 3 additions & 2 deletions examples/coexist_glog/coexist_glog.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import (
"flag"

"github.com/golang/glog"
"k8s.io/klog/examples/util/require"
"k8s.io/klog/v2"
)

func main() {
flag.Set("alsologtostderr", "true")
require.NoError(flag.Set("alsologtostderr", "true"))
flag.Parse()

klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
Expand All @@ -19,7 +20,7 @@ func main() {
f2 := klogFlags.Lookup(f1.Name)
if f2 != nil {
value := f1.Value.String()
f2.Value.Set(value)
require.NoError(f2.Value.Set(value))
}
})

Expand Down
6 changes: 4 additions & 2 deletions examples/flushing/flushing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"

"go.uber.org/goleak"

"k8s.io/klog/examples/util/require"
"k8s.io/klog/v2"
)

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

// By default klog writes to stderr. Setting logtostderr to false makes klog
// write to a log file.
flag.Set("logtostderr", "false")
flag.Set("log_file", "myfile.log")
require.NoError(flag.Set("logtostderr", "false"))
require.NoError(flag.Set("log_file", "myfile.log"))
flag.Parse()

// Info writes the first log message. When the first log file is created,
Expand Down
2 changes: 1 addition & 1 deletion examples/go.mod
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module example
module k8s.io/klog/examples

go 1.13

Expand Down
3 changes: 2 additions & 1 deletion examples/klogr/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"flag"

"k8s.io/klog/examples/util/require"
"k8s.io/klog/v2"
"k8s.io/klog/v2/klogr"
)
Expand All @@ -17,7 +18,7 @@ func (e myError) Error() string {

func main() {
klog.InitFlags(nil)
flag.Set("v", "3")
require.NoError(flag.Set("v", "3"))
flag.Parse()
log := klogr.New().WithName("MyName").WithValues("user", "you")
log.Info("hello", "val1", 1, "val2", map[string]int{"k": 1})
Expand Down
5 changes: 3 additions & 2 deletions examples/log_file/usage_log_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ package main
import (
"flag"

"k8s.io/klog/examples/util/require"
"k8s.io/klog/v2"
)

func main() {
klog.InitFlags(nil)
// By default klog writes to stderr. Setting logtostderr to false makes klog
// write to a log file.
flag.Set("logtostderr", "false")
flag.Set("log_file", "myfile.log")
require.NoError(flag.Set("logtostderr", "false"))
require.NoError(flag.Set("log_file", "myfile.log"))
flag.Parse()
klog.Info("nice to meet you")
klog.Flush()
Expand Down
6 changes: 4 additions & 2 deletions examples/set_output/usage_set_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import (
"bytes"
"flag"
"fmt"

"k8s.io/klog/examples/util/require"
"k8s.io/klog/v2"
)

func main() {
klog.InitFlags(nil)
flag.Set("logtostderr", "false")
flag.Set("alsologtostderr", "false")
require.NoError(flag.Set("logtostderr", "false"))
require.NoError(flag.Set("alsologtostderr", "false"))
flag.Parse()

buf := new(bytes.Buffer)
Expand Down
5 changes: 3 additions & 2 deletions examples/structured_logging/structured_logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ func main() {
flag.Parse()

someData := MyStruct{
Name: "hello",
Data: "world",
Name: "hello",
Data: "world",
internal: 42,
}

longData := MyStruct{
Expand Down
7 changes: 7 additions & 0 deletions examples/util/require/require.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package require

func NoError(err error) {
if err != nil {
panic(err)
}
}
13 changes: 8 additions & 5 deletions exit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ func ExampleFlushAndExit() {

var fs flag.FlagSet
klog.InitFlags(&fs)
fs.Set("skip_headers", "true")
defer flag.Set("skip_headers", "false")
fs.Set("logtostderr", "false")
defer fs.Set("logtostderr", "true")
state := klog.CaptureState()
defer state.Restore()
if err := fs.Set("skip_headers", "true"); err != nil {
panic(err)
}
if err := fs.Set("logtostderr", "false"); err != nil {
panic(err)
}
klog.SetOutput(os.Stdout)
defer klog.SetOutput(nil)
klog.OsExit = func(exitCode int) {
fmt.Printf("os.Exit(%d)\n", exitCode)
}
Expand Down
2 changes: 1 addition & 1 deletion format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestFormat(t *testing.T) {
`, klog.Format(obj).(fmt.Stringer).String(), "Format(config).String()")
// fmt.Sprintf would call String if it was available.
str := fmt.Sprintf("%s", klog.Format(obj).(logr.Marshaler).MarshalLog())
if strings.Index(str, "kind is config") >= 0 {
if strings.Contains(str, "kind is config") {
t.Errorf("fmt.Sprintf called TypeMeta.String for klog.Format(obj).MarshalLog():\n%s", str)
}

Expand Down
3 changes: 1 addition & 2 deletions internal/buffer/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ var (
// use. It also provides some helper methods for output formatting.
type Buffer struct {
bytes.Buffer
Tmp [64]byte // temporary byte array for creating headers.
next *Buffer
Tmp [64]byte // temporary byte array for creating headers.
}

var buffers = sync.Pool{
Expand Down
21 changes: 2 additions & 19 deletions internal/clock/clock.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,6 @@ type Clock interface {
// Sleep sleeps for the provided duration d.
// Consider making the sleep interruptible by using 'select' on a context channel and a timer channel.
Sleep(d time.Duration)
// Tick returns the channel of a new Ticker.
// This method does not allow to free/GC the backing ticker. Use
// NewTicker from WithTicker instead.
Tick(d time.Duration) <-chan time.Time
}

// WithTicker allows for injecting fake or real clocks into code that
// needs to do arbitrary things based on time.
type WithTicker interface {
Clock
// NewTicker returns a new Ticker.
NewTicker(time.Duration) Ticker
}
Expand All @@ -66,7 +56,7 @@ type WithDelayedExecution interface {
// WithTickerAndDelayedExecution allows for injecting fake or real clocks
// into code that needs Ticker and AfterFunc functionality
type WithTickerAndDelayedExecution interface {
WithTicker
Clock
// AfterFunc executes f in its own goroutine after waiting
// for d duration and returns a Timer whose channel can be
// closed by calling Stop() on the Timer.
Expand All @@ -79,7 +69,7 @@ type Ticker interface {
Stop()
}

var _ = WithTicker(RealClock{})
var _ Clock = RealClock{}

// RealClock really calls time.Now()
type RealClock struct{}
Expand Down Expand Up @@ -115,13 +105,6 @@ func (RealClock) AfterFunc(d time.Duration, f func()) Timer {
}
}

// Tick is the same as time.Tick(d)
// This method does not allow to free/GC the backing ticker. Use
// NewTicker instead.
func (RealClock) Tick(d time.Duration) <-chan time.Time {
return time.Tick(d)
}

// NewTicker returns a new Ticker.
func (RealClock) NewTicker(d time.Duration) Ticker {
return &realTicker{
Expand Down
7 changes: 0 additions & 7 deletions internal/clock/testing/fake_clock.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

var (
_ = clock.PassiveClock(&FakePassiveClock{})
_ = clock.WithTicker(&FakeClock{})
_ = clock.Clock(&IntervalClock{})
)

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

// Tick is unimplemented, will panic.
// TODO: make interval clock use FakeClock so this can be implemented.
func (*IntervalClock) Tick(d time.Duration) <-chan time.Time {
panic("IntervalClock doesn't implement Tick")
}

// 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 {
Expand Down
26 changes: 26 additions & 0 deletions internal/test/require/require.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package require

import "testing"

func NoError(tb testing.TB, err error) {
if err != nil {
tb.Helper()
tb.Fatalf("Unexpected error: %v", err)
}
}
4 changes: 1 addition & 3 deletions internal/verbosity/verbosity.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,7 @@ func (vs *VState) setV(pc uintptr) Level {
fn := runtime.FuncForPC(pc)
file, _ := fn.FileLine(pc)
// The file is something like /a/b/c/d.go. We want just the d.
if strings.HasSuffix(file, ".go") {
file = file[:len(file)-3]
}
file = strings.TrimSuffix(file, ".go")
if slash := strings.LastIndex(file, "/"); slash >= 0 {
file = file[slash+1:]
}
Expand Down
8 changes: 5 additions & 3 deletions internal/verbosity/verbosity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ package verbosity

import (
"testing"

"k8s.io/klog/v2/internal/test/require"
)

func TestV(t *testing.T) {
vs := New()
vs.verbosity.Set("2")
require.NoError(t, vs.verbosity.Set("2"))
depth := 0
if !vs.Enabled(1, depth) {
t.Error("not enabled for 1")
Expand All @@ -38,7 +40,7 @@ func TestV(t *testing.T) {

func TestVmoduleOn(t *testing.T) {
vs := New()
vs.vmodule.Set("verbosity_test=2")
require.NoError(t, vs.vmodule.Set("verbosity_test=2"))
depth := 0
if !vs.Enabled(1, depth) {
t.Error("not enabled for 1")
Expand Down Expand Up @@ -81,7 +83,7 @@ var vGlobs = map[string]bool{
// Test that vmodule globbing works as advertised.
func testVmoduleGlob(pat string, match bool, t *testing.T) {
vs := New()
vs.vmodule.Set(pat)
require.NoError(t, vs.vmodule.Set(pat))
depth := 0
actual := vs.Enabled(2, depth)
if actual != match {
Expand Down
Loading

0 comments on commit a4f9060

Please sign in to comment.