Skip to content

Commit a18bc97

Browse files
mowangdkmowangdk
committedMar 20, 2021
Fix by pr suggestions
1 parent 4e4135c commit a18bc97

File tree

2 files changed

+43
-37
lines changed

2 files changed

+43
-37
lines changed
 

‎hack/tools/logcheck/main.go

+34-31
Original file line numberDiff line numberDiff line change
@@ -82,21 +82,22 @@ func checkForFunctionExpr(fun ast.Expr, args []ast.Expr, pass *analysis.Pass) {
8282
// extracting package name
8383
pName, ok := selExpr.X.(*ast.Ident)
8484

85-
if ok && pName.Name == "klog" && !isUnstructured((fName)) {
86-
if fName == "InfoS" {
87-
isKeysValid(args[1:], fun, pass, fName)
88-
} else if fName == "ErrorS" {
89-
isKeysValid(args[2:], fun, pass, fName)
85+
if ok && pName.Name == "klog" {
86+
// Matching if any unstructured logging function is used.
87+
if !isUnstructured((fName)) {
88+
if fName == "InfoS" {
89+
isKeysValid(args[1:], fun, pass, fName)
90+
} else if fName == "ErrorS" {
91+
isKeysValid(args[2:], fun, pass, fName)
92+
}
93+
} else {
94+
msg := fmt.Sprintf("unstructured logging function %q should not be used", fName)
95+
pass.Report(analysis.Diagnostic{
96+
Pos: fun.Pos(),
97+
Message: msg,
98+
})
9099
}
91-
}
92-
// Matching if package name is klog and any unstructured logging function is used.
93-
if ok && pName.Name == "klog" && isUnstructured((fName)) {
94100

95-
msg := fmt.Sprintf("unstructured logging function %q should not be used", fName)
96-
pass.Report(analysis.Diagnostic{
97-
Pos: fun.Pos(),
98-
Message: msg,
99-
})
100101
}
101102
}
102103
}
@@ -125,33 +126,35 @@ func isKeysValid(keyValues []ast.Expr, fun ast.Expr, pass *analysis.Pass, funNam
125126
if len(keyValues)%2 != 0 {
126127
pass.Report(analysis.Diagnostic{
127128
Pos: fun.Pos(),
128-
Message: fmt.Sprintf("Invalid Number of arguments for %s", funName),
129+
Message: fmt.Sprintf("Additional arguments to %s should always be Key Value pairs. Please check if there is any key or value missing.", funName),
129130
})
131+
return
130132
}
131133

132134
for index, arg := range keyValues {
133135
if index%2 != 0 {
134136
continue
135137
}
136-
if lit, ok := arg.(*ast.BasicLit); ok {
137-
if lit.Kind != token.STRING {
138-
pass.Report(analysis.Diagnostic{
139-
Pos: fun.Pos(),
140-
Message: fmt.Sprintf("Invalid value type for key %v", lit.Value),
141-
})
142-
continue
143-
}
144-
isASCII := utf8string.NewString(lit.Value).IsASCII()
145-
if !isASCII {
146-
pass.Report(analysis.Diagnostic{
147-
Pos: fun.Pos(),
148-
Message: fmt.Sprintf("Invalid value for key %v, it's must be ascii", lit.Value),
149-
})
150-
}
151-
} else {
138+
lit, ok := arg.(*ast.BasicLit)
139+
if !ok {
140+
pass.Report(analysis.Diagnostic{
141+
Pos: fun.Pos(),
142+
Message: fmt.Sprintf("Key positional arguments are expected to be inlined constant strings. Please replace %v provided with string value", arg),
143+
})
144+
continue
145+
}
146+
if lit.Kind != token.STRING {
147+
pass.Report(analysis.Diagnostic{
148+
Pos: fun.Pos(),
149+
Message: fmt.Sprintf("Key positional arguments are expected to be inlined constant strings. Please replace %v provided with string value", lit.Value),
150+
})
151+
continue
152+
}
153+
isASCII := utf8string.NewString(lit.Value).IsASCII()
154+
if !isASCII {
152155
pass.Report(analysis.Diagnostic{
153156
Pos: fun.Pos(),
154-
Message: "Invalid value type for key",
157+
Message: fmt.Sprintf("Key positional arguments %s are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.", lit.Value),
155158
})
156159
}
157160
}

‎hack/tools/logcheck/testdata/data.go

+9-6
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,13 @@ func printUnstructuredLog() {
4444
func printStructuredLog() {
4545
klog.InfoS("test log")
4646
klog.ErrorS(nil, "test log")
47-
klog.InfoS("Starting container in a pod", "containerID", "containerID", "pod") // want `Invalid Number of arguments for InfoS`
48-
klog.ErrorS(nil, "Starting container in a pod", "containerID", "containerID", "pod") // want `Invalid Number of arguments for ErrorS`
49-
klog.InfoS("Starting container in a pod", "测试", "containerID") // want `Invalid value for key`
50-
klog.ErrorS(nil, "Starting container in a pod", "测试", "containerID") // want `Invalid value for key`
51-
klog.InfoS("Starting container in a pod", 7, "containerID") // want `Invalid value type for key 7`
52-
klog.ErrorS(nil, "Starting container in a pod", 7, "containerID") // want `Invalid value type for key 7`
47+
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.`
48+
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.`
49+
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.`
50+
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.`
51+
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`
52+
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`
53+
klog.InfoS("Starting container in a pod", map[string]string{"test1": "value"}, "containerID") // want `Key positional arguments are expected to be inlined constant strings. `
54+
testKey := "a"
55+
klog.ErrorS(nil, "Starting container in a pod", testKey, "containerID") // want `Key positional arguments are expected to be inlined constant strings. `
5356
}

0 commit comments

Comments
 (0)
Please sign in to comment.