diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 870d1a2f..48f0aca9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,7 +4,7 @@ jobs: test: strategy: matrix: - go-version: [1.17, 1.18, 1.19] + go-version: [1.18, 1.19, 1.20, 1.21] platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} steps: diff --git a/examples/go.mod b/examples/go.mod index b7750acf..323dd64b 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -8,6 +8,7 @@ require ( github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b go.uber.org/goleak v1.1.12 go.uber.org/zap v1.19.0 + golang.org/x/tools v0.1.5 k8s.io/klog/v2 v2.30.0 ) diff --git a/examples/go_vet/go_vet_test.go b/examples/go_vet/go_vet_test.go new file mode 100644 index 00000000..62a65c94 --- /dev/null +++ b/examples/go_vet/go_vet_test.go @@ -0,0 +1,50 @@ +/* +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 main + +import ( + "os" + "os/exec" + "path" + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/printf" +) + +// TestGoVet checks that "go vet" detects incorrect klog calls like +// mismatched format specifiers and arguments. +func TestGoVet(t *testing.T) { + testdata := analysistest.TestData() + src := path.Join(testdata, "src") + t.Cleanup(func() { + os.RemoveAll(src) + }) + + // analysistest doesn't support using existing code + // via modules (https://github.com/golang/go/issues/37054). + // Populating the "testdata/src" directory with the + // result of "go mod vendor" is a workaround. + cmd := exec.Command("go", "mod", "vendor", "-o", src) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%s failed: %v\nOutput: %s", cmd, err, string(out)) + } + + analyzer := printf.Analyzer + analysistest.Run(t, testdata, analyzer, "") +} diff --git a/examples/go_vet/testdata/calls.go b/examples/go_vet/testdata/calls.go new file mode 100644 index 00000000..60d8ab3c --- /dev/null +++ b/examples/go_vet/testdata/calls.go @@ -0,0 +1,98 @@ +/* +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 testdata + +import ( + "k8s.io/klog/v2" +) + +func calls() { + klog.Infof("%s") // want `k8s.io/klog/v2.Infof format %s reads arg #1, but call has 0 args` + klog.Infof("%s", "world") + klog.Info("%s", "world") // want `k8s.io/klog/v2.Info call has possible formatting directive %s` + klog.Info("world") + klog.Infoln("%s", "world") // want `k8s.io/klog/v2.Infoln call has possible formatting directive %s` + klog.Infoln("world") + + klog.InfofDepth(1, "%s") // want `k8s.io/klog/v2.InfofDepth format %s reads arg #1, but call has 0 args` + klog.InfofDepth(1, "%s", "world") + klog.InfoDepth(1, "%s", "world") // want `k8s.io/klog/v2.InfoDepth call has possible formatting directive %s` + klog.InfoDepth(1, "world") + klog.InfolnDepth(1, "%s", "world") // want `k8s.io/klog/v2.InfolnDepth call has possible formatting directive %s` + klog.InfolnDepth(1, "world") + + klog.Warningf("%s") // want `k8s.io/klog/v2.Warningf format %s reads arg #1, but call has 0 args` + klog.Warningf("%s", "world") + klog.Warning("%s", "world") // want `k8s.io/klog/v2.Warning call has possible formatting directive %s` + klog.Warning("world") + klog.Warningln("%s", "world") // want `k8s.io/klog/v2.Warningln call has possible formatting directive %s` + klog.Warningln("world") + + klog.WarningfDepth(1, "%s") // want `k8s.io/klog/v2.WarningfDepth format %s reads arg #1, but call has 0 args` + klog.WarningfDepth(1, "%s", "world") + klog.WarningDepth(1, "%s", "world") // want `k8s.io/klog/v2.WarningDepth call has possible formatting directive %s` + klog.WarningDepth(1, "world") + klog.WarninglnDepth(1, "%s", "world") // want `k8s.io/klog/v2.WarninglnDepth call has possible formatting directive %s` + klog.WarninglnDepth(1, "world") + + klog.Errorf("%s") // want `k8s.io/klog/v2.Errorf format %s reads arg #1, but call has 0 args` + klog.Errorf("%s", "world") + klog.Error("%s", "world") // want `k8s.io/klog/v2.Error call has possible formatting directive %s` + klog.Error("world") + klog.Errorln("%s", "world") // want `k8s.io/klog/v2.Errorln call has possible formatting directive %s` + klog.Errorln("world") + + klog.ErrorfDepth(1, "%s") // want `k8s.io/klog/v2.ErrorfDepth format %s reads arg #1, but call has 0 args` + klog.ErrorfDepth(1, "%s", "world") + klog.ErrorDepth(1, "%s", "world") // want `k8s.io/klog/v2.ErrorDepth call has possible formatting directive %s` + klog.ErrorDepth(1, "world") + klog.ErrorlnDepth(1, "%s", "world") // want `k8s.io/klog/v2.ErrorlnDepth call has possible formatting directive %s` + klog.ErrorlnDepth(1, "world") + + klog.Fatalf("%s") // want `k8s.io/klog/v2.Fatalf format %s reads arg #1, but call has 0 args` + klog.Fatalf("%s", "world") + klog.Fatal("%s", "world") // want `k8s.io/klog/v2.Fatal call has possible formatting directive %s` + klog.Fatal("world") + klog.Fatalln("%s", "world") // want `k8s.io/klog/v2.Fatalln call has possible formatting directive %s` + klog.Fatalln("world") + + klog.FatalfDepth(1, "%s") // want `k8s.io/klog/v2.FatalfDepth format %s reads arg #1, but call has 0 args` + klog.FatalfDepth(1, "%s", "world") + klog.FatalDepth(1, "%s", "world") // want `k8s.io/klog/v2.FatalDepth call has possible formatting directive %s` + klog.FatalDepth(1, "world") + klog.FatallnDepth(1, "%s", "world") // want `k8s.io/klog/v2.FatallnDepth call has possible formatting directive %s` + klog.FatallnDepth(1, "world") + + klog.V(1).Infof("%s") // want `\(k8s.io/klog/v2.Verbose\).Infof format %s reads arg #1, but call has 0 args` + klog.V(1).Infof("%s", "world") + klog.V(1).Info("%s", "world") // want `\(k8s.io/klog/v2.Verbose\).Info call has possible formatting directive %s` + klog.V(1).Info("world") + klog.V(1).Infoln("%s", "world") // want `\(k8s.io/klog/v2.Verbose\).Infoln call has possible formatting directive %s` + klog.V(1).Infoln("world") + + klog.V(1).InfofDepth(1, "%s") // want `\(k8s.io/klog/v2.Verbose\).InfofDepth format %s reads arg #1, but call has 0 args` + klog.V(1).InfofDepth(1, "%s", "world") + klog.V(1).InfoDepth(1, "%s", "world") // want `\(k8s.io/klog/v2.Verbose\).InfoDepth call has possible formatting directive %s` + klog.V(1).InfoDepth(1, "world") + klog.V(1).InfolnDepth(1, "%s", "world") // want `\(k8s.io/klog/v2.Verbose\).InfolnDepth call has possible formatting directive %s` + klog.V(1).InfolnDepth(1, "world") + + // Detecting format specifiers for klog.InfoS and other structured logging calls would be nice, + // but doesn't work the same way because of the extra "msg" string parameter. logcheck + // can be used instead of "go vet". + klog.InfoS("%s", "world") +} diff --git a/klog.go b/klog.go index 8b40a6b5..f42d8c9a 100644 --- a/klog.go +++ b/klog.go @@ -676,6 +676,10 @@ func (l *loggingT) println(s severity.Severity, logger *logWriter, filter LogFil } func (l *loggingT) printlnDepth(s severity.Severity, logger *logWriter, filter LogFilter, depth int, args ...interface{}) { + if false { + _ = fmt.Sprintln(args...) // cause vet to treat this function like fmt.Println + } + buf, file, line := l.header(s, depth) // If a logger is set and doesn't support writing a formatted buffer, // we clear the generated header as we rely on the backing @@ -696,6 +700,10 @@ func (l *loggingT) print(s severity.Severity, logger *logWriter, filter LogFilte } func (l *loggingT) printDepth(s severity.Severity, logger *logWriter, filter LogFilter, depth int, args ...interface{}) { + if false { + _ = fmt.Sprint(args...) // // cause vet to treat this function like fmt.Print + } + buf, file, line := l.header(s, depth) // If a logger is set and doesn't support writing a formatted buffer, // we clear the generated header as we rely on the backing @@ -719,6 +727,10 @@ func (l *loggingT) printf(s severity.Severity, logger *logWriter, filter LogFilt } func (l *loggingT) printfDepth(s severity.Severity, logger *logWriter, filter LogFilter, depth int, format string, args ...interface{}) { + if false { + _ = fmt.Sprintf(format, args...) // cause vet to treat this function like fmt.Printf + } + buf, file, line := l.header(s, depth) // If a logger is set and doesn't support writing a formatted buffer, // we clear the generated header as we rely on the backing