From bfb2d07932c9d15442017a1bfe24c472004a3ab4 Mon Sep 17 00:00:00 2001 From: Tom <73077675+tmzane@users.noreply.github.com> Date: Tue, 24 Oct 2023 20:24:02 +0300 Subject: [PATCH] feat: implement `-key-naming-case` --- README.md | 5 + go.mod | 5 +- go.sum | 12 +++ sloglint.go | 99 ++++++++++++++++++- sloglint_test.go | 5 + .../src/key_naming_case/key_naming_case.go | 34 +++++++ testdata/src/mixed_args/mixed_args.go | 8 +- 7 files changed, 157 insertions(+), 11 deletions(-) create mode 100644 testdata/src/key_naming_case/key_naming_case.go diff --git a/README.md b/README.md index a839741..1f10acd 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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. diff --git a/go.mod b/go.mod index 8a494be..9953d4c 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 19878d6..ea74573 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,12 @@ +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= @@ -5,3 +14,6 @@ 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= diff --git a/sloglint.go b/sloglint.go index 2e0bbfc..ce74bef 100644 --- a/sloglint.go +++ b/sloglint.go @@ -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" @@ -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. @@ -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) @@ -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 } @@ -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") + } }) } @@ -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. diff --git a/sloglint_test.go b/sloglint_test.go index 9c40464..1da9ddf 100644 --- a/sloglint_test.go +++ b/sloglint_test.go @@ -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") diff --git a/testdata/src/key_naming_case/key_naming_case.go b/testdata/src/key_naming_case/key_naming_case.go new file mode 100644 index 0000000..c7fc5ea --- /dev/null +++ b/testdata/src/key_naming_case/key_naming_case.go @@ -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` +} diff --git a/testdata/src/mixed_args/mixed_args.go b/testdata/src/mixed_args/mixed_args.go index 39d552c..94ab46b 100644 --- a/testdata/src/mixed_args/mixed_args.go +++ b/testdata/src/mixed_args/mixed_args.go @@ -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` @@ -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`