Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement --forbidden-keys #40

Merged
merged 6 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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`
}
Loading