feat: implement --forbidden-key

This PR implements a linter rule to deny usages of certain keys as
attributes to enforce that developers don't override reserved keys managed by
custom handlers.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>

README.md

@@ -23,6 +23,7 @@ With `sloglint` you can enforce various rules for `log/slog` based on your prefe
 * Enforce using constants instead of raw keys (optional)
 * Enforce a single key naming convention (optional)
 * Enforce putting arguments on separate lines (optional)
+* Prevent usage of reserved keys in key-value pairs (optional)
 
 ## 📦 Install
 
@@ -167,6 +168,15 @@ slog.Info("a user has logged in",
 )
 ```
 
+### Arguments with forbidden keys
+
+To ensure users don't use reserved words in key-value pairs, you may want to enforce rules to avoid misusages.
+The `forbidden-key` option causes `sloglint` to report usages of forbidden keys. These keys could be typically controlled by slog handlers and its usage should be preserved to avoid key duplication.
+
+```go
+slog.Info("a user has logged in", "reserved_key", 49) // sloglint: keys include forbidden values
+```
+
 [1]: https://golangci-lint.run
 [2]: https://github.com/go-simpler/sloglint/releases
 [3]: https://golangci-lint.run/usage/linters/#sloglint

sloglint.go

@@ -8,6 +8,7 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -20,15 +21,16 @@ import (
 
 // Options are options for the sloglint analyzer.
 type Options struct {
-	NoMixedArgs    bool   // Enforce not mixing key-value pairs and attributes (default true).
-	KVOnly         bool   // Enforce using key-value pairs only (overrides NoMixedArgs, incompatible with AttrOnly).
-	AttrOnly       bool   // Enforce using attributes only (overrides NoMixedArgs, incompatible with KVOnly).
-	NoGlobal       string // Enforce not using global loggers ("all" or "default").
-	ContextOnly    string // Enforce using methods that accept a context ("all" or "scope").
-	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.
+	NoMixedArgs    bool     // Enforce not mixing key-value pairs and attributes (default true).
+	KVOnly         bool     // Enforce using key-value pairs only (overrides NoMixedArgs, incompatible with AttrOnly).
+	AttrOnly       bool     // Enforce using attributes only (overrides NoMixedArgs, incompatible with KVOnly).
+	NoGlobal       string   // Enforce not using global loggers ("all" or "default").
+	ContextOnly    string   // Enforce using methods that accept a context ("all" or "scope").
+	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.
+	ForbiddenKeys  []string // Forbid using specific keys in key-value pairs.
 }
 
 // New creates a new sloglint analyzer.
@@ -94,6 +96,13 @@ func flags(opts *Options) flag.FlagSet {
 		})
 	}
 
+	repeatedStrVar := func(value *[]string, name, usage string) {
+		fset.Func(name, usage, func(s string) error {
+			*value = append(*value, strings.Split(s, ",")...)
+			return nil
+		})
+	}
+
 	boolVar(&opts.NoMixedArgs, "no-mixed-args", "enforce not mixing key-value pairs and attributes (default true)")
 	boolVar(&opts.KVOnly, "kv-only", "enforce using key-value pairs only (overrides -no-mixed-args, incompatible with -attr-only)")
 	boolVar(&opts.AttrOnly, "attr-only", "enforce using attributes only (overrides -no-mixed-args, incompatible with -kv-only)")
@@ -103,11 +112,13 @@ func flags(opts *Options) flag.FlagSet {
 	boolVar(&opts.NoRawKeys, "no-raw-keys", "enforce using constants instead of raw keys")
 	strVar(&opts.KeyNamingCase, "key-naming-case", "enforce a single key naming convention (snake|kebab|camel|pascal)")
 	boolVar(&opts.ArgsOnSepLines, "args-on-sep-lines", "enforce putting arguments on separate lines")
+	repeatedStrVar(&opts.ForbiddenKeys, "forbidden-key", "enforce not using specific keys. Can be repeated or comma separated")
 
 	return *fset
 }
 
 var slogFuncs = map[string]int{ // funcName:argsPos
+	"log/slog.With":                   0,
 	"log/slog.Log":                    3,
 	"log/slog.Debug":                  1,
 	"log/slog.Info":                   1,
@@ -117,6 +128,7 @@ var slogFuncs = map[string]int{ // funcName:argsPos
 	"log/slog.InfoContext":            2,
 	"log/slog.WarnContext":            2,
 	"log/slog.ErrorContext":           2,
+	"(*log/slog.Logger).With":         0,
 	"(*log/slog.Logger).Log":          3,
 	"(*log/slog.Logger).Debug":        1,
 	"(*log/slog.Logger).Info":         1,
@@ -249,14 +261,18 @@ func visit(pass *analysis.Pass, opts *Options, node ast.Node, stack []ast.Node)
 		pass.Reportf(call.Pos(), "arguments should be put on separate lines")
 	}
 
+	if len(opts.ForbiddenKeys) > 0 && badKeyNames(pass.TypesInfo, forbiddenKeysUsed(opts.ForbiddenKeys), keys, attrs) {
+		pass.Reportf(call.Pos(), "keys include forbidden values")
+	}
+
 	switch {
-	case opts.KeyNamingCase == snakeCase && badKeyNames(pass.TypesInfo, strcase.ToSnake, keys, attrs):
+	case opts.KeyNamingCase == snakeCase && badKeyNames(pass.TypesInfo, valueChanged(strcase.ToSnake), keys, attrs):
 		pass.Reportf(call.Pos(), "keys should be written in snake_case")
-	case opts.KeyNamingCase == kebabCase && badKeyNames(pass.TypesInfo, strcase.ToKebab, keys, attrs):
+	case opts.KeyNamingCase == kebabCase && badKeyNames(pass.TypesInfo, valueChanged(strcase.ToKebab), keys, attrs):
 		pass.Reportf(call.Pos(), "keys should be written in kebab-case")
-	case opts.KeyNamingCase == camelCase && badKeyNames(pass.TypesInfo, strcase.ToCamel, keys, attrs):
+	case opts.KeyNamingCase == camelCase && badKeyNames(pass.TypesInfo, valueChanged(strcase.ToCamel), keys, attrs):
 		pass.Reportf(call.Pos(), "keys should be written in camelCase")
-	case opts.KeyNamingCase == pascalCase && badKeyNames(pass.TypesInfo, strcase.ToPascal, keys, attrs):
+	case opts.KeyNamingCase == pascalCase && badKeyNames(pass.TypesInfo, valueChanged(strcase.ToPascal), keys, attrs):
 		pass.Reportf(call.Pos(), "keys should be written in PascalCase")
 	}
 }
@@ -351,9 +367,9 @@ func rawKeysUsed(info *types.Info, keys, attrs []ast.Expr) bool {
 	return false
 }
 
-func badKeyNames(info *types.Info, caseFn func(string) string, keys, attrs []ast.Expr) bool {
+func badKeyNames(info *types.Info, caseFn func(string) bool, keys, attrs []ast.Expr) bool {
 	for _, key := range keys {
-		if name, ok := getKeyName(key); ok && name != caseFn(name) {
+		if name, ok := getKeyName(key); ok && caseFn(name) {
 			return true
 		}
 	}
@@ -389,7 +405,7 @@ func badKeyNames(info *types.Info, caseFn func(string) string, keys, attrs []ast
 			}
 		}
 
-		if name, ok := getKeyName(expr); ok && name != caseFn(name) {
+		if name, ok := getKeyName(expr); ok && caseFn(name) {
 			return true
 		}
 	}
@@ -438,3 +454,26 @@ func argsOnSameLine(fset *token.FileSet, call ast.Expr, keys, attrs []ast.Expr)
 
 	return false
 }
+
+func forbiddenKeysUsed(forbiddenKeys []string) func(string) bool {
+	return func(s string) bool {
+		// Strip quotes if any.
+		unquoted := trimQuotes(s)
+		return slices.Contains(forbiddenKeys, unquoted)
+	}
+}
+
+func valueChanged(handler func(string) string) func(string) bool {
+	return func(s string) bool {
+		return handler(s) != s
+	}
+}
+
+func trimQuotes(s string) string {
+	if len(s) >= 2 {
+		if c := s[len(s)-1]; s[0] == c && (c == '"' || c == '\'' || c == '`') {
+			return s[1 : len(s)-1]
+		}
+	}
+	return s
+}

