feat: suggest to use slog.DiscardHandler (#81)

Co-authored-by: Ludovic Fernandez <[email protected]>

Author: tmzane

.github/workflows/checks.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches: [ main ]
   pull_request:
-    branches: [ main ]
   workflow_dispatch:
 
 jobs:

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
 	}

sloglint_internal_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)
+			}
+		})
+	}
 }

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

testdata/src/discard_handler/discard_handler.go

@@ -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`
+}