Skip to content

Commit

Permalink
Merge pull request #229 from mowangdk/check_func_params
Browse files Browse the repository at this point in the history 
Add check for InfoS & ErrorS parameters
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Mar 20, 2021
2 parents 407242c + a18bc97 commit e158208
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 11 deletions.
5 changes: 4 additions & 1 deletion hack/tools/go.mod
View file
Expand Up @@ -2,4 +2,7 @@ module k8s.io/klog/hack/tools

go 1.15

require golang.org/x/tools v0.1.0
require (
golang.org/x/exp v0.0.0-20210220032938-85be41e4509f
golang.org/x/tools v0.1.0
)
22 changes: 21 additions & 1 deletion hack/tools/go.sum
View file
@@ -1,23 +1,43 @@
dmitri.shuralyov.com/gpu/mtl v0.0.0-20201218220906-28db891af037/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56/go.mod h1:JhuoJpWY28nO4Vef9tZUw9qufEGTyX1+7lmHxV5q5G4=
golang.org/x/exp v0.0.0-20210220032938-85be41e4509f h1:GrkO5AtFUU9U/1f5ctbIBXtBGeSJbWwIYfIsTcFMaX4=
golang.org/x/exp v0.0.0-20210220032938-85be41e4509f/go.mod h1:I6l2HNBLBZEcrOoCpyKLdY2lHoRZ8lI4x60KMCQDft4=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE=
golang.org/x/mobile v0.0.0-20201217150744-e6ae53a27f4f/go.mod h1:skQtrUTUwhdJvXM/2KKJzY8pDgNr9I/FOMqDVRPBUS4=
golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY=
golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
golang.org/x/mod v0.1.1-0.20191209134235-331c550502dd/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449 h1:xUIPaMhvROX9dhPvRCenIJtU78+lbEenGbgqB5hfHCQ=
golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190312151545-0bb0c0a6e846/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20200117012304-6edc0a871e69/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY=
golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
Expand Down
67 changes: 58 additions & 9 deletions hack/tools/logcheck/main.go
View file
Expand Up @@ -19,7 +19,9 @@ package main
import (
"fmt"
"go/ast"
"go/token"

"golang.org/x/exp/utf8string"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/singlechecker"
)
Expand Down Expand Up @@ -48,7 +50,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
// passing all function calls to checkForFunctionExpr
if fexpr, ok := n.(*ast.CallExpr); ok {

checkForFunctionExpr(fexpr.Fun, pass)
checkForFunctionExpr(fexpr.Fun, fexpr.Args, pass)
}

return true
Expand All @@ -58,7 +60,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
}

// checkForFunctionExpr checks for unstructured logging function, prints error if found any.
func checkForFunctionExpr(fun ast.Expr, pass *analysis.Pass) {
func checkForFunctionExpr(fun ast.Expr, args []ast.Expr, pass *analysis.Pass) {

/* we are extracting external package function calls e.g. klog.Infof fmt.Printf
and eliminating calls like setLocalHost()
Expand All @@ -80,14 +82,22 @@ func checkForFunctionExpr(fun ast.Expr, pass *analysis.Pass) {
// extracting package name
pName, ok := selExpr.X.(*ast.Ident)

// Matching if package name is klog and any unstructured logging function is used.
if ok && pName.Name == "klog" && isUnstructured((fName)) {
if ok && pName.Name == "klog" {
// Matching if any unstructured logging function is used.
if !isUnstructured((fName)) {
if fName == "InfoS" {
isKeysValid(args[1:], fun, pass, fName)
} else if fName == "ErrorS" {
isKeysValid(args[2:], fun, pass, fName)
}
} else {
msg := fmt.Sprintf("unstructured logging function %q should not be used", fName)
pass.Report(analysis.Diagnostic{
Pos: fun.Pos(),
Message: msg,
})
}

msg := fmt.Sprintf("unstructured logging function %q should not be used", fName)
pass.Report(analysis.Diagnostic{
Pos: fun.Pos(),
Message: msg,
})
}
}
}
Expand All @@ -110,3 +120,42 @@ func isUnstructured(fName string) bool {

return false
}

// isKeysValid check if all keys in keyAndValues is string type
func isKeysValid(keyValues []ast.Expr, fun ast.Expr, pass *analysis.Pass, funName string) {
if len(keyValues)%2 != 0 {
pass.Report(analysis.Diagnostic{
Pos: fun.Pos(),
Message: fmt.Sprintf("Additional arguments to %s should always be Key Value pairs. Please check if there is any key or value missing.", funName),
})
return
}

for index, arg := range keyValues {
if index%2 != 0 {
continue
}
lit, ok := arg.(*ast.BasicLit)
if !ok {
pass.Report(analysis.Diagnostic{
Pos: fun.Pos(),
Message: fmt.Sprintf("Key positional arguments are expected to be inlined constant strings. Please replace %v provided with string value", arg),
})
continue
}
if lit.Kind != token.STRING {
pass.Report(analysis.Diagnostic{
Pos: fun.Pos(),
Message: fmt.Sprintf("Key positional arguments are expected to be inlined constant strings. Please replace %v provided with string value", lit.Value),
})
continue
}
isASCII := utf8string.NewString(lit.Value).IsASCII()
if !isASCII {
pass.Report(analysis.Diagnostic{
Pos: fun.Pos(),
Message: fmt.Sprintf("Key positional arguments %s are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.", lit.Value),
})
}
}
}
9 changes: 9 additions & 0 deletions hack/tools/logcheck/testdata/data.go
View file
Expand Up @@ -44,4 +44,13 @@ func printUnstructuredLog() {
func printStructuredLog() {
klog.InfoS("test log")
klog.ErrorS(nil, "test log")
klog.InfoS("Starting container in a pod", "containerID", "containerID", "pod") // want `Additional arguments to InfoS should always be Key Value pairs. Please check if there is any key or value missing.`
klog.ErrorS(nil, "Starting container in a pod", "containerID", "containerID", "pod") // want `Additional arguments to ErrorS should always be Key Value pairs. Please check if there is any key or value missing.`
klog.InfoS("Starting container in a pod", "测试", "containerID") // want `Key positional arguments "测试" are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.`
klog.ErrorS(nil, "Starting container in a pod", "测试", "containerID") // want `Key positional arguments "测试" are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.`
klog.InfoS("Starting container in a pod", 7, "containerID") // want `Key positional arguments are expected to be inlined constant strings. Please replace 7 provided with string value`
klog.ErrorS(nil, "Starting container in a pod", 7, "containerID") // want `Key positional arguments are expected to be inlined constant strings. Please replace 7 provided with string value`
klog.InfoS("Starting container in a pod", map[string]string{"test1": "value"}, "containerID") // want `Key positional arguments are expected to be inlined constant strings. `
testKey := "a"
klog.ErrorS(nil, "Starting container in a pod", testKey, "containerID") // want `Key positional arguments are expected to be inlined constant strings. `
}

0 comments on commit e158208

Please sign in to comment.