-
Notifications
You must be signed in to change notification settings - Fork 8
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: key value check is special for zap (#28)
* fix: key value check is special for zap Fixes #27 Signed-off-by: Timon Wong <[email protected]> * add comments Signed-off-by: Timon Wong <[email protected]> * test: make zap Field test cover both default sugared and custom sugared, add `With` test case with Field * chore: fix zap checker comment Signed-off-by: Timon Wong <[email protected]> Co-authored-by: ttyS3 <[email protected]>
- Loading branch information
Showing
8 changed files
with
278 additions
and
129 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package checkers | ||
|
|
||
| import ( | ||
| "go/ast" | ||
| "go/types" | ||
|
|
||
| "golang.org/x/tools/go/analysis" | ||
| ) | ||
|
|
||
| type Config struct { | ||
| RequireStringKey bool | ||
| } | ||
|
|
||
| type CallContext struct { | ||
| Expr *ast.CallExpr | ||
| Func *types.Func | ||
| Signature *types.Signature | ||
| } | ||
|
|
||
| type Checker interface { | ||
| CheckAndReport(pass *analysis.Pass, call CallContext, cfg Config) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| package checkers | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "go/ast" | ||
| "go/printer" | ||
| "go/token" | ||
| "go/types" | ||
| "unicode/utf8" | ||
|
|
||
| "golang.org/x/tools/go/analysis" | ||
|
|
||
| "github.com/timonwong/loggercheck/internal/bytebufferpool" | ||
| "github.com/timonwong/loggercheck/internal/stringutil" | ||
| ) | ||
|
|
||
| // getStringValueFromArg returns true if the argument is string literal or string constant. | ||
| func getStringValueFromArg(pass *analysis.Pass, arg ast.Expr) (value string, ok bool) { | ||
| switch arg := arg.(type) { | ||
| case *ast.BasicLit: // literals, must be string | ||
| if arg.Kind == token.STRING { | ||
| return arg.Value, true | ||
| } | ||
| case *ast.Ident: // identifiers, we require constant string key | ||
| if arg.Obj != nil && arg.Obj.Kind == ast.Con { | ||
| typeAndValue := pass.TypesInfo.Types[arg] | ||
| if typ, ok := typeAndValue.Type.(*types.Basic); ok { | ||
| if typ.Kind() == types.String { | ||
| return typeAndValue.Value.ExactString(), true | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return "", false | ||
| } | ||
|
|
||
| func checkLoggingKey(pass *analysis.Pass, keyValuesArgs []ast.Expr) { | ||
| for i := 0; i < len(keyValuesArgs); i += 2 { | ||
| arg := keyValuesArgs[i] | ||
| if value, ok := getStringValueFromArg(pass, arg); ok { | ||
| if stringutil.IsASCII(value) { | ||
| continue | ||
| } | ||
|
|
||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: arg.Pos(), | ||
| End: arg.End(), | ||
| Category: "logging", | ||
| Message: fmt.Sprintf( | ||
| "logging keys are expected to be alphanumeric strings, please remove any non-latin characters from %s", | ||
| value), | ||
| }) | ||
| } else { | ||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: arg.Pos(), | ||
| End: arg.End(), | ||
| Category: "logging", | ||
| Message: fmt.Sprintf( | ||
| "logging keys are expected to be inlined constant strings, please replace %q provided with string", | ||
| renderNodeEllipsis(pass.Fset, arg)), | ||
| }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func renderNodeEllipsis(fset *token.FileSet, v interface{}) string { | ||
| const maxLen = 20 | ||
|
|
||
| buf := bytebufferpool.Get() | ||
| defer bytebufferpool.Put(buf) | ||
|
|
||
| _ = printer.Fprint(buf, fset, v) | ||
| s := buf.String() | ||
| if utf8.RuneCountInString(s) > maxLen { | ||
| // Copied from go/constant/value.go | ||
| i := 0 | ||
| for n := 0; n < maxLen-3; n++ { | ||
| _, size := utf8.DecodeRuneInString(s[i:]) | ||
| i += size | ||
| } | ||
| s = s[:i] + "..." | ||
| } | ||
| return s | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package checkers | ||
|
|
||
| import ( | ||
| "golang.org/x/tools/go/analysis" | ||
| ) | ||
|
|
||
| type General struct{} | ||
|
|
||
| var _ Checker = (*General)(nil) | ||
|
|
||
| func (g General) CheckAndReport(pass *analysis.Pass, call CallContext, cfg Config) { | ||
| args := call.Expr.Args | ||
| params := call.Signature.Params() | ||
|
|
||
| nparams := params.Len() // variadic => nonzero | ||
| startIndex := nparams - 1 | ||
| nargs := len(args) | ||
|
|
||
| // Check the argument count | ||
| variadicLen := nargs - startIndex | ||
| if variadicLen%2 != 0 { | ||
| firstArg := args[startIndex] | ||
| lastArg := args[nargs-1] | ||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: firstArg.Pos(), | ||
| End: lastArg.End(), | ||
| Category: "logging", | ||
| Message: "odd number of arguments passed as key-value pairs for logging", | ||
| }) | ||
| } | ||
|
|
||
| // Check the "key" type | ||
| if cfg.RequireStringKey { | ||
| checkLoggingKey(pass, args[startIndex:]) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package checkers | ||
|
|
||
| import ( | ||
| "go/ast" | ||
| "go/types" | ||
|
|
||
| "golang.org/x/tools/go/analysis" | ||
| ) | ||
|
|
||
| type Zap struct { | ||
| General | ||
| } | ||
|
|
||
| var _ Checker = (*Zap)(nil) | ||
|
|
||
| func (z Zap) CheckAndReport(pass *analysis.Pass, call CallContext, cfg Config) { | ||
| args := call.Expr.Args | ||
| params := call.Signature.Params() | ||
|
|
||
| nparams := params.Len() // variadic => nonzero | ||
| startIndex := nparams - 1 | ||
| nargs := len(args) | ||
|
|
||
| // Check the argument count | ||
| keyValuesArgs := make([]ast.Expr, 0, nargs-startIndex) | ||
| for i := startIndex; i < nargs; i++ { | ||
| arg := args[i] | ||
| switch arg := arg.(type) { | ||
| case *ast.CallExpr, *ast.Ident: | ||
| typ := pass.TypesInfo.TypeOf(arg) | ||
| switch typ := typ.(type) { | ||
| case *types.Named: | ||
| obj := typ.Obj() | ||
| // This is a strongly-typed field. Consume it and move on. | ||
| // Actually it's go.uber.org/zap/zapcore.Field, however for simplicity | ||
| // we don't check the import path | ||
| if obj != nil && obj.Name() == "Field" { | ||
| continue | ||
| } | ||
| default: | ||
| // pass | ||
| } | ||
| } | ||
| keyValuesArgs = append(keyValuesArgs, arg) | ||
| } | ||
|
|
||
| if len(keyValuesArgs)%2 != 0 { | ||
| firstArg := args[startIndex] | ||
| lastArg := args[nargs-1] | ||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: firstArg.Pos(), | ||
| End: lastArg.End(), | ||
| Category: "logging", | ||
| Message: "odd number of arguments passed as key-value pairs for logging", | ||
| }) | ||
| } | ||
|
|
||
| // Check the "key" type | ||
| if cfg.RequireStringKey { | ||
| checkLoggingKey(pass, keyValuesArgs) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.