Skip to content

Commit 61b308a

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #390 from pohly/go-vet-print
enable "go vet" checks for parameters
2 parents 6632ba5 + 6af4ad1 commit 61b308a

File tree

5 files changed

+162
-1
lines changed

5 files changed

+162
-1
lines changed

‎.github/workflows/test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ jobs:
44
test:
55
strategy:
66
matrix:
7-
go-version: [1.17, 1.18, 1.19]
7+
go-version: [1.18, 1.19, 1.20, 1.21]
88
platform: [ubuntu-latest, macos-latest, windows-latest]
99
runs-on: ${{ matrix.platform }}
1010
steps:

‎examples/go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
99
go.uber.org/goleak v1.1.12
1010
go.uber.org/zap v1.19.0
11+
golang.org/x/tools v0.1.5
1112
k8s.io/klog/v2 v2.30.0
1213
)
1314

‎examples/go_vet/go_vet_test.go

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
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 main
18+
19+
import (
20+
"os"
21+
"os/exec"
22+
"path"
23+
"testing"
24+
25+
"golang.org/x/tools/go/analysis/analysistest"
26+
"golang.org/x/tools/go/analysis/passes/printf"
27+
)
28+
29+
// TestGoVet checks that "go vet" detects incorrect klog calls like
30+
// mismatched format specifiers and arguments.
31+
func TestGoVet(t *testing.T) {
32+
testdata := analysistest.TestData()
33+
src := path.Join(testdata, "src")
34+
t.Cleanup(func() {
35+
os.RemoveAll(src)
36+
})
37+
38+
// analysistest doesn't support using existing code
39+
// via modules (https://github.com/golang/go/issues/37054).
40+
// Populating the "testdata/src" directory with the
41+
// result of "go mod vendor" is a workaround.
42+
cmd := exec.Command("go", "mod", "vendor", "-o", src)
43+
out, err := cmd.CombinedOutput()
44+
if err != nil {
45+
t.Fatalf("%s failed: %v\nOutput: %s", cmd, err, string(out))
46+
}
47+
48+
analyzer := printf.Analyzer
49+
analysistest.Run(t, testdata, analyzer, "")
50+
}

