Skip to content

Commit

Permalink
Add -context-only option
Browse files Browse the repository at this point in the history
As described in go-simpler#13, third-party implementations of `slog.Handler` can choose to take values out of
a context to supplement the attributes already used. An example of this in my day job is adding
trace IDs to enhance serviceability.

This change adds the ability to optionally identify `slog` methods that do not take a context and so
prevent the ability to take values from that context. Any matches can be resolved by appending
`Context` to the method. For example, `slog.Info` becomes `slog.Context`.
  • Loading branch information
Matthew Dowdell committed Oct 21, 2023
1 parent bc45156 commit 058e2f9
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 0 deletions.
18 changes: 18 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 constants instead of raw keys (optional)
* Enforce putting arguments on separate lines (optional)
* Enforce using methods that take a context (optional)

## 📦 Install

Expand Down Expand Up @@ -98,5 +99,22 @@ slog.Info("a user has logged in",
)
```

### Context only

The `-context-only` flag causes `sloglint` to report the use of any methods that do not take a `context.Context`.

```go
slog.Info("a user has logged in") // sloglint: methods that do not take a context should not be used"
```

This report can be fixed by using the equivalent method with a `Context` suffix.

```go
slog.InfoContext(ctx, "a user has logged in")
```

The `slog.Handler` implementations in the standard library do not currently utilise the given
context. However, third-party implementations can create `slog.Attr` instances from the context.

[1]: https://github.com/go-simpler/sloglint/releases
[2]: https://github.com/go-simpler/sloggen
11 changes: 11 additions & 0 deletions sloglint.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"go/token"
"go/types"
"strconv"
"strings"

"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
Expand All @@ -21,6 +22,7 @@ type Options struct {
AttrOnly bool // Enforce using attributes only (incompatible with KVOnly).
NoRawKeys bool // Enforce using constants instead of raw keys.
ArgsOnSepLines bool // Enforce putting arguments on separate lines.
ContextOnly bool // Enforce using *Context methods only.
}

// New creates a new sloglint analyzer.
Expand Down Expand Up @@ -58,6 +60,7 @@ func flags(opts *Options) flag.FlagSet {
boolVar(&opts.AttrOnly, "attr-only", "enforce using attributes only (incompatible with -kv-only)")
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")
boolVar(&opts.ContextOnly, "context-only", "enforce using methods that take a context")

return *fs
}
Expand Down Expand Up @@ -113,6 +116,14 @@ func run(pass *analysis.Pass, opts *Options) {
return
}

if opts.ContextOnly {
fnName := fn.FullName()

if !strings.HasSuffix(fnName, ".Log") && !strings.HasSuffix(fnName, "Context") {
pass.Reportf(call.Pos(), "methods that do not take a context should not be used")
}
}

// NOTE: we assume that the arguments have already been validated by govet.
args := call.Args[argsPos:]
if len(args) == 0 {
Expand Down
5 changes: 5 additions & 0 deletions sloglint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,9 @@ func TestAnalyzer(t *testing.T) {
analyzer := sloglint.New(&sloglint.Options{ArgsOnSepLines: true})
analysistest.Run(t, testdata, analyzer, "args_on_sep_lines")
})

t.Run("context only", func(t *testing.T) {
analyzer := sloglint.New(&sloglint.Options{ContextOnly: true})
analysistest.Run(t, testdata, analyzer, "context_only")
})
}
35 changes: 35 additions & 0 deletions testdata/src/context_only/context_only.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package context_only

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

func tests() {
ctx := context.Background()

slog.Debug("msg") // want `methods that do not take a context should not be used`
slog.Info("msg") // want `methods that do not take a context should not be used`
slog.Warn("msg") // want `methods that do not take a context should not be used`
slog.Error("msg") // want `methods that do not take a context should not be used`

slog.Log(ctx, slog.LevelInfo, "msg")
slog.DebugContext(ctx, "msg")
slog.InfoContext(ctx, "msg")
slog.WarnContext(ctx, "msg")
slog.ErrorContext(ctx, "msg")

logger := slog.New(slog.NewJSONHandler(io.Discard, &slog.HandlerOptions{}))

logger.Debug("msg") // want `methods that do not take a context should not be used`
logger.Info("msg") // want `methods that do not take a context should not be used`
logger.Warn("msg") // want `methods that do not take a context should not be used`
logger.Error("msg") // want `methods that do not take a context should not be used`

logger.Log(ctx, slog.LevelInfo, "msg")
logger.DebugContext(ctx, "msg")
logger.InfoContext(ctx, "msg")
logger.WarnContext(ctx, "msg")
logger.ErrorContext(ctx, "msg")
}

0 comments on commit 058e2f9

Please sign in to comment.