Skip to content

Commit

Permalink
logcheck: harmonize report messages
Browse files Browse the repository at this point in the history
If the message consists of full sentences, it should use valid spelling (upper
capital at the beginning, full stop at the end). Shorter messages should be
consistent with messages from other tools where lower case seems to be more
common (https://grep.app/search?q=pass.Report).
  • Loading branch information
pohly committed Mar 11, 2022
1 parent 085c4ea commit afe3a50
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
6 changes: 3 additions & 3 deletions hack/tools/logcheck/pkg/logcheck.go
Expand Up @@ -331,14 +331,14 @@ func isKeysValid(keyValues []ast.Expr, fun ast.Expr, pass *analysis.Pass, funNam
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),
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),
Message: fmt.Sprintf("Key positional arguments are expected to be inlined constant strings. Please replace %v provided with string value.", lit.Value),
})
continue
}
Expand Down Expand Up @@ -483,7 +483,7 @@ func checkForIfEnabled(i *ast.IfStmt, pass *analysis.Pass, c *config) {
pass.Report(analysis.Diagnostic{
Pos: i.Pos(),
End: i.End(),
Message: fmt.Sprintf("The result of %s should be stored in a variable and then be used multiple times: if %s := %s(); %s.Enabled() { ... %s.Info ... }",
Message: fmt.Sprintf("the result of %s should be stored in a variable and then be used multiple times: if %s := %s(); %s.Enabled() { ... %s.Info ... }",
funcCall, varName, funcCall, varName, varName),
})
}
4 changes: 2 additions & 2 deletions hack/tools/logcheck/testdata/src/verbose/verbose.go
Expand Up @@ -37,11 +37,11 @@ func verboseLogging() {
// \(\) is actually () in the diagnostic output. We have to escape here
// because `want` expects a regular expression.

if klog.V(1).Enabled() { // want `The result of klog.V should be stored in a variable and then be used multiple times: if klogV := klog.V\(\); klogV.Enabled\(\) { ... klogV.Info ... }`
if klog.V(1).Enabled() { // want `the result of klog.V should be stored in a variable and then be used multiple times: if klogV := klog.V\(\); klogV.Enabled\(\) { ... klogV.Info ... }`
klog.V(1).InfoS("I'm logging at level 1.")
}

if l.V(1).Enabled() { // want `The result of l.V should be stored in a variable and then be used multiple times: if l := l.V\(\); l.Enabled\(\) { ... l.Info ... }`
if l.V(1).Enabled() { // want `the result of l.V should be stored in a variable and then be used multiple times: if l := l.V\(\); l.Enabled\(\) { ... l.Info ... }`
l.V(1).Info("I'm logging at level 1.")
}

Expand Down

0 comments on commit afe3a50

Please sign in to comment.