Skip to content

Commit

Permalink
feat: enforce lint rules to With func
Browse files Browse the repository at this point in the history 
This PR enforces the lint rules to `With` functions that are enforced to arguments in other logger funcs.

Signed-off-by: Tiago Silva <[email protected]>
  • Loading branch information
@tigrato
tigrato committed May 9, 2024
1 parent f4014dd commit 034e4bc
Show file tree
Hide file tree
Showing 9 changed files with 136 additions and 53 deletions.
76 changes: 44 additions & 32 deletions sloglint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
13 changes: 13 additions & 0 deletions testdata/src/args_on_sep_lines/args_on_sep_lines.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand All @@ -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`
}
8 changes: 5 additions & 3 deletions testdata/src/attr_only/attr_only.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
10 changes: 6 additions & 4 deletions testdata/src/context_only_all/context_only_all.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
}
36 changes: 35 additions & 1 deletion testdata/src/key_naming_case/key_naming_case.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package key_naming_case

import "log/slog"
import (
"context"
"log/slog"
)

const (
snakeKey = "foo_bar"
Expand Down Expand Up @@ -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() {
Expand Down
11 changes: 8 additions & 3 deletions testdata/src/kv_only/kv_only.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
4 changes: 3 additions & 1 deletion testdata/src/no_global_default/no_global_default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
24 changes: 15 additions & 9 deletions testdata/src/no_mixed_args/no_mixed_args.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
}
7 changes: 7 additions & 0 deletions testdata/src/no_raw_keys/no_raw_keys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}

0 comments on commit 034e4bc

Please sign in to comment.