diff --git a/README.md b/README.md index c080c2b..0a35818 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ With `sloglint` you can enforce various rules for `log/slog` based on your prefe * Enforce using static log messages (optional) * Enforce using constants instead of raw keys (optional) * Enforce a single key naming convention (optional) +* Enforce not using specific keys (optional) * Enforce putting arguments on separate lines (optional) ## 📦 Install @@ -74,7 +75,7 @@ slog.Info("a user has logged in", "user_id", 42) // sloglint: key-value pairs sh ### No global Some projects prefer to pass loggers as explicit dependencies. -The `no-global` option causes `sloglint` to report the usage of global loggers. +The `no-global` option causes `sloglint` to report the use of global loggers. ```go slog.Info("a user has logged in", "user_id", 42) // sloglint: global logger should not be used @@ -149,6 +150,18 @@ slog.Info("a user has logged in", "user-id", 42) // sloglint: keys should be wri Possible values are `snake`, `kebab`, `camel`, or `pascal`. +### Forbidden keys + +To prevent accidental use of reserved log keys, you may want to forbid specific keys altogether. +The `forbidden-keys` option causes `sloglint` to report the use of forbidden keys: + +```go +slog.Info("a user has logged in", "reserved", 42) // sloglint: "reserved" key is forbidden and should not be used +``` + +For example, when using the standard `slog.JSONHandler` and `slog.TextHandler`, +you may want to forbid the `time`, `level`, `msg`, and `source` keys, as these are used by the handlers. + ### Arguments on separate lines To improve code readability, you may want to put arguments on separate lines, especially when using key-value pairs. diff --git a/sloglint.go b/sloglint.go index d3b0176..c678267 100644 --- a/sloglint.go +++ b/sloglint.go @@ -8,6 +8,7 @@ import ( "go/ast" "go/token" "go/types" + "slices" "strconv" "strings" @@ -20,15 +21,16 @@ import ( // Options are options for the sloglint analyzer. type Options struct { - NoMixedArgs bool // Enforce not mixing key-value pairs and attributes (default true). - KVOnly bool // Enforce using key-value pairs only (overrides NoMixedArgs, incompatible with AttrOnly). - AttrOnly bool // Enforce using attributes only (overrides NoMixedArgs, incompatible with KVOnly). - NoGlobal string // Enforce not using global loggers ("all" or "default"). - ContextOnly string // Enforce using methods that accept a context ("all" or "scope"). - StaticMsg bool // Enforce using static log messages. - NoRawKeys bool // Enforce using constants instead of raw keys. - KeyNamingCase string // Enforce a single key naming convention ("snake", "kebab", "camel", or "pascal"). - ArgsOnSepLines bool // Enforce putting arguments on separate lines. + NoMixedArgs bool // Enforce not mixing key-value pairs and attributes (default true). + KVOnly bool // Enforce using key-value pairs only (overrides NoMixedArgs, incompatible with AttrOnly). + AttrOnly bool // Enforce using attributes only (overrides NoMixedArgs, incompatible with KVOnly). + NoGlobal string // Enforce not using global loggers ("all" or "default"). + ContextOnly string // Enforce using methods that accept a context ("all" or "scope"). + StaticMsg bool // Enforce using static log messages. + NoRawKeys bool // Enforce using constants instead of raw keys. + KeyNamingCase string // Enforce a single key naming convention ("snake", "kebab", "camel", or "pascal"). + ForbiddenKeys []string // Enforce not using specific keys. + ArgsOnSepLines bool // Enforce putting arguments on separate lines. } // New creates a new sloglint analyzer. @@ -104,6 +106,11 @@ func flags(opts *Options) flag.FlagSet { strVar(&opts.KeyNamingCase, "key-naming-case", "enforce a single key naming convention (snake|kebab|camel|pascal)") boolVar(&opts.ArgsOnSepLines, "args-on-sep-lines", "enforce putting arguments on separate lines") + fset.Func("forbidden-keys", "enforce not using specific keys (comma-separated)", func(s string) error { + opts.ForbiddenKeys = append(opts.ForbiddenKeys, strings.Split(s, ",")...) + return nil + }) + return *fset } @@ -249,15 +256,41 @@ func visit(pass *analysis.Pass, opts *Options, node ast.Node, stack []ast.Node) pass.Reportf(call.Pos(), "arguments should be put on separate lines") } + if len(opts.ForbiddenKeys) > 0 { + if name, found := badKeyNames(pass.TypesInfo, isForbiddenKey(opts.ForbiddenKeys), keys, attrs); found { + pass.Reportf(call.Pos(), "%q key is forbidden and should not be used", name) + } + } + switch { - case opts.KeyNamingCase == snakeCase && badKeyNames(pass.TypesInfo, strcase.ToSnake, keys, attrs): - pass.Reportf(call.Pos(), "keys should be written in snake_case") - case opts.KeyNamingCase == kebabCase && badKeyNames(pass.TypesInfo, strcase.ToKebab, keys, attrs): - pass.Reportf(call.Pos(), "keys should be written in kebab-case") - case opts.KeyNamingCase == camelCase && badKeyNames(pass.TypesInfo, strcase.ToCamel, keys, attrs): - pass.Reportf(call.Pos(), "keys should be written in camelCase") - case opts.KeyNamingCase == pascalCase && badKeyNames(pass.TypesInfo, strcase.ToPascal, keys, attrs): - pass.Reportf(call.Pos(), "keys should be written in PascalCase") + case opts.KeyNamingCase == snakeCase: + if _, found := badKeyNames(pass.TypesInfo, valueChanged(strcase.ToSnake), keys, attrs); found { + pass.Reportf(call.Pos(), "keys should be written in snake_case") + } + case opts.KeyNamingCase == kebabCase: + if _, found := badKeyNames(pass.TypesInfo, valueChanged(strcase.ToKebab), keys, attrs); found { + pass.Reportf(call.Pos(), "keys should be written in kebab-case") + } + case opts.KeyNamingCase == camelCase: + if _, found := badKeyNames(pass.TypesInfo, valueChanged(strcase.ToCamel), keys, attrs); found { + pass.Reportf(call.Pos(), "keys should be written in camelCase") + } + case opts.KeyNamingCase == pascalCase: + if _, found := badKeyNames(pass.TypesInfo, valueChanged(strcase.ToPascal), keys, attrs); found { + pass.Reportf(call.Pos(), "keys should be written in PascalCase") + } + } +} + +func isForbiddenKey(forbiddenKeys []string) func(string) bool { + return func(name string) bool { + return slices.Contains(forbiddenKeys, name) + } +} + +func valueChanged(handler func(string) string) func(string) bool { + return func(name string) bool { + return handler(name) != name } } @@ -351,10 +384,10 @@ func rawKeysUsed(info *types.Info, keys, attrs []ast.Expr) bool { return false } -func badKeyNames(info *types.Info, caseFn func(string) string, keys, attrs []ast.Expr) bool { +func badKeyNames(info *types.Info, validationFn func(string) bool, keys, attrs []ast.Expr) (string, bool) { for _, key := range keys { - if name, ok := getKeyName(key); ok && name != caseFn(name) { - return true + if name, ok := getKeyName(key); ok && validationFn(name) { + return name, true } } @@ -389,12 +422,12 @@ func badKeyNames(info *types.Info, caseFn func(string) string, keys, attrs []ast } } - if name, ok := getKeyName(expr); ok && name != caseFn(name) { - return true + if name, ok := getKeyName(expr); ok && validationFn(name) { + return name, true } } - return false + return "", false } func getKeyName(expr ast.Expr) (string, bool) { @@ -411,7 +444,12 @@ func getKeyName(expr ast.Expr) (string, bool) { } } if lit, ok := expr.(*ast.BasicLit); ok && lit.Kind == token.STRING { - return lit.Value, true + // string literals are always quoted. + value, err := strconv.Unquote(lit.Value) + if err != nil { + panic("unreachable") + } + return value, true } return "", false } diff --git a/sloglint_test.go b/sloglint_test.go index 90622e7..a84c70b 100644 --- a/sloglint_test.go +++ b/sloglint_test.go @@ -64,4 +64,9 @@ func TestAnalyzer(t *testing.T) { analyzer := sloglint.New(&sloglint.Options{ArgsOnSepLines: true}) analysistest.Run(t, testdata, analyzer, "args_on_sep_lines") }) + + t.Run("forbidden keys", func(t *testing.T) { + analyzer := sloglint.New(&sloglint.Options{ForbiddenKeys: []string{"foo_bar"}}) + analysistest.Run(t, testdata, analyzer, "forbidden_keys") + }) } diff --git a/testdata/src/forbidden_keys/forbidden_keys.go b/testdata/src/forbidden_keys/forbidden_keys.go new file mode 100644 index 0000000..ebd23d8 --- /dev/null +++ b/testdata/src/forbidden_keys/forbidden_keys.go @@ -0,0 +1,26 @@ +package forbidden_keys + +import "log/slog" + +const ( + snakeKey = "foo_bar" +) + +func tests() { + slog.Info("msg") + slog.Info("msg", "foo-bar", 1) + slog.Info("msg", "foo_bar", 1) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", snakeKey, 1) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Int("foo_bar", 1)) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Int(snakeKey, 1)) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{}) + slog.Info("msg", slog.Attr{"foo_bar", slog.IntValue(1)}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{snakeKey, slog.IntValue(1)}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{Key: "foo_bar"}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{Key: snakeKey}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{Key: "foo_bar", Value: slog.IntValue(1)}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{Key: snakeKey, Value: slog.IntValue(1)}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: "foo_bar"}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: snakeKey}) // want `"foo_bar" key is forbidden and should not be used` + slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: `foo_bar`}) // want `"foo_bar" key is forbidden and should not be used` +}