Skip to content

Commit

Permalink
feat: implement -static-msg (#18)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattdowdell authored Oct 29, 2023
1 parent 61cece7 commit 43ec8d8
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 0 deletions.
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ 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 a single key naming convention (optional)
* Enforce putting arguments on separate lines (optional)
Expand Down Expand Up @@ -82,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.
Expand Down
16 changes: 16 additions & 0 deletions sloglint.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ 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.
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 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")

Expand Down Expand Up @@ -146,6 +148,9 @@ func run(pass *analysis.Pass, opts *Options) {
pass.Reportf(call.Pos(), "methods without a context should not be used")
}
}
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.
args := call.Args[argsPos:]
Expand Down Expand Up @@ -199,6 +204,17 @@ func run(pass *analysis.Pass, opts *Options) {
})
}

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
}
}

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 @@ -30,6 +30,11 @@ 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")
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.Info("msg")
slog.InfoContext(ctx, "msg")
slog.Log(ctx, slog.LevelInfo, "msg")

slog.Info(constMsg)
slog.InfoContext(ctx, constMsg)
slog.Log(ctx, slog.LevelInfo, constMsg)

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.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`
}

0 comments on commit 43ec8d8

Please sign in to comment.