sloglint_test.go

@@ -64,4 +64,9 @@ func TestAnalyzer(t *testing.T) {
 		analyzer := sloglint.New(&sloglint.Options{ArgsOnSepLines: true})
 		analysistest.Run(t, testdata, analyzer, "args_on_sep_lines")
 	})
+
+	t.Run("key foo_bar is forbbiden case", func(t *testing.T) {
+		analyzer := sloglint.New(&sloglint.Options{ForbiddenKeys: []string{"foo_bar"}})
+		analysistest.Run(t, testdata, analyzer, "key_forbidden")
+	})
 }

testdata/src/key_forbidden/key_forbidden.go

@@ -0,0 +1,30 @@
+package key_forbidden
+
+import "log/slog"
+
+const (
+	snakeKey = "foo_bar"
+)
+
+func tests() {
+	slog.Info("msg")
+	slog.Info("msg", "foo-bar", 1)
+	slog.Info("msg", "foo_bar", 1)           // want `keys include forbidden values`
+	slog.Info("msg", snakeKey, 1)            // want `keys include forbidden values`
+	slog.Info("msg", slog.Int("foo_bar", 1)) // want `keys include forbidden values`
+	slog.Info("msg", slog.Int(snakeKey, 1))  // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{})
+	slog.Info("msg", slog.Attr{"foo_bar", slog.IntValue(1)})             // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{snakeKey, slog.IntValue(1)})              // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{Key: "foo_bar"})                          // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{Key: snakeKey})                           // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{Key: "foo_bar", Value: slog.IntValue(1)}) // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{Key: snakeKey, Value: slog.IntValue(1)})  // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: "foo_bar"}) // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: snakeKey})  // want `keys include forbidden values`
+	slog.With(slog.Attr{"foo_bar", slog.IntValue(1)}).Info("msg")        // want `keys include forbidden values`
+	slog.With(slog.Attr{snakeKey, slog.IntValue(1)})                     // want `keys include forbidden values`
+	slog.With(snakeKey, slog.IntValue(1))                                // want `keys include forbidden values`
+	slog.With("foo_bar", slog.IntValue(1))                               // want `keys include forbidden values`
+	slog.Info("msg", slog.Attr{Value: slog.IntValue(1), Key: `foo_bar`}) // want `keys include forbidden values`
+}