diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index b81ecd8..9868a07 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -4,7 +4,6 @@ on: push: branches: [ main ] pull_request: - branches: [ main ] workflow_dispatch: jobs: diff --git a/sloglint.go b/sloglint.go index d4dc964..48e7591 100644 --- a/sloglint.go +++ b/sloglint.go @@ -8,6 +8,7 @@ import ( "go/ast" "go/token" "go/types" + "go/version" "slices" "strconv" "strings" @@ -33,6 +34,8 @@ type Options struct { KeyNamingCase string // Enforce key naming convention ("snake", "kebab", "camel", or "pascal"). ForbiddenKeys []string // Enforce not using specific keys. ArgsOnSepLines bool // Enforce putting arguments on separate lines. + + go124 bool } // New creates a new sloglint analyzer. @@ -75,6 +78,10 @@ func New(opts *Options) *analysis.Analyzer { return nil, fmt.Errorf("sloglint: Options.KeyNamingCase=%s: %w", opts.KeyNamingCase, errInvalidValue) } + if version.Compare("go"+pass.Module.GoVersion, "go1.24") >= 0 { + opts.go124 = true + } + run(pass, opts) return nil, nil }, @@ -206,6 +213,27 @@ func visit(pass *analysis.Pass, opts *Options, node ast.Node, stack []ast.Node) } name := fn.FullName() + + if opts.go124 && (name == "log/slog.NewTextHandler" || name == "log/slog.NewJSONHandler") { + if sel, ok := call.Args[0].(*ast.SelectorExpr); ok { + if obj := pass.TypesInfo.ObjectOf(sel.Sel); obj != nil { + if obj.Pkg().Name() == "io" && obj.Name() == "Discard" { + pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + Message: "use slog.DiscardHandler instead", + SuggestedFixes: []analysis.SuggestedFix{{ + TextEdits: []analysis.TextEdit{{ + Pos: call.Pos(), + End: call.End(), + NewText: []byte("slog.DiscardHandler"), + }}, + }}, + }) + } + } + } + } + funcInfo, ok := slogFuncs[name] if !ok { return @@ -293,8 +321,8 @@ func visit(pass *analysis.Pass, opts *Options, node ast.Node, stack []ast.Node) if opts.NoRawKeys { forEachKey(pass.TypesInfo, keys, attrs, func(key ast.Expr) { - if selector, ok := key.(*ast.SelectorExpr); ok { - key = selector.Sel // the key is defined in another package, e.g. pkg.ConstKey. + if sel, ok := key.(*ast.SelectorExpr); ok { + key = sel.Sel // the key is defined in another package, e.g. pkg.ConstKey. } isConst := false if ident, ok := key.(*ast.Ident); ok { @@ -343,11 +371,11 @@ func visit(pass *analysis.Pass, opts *Options, node ast.Node, stack []ast.Node) } func isGlobalLoggerUsed(info *types.Info, call ast.Expr) bool { - selector, ok := call.(*ast.SelectorExpr) + sel, ok := call.(*ast.SelectorExpr) if !ok { return false } - ident, ok := selector.X.(*ast.Ident) + ident, ok := sel.X.(*ast.Ident) if !ok { return false } diff --git a/sloglint_internal_test.go b/sloglint_internal_test.go deleted file mode 100644 index 2295830..0000000 --- a/sloglint_internal_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package sloglint - -import ( - "errors" - "testing" -) - -func TestOptions(t *testing.T) { - tests := map[string]struct { - opts Options - err error - }{ - "KVOnly+AttrOnly: incompatible": { - opts: Options{KVOnly: true, AttrOnly: true}, - err: errIncompatible, - }, - "NoGlobal: invalid value": { - opts: Options{NoGlobal: "-"}, - err: errInvalidValue, - }, - "ContextOnly: invalid value": { - opts: Options{ContextOnly: "-"}, - err: errInvalidValue, - }, - "MsgStyle: invalid value": { - opts: Options{MsgStyle: "-"}, - err: errInvalidValue, - }, - "KeyNamingCase: invalid value": { - opts: Options{KeyNamingCase: "-"}, - err: errInvalidValue, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - analyzer := New(&test.opts) - if _, err := analyzer.Run(nil); !errors.Is(err, test.err) { - t.Errorf("errors.Is() mismatch\ngot: %v\nwant: %v", err, test.err) - } - }) - } -} diff --git a/sloglint_test.go b/sloglint_test.go index d88eeb5..40f7366 100644 --- a/sloglint_test.go +++ b/sloglint_test.go @@ -1,82 +1,61 @@ -package sloglint_test +package sloglint import ( + "errors" "testing" - "go-simpler.org/sloglint" "golang.org/x/tools/go/analysis/analysistest" ) func TestAnalyzer(t *testing.T) { - testdata := analysistest.TestData() - - t.Run("no mixed arguments", func(t *testing.T) { - analyzer := sloglint.New(nil) - analysistest.Run(t, testdata, analyzer, "no_mixed_args") - }) - - t.Run("key-value pairs only", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{KVOnly: true}) - analysistest.Run(t, testdata, analyzer, "kv_only") - }) - - t.Run("attributes only", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{AttrOnly: true}) - analysistest.Run(t, testdata, analyzer, "attr_only") - }) - - t.Run("no global (all)", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{NoGlobal: "all"}) - analysistest.Run(t, testdata, analyzer, "no_global_all") - }) - - t.Run("no global (default)", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{NoGlobal: "default"}) - analysistest.Run(t, testdata, analyzer, "no_global_default") - }) - - t.Run("context only (all)", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{ContextOnly: "all"}) - analysistest.Run(t, testdata, analyzer, "context_only_all") - }) - - t.Run("context only (scope)", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{ContextOnly: "scope"}) - analysistest.Run(t, testdata, analyzer, "context_only_scope") - }) - - 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("key naming case", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{KeyNamingCase: "snake"}) - analysistest.Run(t, testdata, analyzer, "key_naming_case") - }) - - t.Run("arguments on separate lines", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{ArgsOnSepLines: true}) - analysistest.Run(t, testdata, analyzer, "args_on_sep_lines") - }) - - t.Run("forbidden keys", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{ForbiddenKeys: []string{"foo_bar"}}) - analysistest.Run(t, testdata, analyzer, "forbidden_keys") - }) - - t.Run("message style (lowercased)", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{MsgStyle: "lowercased"}) - analysistest.Run(t, testdata, analyzer, "msg_style_lowercased") - }) + tests := map[string]struct { + opts Options + dir string + }{ + "no mixed arguments": {Options{NoMixedArgs: true}, "no_mixed_args"}, + "key-value pairs only": {Options{KVOnly: true}, "kv_only"}, + "attributes only": {Options{AttrOnly: true}, "attr_only"}, + "no global (all)": {Options{NoGlobal: "all"}, "no_global_all"}, + "no global (default)": {Options{NoGlobal: "default"}, "no_global_default"}, + "context only (all)": {Options{ContextOnly: "all"}, "context_only_all"}, + "context only (scope)": {Options{ContextOnly: "scope"}, "context_only_scope"}, + "static message": {Options{StaticMsg: true}, "static_msg"}, + "no raw keys": {Options{NoRawKeys: true}, "no_raw_keys"}, + "key naming case": {Options{KeyNamingCase: "snake"}, "key_naming_case"}, + "arguments on separate lines": {Options{ArgsOnSepLines: true}, "args_on_sep_lines"}, + "forbidden keys": {Options{ForbiddenKeys: []string{"foo_bar"}}, "forbidden_keys"}, + "message style (lowercased)": {Options{MsgStyle: "lowercased"}, "msg_style_lowercased"}, + "message style (capitalized)": {Options{MsgStyle: "capitalized"}, "msg_style_capitalized"}, + "slog.DiscardHandler": {Options{go124: true}, "discard_handler"}, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + analyzer := New(&tt.opts) + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, analyzer, tt.dir) + }) + } +} - t.Run("message style (capitalized)", func(t *testing.T) { - analyzer := sloglint.New(&sloglint.Options{MsgStyle: "capitalized"}) - analysistest.Run(t, testdata, analyzer, "msg_style_capitalized") - }) +func TestOptions(t *testing.T) { + tests := map[string]struct { + opts Options + err error + }{ + "KVOnly+AttrOnly: incompatible": {Options{KVOnly: true, AttrOnly: true}, errIncompatible}, + "NoGlobal: invalid value": {Options{NoGlobal: "-"}, errInvalidValue}, + "ContextOnly: invalid value": {Options{ContextOnly: "-"}, errInvalidValue}, + "MsgStyle: invalid value": {Options{MsgStyle: "-"}, errInvalidValue}, + "KeyNamingCase: invalid value": {Options{KeyNamingCase: "-"}, errInvalidValue}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + analyzer := New(&test.opts) + if _, err := analyzer.Run(nil); !errors.Is(err, test.err) { + t.Errorf("errors.Is() mismatch\ngot: %v\nwant: %v", err, test.err) + } + }) + } } diff --git a/testdata/src/discard_handler/discard_handler.go b/testdata/src/discard_handler/discard_handler.go new file mode 100644 index 0000000..ff64573 --- /dev/null +++ b/testdata/src/discard_handler/discard_handler.go @@ -0,0 +1,11 @@ +package discard_handler + +import ( + "io" + "log/slog" +) + +func _() { + _ = slog.NewTextHandler(io.Discard, nil) // want `use slog.DiscardHandler instead` + _ = slog.NewJSONHandler(io.Discard, nil) // want `use slog.DiscardHandler instead` +} diff --git a/testdata/src/discard_handler/discard_handler.go.golden b/testdata/src/discard_handler/discard_handler.go.golden new file mode 100644 index 0000000..0861c7a --- /dev/null +++ b/testdata/src/discard_handler/discard_handler.go.golden @@ -0,0 +1,11 @@ +package discard_handler + +import ( + "io" + "log/slog" +) + +func _() { + _ = slog.DiscardHandler // want `use slog.DiscardHandler instead` + _ = slog.DiscardHandler // want `use slog.DiscardHandler instead` +}