‎examples/go_vet/testdata/calls.go

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
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 testdata
18+
19+
import (
20+
"k8s.io/klog/v2"
21+
)
22+
23+
func calls() {
24+
klog.Infof("%s") // want `k8s.io/klog/v2.Infof format %s reads arg #1, but call has 0 args`
25+
klog.Infof("%s", "world")
26+
klog.Info("%s", "world") // want `k8s.io/klog/v2.Info call has possible formatting directive %s`
27+
klog.Info("world")
28+
klog.Infoln("%s", "world") // want `k8s.io/klog/v2.Infoln call has possible formatting directive %s`
29+
klog.Infoln("world")
30+
31+
klog.InfofDepth(1, "%s") // want `k8s.io/klog/v2.InfofDepth format %s reads arg #1, but call has 0 args`
32+
klog.InfofDepth(1, "%s", "world")
33+
klog.InfoDepth(1, "%s", "world") // want `k8s.io/klog/v2.InfoDepth call has possible formatting directive %s`
34+
klog.InfoDepth(1, "world")
35+
klog.InfolnDepth(1, "%s", "world") // want `k8s.io/klog/v2.InfolnDepth call has possible formatting directive %s`
36+
klog.InfolnDepth(1, "world")
37+
38+
klog.Warningf("%s") // want `k8s.io/klog/v2.Warningf format %s reads arg #1, but call has 0 args`
39+
klog.Warningf("%s", "world")
40+
klog.Warning("%s", "world") // want `k8s.io/klog/v2.Warning call has possible formatting directive %s`
41+
klog.Warning("world")
42+
klog.Warningln("%s", "world") // want `k8s.io/klog/v2.Warningln call has possible formatting directive %s`
43+
klog.Warningln("world")
44+
45+
klog.WarningfDepth(1, "%s") // want `k8s.io/klog/v2.WarningfDepth format %s reads arg #1, but call has 0 args`
46+
klog.WarningfDepth(1, "%s", "world")
47+
klog.WarningDepth(1, "%s", "world") // want `k8s.io/klog/v2.WarningDepth call has possible formatting directive %s`
48+
klog.WarningDepth(1, "world")
49+
klog.WarninglnDepth(1, "%s", "world") // want `k8s.io/klog/v2.WarninglnDepth call has possible formatting directive %s`
50+
klog.WarninglnDepth(1, "world")
51+
52+
klog.Errorf("%s") // want `k8s.io/klog/v2.Errorf format %s reads arg #1, but call has 0 args`
53+
klog.Errorf("%s", "world")
54+
klog.Error("%s", "world") // want `k8s.io/klog/v2.Error call has possible formatting directive %s`
55+
klog.Error("world")
56+
klog.Errorln("%s", "world") // want `k8s.io/klog/v2.Errorln call has possible formatting directive %s`
57+
klog.Errorln("world")
58+
59+
klog.ErrorfDepth(1, "%s") // want `k8s.io/klog/v2.ErrorfDepth format %s reads arg #1, but call has 0 args`
60+
klog.ErrorfDepth(1, "%s", "world")
61+
klog.ErrorDepth(1, "%s", "world") // want `k8s.io/klog/v2.ErrorDepth call has possible formatting directive %s`
62+
klog.ErrorDepth(1, "world")
63+
klog.ErrorlnDepth(1, "%s", "world") // want `k8s.io/klog/v2.ErrorlnDepth call has possible formatting directive %s`
64+
klog.ErrorlnDepth(1, "world")
65+
66+
klog.Fatalf("%s") // want `k8s.io/klog/v2.Fatalf format %s reads arg #1, but call has 0 args`
67+
klog.Fatalf("%s", "world")
68+
klog.Fatal("%s", "world") // want `k8s.io/klog/v2.Fatal call has possible formatting directive %s`
69+
klog.Fatal("world")
70+
klog.Fatalln("%s", "world") // want `k8s.io/klog/v2.Fatalln call has possible formatting directive %s`
71+
klog.Fatalln("world")
72+
73+
klog.FatalfDepth(1, "%s") // want `k8s.io/klog/v2.FatalfDepth format %s reads arg #1, but call has 0 args`
74+
klog.FatalfDepth(1, "%s", "world")
75+
klog.FatalDepth(1, "%s", "world") // want `k8s.io/klog/v2.FatalDepth call has possible formatting directive %s`
76+
klog.FatalDepth(1, "world")
77+
klog.FatallnDepth(1, "%s", "world") // want `k8s.io/klog/v2.FatallnDepth call has possible formatting directive %s`
78+
klog.FatallnDepth(1, "world")
79+
80+
klog.V(1).Infof("%s") // want `\(k8s.io/klog/v2.Verbose\).Infof format %s reads arg #1, but call has 0 args`
81+
klog.V(1).Infof("%s", "world")
82+
klog.V(1).Info("%s", "world") // want `\(k8s.io/klog/v2.Verbose\).Info call has possible formatting directive %s`
83+
klog.V(1).Info("world")
84+
klog.V(1).Infoln("%s", "world") // want `\(k8s.io/klog/v2.Verbose\).Infoln call has possible formatting directive %s`
85+
klog.V(1).Infoln("world")
86+
87+
klog.V(1).InfofDepth(1, "%s") // want `\(k8s.io/klog/v2.Verbose\).InfofDepth format %s reads arg #1, but call has 0 args`
88+
klog.V(1).InfofDepth(1, "%s", "world")
89+
klog.V(1).InfoDepth(1, "%s", "world") // want `\(k8s.io/klog/v2.Verbose\).InfoDepth call has possible formatting directive %s`
90+
klog.V(1).InfoDepth(1, "world")
91+
klog.V(1).InfolnDepth(1, "%s", "world") // want `\(k8s.io/klog/v2.Verbose\).InfolnDepth call has possible formatting directive %s`
92+
klog.V(1).InfolnDepth(1, "world")
93+
94+
// Detecting format specifiers for klog.InfoS and other structured logging calls would be nice,
95+
// but doesn't work the same way because of the extra "msg" string parameter. logcheck
96+
// can be used instead of "go vet".
97+
klog.InfoS("%s", "world")
98+
}

‎klog.go

+12
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,10 @@ func (l *loggingT) println(s severity.Severity, logger *logWriter, filter LogFil
676676
}
677677

678678
func (l *loggingT) printlnDepth(s severity.Severity, logger *logWriter, filter LogFilter, depth int, args ...interface{}) {
679+
if false {
680+
_ = fmt.Sprintln(args...) // cause vet to treat this function like fmt.Println
681+
}
682+
679683
buf, file, line := l.header(s, depth)
680684
// If a logger is set and doesn't support writing a formatted buffer,
681685
// 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
696700
}
697701

698702
func (l *loggingT) printDepth(s severity.Severity, logger *logWriter, filter LogFilter, depth int, args ...interface{}) {
703+
if false {
704+
_ = fmt.Sprint(args...) // // cause vet to treat this function like fmt.Print
705+
}
706+
699707
buf, file, line := l.header(s, depth)
700708
// If a logger is set and doesn't support writing a formatted buffer,
701709
// 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
719727
}
720728

721729
func (l *loggingT) printfDepth(s severity.Severity, logger *logWriter, filter LogFilter, depth int, format string, args ...interface{}) {
730+
if false {
731+
_ = fmt.Sprintf(format, args...) // cause vet to treat this function like fmt.Printf
732+
}
733+
722734
buf, file, line := l.header(s, depth)
723735
// If a logger is set and doesn't support writing a formatted buffer,
724736
// we clear the generated header as we rely on the backing

0 commit comments

Comments
 (0)