Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ Each enforceable guide in `docs/` should have a matching agent in `agents/`. The
| `go-json-error-handler-guide.md` | `go-http-handler-assistant` |
| `go-linting-guide.md` | `go-quality-assistant` |
| `go-state-machine-pattern.md` | `go-architecture-assistant` |
| `go-service-implementation-patterns.md` | `go-architecture-assistant` |
| `go-kubernetes-crd-controller-guide.md` | `go-architecture-assistant` |
| `go-functional-options-pattern.md` | `go-quality-assistant` |
| `go-doc-best-practices.md` | `godoc-assistant` |
| `go-testing-guide.md` | `go-test-quality-assistant` |
| `go-security-linting.md` | `go-security-specialist` |
Expand Down
73 changes: 73 additions & 0 deletions docs/go-functional-options-pattern.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,79 @@ func NewConsumer(options ...func(*ConsumerOptions)) Consumer {

## Naming Conventions

### RULE go-functional-options/singular-option-type (SHOULD)

**Owner**: go-quality-assistant
**Applies when**: a Go package using the functional-options pattern uses suboptimal pair naming — function type with plural `XxxOptions` (matching the struct so they collide semantically), OR config struct with singular `XxxOption` (clashing with the function type). Both shapes work; this rule promotes the industry-standard singular-function / plural-struct pair for clarity, not as a correctness fix.
**Enforcement**: judgment (ast-grep follow-up: `type_alias_declaration` / `type_declaration` with `XxxOptions func(...)` matching the wrong-singular pattern, and `struct_type` named `XxxOption`)
**Why**: The pair-naming convention — `XxxOption` (singular function type) + `XxxOptions` (plural config struct) — makes the relationship at the type signature unambiguous: one `XxxOption` modifies one `XxxOptions`. Swapping them or using the same word for both makes every reader pause to remember which is which. Industry standard (Dave Cheney's original pattern, gRPC, OpenTelemetry, k8s client-go) follows the singular-function / plural-struct shape; deviating costs reader-onboarding effort with no upside.

#### Bad

```go
// Function type uses plural — semantically collides with the typical
// XxxOptions struct name. To avoid a duplicate declaration, the config
// struct is then forced to a different word (Config), splitting the
// naming pair across two unrelated nouns.
type ConsumerOptions func(*ConsumerConfig)
type ConsumerConfig struct {
BatchSize int
Timeout time.Duration
}

// Or: config struct uses singular — collides with the typical XxxOption
// function-type name. Function type then needs a different word.
type ServerOption struct {
Port int
}
type ServerOptionFunc func(*ServerOption)
```

#### Good

```go
// Function type singular; config struct plural
type ConsumerOption func(*ConsumerOptions)
type ServerOption func(*ServerOptions)
type ClientOption func(*ClientOptions)

type ConsumerOptions struct {
BatchSize int
Timeout time.Duration
}
```

### RULE go-functional-options/with-prefix-option-functions (SHOULD)

**Owner**: go-quality-assistant
**Applies when**: a Go package using the functional-options pattern names the option constructors with prefixes other than `With*` (e.g. `Set*`, `Use*`, `Configure*`, bare nouns like `BatchSize(n)`).
**Enforcement**: judgment (ast-grep follow-up: `function_declaration` returning a type matching `*Option` / `*ConfigFunc` with name not starting with `With`)
**Why**: `With*` is the unambiguous signal that this function is an option constructor, not an action — `WithBatchSize(100)` reads as "with a batch size of 100" while `SetBatchSize(100)` reads as a mutating call against an existing config. Mixed prefixes in the same package leave the reader guessing which functions belong in the option list at the call site (`NewConsumer(WithX(...), SetY(...), Z(...))`) and which are unrelated calls. Picking `With*` and sticking to it makes the pattern self-describing.

#### Bad

```go
// Mixed prefixes — call site is unreadable
consumer := NewConsumer(
BatchSize(100),
UseTimeout(30 * time.Second),
ConfigureRetry(3),
WithTLS(tlsCfg), // which of these are option constructors?
)
```

#### Good

```go
// Uniform With* prefix — every arg is obviously an option
consumer := NewConsumer(
WithBatchSize(100),
WithTimeout(30 * time.Second),
WithRetry(3),
WithTLS(tlsCfg),
)
```

### Option Type Names

**Recommended Pattern**: Use **singular** for the function type (represents one option):
Expand Down
52 changes: 52 additions & 0 deletions docs/go-kubernetes-crd-controller-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,32 @@ How to define and consume a Kubernetes Custom Resource Definition (CRD) in a Go

If your service already depends on `bborbe/k8s` (most do), use these instead of hand-writing the store and adapter. The hand-written skeletons in sections 5–6 are documented only for services that cannot take the dependency.

### RULE go-k8s-crd/use-bborbe-k8s (SHOULD)

**Owner**: go-architecture-assistant
**Applies when**: a Go service consuming a CRD hand-writes the event-handler cast adapter, the typed event handler, or the in-memory state store, while the service already depends on (or could depend on) `github.com/bborbe/k8s`.
**Enforcement**: judgment (file-existence + dependency-graph check: presence of `pkg/event-handler*.go` or `pkg/<resource>-store.go` alongside a non-trivial `bborbe/k8s` import means hand-written boilerplate that should use the generic primitives)
**Why**: `bborbe/k8s` collapses ~300 lines of hand-written event-handler + adapter + store boilerplate per CRD into one `k8s.NewEventHandler[T]()` + `k8s.NewResourceEventHandler[T]()` call. The generic primitives are typed (no `interface{}` casts), thread-safe by construction, and tested upstream. Hand-written equivalents re-invent the same bugs (race conditions on the store, missed `OnDelete` events, type assertions that panic on cache resync) across every service. Take the dependency; delete the boilerplate.

#### Bad

```go
// pkg/event-handler.go — hand-written cast adapter
// pkg/event-handler-user.go — typed handler with OnAdd/OnUpdate/OnDelete
// pkg/user-store.go — sync.RWMutex + map[string]*User, 60+ lines
// (three files of boilerplate per CRD — re-invents the same bugs every service)
```

#### Good

```go
// pkg/factory/factory.go
store := k8s.NewEventHandler[*v1.User]()
adapter := k8s.NewResourceEventHandler[*v1.User](ctx, store)
informer.AddEventHandler(adapter)
// done — typed, thread-safe, tested upstream
```

## 1. When to use this pattern

Use a CRD when:
Expand Down Expand Up @@ -248,6 +274,32 @@ No `go mod vendor` step is required before `generatek8s` — the generator reads

## 4. K8sConnector interface (consumer side)

### RULE go-k8s-crd/generated-client-not-dynamic (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go consumer service interacts with a first-party CRD (the CR schema is known at compile time, defined in the same repo or a sibling library) using `k8s.io/client-go/dynamic` instead of the typed clientset generated via `hack/update-codegen.sh`.
**Enforcement**: judgment (import-graph check on the consumer service: `client-go/dynamic` imported alongside a known-schema CR type is the smell; dynamic-client use is acceptable only when the CR is third-party with unknown schema)
**Why**: The dynamic client returns `*unstructured.Unstructured` — every field access is a string lookup, every value comes back as `interface{}`, every typo is a runtime error. Generated clientsets return typed Go structs: typos fail at compile time, IDE auto-complete works, refactors propagate. The dynamic client is the right tool when you don't know the schema (admin tools, generic operators, schema discovery); for first-party CRDs where you wrote the types, it's the wrong tool — it discards every type-safety guarantee Go offers.

#### Bad

```go
// Dynamic client for a first-party CRD — every field access is stringly-typed
client := dynamic.NewForConfigOrDie(config)
gvr := schema.GroupVersionResource{Group: "example.com", Version: "v1", Resource: "users"}
obj, err := client.Resource(gvr).Namespace("default").Get(ctx, "alice", metav1.GetOptions{})
// obj.Object["spec"].(map[string]interface{})["email"].(string) ← typo-prone, runtime-only
```

#### Good

```go
// Generated typed clientset — fields are real Go types
clientset := versioned.NewForConfigOrDie(config)
user, err := clientset.ExampleV1().Users("default").Get(ctx, "alice", metav1.GetOptions{})
// user.Spec.Email ← compile-time checked
```

Every bborbe CRD consumer has this exact interface:

```go
Expand Down
104 changes: 104 additions & 0 deletions docs/go-service-implementation-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,61 @@ This guide captures practical decision-making patterns and implementation choice

## Service Architecture Decision Framework

### RULE go-service-impl/provider-vs-registry-choice (SHOULD)

**Owner**: go-architecture-assistant
**Applies when**: a Go service needs to dispatch to one of several implementations and the code uses the wrong dispatch shape for the set's openness: a `map[string]Creator` registry for a closed compile-time set, or a compiled `switch` for an open runtime-extensible set.
**Enforcement**: judgment (semantic — depends on whether the implementation set is closed or open)
**Why**: Static `switch` is faster (compiled jump table vs. map lookup + indirect call), trivially exhaustive (compiler-checked via `default:` + `errors.Errorf`), and refactor-friendly (renames propagate). Dynamic map-based registries are necessary when implementations register themselves at runtime (plugin systems, third-party extensions) but the cost — lost compile-time exhaustiveness, harder-to-test, registration-order sensitivity — is real. Match the shape to the problem: closed set → switch; open set → registry. Defaulting to one or the other regardless of context produces both unnecessary overhead and brittle systems.

#### Bad

```go
// Fixed 3-implementation set, runtime registry — overkill.
// Also violates `go-architecture/no-globals-or-singletons` (MUST): init()
// initialises a package-level Registry with all the bound creators —
// untestable in parallel, hidden dependency graph.
type ProcessorRegistry struct {
processors map[string]ProcessorCreator
}

func init() {
r := &ProcessorRegistry{processors: make(map[string]ProcessorCreator)}
r.Register("image", NewImageProcessor) // these never change
r.Register("video", NewVideoProcessor)
r.Register("document", NewDocumentProcessor)
}
```

#### Good

```go
// Fixed set → compiled switch; exhaustiveness checked at compile time
type ProcessorProvider interface {
Get(ctx context.Context, t ProcessType) (Processor, error)
}

func (p *processorProvider) Get(ctx context.Context, t ProcessType) (Processor, error) {
switch t {
case ProcessImage:
return NewImageProcessor(p.deps...), nil
case ProcessVideo:
return NewVideoProcessor(p.deps...), nil
case ProcessDocument:
return NewDocumentProcessor(p.deps...), nil
default:
return nil, errors.Errorf(ctx, "unknown processor type: %s", t)
}
}

// Plugin-extensible set → registry; new types register at startup
type PluginRegistry struct {
plugins map[string]PluginCreator
}

func (r *PluginRegistry) Register(name string, creator PluginCreator) { r.plugins[name] = creator }
```

### Provider vs Registry Pattern

**Use Static Provider Pattern When:**
Expand Down Expand Up @@ -141,6 +196,55 @@ type PaymentHandler interface { // Focuses on technical aspect

## Dependency Injection Best Practices

### RULE go-service-impl/no-context-object-injection (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go service method receives a struct (by value OR by pointer) named `Context` / `ServiceContext` / `Deps` / etc. that bundles multiple service dependencies (logger, repository, validator, etc.) and passes them through method calls instead of through the constructor.
**Enforcement**: judgment (ast-grep follow-up: method signature with a parameter type matching `<Name>Context` or `*<Name>Context` / `<Name>Deps` or `*<Name>Deps` where the struct contains 2+ service-interface fields — value and pointer shapes share the anti-pattern)
**Why**: Context-object injection is the rebrand of global state — every method becomes "give me everything, I'll pick what I need." Three failure modes: (1) compile-time can't tell which deps a method actually uses, so refactors and dead-code detection break; (2) tests need to construct a full context object for every call site, even for methods that touch one dependency; (3) the context grows over time (the "we'll just add one more field" trap), and unused fields linger forever. Constructor injection forces the dep set to be minimal and visible at the type signature.

#### Bad

```go
type ServiceContext struct {
SMTPClient SMTPClient
TemplateRepo TemplateRepository
Logger log.Logger
}

func (e *EmailService) Send(ctx context.Context, msg Message, svcCtx ServiceContext) error {
svcCtx.Logger.Info("sending")
return svcCtx.SMTPClient.Send(ctx, msg) // hidden dep set; compile-time can't see it
}
```

#### Good

```go
type EmailService interface {
Send(ctx context.Context, msg Message) error
}

type emailService struct {
smtpClient SMTPClient
templateRepo TemplateRepository
logger log.Logger
}

func NewEmailService(
smtpClient SMTPClient,
templateRepo TemplateRepository,
logger log.Logger,
) EmailService {
return &emailService{smtpClient: smtpClient, templateRepo: templateRepo, logger: logger}
}

func (e *emailService) Send(ctx context.Context, msg Message) error {
e.logger.Info("sending") // dep is a struct field, visible at type
return e.smtpClient.Send(ctx, msg)
}
```

### Constructor Injection vs Context Objects

**✅ Good: Constructor Dependency Injection**
Expand Down
54 changes: 54 additions & 0 deletions rules/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,24 @@
"level": "MUST",
"owner": "go-factory-pattern-assistant"
},
{
"anchor": "go-functional-options/singular-option-type",
"applies_when": "a Go package using the functional-options pattern uses suboptimal pair naming — function type with plural `XxxOptions` (matching the struct so they collide semantically), OR config struct with singular `XxxOption` (clashing with the function type). Both shapes work; this rule promotes the industry-standard singular-function / plural-struct pair for clarity, not as a correctness fix.",
"doc_path": "docs/go-functional-options-pattern.md",
"enforcement": "judgment (ast-grep follow-up: `type_alias_declaration` / `type_declaration` with `XxxOptions func(...)` matching the wrong-singular pattern, and `struct_type` named `XxxOption`)",
"id": "go-functional-options/singular-option-type",
"level": "SHOULD",
"owner": "go-quality-assistant"
},
{
"anchor": "go-functional-options/with-prefix-option-functions",
"applies_when": "a Go package using the functional-options pattern names the option constructors with prefixes other than `With*` (e.g. `Set*`, `Use*`, `Configure*`, bare nouns like `BatchSize(n)`).",
"doc_path": "docs/go-functional-options-pattern.md",
"enforcement": "judgment (ast-grep follow-up: `function_declaration` returning a type matching `*Option` / `*ConfigFunc` with name not starting with `With`)",
"id": "go-functional-options/with-prefix-option-functions",
"level": "SHOULD",
"owner": "go-quality-assistant"
},
{
"anchor": "go-http-handler/kebab-case-handler-files",
"applies_when": "a `*.go` file under `**/pkg/handler/**` is named with non-kebab-case style (e.g. `exists_handler.go`, `existshandler.go`, or `handler.go`) instead of the documented `<action>-<noun>.go` kebab-case (e.g. `exists.go`, `forward-invoice.go`). Pure ast-grep operates on file contents, not filenames — this is a filesystem convention check.",
Expand Down Expand Up @@ -296,6 +314,24 @@
"level": "MUST",
"owner": "go-http-handler-assistant"
},
{
"anchor": "go-k8s-crd/generated-client-not-dynamic",
"applies_when": "a Go consumer service interacts with a first-party CRD (the CR schema is known at compile time, defined in the same repo or a sibling library) using `k8s.io/client-go/dynamic` instead of the typed clientset generated via `hack/update-codegen.sh`.",
"doc_path": "docs/go-kubernetes-crd-controller-guide.md",
"enforcement": "judgment (import-graph check on the consumer service: `client-go/dynamic` imported alongside a known-schema CR type is the smell; dynamic-client use is acceptable only when the CR is third-party with unknown schema)",
"id": "go-k8s-crd/generated-client-not-dynamic",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-k8s-crd/use-bborbe-k8s",
"applies_when": "a Go service consuming a CRD hand-writes the event-handler cast adapter, the typed event handler, or the in-memory state store, while the service already depends on (or could depend on) `github.com/bborbe/k8s`.",
"doc_path": "docs/go-kubernetes-crd-controller-guide.md",
"enforcement": "judgment (file-existence + dependency-graph check: presence of `pkg/event-handler*.go` or `pkg/<resource>-store.go` alongside a non-trivial `bborbe/k8s` import means hand-written boilerplate that should use the generic primitives)",
"id": "go-k8s-crd/use-bborbe-k8s",
"level": "SHOULD",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-licensing/copyright-year-discipline",
"applies_when": "a PR diff modifies copyright years in `*.go` source-file headers — either bulk-updating across many files or setting future / non-numeric years (`2099`, `present`, etc.).",
Expand Down Expand Up @@ -440,6 +476,24 @@
"level": "MUST",
"owner": "go-security-specialist"
},
{
"anchor": "go-service-impl/no-context-object-injection",
"applies_when": "a Go service method receives a struct (by value OR by pointer) named `Context` / `ServiceContext` / `Deps` / etc. that bundles multiple service dependencies (logger, repository, validator, etc.) and passes them through method calls instead of through the constructor.",
"doc_path": "docs/go-service-implementation-patterns.md",
"enforcement": "judgment (ast-grep follow-up: method signature with a parameter type matching `<Name>Context` or `*<Name>Context` / `<Name>Deps` or `*<Name>Deps` where the struct contains 2+ service-interface fields — value and pointer shapes share the anti-pattern)",
"id": "go-service-impl/no-context-object-injection",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-service-impl/provider-vs-registry-choice",
"applies_when": "a Go service needs to dispatch to one of several implementations and the code uses the wrong dispatch shape for the set's openness: a `map[string]Creator` registry for a closed compile-time set, or a compiled `switch` for an open runtime-extensible set.",
"doc_path": "docs/go-service-implementation-patterns.md",
"enforcement": "judgment (semantic — depends on whether the implementation set is closed or open)",
"id": "go-service-impl/provider-vs-registry-choice",
"level": "SHOULD",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-state-machine/forward-only-by-default",
"applies_when": "a Go FSM worker emits `NextPhase` referring to an earlier phase in the workflow's declared order, without an explicit circuit-breaker `attempt` counter and a controller-side cap.",
Expand Down
Loading