Merge pull request #390 from pohly/go-vet-print

enable "go vet" checks for parameters

.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:

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
 )
 

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, "")
+}

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")
+}

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