From 44f7796438e567ee014586c24a7afbab9a1ebed1 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Fri, 1 Sep 2023 14:04:38 -0400 Subject: [PATCH] gopls: add and enable the slog analyzer This analyzer is included in go/vet, and so gopls should have it as well. Change-Id: Ib5cbee44a1f38c4aa45d75bcaa7a345d88099d8e Reviewed-on: https://go-review.googlesource.com/c/tools/+/524764 Reviewed-by: Alan Donovan Auto-Submit: Robert Findley gopls-CI: kokoro LUCI-TryBot-Result: Go LUCI --- gopls/doc/analyzers.md | 18 +++++++++++++++ gopls/internal/lsp/source/api_json.go | 11 +++++++++ gopls/internal/lsp/source/options.go | 2 ++ .../marker/testdata/diagnostics/analyzers.txt | 23 +++++++++++++++++-- 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 9a592d4b890..2ff9434d0b6 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -494,6 +494,24 @@ This is one of the simplifications that "gofmt -s" applies. **Enabled by default.** +## **slog** + +check for invalid structured logging calls + +The slog checker looks for calls to functions from the log/slog +package that take alternating key-value pairs. It reports calls +where an argument in a key position is neither a string nor a +slog.Attr, and where a final key is missing its value. +For example,it would report + + slog.Warn("message", 11, "k") // slog.Warn arg "11" should be a string or a slog.Attr + +and + + slog.Info("message", "k1", v1, "k2") // call to slog.Info missing a final value + +**Enabled by default.** + ## **sortslice** check the argument type of sort.Slice diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go index 7635edba2e6..60425db2c5c 100644 --- a/gopls/internal/lsp/source/api_json.go +++ b/gopls/internal/lsp/source/api_json.go @@ -358,6 +358,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "check for slice simplifications\n\nA slice expression of the form:\n\ts[a:len(s)]\nwill be simplified to:\n\ts[a:]\n\nThis is one of the simplifications that \"gofmt -s\" applies.", Default: "true", }, + { + Name: "\"slog\"", + Doc: "check for invalid structured logging calls\n\nThe slog checker looks for calls to functions from the log/slog\npackage that take alternating key-value pairs. It reports calls\nwhere an argument in a key position is neither a string nor a\nslog.Attr, and where a final key is missing its value.\nFor example,it would report\n\n\tslog.Warn(\"message\", 11, \"k\") // slog.Warn arg \"11\" should be a string or a slog.Attr\n\nand\n\n\tslog.Info(\"message\", \"k1\", v1, \"k2\") // call to slog.Info missing a final value", + Default: "true", + }, { Name: "\"sortslice\"", Doc: "check the argument type of sort.Slice\n\nsort.Slice requires an argument of a slice type. Check that\nthe interface{} value passed to sort.Slice is actually a slice.", @@ -1070,6 +1075,12 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "check for slice simplifications\n\nA slice expression of the form:\n\ts[a:len(s)]\nwill be simplified to:\n\ts[a:]\n\nThis is one of the simplifications that \"gofmt -s\" applies.", Default: true, }, + { + Name: "slog", + Doc: "check for invalid structured logging calls\n\nThe slog checker looks for calls to functions from the log/slog\npackage that take alternating key-value pairs. It reports calls\nwhere an argument in a key position is neither a string nor a\nslog.Attr, and where a final key is missing its value.\nFor example,it would report\n\n\tslog.Warn(\"message\", 11, \"k\") // slog.Warn arg \"11\" should be a string or a slog.Attr\n\nand\n\n\tslog.Info(\"message\", \"k1\", v1, \"k2\") // call to slog.Info missing a final value", + URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/slog", + Default: true, + }, { Name: "sortslice", Doc: "check the argument type of sort.Slice\n\nsort.Slice requires an argument of a slice type. Check that\nthe interface{} value passed to sort.Slice is actually a slice.", diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go index 67818fad34a..2b91f834d6a 100644 --- a/gopls/internal/lsp/source/options.go +++ b/gopls/internal/lsp/source/options.go @@ -39,6 +39,7 @@ import ( "golang.org/x/tools/go/analysis/passes/printf" "golang.org/x/tools/go/analysis/passes/shadow" "golang.org/x/tools/go/analysis/passes/shift" + "golang.org/x/tools/go/analysis/passes/slog" "golang.org/x/tools/go/analysis/passes/sortslice" "golang.org/x/tools/go/analysis/passes/stdmethods" "golang.org/x/tools/go/analysis/passes/stringintconv" @@ -1549,6 +1550,7 @@ func defaultAnalyzers() map[string]*Analyzer { nilfunc.Analyzer.Name: {Analyzer: nilfunc.Analyzer, Enabled: true}, printf.Analyzer.Name: {Analyzer: printf.Analyzer, Enabled: true}, shift.Analyzer.Name: {Analyzer: shift.Analyzer, Enabled: true}, + slog.Analyzer.Name: {Analyzer: slog.Analyzer, Enabled: true}, stdmethods.Analyzer.Name: {Analyzer: stdmethods.Analyzer, Enabled: true}, stringintconv.Analyzer.Name: {Analyzer: stringintconv.Analyzer, Enabled: true}, structtag.Analyzer.Name: {Analyzer: structtag.Analyzer, Enabled: true}, diff --git a/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt b/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt index 6e7e4650578..e98674b94f4 100644 --- a/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt +++ b/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt @@ -1,24 +1,32 @@ Test of warning diagnostics from various analyzers: -tests, copylocks, printf, and timeformat. +copylocks, printf, slog, tests, and timeformat. -- go.mod -- module example.com go 1.12 +-- flags -- +-min_go=go1.21 + -- bad_test.go -- package analyzer import ( "fmt" + "log/slog" "sync" "testing" "time" ) -func Testbad(t *testing.T) { //@diag("", re"Testbad has malformed name: first letter after 'Test' must not be lowercase") +// copylocks +func _() { var x sync.Mutex _ = x //@diag("x", re"assignment copies lock value to _: sync.Mutex") +} +// printf +func _() { printfWrapper("%s") //@diag(re`printfWrapper\(.*\)`, re"example.com.printfWrapper format %s reads arg #1, but call has 0 args") } @@ -26,7 +34,18 @@ func printfWrapper(format string, args ...interface{}) { fmt.Printf(format, args...) } +// slog +func _() { + slog.Info("msg", 1) //@diag("1", re`slog.Info arg "1" should be a string or a slog.Attr`) +} + +// tests +func Testbad(t *testing.T) { //@diag("", re"Testbad has malformed name: first letter after 'Test' must not be lowercase") +} + +// timeformat func _() { now := time.Now() fmt.Println(now.Format("2006-02-01")) //@diag("2006-02-01", re"2006-02-01 should be 2006-01-02") } +