Skip to content

Commit

Permalink
feat: implement -key-naming-case (#15)
Browse files Browse the repository at this point in the history
  • Loading branch information
tmzane authored Oct 25, 2023
1 parent 1301dc6 commit b2f5791
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 20 deletions.
29 changes: 20 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ A Go linter that ensures consistent code style when using `log/slog`.

The `log/slog` API allows two different types of arguments: key-value pairs and attributes.
People may have different opinions about which one is better,
but nobody probably wants to mix them up because it makes the code harder to read.
but nobody probably wants to mix them up because it makes the code harder to read:

```go
slog.Info("a user has logged in", "user_id", 42, slog.String("ip_address", "192.0.2.0")) // ugh
Expand All @@ -26,6 +26,7 @@ The linter has several options, so you can adjust it to your own code style.
* Enforce using either key-value pairs or attributes for the entire project (optional)
* Enforce using methods that accept a context (optional)
* Enforce using constants instead of raw keys (optional)
* Enforce a single key naming convention (optional)
* Enforce putting arguments on separate lines (optional)

## 📦 Install
Expand All @@ -40,15 +41,15 @@ sloglint [flags] ./...

### Key-value pairs only

The `-kv-only` flag causes `sloglint` to report any use of attributes.
The `-kv-only` flag causes `sloglint` to report any use of attributes:

```go
slog.Info("a user has logged in", slog.Int("user_id", 42)) // sloglint: attributes should not be used
```

### Attributes only

In contrast, the `-attr-only` flag causes `sloglint` to report any use of key-value pairs.
In contrast, the `-attr-only` flag causes `sloglint` to report any use of key-value pairs:

```go
slog.Info("a user has logged in", "user_id", 42) // sloglint: key-value pairs should not be used
Expand All @@ -58,13 +59,13 @@ slog.Info("a user has logged in", "user_id", 42) // sloglint: key-value pairs sh

Some `slog.Handler` implementations make use of the given `context.Context` (e.g. to access context values).
For them to work properly, you need to pass a context to all logger calls.
The `-context-only` flag causes `sloglint` to report the use of methods without a context.
The `-context-only` flag causes `sloglint` to report the use of methods without a context:

```go
slog.Info("a user has logged in") // sloglint: methods without a context should not be used
```

This report can be fixed by using the equivalent method with the `Context` suffix.
This report can be fixed by using the equivalent method with the `Context` suffix:

```go
slog.InfoContext(ctx, "a user has logged in")
Expand All @@ -73,7 +74,8 @@ slog.InfoContext(ctx, "a user has logged in")
### No raw keys

To prevent typos, you may want to forbid the use of raw keys altogether.
The `-no-raw-keys` flag causes `sloglint` to report the use of strings as keys (including `slog.Attr` calls, e.g. `slog.Int("user_id", 42)`).
The `-no-raw-keys` flag causes `sloglint` to report the use of strings as keys
(including `slog.Attr` calls, e.g. `slog.Int("user_id", 42)`):

```go
slog.Info("a user has logged in", "user_id", 42) // sloglint: raw keys should not be used
Expand All @@ -87,7 +89,7 @@ const UserId = "user_id"
slog.Info("a user has logged in", UserId, 42)
```

...or custom `slog.Attr` constructors.
...or custom `slog.Attr` constructors:

```go
func UserId(value int) slog.Attr { return slog.Int("user_id", value) }
Expand All @@ -97,16 +99,25 @@ slog.Info("a user has logged in", UserId(42))

> 💡 Such helpers can be automatically generated for you by the [`sloggen`][2] tool. Give it a try too!
### Key naming convention

To ensure consistency in logs, you may want to enforce a single key naming convention.
The `-key-naming-case=<snake|kebab|camel|pascal>` flag causes `sloglint` to report keys written in a case other than the given one:

```go
slog.Info("a user has logged in", "user-id", 42) // sloglint: keys should be written in snake_case
```

### Arguments on separate lines

To improve code readability, you may want to put arguments on separate lines, especially when using key-value pairs.
The `-args-on-sep-lines` flag causes `sloglint` to report 2+ arguments on the same line.
The `-args-on-sep-lines` flag causes `sloglint` to report 2+ arguments on the same line:

```go
slog.Info("a user has logged in", "user_id", 42, "ip_address", "192.0.2.0") // sloglint: arguments should be put on separate lines
```

This report can be fixed by reformatting the code.
This report can be fixed by reformatting the code:

```go
slog.Info("a user has logged in",
Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ module go-simpler.org/sloglint

go 1.20

require golang.org/x/tools v0.14.0
require (
github.com/ettle/strcase v0.1.1
golang.org/x/tools v0.14.0
)

require (
golang.org/x/mod v0.13.0 // indirect
Expand Down
12 changes: 12 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/ettle/strcase v0.1.1 h1:htFueZyVeE1XNnMEfbqp5r67qAN/4r6ya1ysq8Q+Zcw=
github.com/ettle/strcase v0.1.1/go.mod h1:hzDLsPC7/lwKyBOywSHEP89nt2pDgdy+No1NBA9o9VY=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY=
golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/tools v0.14.0 h1:jvNa2pY0M4r62jkRQ6RwEZZyPcymeL9XZMLBbV7U2nc=
golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
99 changes: 94 additions & 5 deletions sloglint.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ package sloglint
import (
"errors"
"flag"
"fmt"
"go/ast"
"go/token"
"go/types"
"strconv"

"github.com/ettle/strcase"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
Expand All @@ -17,11 +19,12 @@ import (

// Options are options for the sloglint analyzer.
type Options struct {
KVOnly bool // Enforce using key-value pairs only (incompatible with AttrOnly).
AttrOnly bool // Enforce using attributes only (incompatible with KVOnly).
ContextOnly bool // Enforce using methods that accept a context.
NoRawKeys bool // Enforce using constants instead of raw keys.
ArgsOnSepLines bool // Enforce putting arguments on separate lines.
KVOnly bool // Enforce using key-value pairs only (incompatible with AttrOnly).
AttrOnly bool // Enforce using attributes only (incompatible with KVOnly).
ContextOnly bool // Enforce using methods that accept a context.
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.
}

// New creates a new sloglint analyzer.
Expand All @@ -44,6 +47,13 @@ func New(opts *Options) *analysis.Analyzer {
}
}

const (
snakeCase = "snake"
kebabCase = "kebab"
camelCase = "camel"
pascalCase = "pascal"
)

func flags(opts *Options) flag.FlagSet {
fs := flag.NewFlagSet("sloglint", flag.ContinueOnError)

Expand All @@ -61,6 +71,16 @@ func flags(opts *Options) flag.FlagSet {
boolVar(&opts.NoRawKeys, "no-raw-keys", "enforce using constants instead of raw keys")
boolVar(&opts.ArgsOnSepLines, "args-on-sep-lines", "enforce putting arguments on separate lines")

fs.Func("key-naming-case", "enforce a single key naming convention (snake|kebab|camel|pascal)", func(s string) error {
switch s {
case snakeCase, kebabCase, camelCase, pascalCase:
opts.KeyNamingCase = s
return nil
default:
return fmt.Errorf("sloglint: -key-naming-case=%s: invalid value", s)
}
})

return *fs
}

Expand Down Expand Up @@ -160,6 +180,17 @@ func run(pass *analysis.Pass, opts *Options) {
if opts.ArgsOnSepLines && argsOnSameLine(pass.Fset, call, keys, attrs) {
pass.Reportf(call.Pos(), "arguments should be put on separate lines")
}

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")
}
})
}

Expand Down Expand Up @@ -211,6 +242,64 @@ 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 {
for _, key := range keys {
if name, ok := getKeyName(key); ok && name != caseFn(name) {
return true
}
}

for _, attr := range attrs {
var expr ast.Expr
switch attr := attr.(type) {
case *ast.CallExpr: // e.g. slog.Int()
fn := typeutil.StaticCallee(info, attr)
if _, ok := attrFuncs[fn.FullName()]; ok {
expr = attr.Args[0]
}
case *ast.CompositeLit: // slog.Attr{}
switch len(attr.Elts) {
case 1: // slog.Attr{Key: ...} | slog.Attr{Value: ...}
if kv := attr.Elts[0].(*ast.KeyValueExpr); kv.Key.(*ast.Ident).Name == "Key" {
expr = kv.Value
}
case 2: // slog.Attr{..., ...} | slog.Attr{Key: ..., Value: ...}
expr = attr.Elts[0]
if kv1, ok := attr.Elts[0].(*ast.KeyValueExpr); ok && kv1.Key.(*ast.Ident).Name == "Key" {
expr = kv1.Value
}
if kv2, ok := attr.Elts[1].(*ast.KeyValueExpr); ok && kv2.Key.(*ast.Ident).Name == "Key" {
expr = kv2.Value
}
}
}
if name, ok := getKeyName(expr); ok && name != caseFn(name) {
return true
}
}

return false
}

func getKeyName(expr ast.Expr) (string, bool) {
if expr == nil {
return "", false
}
if ident, ok := expr.(*ast.Ident); ok {
if ident.Obj == nil || ident.Obj.Decl == nil || ident.Obj.Kind != ast.Con {
return "", false
}
if spec, ok := ident.Obj.Decl.(*ast.ValueSpec); ok && len(spec.Values) > 0 {
// TODO: support len(spec.Values) > 1; e.g. "const foo, bar = 1, 2"
expr = spec.Values[0]
}
}
if lit, ok := expr.(*ast.BasicLit); ok && lit.Kind == token.STRING {
return lit.Value, true
}
return "", false
}

func argsOnSameLine(fset *token.FileSet, call ast.Expr, keys, attrs []ast.Expr) bool {
if len(keys)+len(attrs) <= 1 {
return false // special case: slog.Info("msg", "key", "value") is ok.
Expand Down
5 changes: 5 additions & 0 deletions sloglint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ func TestAnalyzer(t *testing.T) {
analysistest.Run(t, testdata, analyzer, "no_raw_keys")
})

t.Run("key naming case", func(t *testing.T) {
analyzer := sloglint.New(&sloglint.Options{KeyNamingCase: "snake"})
analysistest.Run(t, testdata, analyzer, "key_naming_case")
})

t.Run("arguments on separate lines", func(t *testing.T) {
analyzer := sloglint.New(&sloglint.Options{ArgsOnSepLines: true})
analysistest.Run(t, testdata, analyzer, "args_on_sep_lines")
Expand Down
38 changes: 38 additions & 0 deletions testdata/src/key_naming_case/key_naming_case.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package key_naming_case

import "log/slog"

const (
snakeKey = "foo_bar"
kebabKey = "foo-bar"
)

func tests() {
slog.Info("msg")
slog.Info("msg", "foo_bar", 1)
slog.Info("msg", snakeKey, 1)
slog.Info("msg", slog.Int("foo_bar", 1))
slog.Info("msg", slog.Int(snakeKey, 1))
slog.Info("msg", slog.Attr{})
slog.Info("msg", slog.Attr{"foo_bar", slog.IntValue(1)})
slog.Info("msg", slog.Attr{snakeKey, slog.IntValue(1)})
slog.Info("msg", slog.Attr{Key: "foo_bar"})
slog.Info("msg", slog.Attr{Key: snakeKey})
slog.Info("msg", slog.Attr{Key: "foo_bar", Value: slog.IntValue(1)})
slog.Info("msg", slog.Attr{Key: snakeKey, Value: slog.IntValue(1)})
slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: "foo_bar"})
slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: snakeKey})

slog.Info("msg", "foo-bar", 1) // want `keys should be written in snake_case`
slog.Info("msg", kebabKey, 1) // want `keys should be written in snake_case`
slog.Info("msg", slog.Int("foo-bar", 1)) // want `keys should be written in snake_case`
slog.Info("msg", slog.Int(kebabKey, 1)) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{"foo-bar", slog.IntValue(1)}) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{kebabKey, slog.IntValue(1)}) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{Key: "foo-bar"}) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{Key: kebabKey}) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{Key: "foo-bar", Value: slog.IntValue(1)}) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{Key: kebabKey, Value: slog.IntValue(1)}) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: "foo-bar"}) // want `keys should be written in snake_case`
slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: kebabKey}) // want `keys should be written in snake_case`
}
8 changes: 3 additions & 5 deletions testdata/src/mixed_args/mixed_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ import (
)

func tests() {
logger := slog.New(nil)
ctx := context.Background()

slog.Info("msg")
slog.Info("msg", "foo", 1)
slog.Info("msg", "foo", 1, "bar", 2)
slog.Info("msg", slog.Int("foo", 1))
slog.Info("msg", slog.Int("foo", 1), slog.Int("bar", 2))
}

func allFuncs() {
ctx := context.Background()

slog.Log(ctx, slog.LevelInfo, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed`
slog.Debug("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed`
Expand All @@ -26,7 +25,6 @@ func allFuncs() {
slog.WarnContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed`
slog.ErrorContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed`

logger := slog.New(nil)
logger.Log(ctx, slog.LevelInfo, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed`
logger.Debug("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed`
logger.Info("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed`
Expand Down
2 changes: 2 additions & 0 deletions testdata/src/no_raw_keys/no_raw_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ func tests() {
slog.Info("msg", slog.Attr{Key: foo})
slog.Info("msg", slog.Attr{Value: slog.IntValue(1)})
slog.Info("msg", slog.Attr{Key: foo, Value: slog.IntValue(1)})
slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: foo})

slog.Info("msg", "foo", 1) // want `raw keys should not be used`
slog.Info("msg", slog.Int("foo", 1)) // want `raw keys should not be used`
slog.Info("msg", slog.Attr{"foo", slog.IntValue(1)}) // want `raw keys should not be used`
slog.Info("msg", slog.Attr{Key: "foo"}) // want `raw keys should not be used`
slog.Info("msg", slog.Attr{Key: "foo", Value: slog.IntValue(1)}) // want `raw keys should not be used`
slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: "foo"}) // want `raw keys should not be used`
}

0 comments on commit b2f5791

Please sign in to comment.