From 1f6fb8a48b813f74433eb904753d5668f310ccc0 Mon Sep 17 00:00:00 2001 From: Matthew Dowdell Date: Sat, 28 Oct 2023 09:30:01 +0100 Subject: [PATCH 1/2] feat: Add -static-msg option This adds support for enforcing log messages are static by requiring the use of string literals or constants. Fixes #17. --- README.md | 24 +++++++++++++++++++++ sloglint.go | 17 +++++++++++++++ sloglint_test.go | 5 +++++ testdata/src/static_msg/static_msg.go | 31 +++++++++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 testdata/src/static_msg/static_msg.go diff --git a/README.md b/README.md index cb5ba18..13107ca 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 using static values for log messages (optional) * Enforce a single key naming convention (optional) * Enforce putting arguments on separate lines (optional) @@ -110,6 +111,29 @@ slog.Info("a user has logged in", UserId(42)) > 💡 Such helpers can be automatically generated for you by the [`sloggen`][4] tool. Give it a try too! +### Static messages + +To maximise the utility of structured logging, you may want to require log messages are static and +move dynamic values to attributes. The `static-msg` option causes `sloglint` to report log messages +that are not string literals or constants: + +```go +myMsg := generateMsg() +slog.Info(myMsg) // sloglint: messages should be string literals or constants + +slog.Info(fmt.Sprintf("message: %s", value)) // sloglint: messages should be string literals or constants +``` + +The report can be fixed by moving any dynamic values to attributes and useing a constant or a string +literal for the message: + +```go +const myMsg = "message" +slog.Info(myMsg) + +slog.Info("message", slog.String("value", value)) +``` + ### Key naming convention To ensure consistency in logs, you may want to enforce a single key naming convention. diff --git a/sloglint.go b/sloglint.go index e480c22..57037e0 100644 --- a/sloglint.go +++ b/sloglint.go @@ -23,6 +23,7 @@ type Options struct { 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. + StaticMsg bool // Enforce using static values for messages. KeyNamingCase string // Enforce a single key naming convention ("snake", "kebab", "camel", or "pascal"). ArgsOnSepLines bool // Enforce putting arguments on separate lines. } @@ -71,6 +72,7 @@ func flags(opts *Options) flag.FlagSet { boolVar(&opts.KVOnly, "kv-only", "enforce using key-value pairs only (incompatible with -attr-only)") boolVar(&opts.AttrOnly, "attr-only", "enforce using attributes only (incompatible with -kv-only)") boolVar(&opts.ContextOnly, "context-only", "enforce using methods that accept a context") + boolVar(&opts.StaticMsg, "static-msg", "enforce using static values for messages") 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") @@ -147,6 +149,10 @@ func run(pass *analysis.Pass, opts *Options) { } } + if opts.StaticMsg && !isStaticMsg(call.Args[argsPos-1]) { + pass.Reportf(call.Pos(), "messages should be string literals or constants") + } + // NOTE: we assume that the arguments have already been validated by govet. args := call.Args[argsPos:] if len(args) == 0 { @@ -199,6 +205,17 @@ func run(pass *analysis.Pass, opts *Options) { }) } +func isStaticMsg(arg ast.Expr) bool { + switch val := arg.(type) { + case *ast.BasicLit: + return true + case *ast.Ident: + return val.Obj != nil && val.Obj.Kind == ast.Con + default: + return false + } +} + func rawKeysUsed(info *types.Info, keys, attrs []ast.Expr) bool { isConst := func(expr ast.Expr) bool { ident, ok := expr.(*ast.Ident) diff --git a/sloglint_test.go b/sloglint_test.go index 1da9ddf..ad47a77 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("static msg", func(t *testing.T) { + analyzer := sloglint.New(&sloglint.Options{StaticMsg: true}) + analysistest.Run(t, testdata, analyzer, "static_msg") + }) + t.Run("key naming case", func(t *testing.T) { analyzer := sloglint.New(&sloglint.Options{KeyNamingCase: "snake"}) analysistest.Run(t, testdata, analyzer, "key_naming_case") diff --git a/testdata/src/static_msg/static_msg.go b/testdata/src/static_msg/static_msg.go new file mode 100644 index 0000000..f4724ef --- /dev/null +++ b/testdata/src/static_msg/static_msg.go @@ -0,0 +1,31 @@ +package static_msg + +import ( + "context" + "fmt" + "log/slog" +) + +const constMsg = "msg" + +var varMsg = "msg" + +func tests() { + ctx := context.Background() + + slog.Log(ctx, slog.LevelInfo, "msg") + slog.Debug("msg") + slog.DebugContext(ctx, "msg") + + slog.Log(ctx, slog.LevelInfo, constMsg) + slog.Debug(constMsg) + slog.DebugContext(ctx, constMsg) + + slog.Log(ctx, slog.LevelInfo, fmt.Sprintf("msg")) // want `messages should be string literals or constants` + slog.Debug(fmt.Sprintf("msg")) // want `messages should be string literals or constants` + slog.DebugContext(ctx, fmt.Sprintf("msg")) // want `messages should be string literals or constants` + + slog.Log(ctx, slog.LevelInfo, varMsg) // want `messages should be string literals or constants` + slog.Debug(varMsg) // want `messages should be string literals or constants` + slog.DebugContext(ctx, varMsg) // want `messages should be string literals or constants` +} From 4a41eaa24f979f27d44173df0602e0ab94caf506 Mon Sep 17 00:00:00 2001 From: Tom <73077675+tmzane@users.noreply.github.com> Date: Sun, 29 Oct 2023 15:23:52 +0300 Subject: [PATCH 2/2] minor tweaks --- README.md | 40 +++++++++++---------------- sloglint.go | 21 +++++++------- sloglint_test.go | 10 +++---- testdata/src/static_msg/static_msg.go | 20 +++++++------- 4 files changed, 41 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 13107ca..e2c719c 100644 --- a/README.md +++ b/README.md @@ -25,8 +25,8 @@ The linter has several options, so you can adjust it to your own code style. * Forbid mixing key-value pairs and attributes within a single function call (default) * Enforce using either key-value pairs or attributes for the entire project (optional) * Enforce using methods that accept a context (optional) +* Enforce using static log messages (optional) * Enforce using constants instead of raw keys (optional) -* Enforce using static values for log messages (optional) * Enforce a single key naming convention (optional) * Enforce putting arguments on separate lines (optional) @@ -83,6 +83,21 @@ This report can be fixed by using the equivalent method with the `Context` suffi slog.InfoContext(ctx, "a user has logged in") ``` +### Static messages + +To get the most out of structured logging, you may want to require log messages to be static. +The `static-msg` option causes `sloglint` to report non-static messages: + +```go +slog.Info(fmt.Sprintf("a user with id %d has logged in", 42)) // sloglint: message should be a string literal or a constant +``` + +The report can be fixed by moving dynamic values to arguments: + +```go +slog.Info("a user has logged in", "user_id", 42) +``` + ### No raw keys To prevent typos, you may want to forbid the use of raw keys altogether. @@ -111,29 +126,6 @@ slog.Info("a user has logged in", UserId(42)) > 💡 Such helpers can be automatically generated for you by the [`sloggen`][4] tool. Give it a try too! -### Static messages - -To maximise the utility of structured logging, you may want to require log messages are static and -move dynamic values to attributes. The `static-msg` option causes `sloglint` to report log messages -that are not string literals or constants: - -```go -myMsg := generateMsg() -slog.Info(myMsg) // sloglint: messages should be string literals or constants - -slog.Info(fmt.Sprintf("message: %s", value)) // sloglint: messages should be string literals or constants -``` - -The report can be fixed by moving any dynamic values to attributes and useing a constant or a string -literal for the message: - -```go -const myMsg = "message" -slog.Info(myMsg) - -slog.Info("message", slog.String("value", value)) -``` - ### Key naming convention To ensure consistency in logs, you may want to enforce a single key naming convention. diff --git a/sloglint.go b/sloglint.go index 57037e0..0f12f82 100644 --- a/sloglint.go +++ b/sloglint.go @@ -22,8 +22,8 @@ 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. + StaticMsg bool // Enforce using static log messages. NoRawKeys bool // Enforce using constants instead of raw keys. - StaticMsg bool // Enforce using static values for messages. KeyNamingCase string // Enforce a single key naming convention ("snake", "kebab", "camel", or "pascal"). ArgsOnSepLines bool // Enforce putting arguments on separate lines. } @@ -72,7 +72,7 @@ func flags(opts *Options) flag.FlagSet { boolVar(&opts.KVOnly, "kv-only", "enforce using key-value pairs only (incompatible with -attr-only)") boolVar(&opts.AttrOnly, "attr-only", "enforce using attributes only (incompatible with -kv-only)") boolVar(&opts.ContextOnly, "context-only", "enforce using methods that accept a context") - boolVar(&opts.StaticMsg, "static-msg", "enforce using static values for messages") + boolVar(&opts.StaticMsg, "static-msg", "enforce using static log messages") 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") @@ -148,9 +148,8 @@ func run(pass *analysis.Pass, opts *Options) { pass.Reportf(call.Pos(), "methods without a context should not be used") } } - - if opts.StaticMsg && !isStaticMsg(call.Args[argsPos-1]) { - pass.Reportf(call.Pos(), "messages should be string literals or constants") + if opts.StaticMsg && !staticMsg(call.Args[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. @@ -205,12 +204,12 @@ func run(pass *analysis.Pass, opts *Options) { }) } -func isStaticMsg(arg ast.Expr) bool { - switch val := arg.(type) { - case *ast.BasicLit: - return true - case *ast.Ident: - return val.Obj != nil && val.Obj.Kind == ast.Con +func staticMsg(expr ast.Expr) bool { + switch msg := expr.(type) { + case *ast.BasicLit: // e.g. slog.Info("msg") + return msg.Kind == token.STRING + case *ast.Ident: // e.g. const msg = "msg"; slog.Info(msg) + return msg.Obj != nil && msg.Obj.Kind == ast.Con default: return false } diff --git a/sloglint_test.go b/sloglint_test.go index ad47a77..17e81e7 100644 --- a/sloglint_test.go +++ b/sloglint_test.go @@ -30,16 +30,16 @@ func TestAnalyzer(t *testing.T) { analysistest.Run(t, testdata, analyzer, "context_only") }) + t.Run("static message", func(t *testing.T) { + analyzer := sloglint.New(&sloglint.Options{StaticMsg: true}) + analysistest.Run(t, testdata, analyzer, "static_msg") + }) + t.Run("no raw keys", func(t *testing.T) { analyzer := sloglint.New(&sloglint.Options{NoRawKeys: true}) analysistest.Run(t, testdata, analyzer, "no_raw_keys") }) - t.Run("static msg", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{StaticMsg: true}) - analysistest.Run(t, testdata, analyzer, "static_msg") - }) - t.Run("key naming case", func(t *testing.T) { analyzer := sloglint.New(&sloglint.Options{KeyNamingCase: "snake"}) analysistest.Run(t, testdata, analyzer, "key_naming_case") diff --git a/testdata/src/static_msg/static_msg.go b/testdata/src/static_msg/static_msg.go index f4724ef..bcf4cce 100644 --- a/testdata/src/static_msg/static_msg.go +++ b/testdata/src/static_msg/static_msg.go @@ -13,19 +13,19 @@ var varMsg = "msg" func tests() { ctx := context.Background() + slog.Info("msg") + slog.InfoContext(ctx, "msg") slog.Log(ctx, slog.LevelInfo, "msg") - slog.Debug("msg") - slog.DebugContext(ctx, "msg") + slog.Info(constMsg) + slog.InfoContext(ctx, constMsg) slog.Log(ctx, slog.LevelInfo, constMsg) - slog.Debug(constMsg) - slog.DebugContext(ctx, constMsg) - slog.Log(ctx, slog.LevelInfo, fmt.Sprintf("msg")) // want `messages should be string literals or constants` - slog.Debug(fmt.Sprintf("msg")) // want `messages should be string literals or constants` - slog.DebugContext(ctx, fmt.Sprintf("msg")) // want `messages should be string literals or constants` + slog.Info(fmt.Sprintf("msg")) // want `message should be a string literal or a constant` + slog.InfoContext(ctx, fmt.Sprintf("msg")) // want `message should be a string literal or a constant` + slog.Log(ctx, slog.LevelInfo, fmt.Sprintf("msg")) // want `message should be a string literal or a constant` - slog.Log(ctx, slog.LevelInfo, varMsg) // want `messages should be string literals or constants` - slog.Debug(varMsg) // want `messages should be string literals or constants` - slog.DebugContext(ctx, varMsg) // want `messages should be string literals or constants` + slog.Info(varMsg) // want `message should be a string literal or a constant` + slog.InfoContext(ctx, varMsg) // want `message should be a string literal or a constant` + slog.Log(ctx, slog.LevelInfo, varMsg) // want `message should be a string literal or a constant` }