Skip to content

Commit

Permalink
feat: Add -static-msg option
Browse files Browse the repository at this point in the history
This adds support for enforcing log messages are static by requiring the use of string literals or
constants.

Fixes #17.
  • Loading branch information
Matthew Dowdell authored and tmzane committed Oct 28, 2023
1 parent 61cece7 commit 1f6fb8a
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 0 deletions.
24 changes: 24 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 using static values for log messages (optional)
* Enforce a single key naming convention (optional)
* Enforce putting arguments on separate lines (optional)

Expand Down Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions sloglint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
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("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")
Expand Down
31 changes: 31 additions & 0 deletions testdata/src/static_msg/static_msg.go
Original file line number Diff line number Diff line change
@@ -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`
}

0 comments on commit 1f6fb8a

Please sign in to comment.