diff --git a/logcheck/README.md b/logcheck/README.md index e01dd0e..04d2c80 100644 --- a/logcheck/README.md +++ b/logcheck/README.md @@ -80,3 +80,12 @@ disabled for the file. be used. The corresponding helper calls from `k8s.io/klogr` should be used instead. This is relevant when support contextual logging is disabled at runtime in klog. + +## verbosity-zero (enabled by default) + +This check flags all invocation of `klog.V(0)` or any of it's equivalent as errors + +## deprecations (enabled by default) + +This checks detects the usage of deprecated `klog` helper functions such as `KObjs` and suggests +a suitable alternative to replace them with. \ No newline at end of file diff --git a/logcheck/pkg/logcheck.go b/logcheck/pkg/logcheck.go index 92901e1..f74d653 100644 --- a/logcheck/pkg/logcheck.go +++ b/logcheck/pkg/logcheck.go @@ -37,6 +37,7 @@ const ( contextualCheck = "contextual" withHelpersCheck = "with-helpers" verbosityZeroCheck = "verbosity-zero" + deprecationsCheck = "deprecations" ) type checks map[string]*bool @@ -59,6 +60,7 @@ func Analyser() *analysis.Analyzer { contextualCheck: new(bool), withHelpersCheck: new(bool), verbosityZeroCheck: new(bool), + deprecationsCheck: new(bool), }, } c.fileOverrides.validChecks = map[string]bool{} @@ -73,6 +75,7 @@ klog methods (Info, Infof, Error, Errorf, Warningf, etc).`) logcheckFlags.BoolVar(c.enabled[contextualCheck], prefix+contextualCheck, false, `When true, logcheck will only allow log calls for contextual logging (retrieving a Logger from klog or the context and logging through that) and warn about all others.`) logcheckFlags.BoolVar(c.enabled[withHelpersCheck], prefix+withHelpersCheck, false, `When true, logcheck will warn about direct calls to WithName, WithValues and NewContext.`) logcheckFlags.BoolVar(c.enabled[verbosityZeroCheck], prefix+verbosityZeroCheck, true, `When true, logcheck will check whether the parameter for V() is 0.`) + logcheckFlags.BoolVar(c.enabled[deprecationsCheck], prefix+deprecationsCheck, true, `When true, logcheck will analyze the usage of deprecated Klog function calls.`) logcheckFlags.Var(&c.fileOverrides, "config", `A file which overrides the global settings for checks on a per-file basis via regular expressions.`) // Use env variables as defaults. This is necessary when used as plugin @@ -149,6 +152,17 @@ func checkForFunctionExpr(fexpr *ast.CallExpr, pass *analysis.Pass, c *config) { return } + // Check for Deprecated function usage + if c.isEnabled(deprecationsCheck, filename) { + message, deprecatedUse := isDeprecatedContextualCall(fName) + if deprecatedUse { + pass.Report(analysis.Diagnostic{ + Pos: fun.Pos(), + Message: message, + }) + } + } + // Matching if any unstructured logging function is used. if c.isEnabled(structuredCheck, filename) && isUnstructured(fName) { pass.Report(analysis.Diagnostic{ @@ -301,6 +315,18 @@ func isUnstructured(fName string) bool { return false } +func isDeprecatedContextualCall(fName string) (message string, deprecatedUse bool) { + deprecatedContextualLogHelper := map[string]string{ + "KObjs": "KObjSlice", + } + var replacementFunction string + if replacementFunction, deprecatedUse = deprecatedContextualLogHelper[fName]; deprecatedUse { + message = fmt.Sprintf(`Detected usage of deprecated helper "%s". Please switch to "%s" instead.`, fName, replacementFunction) + return + } + return +} + func isContextualCall(fName string) bool { // List of klog functions we still want to use after migration to // contextual logging. This is an allow list, so any new acceptable @@ -315,6 +341,7 @@ func isContextualCall(fName string) bool { "FromContext", "KObj", "KObjs", + "KObjSlice", "KRef", "LoggerWithName", "LoggerWithValues", @@ -519,7 +546,7 @@ func checkForVerbosityZero(fexpr *ast.CallExpr, pass *analysis.Pass) { return } if isVerbosityZero(expr) { - msg := fmt.Sprintf("Logging with V(0) is semantically equivalent to the same expression without it and just causes unnecessary overhead. It should get removed.") + msg := "Logging with V(0) is semantically equivalent to the same expression without it and just causes unnecessary overhead. It should get removed." pass.Report(analysis.Diagnostic{ Pos: fexpr.Fun.Pos(), Message: msg, diff --git a/logcheck/testdata/src/k8s.io/klog/v2/klog.go b/logcheck/testdata/src/k8s.io/klog/v2/klog.go index d62a5d4..f05f566 100644 --- a/logcheck/testdata/src/k8s.io/klog/v2/klog.go +++ b/logcheck/testdata/src/k8s.io/klog/v2/klog.go @@ -221,6 +221,11 @@ func KObjs(obj interface{}) interface{} { return nil } +// KObjSlice emulates klog.KObjSlice +func KObjSlice(obj interface{}) interface{} { + return nil +} + func KRef(namespace, name string) interface{} { return nil } diff --git a/logcheck/testdata/src/onlyAllowContextual/onlyAllowContextual.go b/logcheck/testdata/src/onlyAllowContextual/onlyAllowContextual.go index 4cc5e12..e6a71b2 100644 --- a/logcheck/testdata/src/onlyAllowContextual/onlyAllowContextual.go +++ b/logcheck/testdata/src/onlyAllowContextual/onlyAllowContextual.go @@ -29,4 +29,7 @@ func doNotAlllowKlog() { klog.InfoS("test log") // want `function "InfoS" should not be used, convert to contextual logging` klog.ErrorS(nil, "test log") // want `function "ErrorS" should not be used, convert to contextual logging` klog.V(1).Infof("test log") // want `function "V" should not be used, convert to contextual logging` `function "Infof" should not be used, convert to contextual logging` + + klog.KObjs(nil) // want `Detected usage of deprecated helper "KObjs". Please switch to "KObjSlice" instead.` + klog.InfoS("test log", "key", klog.KObjs(nil)) // want `function "InfoS" should not be used, convert to contextual logging` `Detected usage of deprecated helper "KObjs". Please switch to "KObjSlice" instead.` } diff --git a/logcheck/testdata/src/onlyAllowContextual/whitelistedKlog.go b/logcheck/testdata/src/onlyAllowContextual/whitelistedKlog.go index 07bb2a2..6acce7c 100644 --- a/logcheck/testdata/src/onlyAllowContextual/whitelistedKlog.go +++ b/logcheck/testdata/src/onlyAllowContextual/whitelistedKlog.go @@ -27,7 +27,7 @@ import ( func allowKlog() { klog.KObj(nil) - klog.KObjs(nil) + klog.KObjSlice(nil) klog.KRef("", "") klog.FromContext(nil) klog.TODO()