Skip to content

Commit

Permalink
feat: implement -key-naming-case
Browse files Browse the repository at this point in the history
  • Loading branch information
tmzane committed Oct 24, 2023
1 parent 1301dc6 commit bfb2d07
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 11 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 Down Expand Up @@ -97,6 +98,10 @@ 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

WIP

### 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
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

Check failure on line 77 in sloglint.go

View workflow job for this annotation

GitHub Actions / lint / lint

underef: could simplify (*opts).KeyNamingCase to opts.KeyNamingCase (gocritic)
return nil
default:
return fmt.Errorf("sloglint: -key-naming-case=%s: invalid value", s)

Check warning on line 80 in sloglint.go

View check run for this annotation

Codecov / codecov/patch

sloglint.go#L75-L80

Added lines #L75 - L80 were not covered by tests
}
})

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

Check warning on line 192 in sloglint.go

View check run for this annotation

Codecov / codecov/patch

sloglint.go#L187-L192

Added lines #L187 - L192 were not covered by tests
}
})
}

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
}

Check warning on line 273 in sloglint.go

View check run for this annotation

Codecov / codecov/patch

sloglint.go#L272-L273

Added lines #L272 - L273 were not covered by tests
}
}
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
}

Check warning on line 291 in sloglint.go

View check run for this annotation

Codecov / codecov/patch

sloglint.go#L290-L291

Added lines #L290 - L291 were not covered by tests
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

Check warning on line 300 in sloglint.go

View check run for this annotation

Codecov / codecov/patch

sloglint.go#L300

Added line #L300 was not covered by tests
}

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
34 changes: 34 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,34 @@
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", "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`
}
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

0 comments on commit bfb2d07

Please sign in to comment.