docs: add when-not to structs + testing-advanced + api-conventions

patterns/api-conventions.md

@@ -39,6 +39,41 @@ var emailRegex = regexp.MustCompile(`^[a-z]+@[a-z]+\.[a-z]+$`)
 // One line. Panics on typo (caught immediately in tests). Clean.
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- The input is dynamic or user-provided (URL from a config file, regex from user input)
+- You're inside a request handler or any code path where panicking would crash the server
+- The error is recoverable — the caller should decide how to handle it
+
+**Over-application example:**
+```go
+func HandleSearch(w http.ResponseWriter, r *http.Request) {
+    pattern := r.URL.Query().Get("q")
+    re := regexp.MustCompile(pattern) // PANIC on invalid user input!
+    // One bad query crashes the entire server
+    matches := re.FindAllString(corpus, -1)
+    // ...
+}
+```
+
+**Better alternative:**
+```go
+func HandleSearch(w http.ResponseWriter, r *http.Request) {
+    pattern := r.URL.Query().Get("q")
+    re, err := regexp.Compile(pattern)
+    if err != nil {
+        http.Error(w, "invalid regex: "+err.Error(), 400)
+        return
+    }
+    matches := re.FindAllString(corpus, -1)
+    // ...
+}
+```
+
+**Why:** `Must` is for programmer errors caught at init time, not for runtime input.
+If the input can vary, the error is expected and must be handled — not panicked on.
+
 **Anti-pattern:** Using Must in runtime code where the input is dynamic/user-provided;
 panicking on recoverable errors; naming it something other than Must (e.g., `PanicOnError`).
 
