Skip to content

Commit

Permalink
feat: implement --forbidden-keys (#40)
Browse files Browse the repository at this point in the history
  • Loading branch information
tigrato authored May 9, 2024
1 parent f4014dd commit 431c6b7
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 25 deletions.
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
86 changes: 62 additions & 24 deletions sloglint.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"go/ast"
"go/token"
"go/types"
"slices"
"strconv"
"strings"

Expand All @@ -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.
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions sloglint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
26 changes: 26 additions & 0 deletions testdata/src/forbidden_keys/forbidden_keys.go
Original file line number Diff line number Diff line change
@@ -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`
}

0 comments on commit 431c6b7

Please sign in to comment.