From 034e4bc3fdd7fe6c68a505ea66a4f8ef8ae3f146 Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Thu, 9 May 2024 15:58:05 +0100 Subject: [PATCH] feat: enforce lint rules to `With` func This PR enforces the lint rules to `With` functions that are enforced to arguments in other logger funcs. Signed-off-by: Tiago Silva --- sloglint.go | 76 +++++++++++-------- .../args_on_sep_lines/args_on_sep_lines.go | 13 ++++ testdata/src/attr_only/attr_only.go | 8 +- .../src/context_only_all/context_only_all.go | 10 ++- .../src/key_naming_case/key_naming_case.go | 36 ++++++++- testdata/src/kv_only/kv_only.go | 11 ++- .../no_global_default/no_global_default.go | 4 +- testdata/src/no_mixed_args/no_mixed_args.go | 24 +++--- testdata/src/no_raw_keys/no_raw_keys.go | 7 ++ 9 files changed, 136 insertions(+), 53 deletions(-) diff --git a/sloglint.go b/sloglint.go index d3b0176..723cbf6 100644 --- a/sloglint.go +++ b/sloglint.go @@ -107,25 +107,34 @@ func flags(opts *Options) flag.FlagSet { return *fset } -var slogFuncs = map[string]int{ // funcName:argsPos - "log/slog.Log": 3, - "log/slog.Debug": 1, - "log/slog.Info": 1, - "log/slog.Warn": 1, - "log/slog.Error": 1, - "log/slog.DebugContext": 2, - "log/slog.InfoContext": 2, - "log/slog.WarnContext": 2, - "log/slog.ErrorContext": 2, - "(*log/slog.Logger).Log": 3, - "(*log/slog.Logger).Debug": 1, - "(*log/slog.Logger).Info": 1, - "(*log/slog.Logger).Warn": 1, - "(*log/slog.Logger).Error": 1, - "(*log/slog.Logger).DebugContext": 2, - "(*log/slog.Logger).InfoContext": 2, - "(*log/slog.Logger).WarnContext": 2, - "(*log/slog.Logger).ErrorContext": 2, +type slogFuncInfo struct { + argsPos int + skipContextCheck bool +} + +var slogFuncs = map[string]slogFuncInfo{ // funcName: + "log/slog.With": {argsPos: 0, skipContextCheck: true}, + "log/slog.Log": {argsPos: 3}, + "log/slog.LogAttrs": {argsPos: 3}, + "log/slog.Debug": {argsPos: 1}, + "log/slog.Info": {argsPos: 1}, + "log/slog.Warn": {argsPos: 1}, + "log/slog.Error": {argsPos: 1}, + "log/slog.DebugContext": {argsPos: 2}, + "log/slog.InfoContext": {argsPos: 2}, + "log/slog.WarnContext": {argsPos: 2}, + "log/slog.ErrorContext": {argsPos: 2}, + "(*log/slog.Logger).With": {argsPos: 0, skipContextCheck: true}, + "(*log/slog.Logger).Log": {argsPos: 3}, + "(*log/slog.Logger).LogAttrs": {argsPos: 3}, + "(*log/slog.Logger).Debug": {argsPos: 1}, + "(*log/slog.Logger).Info": {argsPos: 1}, + "(*log/slog.Logger).Warn": {argsPos: 1}, + "(*log/slog.Logger).Error": {argsPos: 1}, + "(*log/slog.Logger).DebugContext": {argsPos: 2}, + "(*log/slog.Logger).InfoContext": {argsPos: 2}, + "(*log/slog.Logger).WarnContext": {argsPos: 2}, + "(*log/slog.Logger).ErrorContext": {argsPos: 2}, } var attrFuncs = map[string]struct{}{ @@ -176,7 +185,7 @@ func visit(pass *analysis.Pass, opts *Options, node ast.Node, stack []ast.Node) } name := fn.FullName() - argsPos, ok := slogFuncs[name] + funcInfo, ok := slogFuncs[name] if !ok { return } @@ -192,25 +201,28 @@ func visit(pass *analysis.Pass, opts *Options, node ast.Node, stack []ast.Node) } } - switch opts.ContextOnly { - case "all": - typ := pass.TypesInfo.TypeOf(call.Args[0]) - if typ != nil && typ.String() != "context.Context" { - pass.Reportf(call.Pos(), "%sContext should be used instead", fn.Name()) - } - case "scope": - typ := pass.TypesInfo.TypeOf(call.Args[0]) - if typ != nil && typ.String() != "context.Context" && hasContextInScope(pass.TypesInfo, stack) { - pass.Reportf(call.Pos(), "%sContext should be used instead", fn.Name()) + // NOTE: with functions are not checked for context.Context. + if !funcInfo.skipContextCheck { + switch opts.ContextOnly { + case "all": + typ := pass.TypesInfo.TypeOf(call.Args[0]) + if typ != nil && typ.String() != "context.Context" { + pass.Reportf(call.Pos(), "%sContext should be used instead", fn.Name()) + } + case "scope": + typ := pass.TypesInfo.TypeOf(call.Args[0]) + if typ != nil && typ.String() != "context.Context" && hasContextInScope(pass.TypesInfo, stack) { + pass.Reportf(call.Pos(), "%sContext should be used instead", fn.Name()) + } } } - if opts.StaticMsg && !staticMsg(call.Args[argsPos-1]) { + if opts.StaticMsg && !staticMsg(call.Args[funcInfo.argsPos-1]) { pass.Reportf(call.Pos(), "message should be a string literal or a constant") } // NOTE: we assume that the arguments have already been validated by govet. - args := call.Args[argsPos:] + args := call.Args[funcInfo.argsPos:] if len(args) == 0 { return } diff --git a/testdata/src/args_on_sep_lines/args_on_sep_lines.go b/testdata/src/args_on_sep_lines/args_on_sep_lines.go index c37c714..6c3cf06 100644 --- a/testdata/src/args_on_sep_lines/args_on_sep_lines.go +++ b/testdata/src/args_on_sep_lines/args_on_sep_lines.go @@ -16,6 +16,16 @@ func tests() { slog.Int("bar", 2), ) + slog.With( + "foo", 1, + "bar", 2, + ).Info("msg") + + slog.With( + slog.Int("foo", 1), + slog.Int("bar", 2), + ).Info("msg") + slog.Info("msg", "foo", 1, "bar", 2) // want `arguments should be put on separate lines` slog.Info("msg", slog.Int("foo", 1), slog.Int("bar", 2)) // want `arguments should be put on separate lines` @@ -31,4 +41,7 @@ func tests() { slog.Info("msg", // want `arguments should be put on separate lines` slog.Int("foo", 1), slog.Int("bar", 2), ) + + slog.With("msg", "foo", 1, "bar", 2).Info("msg") // want `arguments should be put on separate lines` + slog.With("msg", slog.Int("foo", 1), slog.Int("bar", 2)).Info("msg") // want `arguments should be put on separate lines` } diff --git a/testdata/src/attr_only/attr_only.go b/testdata/src/attr_only/attr_only.go index 4933e4a..b189d2a 100644 --- a/testdata/src/attr_only/attr_only.go +++ b/testdata/src/attr_only/attr_only.go @@ -6,8 +6,10 @@ func tests() { slog.Info("msg") slog.Info("msg", slog.Int("foo", 1)) slog.Info("msg", slog.Int("foo", 1), slog.Int("bar", 2)) + slog.With(slog.Int("foo", 1), slog.Int("bar", 2)).Info("msg") - slog.Info("msg", "foo", 1) // want `key-value pairs should not be used` - slog.Info("msg", "foo", 1, "bar", 2) // want `key-value pairs should not be used` - slog.Info("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs should not be used` + slog.Info("msg", "foo", 1) // want `key-value pairs should not be used` + slog.Info("msg", "foo", 1, "bar", 2) // want `key-value pairs should not be used` + slog.Info("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs should not be used` + slog.With("foo", 1, slog.Int("bar", 2)).Info("msg") // want `key-value pairs should not be used` } diff --git a/testdata/src/context_only_all/context_only_all.go b/testdata/src/context_only_all/context_only_all.go index 171a2c7..9732b51 100644 --- a/testdata/src/context_only_all/context_only_all.go +++ b/testdata/src/context_only_all/context_only_all.go @@ -11,6 +11,7 @@ func tests(ctx context.Context) { slog.InfoContext(ctx, "msg") slog.WarnContext(ctx, "msg") slog.ErrorContext(ctx, "msg") + slog.With("key", "value").ErrorContext(ctx, "msg") slog.Debug("msg") // want `DebugContext should be used instead` slog.Info("msg") // want `InfoContext should be used instead` @@ -24,8 +25,9 @@ func tests(ctx context.Context) { logger.WarnContext(ctx, "msg") logger.ErrorContext(ctx, "msg") - logger.Debug("msg") // want `DebugContext should be used instead` - logger.Info("msg") // want `InfoContext should be used instead` - logger.Warn("msg") // want `WarnContext should be used instead` - logger.Error("msg") // want `ErrorContext should be used instead` + logger.Debug("msg") // want `DebugContext should be used instead` + logger.Info("msg") // want `InfoContext should be used instead` + logger.Warn("msg") // want `WarnContext should be used instead` + logger.Error("msg") // want `ErrorContext should be used instead` + logger.With("key", "value").Error("msg") // want `ErrorContext should be used instead` } diff --git a/testdata/src/key_naming_case/key_naming_case.go b/testdata/src/key_naming_case/key_naming_case.go index 4011565..8f5b36a 100644 --- a/testdata/src/key_naming_case/key_naming_case.go +++ b/testdata/src/key_naming_case/key_naming_case.go @@ -1,6 +1,9 @@ package key_naming_case -import "log/slog" +import ( + "context" + "log/slog" +) const ( snakeKey = "foo_bar" @@ -35,6 +38,37 @@ func tests() { 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` + + // With snake_case key + slog.Info("msg") + slog.With("foo_bar", 1).Info("msg") + slog.With(snakeKey, 1).Info("msg") + slog.With(slog.Int("foo_bar", 1)).Info("msg") + slog.With(slog.Int(snakeKey, 1)).Info("msg") + slog.With(slog.Attr{}).Info("msg") + slog.With(slog.Attr{"foo_bar", slog.IntValue(1)}).Info("msg") + slog.With(slog.Attr{snakeKey, slog.IntValue(1)}).Info("msg") + slog.With(slog.Attr{Key: "foo_bar"}).Info("msg") + slog.With(slog.Attr{Key: snakeKey}).Info("msg") + slog.With(slog.Attr{Key: "foo_bar", Value: slog.IntValue(1)}).Info("msg") + slog.With(slog.Attr{Key: snakeKey, Value: slog.IntValue(1)}).Info("msg") + slog.With(slog.Attr{Value: slog.IntValue(1), Key: "foo_bar"}).Info("msg") + slog.With(slog.Attr{Value: slog.IntValue(1), Key: snakeKey}).Info("msg") + + slog.With("foo-bar", 1).Info("msg") // want `keys should be written in snake_case` + slog.With(kebabKey, 1).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Int("foo-bar", 1)).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Int(kebabKey, 1)).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{"foo-bar", slog.IntValue(1)}).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{kebabKey, slog.IntValue(1)}).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{Key: "foo-bar"}).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{Key: kebabKey}).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{Key: "foo-bar", Value: slog.IntValue(1)}).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{Key: kebabKey, Value: slog.IntValue(1)}).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{Value: slog.IntValue(1), Key: "foo-bar"}).Info("msg") // want `keys should be written in snake_case` + slog.With(slog.Attr{Value: slog.IntValue(1), Key: kebabKey}).Info("msg") // want `keys should be written in snake_case` + + slog.LogAttrs(context.TODO(), slog.LevelInfo, "msg", slog.Attr{Value: slog.IntValue(1), Key: kebabKey}) // want `keys should be written in snake_case` } func issue35() { diff --git a/testdata/src/kv_only/kv_only.go b/testdata/src/kv_only/kv_only.go index 280bb54..6b3d37d 100644 --- a/testdata/src/kv_only/kv_only.go +++ b/testdata/src/kv_only/kv_only.go @@ -6,8 +6,13 @@ func tests() { slog.Info("msg") slog.Info("msg", "foo", 1) slog.Info("msg", "foo", 1, "bar", 2) + slog.With("foo", 1).Info("msg") + slog.With("foo", 1, "bar", 2).Info("msg") - slog.Info("msg", slog.Int("foo", 1)) // want `attributes should not be used` - slog.Info("msg", slog.Int("foo", 1), slog.Int("bar", 2)) // want `attributes should not be used` - slog.Info("msg", "foo", 1, slog.Int("bar", 2)) // want `attributes should not be used` + slog.Info("msg", slog.Int("foo", 1)) // want `attributes should not be used` + slog.Info("msg", slog.Int("foo", 1), slog.Int("bar", 2)) // want `attributes should not be used` + slog.Info("msg", "foo", 1, slog.Int("bar", 2)) // want `attributes should not be used` + slog.With(slog.Int("foo", 1)).Info("msg") // want `attributes should not be used` + slog.With(slog.Int("foo", 1), slog.Int("bar", 2)).Info("msg") // want `attributes should not be used` + slog.With("foo", 1, slog.Int("bar", 2)).Info("msg") // want `attributes should not be used` } diff --git a/testdata/src/no_global_default/no_global_default.go b/testdata/src/no_global_default/no_global_default.go index 4bdfb7a..e9164f2 100644 --- a/testdata/src/no_global_default/no_global_default.go +++ b/testdata/src/no_global_default/no_global_default.go @@ -5,6 +5,8 @@ import "log/slog" var logger = slog.New(nil) func tests() { - slog.Info("msg") // want `default logger should not be used` + slog.Info("msg") // want `default logger should not be used` + slog.With("key", "value") // want `default logger should not be used` logger.Info("msg") + logger.With("key", "value") } diff --git a/testdata/src/no_mixed_args/no_mixed_args.go b/testdata/src/no_mixed_args/no_mixed_args.go index 94ab46b..8659336 100644 --- a/testdata/src/no_mixed_args/no_mixed_args.go +++ b/testdata/src/no_mixed_args/no_mixed_args.go @@ -14,6 +14,9 @@ func tests() { 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)) + slog.With("foo", 1, "bar", 2).Info("msg") + slog.With(slog.Int("foo", 1)).Info("msg") + slog.With(slog.Int("foo", 1), slog.Int("bar", 2)).Info("msg") 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` @@ -24,14 +27,17 @@ func tests() { slog.InfoContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` 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` + slog.With("foo", 1, slog.Int("bar", 2)).ErrorContext(ctx, "msg") // want `key-value pairs and attributes should not be mixed` - 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` - logger.Warn("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` - logger.Error("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` - logger.DebugContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` - logger.InfoContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` - logger.WarnContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` - logger.ErrorContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` + 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` + logger.Warn("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` + logger.Error("msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` + logger.DebugContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` + logger.InfoContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` + logger.WarnContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` + logger.ErrorContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` + logger.With("foo", 1, slog.Int("bar", 2)).ErrorContext(ctx, "msg") // want `key-value pairs and attributes should not be mixed` + logger.With("foo", 1).ErrorContext(ctx, "msg", "foo", 1, slog.Int("bar", 2)) // want `key-value pairs and attributes should not be mixed` } diff --git a/testdata/src/no_raw_keys/no_raw_keys.go b/testdata/src/no_raw_keys/no_raw_keys.go index 0cd31ac..c8f5f7e 100644 --- a/testdata/src/no_raw_keys/no_raw_keys.go +++ b/testdata/src/no_raw_keys/no_raw_keys.go @@ -26,4 +26,11 @@ func tests() { 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` + + slog.With("foo", 1).Info("msg") // want `raw keys should not be used` + slog.With(slog.Int("foo", 1)).Info("msg") // want `raw keys should not be used` + slog.With(slog.Attr{"foo", slog.IntValue(1)}).Info("msg") // want `raw keys should not be used` + slog.With(slog.Attr{Key: "foo"}).Info("msg") // want `raw keys should not be used` + slog.With(slog.Attr{Key: "foo", Value: slog.IntValue(1)}).Info("msg") // want `raw keys should not be used` + slog.With(slog.Attr{Value: slog.IntValue(1), Key: "foo"}).Info("msg") // want `raw keys should not be used` }