@@ -260,6 +295,36 @@ f, err := os.Open("data.json") // just reads — no flags to remember
 f, err := os.OpenFile("data.json", os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- There's no clear "common case" — all callers need different flag combinations
+- The convenience wrapper would hide important behavior (e.g., `Create` hides truncation — some callers are surprised)
+- You have 2+ equally common usage patterns that would each need their own wrapper, leading to an explosion of functions
+
+**Over-application example:**
+```go
+// Too many convenience wrappers — which one do I want?
+func OpenForAppend(name string) (*File, error) { ... }
+func OpenOrCreate(name string) (*File, error) { ... }
+func OpenReadWrite(name string) (*File, error) { ... }
+func OpenExclusive(name string) (*File, error) { ... }
+// Users now have to remember 6 functions instead of learning 1 + flags
+```
+
+**Better alternative:**
+```go
+// One convenience for the overwhelmingly common case, full-power for the rest
+func Open(name string) (*File, error) { return OpenFile(name, O_RDONLY, 0) }
+func Create(name string) (*File, error) { return OpenFile(name, O_RDWR|O_CREATE|O_TRUNC, 0666) }
+func OpenFile(name string, flag int, perm FileMode) (*File, error) { ... }
+// Only 2 convenience wrappers for the 2 dominant patterns. Everything else uses OpenFile.
+```
+
+**Why:** Layered APIs work when there's a clear 80/20 split. If you're writing a convenience
+wrapper for every combination, you've just created a larger API surface that's harder to
+navigate than the single configurable function.
+
 **Anti-pattern:** Only exposing the full-power version; making users learn flag
 constants for simple reads; duplicating implementation across convenience functions.
 
@@ -402,6 +467,39 @@ func (s *Server) Shutdown(ctx context.Context) error {
 }
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- Your type doesn't manage long-lived resources (a pure data struct, a stateless transformer)
+- Shutdown order doesn't matter — a simple `Close()` suffices
+- You're building a CLI tool that exits the process — `os.Exit` is your shutdown
+
+**Over-application example:**
+```go
+// Graceful shutdown for a type that holds no connections
+type Calculator struct {
+    precision int
+}
+
+func (c *Calculator) Shutdown(ctx context.Context) error {
+    // ... nothing to drain, nothing to close
+    return nil
+}
+```
+
+**Better alternative:**
+```go
+// No shutdown needed — the GC handles it. Maybe a Reset() if you want to reuse.
+type Calculator struct {
+    precision int
+}
+```
+
+**Why:** The Close/Shutdown duality exists for types that own goroutines, connections, or
+file descriptors that outlive individual method calls. If your type is just data and methods,
+adding shutdown ceremony is over-engineering that confuses users into thinking there are
+hidden resources to manage.
+
 **Anti-pattern:** Only providing one shutdown mode; not accepting a context for
 timeout control; leaking goroutines on shutdown.
 

patterns/interfaces.md

@@ -60,6 +60,45 @@ func countLines(r io.Reader) (int, error) {
 }
 ```
 
+
+### When NOT to Use
+
+**Don't use this when:**
+- You only have one implementation and no tests — you're adding indirection for no reason
+- The function genuinely needs multiple capabilities together (reading + seeking + closing)
+- You're creating an interface to match a single concrete type that you control
+
+**Over-application example:**
+```go
+// Interface with one implementation, no tests, no external consumers
+type Configurer interface {
+    LoadConfig(path string) (*Config, error)
+}
+
+type fileConfigurer struct{}
+
+func (f *fileConfigurer) LoadConfig(path string) (*Config, error) {
+    return parseFile(path)
+}
+
+func NewApp(c Configurer) *App {
+    // c is always *fileConfigurer — the interface adds nothing
+    return &App{cfg: c}
+}
+```
+
+**Better alternative:**
+```go
+// Just use the concrete type until you actually need the abstraction
+func NewApp(cfgPath string) *App {
+    cfg := parseFile(cfgPath)
+    return &App{cfg: cfg}
+}
+```
+
+**Why:** Interfaces in Go should be discovered through usage, not predicted. "Accept interfaces" means accept them at the *boundaries* where multiple types actually flow through. If you have one implementation and no tests that need a mock, you have a premature abstraction.
+
+
 ### Anti-pattern
 
 ```go
@@ -187,6 +226,38 @@ func NewLogger(w io.Writer) *Logger {
 }
 ```
 
+
+### When NOT to Use
+
+**Don't use this when:**
+- Your function is internal and only ever called with one concrete type
+- Returning an interface is genuinely better because the concrete type is an implementation detail that may change
+- The struct's exported fields would expose dangerous internals
+
+**Over-application example:**
+```go
+// Accepting an interface when only one concrete type makes sense
+func NewDatabaseMigrator(db interface {
+    Exec(query string, args ...any) (sql.Result, error)
+    Query(query string, args ...any) (*sql.Rows, error)
+    Begin() (*sql.Tx, error)
+}) *Migrator {
+    // This custom interface exactly matches *sql.DB — just accept *sql.DB
+    return &Migrator{db: db}
+}
+```
+
+**Better alternative:**
+```go
+// Accept the concrete type when the abstraction doesn't buy anything
+func NewDatabaseMigrator(db *sql.DB) *Migrator {
+    return &Migrator{db: db}
+}
+```
+
+**Why:** "Accept interfaces" doesn't mean "always accept interfaces." If you define a bespoke interface that matches exactly one concrete type and no one else will implement it, you've just added indirection. The guideline targets *standard* interfaces (io.Reader, io.Writer) that many types satisfy.
+
+
 ### Anti-pattern
 
 ```go
@@ -317,6 +388,39 @@ pipeline.Use(ProcessorFunc(func(data []byte) error {
 }))
 ```
 
+
+### When NOT to Use
+
+**Don't use this when:**
+- The interface has more than one method — adapters only work for single-method interfaces
+- Implementations typically need state (struct fields) that closures would awkwardly close over
+- The function signature is complex enough that a named type with methods is clearer
+
+**Over-application example:**
+```go
+// Adapter for a multi-method interface — doesn't work
+type StorageFunc func(key string, data []byte) error
+
+func (f StorageFunc) Store(key string, data []byte) error { return f(key, data) }
+func (f StorageFunc) Load(key string) ([]byte, error)     { /* can't implement! */ }
+func (f StorageFunc) Delete(key string) error             { /* can't implement! */ }
+```
+
+**Better alternative:**
+```go
+// For multi-method interfaces, use a struct (or split the interface)
+type MemoryStorage struct {
+    data map[string][]byte
+}
+
+func (m *MemoryStorage) Store(key string, data []byte) error { ... }
+func (m *MemoryStorage) Load(key string) ([]byte, error)     { ... }
+func (m *MemoryStorage) Delete(key string) error             { ... }
+```
+
+**Why:** The adapter pattern bridges functions to interfaces. Functions have one signature, so adapters only work for single-method interfaces. If your interface has multiple methods, callers need a struct anyway — the adapter just adds confusion.
+
+
 ### Anti-pattern
 
 ```go
@@ -394,6 +498,55 @@ func refreshAll(s Store) {
 }
 ```
 
+
+### When NOT to Use
+
+**Don't use this when:**
+- All implementations will always support the capability — just put it in the main interface
+- The capability is required for correctness, not just optimization
+- You have only 2-3 implementations and a simple interface split handles it better
+
+**Over-application example:**
+```go
+// Every HTTP handler MUST write a response — this isn't optional
+type Handler interface {
+    ServeHTTP(w ResponseWriter, r *Request)
+}
+
+// Don't make response-writing "optional"
+type ResponseWriter interface {
+    Header() Header
+}
+
+type BodyWriter interface {
+    Write([]byte) (int, error) // NOT optional — every response needs a body mechanism
+}
+
+func handle(w ResponseWriter) {
+    if bw, ok := w.(BodyWriter); ok { // wrong: Write is fundamental, not optional
+        bw.Write([]byte("hello"))
+    }
+}
+```
+
+**Better alternative:**
+```go
+// Write is fundamental — keep it in the core interface
+type ResponseWriter interface {
+    Header() Header
+    Write([]byte) (int, error)
+    WriteHeader(statusCode int)
+}
+
+// Only truly optional capabilities get separate interfaces
+type Flusher interface {
+    Flush()
+}
+```
+
+**Why:** Optional interfaces are for progressive enhancement — capabilities that some implementations support but others legitimately don't. If every implementation must support it for the system to work, it belongs in the core interface. Overusing type assertions makes code fragile and harder to reason about.
+
+
 ### Anti-pattern
 
 ```go

patterns/package-design.md

@@ -143,6 +143,32 @@ func InternalFormat(s string) Thing { ... } // only importable by pkg/mylib and
 import "pkg/mylib/internal/parse" // ✓ allowed
 ```
 
+### When NOT to Use
+
+**Don't use `internal/` when:**
+- The code is only used by a single package (just keep it unexported in that package)
+- You're hiding code that *should* be public API — `internal/` isn't a staging area for "maybe later"
+- You have a flat package structure with no sub-packages (no one to share with)
+
+**Over-application example:**
+```go
+// pkg/mylib/internal/config/config.go
+package config
+
+// Only used by pkg/mylib itself — no sub-packages import this
+func DefaultTimeout() time.Duration { return 30 * time.Second }
+```
+
+**Better alternative:**
+```go
+// pkg/mylib/config.go — just make it unexported in the parent package
+package mylib
+
+func defaultTimeout() time.Duration { return 30 * time.Second }
+```
+
+**Why:** `internal/` adds directory structure complexity. If you have no sub-packages sharing the code, an unexported function in the parent package is simpler and achieves the same encapsulation.
+
 ### Anti-pattern
 
 ```go
@@ -246,6 +272,47 @@ func init() {
 import _ "github.com/lib/pq" // driver registers itself
 ```
 
+### When NOT to Use
+
+**Don't use `init()` when:**
+- The initialization can fail (you can't return errors from `init()`)
+- The setup requires configuration or parameters (init takes no args)
+- You need to control initialization order across packages
+- It's a one-off application (not a library/driver) — just call setup in `main()`
+
+**Over-application example:**
+```go
+// internal/metrics/metrics.go
+func init() {
+    // Bad: init() hides this dependency, makes testing impossible,
+    // and panics if prometheus isn't reachable
+    prometheus.MustRegister(requestCounter)
+    prometheus.MustRegister(errorCounter)
+    prometheus.MustRegister(latencyHistogram)
+}
+```
+
+**Better alternative:**
+```go
+// internal/metrics/metrics.go
+func Register(reg prometheus.Registerer) error {
+    if err := reg.Register(requestCounter); err != nil {
+        return fmt.Errorf("registering request counter: %w", err)
+    }
+    // ...
+    return nil
+}
+
+// main.go
+func main() {
+    if err := metrics.Register(prometheus.DefaultRegisterer); err != nil {
+        log.Fatal(err)
+    }
+}
+```
+
+**Why:** `init()` is invisible, untestable, and can't fail gracefully. Use it only when the registration pattern demands it (database/sql drivers, codec registration) and failure is impossible.
+
 ### Anti-pattern
 
 ```go
@@ -486,6 +553,37 @@ func UserID(ctx context.Context) (int, bool) {
 }
 ```
 
+### When NOT to Use
+
+**Don't use context values when:**
+- The data is a required function parameter (pass it explicitly)
+- The data controls behavior/logic (timeouts, retry counts) — use function args or config structs
+- You're using it to avoid refactoring function signatures
+- The value is large or expensive to retrieve (context isn't a cache)
+
+**Over-application example:**
+```go
+// Passing database connection through context — it's required everywhere!
+func HandleRequest(ctx context.Context) {
+    db := DatabaseFromContext(ctx) // nil if forgotten — runtime panic
+    users, err := db.Query(ctx, "SELECT ...")
+}
+```
+
+**Better alternative:**
+```go
+// Make the dependency explicit
+type Handler struct {
+    db *sql.DB
+}
+
+func (h *Handler) HandleRequest(ctx context.Context) {
+    users, err := h.db.QueryContext(ctx, "SELECT ...")
+}
+```
+
+**Why:** Context values are untyped, invisible in function signatures, and can silently be nil. They're meant for *request-scoped metadata* that crosses API boundaries (trace IDs, auth tokens), not for dependency injection or configuration.
+
 ### Anti-pattern
 
 ```go

patterns/structs.md

@@ -46,6 +46,40 @@ func (c *Cache) Set(k string, v []byte) {
 }
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- The type has mandatory dependencies that *must* be provided (a DB connection, an io.Reader with no sensible default)
+- The zero value would be dangerous rather than merely useless (e.g., a zero-value security config that disables auth)
+- Lazy initialization adds per-call overhead on a hot path and the constructor is called once
+
+**Over-application example:**
+```go
+// Trying to make a DB-backed store zero-value ready
+type UserStore struct {
+    db *sql.DB // nil means... what? No sensible default exists.
+}
+
+func (s *UserStore) Get(id int) (*User, error) {
+    if s.db == nil {
+        return nil, errors.New("no database configured") // every call pays for nil check
+    }
+    // ...
+}
+```
+
+**Better alternative:**
+```go
+// A constructor makes the requirement explicit
+func NewUserStore(db *sql.DB) *UserStore {
+    return &UserStore{db: db}
+}
+```
+
+**Why:** When there's no meaningful default for a dependency, forcing zero-value usability
+just moves the error from compile time (missing argument) to runtime (nil check on every call).
+Use a constructor instead.
+
 **Anti-pattern:** Requiring a constructor for basic use; panicking on zero-value use;
 requiring all fields be set before the type is functional.
 
@@ -182,6 +216,35 @@ func NewParser(input io.Reader) *Parser {
 p := NewParser(file)
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- The type's zero value is already useful — adding a constructor creates unnecessary ceremony
+- Your constructor takes 5+ optional parameters (use a config struct instead)
+- The "mandatory dependency" is actually optional and has a sensible default (e.g., a logger defaulting to `slog.Default()`)
+
+**Over-application example:**
+```go
+// Constructor for something that doesn't need one
+func NewCounter() *Counter {
+    return &Counter{count: 0} // zero value already does this!
+}
+
+// Forces users to write:
+c := NewCounter()
+```
+
+**Better alternative:**
+```go
+type Counter struct {
+    count int64
+}
+// Users write: var c Counter — done.
+```
+
+**Why:** If the zero value works, a constructor is just noise. It obscures the type's
+actual simplicity and makes users wonder what hidden initialization they're missing.
+
 **Anti-pattern:** Forcing users to manually set unexported fields; having a constructor
 that takes 10 optional parameters (use config struct instead); requiring New when
 zero value would suffice.
@@ -300,6 +363,41 @@ type ServerConfig struct {
 func NewServer(cfg ServerConfig) *Server { ... }
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- You only have 1–2 options — just use function parameters
+- The options are truly required (not optional) — they belong in the constructor signature
+- Your config struct has methods or behavior — it's no longer a plain config, it's becoming a type
+
+**Over-application example:**
+```go
+// Config struct for two required parameters
+type ClientConfig struct {
+    BaseURL string // required!
+    APIKey  string // required!
+}
+
+func NewClient(cfg ClientConfig) (*Client, error) {
+    if cfg.BaseURL == "" { return nil, errors.New("base URL required") }
+    if cfg.APIKey == "" { return nil, errors.New("API key required") }
+    // ...
+}
+```
+
+**Better alternative:**
+```go
+// Required params are function args; only truly optional things go in a config
+func NewClient(baseURL, apiKey string, opts *ClientOptions) (*Client, error) {
+    // baseURL and apiKey are enforced by the signature
+    // opts can be nil for defaults
+}
+```
+
+**Why:** Config structs shine for *optional* configuration. When fields are required,
+the compiler can't enforce them — you end up validating at runtime what the type system
+could have caught. Keep required parameters as explicit function arguments.
+
 **Anti-pattern:** Undocumented fields; requiring all fields set; using sentinel values
 other than zero/nil for defaults; providing setters when direct assignment works.
 

patterns/testing-advanced.md

@@ -65,6 +65,49 @@ func TestParseSize(t *testing.T) {
 }
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- You have 1–2 test cases with significantly different setup logic — a table adds indirection for no gain
+- Each case requires unique assertions or error-checking logic that can't be unified
+- The test is inherently sequential (step 2 depends on step 1's output)
+
+**Over-application example:**
+```go
+func TestMigration(t *testing.T) {
+    tests := []struct {
+        name string
+        // ... 15 fields for setup, teardown, assertions, side effects
+    }{
+        {"migrate v1 to v2", /* massive struct literal */},
+        {"migrate v2 to v3", /* completely different struct literal */},
+    }
+    for _, tt := range tests {
+        // 50 lines of conditional logic because each case is fundamentally different
+    }
+}
+```
+
+**Better alternative:**
+```go
+func TestMigrateV1ToV2(t *testing.T) {
+    // Clear, self-contained, readable
+    db := setupV1(t)
+    err := MigrateToV2(db)
+    // specific assertions for this migration
+}
+
+func TestMigrateV2ToV3(t *testing.T) {
+    db := setupV2(t)
+    err := MigrateToV3(db)
+    // different assertions entirely
+}
+```
+
+**Why:** Table-driven tests shine when cases share identical setup/assertion logic and differ
+only in inputs and expected outputs. When each "case" needs its own control flow, the table
+becomes a mini-DSL that's harder to read than separate functions.
+
 **Anti-pattern:** Writing individual assertions for each case, or copy-pasting test functions that differ by one input.
 
 **Code example (stdlib):**
@@ -357,6 +400,47 @@ func TestRenderTemplate(t *testing.T) {
 // Golden file lives at testdata/TestRenderTemplate.golden
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- Expected output is short (< 5 lines) — inline it directly for readability
+- Output is non-deterministic (timestamps, random IDs, goroutine ordering) without normalization
+- The golden file would need updating on every minor refactor — brittle and noisy diffs
+
+**Over-application example:**
+```go
+// Golden file for a one-line output
+var update = flag.Bool("update", false, "update golden files")
+
+func TestVersion(t *testing.T) {
+    got := Version()
+    golden := "testdata/TestVersion.golden"
+    if *update {
+        os.WriteFile(golden, []byte(got), 0644)
+        return
+    }
+    want, _ := os.ReadFile(golden)
+    if got != string(want) {
+        t.Error("mismatch")
+    }
+}
+// testdata/TestVersion.golden contains: "v1.2.3"  — seriously?
+```
+
+**Better alternative:**
+```go
+func TestVersion(t *testing.T) {
+    got := Version()
+    if got != "v1.2.3" {
+        t.Errorf("Version() = %q, want %q", got, "v1.2.3")
+    }
+}
+```
+
+**Why:** Golden files add process overhead (the `-update` workflow, reviewing diffs in a separate
+file). For short, stable outputs, inline comparison is simpler, faster to read, and keeps the
+expected value next to the assertion.
+
 **Anti-pattern:** Comparing against inline expected strings that span 50+ lines, or manually constructing expected output.
 
 **Code example (stdlib):**
@@ -442,6 +526,45 @@ func TestHealthHandler(t *testing.T) {
 }
 ```
 
+### When NOT to Use
+
+**Don't use this when:**
+- You need to test real HTTP behavior: TLS handshakes, connection pooling, timeouts, keep-alive
+- Your handler depends on server-level middleware (e.g., `http.Server.ConnContext`, TLS client certs)
+- You're testing client behavior or redirect-following (need a real URL to connect to)
+
+**Over-application example:**
+```go
+func TestClientRetries(t *testing.T) {
+    rec := httptest.NewRecorder()
+    // Can't test retry logic — there's no real server for the client to connect to!
+    // rec doesn't have a URL, no TCP, no connection reset simulation
+}
+```
+
+**Better alternative:**
+```go
+func TestClientRetries(t *testing.T) {
+    attempts := 0
+    srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+        attempts++
+        if attempts < 3 {
+            w.WriteHeader(503)
+            return
+        }
+        w.WriteHeader(200)
+    }))
+    defer srv.Close()
+    // Now test the client's retry behavior against a real server
+    resp, err := myClient.Get(srv.URL + "/resource")
+    // ...
+}
+```
+
+**Why:** `httptest.NewRecorder` tests handler logic in isolation — it has no network, no URL,
+no connection lifecycle. When you need to test anything that crosses the network boundary
+(clients, retries, TLS, timeouts), you need `httptest.NewServer`.
+
 **Anti-pattern:** Spinning up a full server to test handler logic that doesn't need networking.
 
 **Code example (stdlib):**