idiomatic-go

Files (17)

• SKILL.md
• evals.json · evals/
• ast.go · evals/files/ast-navigation/
• go.mod · evals/files/ast-navigation/
• go.mod · evals/files/concurrency-poller/
• poller.go · evals/files/concurrency-poller/
• go.mod · evals/files/interfaces-errors/
• orders.go · evals/files/interfaces-errors/orders/
• userstore.go · evals/files/interfaces-errors/userstore/
• go.mod · evals/files/reference-resolution/
• types.go · evals/files/reference-resolution/
• accounts.go · evals/files/typescript-port/
• go.mod · evals/files/typescript-port/
• concurrency-patterns.md · references/
• interfaces-and-errors.md · references/
• language-engineering.md · references/
• naming.md · references/

SKILL.md (first lines)

# Idiomatic Go

The conventions here come from [Effective Go](https://go.dev/doc/effective_go) and [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments) — the documents that codify what Go reviewers reject pull requests over. The content is filtered to the rules that strong models still slip on, so basic syntax, `gofmt` usage, and well-known mechanics (LIFO `defer`, `new` vs `make`, channel basics) are not repeated here.

The guiding principle is **clear is better than clever** (Rob Pike's phrasing, elaborated in [Dave Cheney's article of the same name](https://dave.cheney.net/2019/07/09/clear-is-better-than-clever)). When choosing between a concise/clever construction and an explicit/plain one, pick the explicit one. Code is decoded, not skimmed, and it outlives the person who wrote it.

APIs that require **Go 1.22 or later** are called out inline below. Older stable symbols (`errors.Is`, `fmt.Errorf` with `%w`, `slices.Clone`, and similar) are not annotated.

## Reference files

Load these as needed — don't read them upfront. This file has the rules and decision points; the references hold the long tail.

- `references/naming.md` — the full initialism table with the awkward cases (`OAuth`, `IPv4`, `gRPC`, `IDs`, `HTTPSProxy`), worked naming examples, deeper package-naming guidance (why `util`/`common`/`misc` collect garbage and how to split them), import grouping, and file naming with build constraints. Read this when naming anything exported or organizing a new package.
- `references/interfaces-and-errors.md` — interface placement worked end-to-end, embedding, compile-time satisfaction checks; sentinel errors vs custom error types vs `fmt.Errorf`; the `errors.Is`/`As`/`Join` model and when *not* to wrap; structured errors; `panic`/`recover` discipline. Read this when designing an API, writing a non-trivial error type, or wrapping errors across abstraction boundaries.
- `references/concurrency-patterns.md` — the `context.Context` contract end-to-end, the four ways a goroutine exits, the `for select` skeleton, channel-direction signatures, semaphore and leaky-buffer patterns, `sync.Once`/`WaitGroup`/`errgroup` decisions, channel-closing rules, idempotent shutdown against concurrent registration (a coordinator's shutdown safe to call twice and race-free against concurrent subscriber registration), and the anti-patterns (sleeping in tests, polling, fire-and-forget goroutines). Read this when starting a goroutine, designing a cancellable operation, or reviewing concurrent code.
- `references/language-engineering.md` — patterns specific to Go tools for *programming languages* (parsers, language servers/LSP, type checkers, validators, DSLs) that go beyond baseline idiomatic Go: nil-safe AST navigation, callback-vs-iterator traversal, optional capability interfaces, non-nil empty sentinels, typed source coordinates and UTF-16 columns, editor-overlay text handles, lazy scope chains, typed/untyped references and once-only resolution with cycle detection, phased incremental builds and the write-to-read lock downgrade, keeping the domain free of the LSP protocol, and completion as a parser capability. Read this when working on a language implementation, a DSL, or a toolkit built on a Go language framework — not for ordinary application code.

## Naming

Naming is where Go diverges most visibly from other languages and where models slip most often.

**Initialisms keep one case throughout.** `URL`, `ID`, `HTTP`, `XML`, `JSON`, `API`, `IO`, `DB`, `URI` — either all uppercase or all lowercase, never mixed. So `userID` not `userId`, `ServeHTTP` not `ServeHttp`, `parseJSON` not `parseJson`, `xmlHTTPRequest` not `xmlHttpRequest`. The rule applies even when the initialism appears in the middle of a name. The reference has the edge cases.

**MixedCaps, not snake_case.** Both exported (`MaxRetries`) and unexported (`maxRetries`) names use mixed caps. Unexported constants are *not* `MAX_RETRIES`, even if other languages would write them that way.

**Package names: short, lowercase, no underscores.** The package name prefixes every call site (`http.Get`, `json.Marshal`), so it must read well in context. Avoid `util`, `common`, `helpers`, `misc`, `base`, `shared` — they collect unrelated code and never repay the loan. If a function doesn't fit anywhere, find a more specific home or start a narrower package.

**Avoid stutter.** Package `chubby` exports `File`, not `ChubbyFile`. Callers write `chubby.File` and the package prefix supplies the namespacing. Same for functions: `bytes.NewBuffer`, not `bytes.NewBytesBuffer`.

**Variable name length scales with scope.** Use `i` for a tight loop index, `r` for a `Reader` in a five-line function, `customerID` for a field on an exported struct, `defaultTimeout` for a package-level constant. The further from declaration a name is used, the more descriptive it must be.

**Receiver names are short and consistent.** One or two letters that reflect the type — `c` for `Client`, `b` for `Buffer`, `srv` for `Server`. Use the same name across every method on a type. Never `self`, `this`, `me` — receivers are just parameters in Go.

**No `Get` prefix on getters.** `owner.Name()` not `owner.GetName()`. Setters keep their `Set` prefix to mark the asymmetry: `owner.SetName(n)`.

**Single-method interfaces end in `-er`.** `Reader`, `Writer`, `Stringer`, `Closer`, `Formatter`. Multi-method interfaces don't force the suffix.

## Interfaces

**Define interfaces in the consumer package, not the producer's.** The package that *needs* a `Fetcher` declares the shape it requires. The package that *implements* fetching exports a concrete `*Client` and lets consumers wrap it in whatever interface they need. This keeps the producer free to add methods without breaking consumers, and avoids a forest of speculative interfaces in the implementer.

**Accept interfaces, return concrete types.** Function parameters use interfaces to describe the minimum needed; return values are concrete so callers see every method and the package can add fields without breaking callers.

```go
// Good: minimal interface input, concrete return.
func NewServer(log Logger) *Server

// Bad: hides fields, blocks future methods, forces type assertions.
func NewServer(log Logger) HTTPHandler
```

**Don't pre-create interfaces "for mocking".** Mocks belong in the consumer's test package, against the consumer's interface. A speculative interface in the producer is dead weight and a future API hazard.

**Small interfaces compose better.** Prefer `io.Reader` to a six-method `Source`. If you need read+write, embed: `io.ReadWriter` is just `io.Reader` plus `io.Writer`.

See `references/interfaces-and-errors.md` for type-assertion forms, embedding, and the compile-time satisfaction check `var _ io.Reader = (*MyReader)(nil)`.

## Generics

**Write functions first; add type parameters when you would duplicate the same logic for different types.** Starting with constraint interfaces is usually the wrong path ([When To Use Generics](https://go.dev/blog/when-generics)).

**Prefer interfaces over type parameters** when callers only need methods (`io.Reader`, `io.Writer`). Do not rewrite `func Read(r io.Reader)` into `func Read[T io.Reader](r T)` — harder to read, rarely faster.

**Prefer the standard library** (`slices`, `maps`, `cmp`) over custom generic utilities; use `cmp.Ordered`, not `golang.org/x/exp/constraints`.

**Use type parameters** for language containers (slice/map/channel ops with no element-specific logic) and shared data structures — not when each type needs a different method body (use interfaces and separate implementations).

**Preserve named slice types:** constrain slice args as `S ~[]E` and return `S`, not `[]E`, when callers may pass a defined slice type ([An Introduction To Generics](https://go.dev/blog/intro-generics) Scale example).

**Map keys need `comparable`** in the type parameter list when a generic type uses `map[K]V`.

**Go 1.27 — generic methods:** From Go 1.27, a method may declare type parameters (`func (t *T) M[P any](...)`). Use them for helpers that naturally live on the receiver type instead of polluting the package with `func helper[T, P any](t *T, ...)`. A generic method **does not** implement a non-generic interface method: `func (*R) Read[E any]([]E)` does not satisfy `io.Reader`. For interface satisfaction, keep ordinary methods or package-level generic functions.

## Errors

**Return errors as values.** Never use in-band sentinels like `-1`, `""`, or `nil` to signal failure when the real return type can take those values legitimately. An explicit `(T, error)` return makes the failure case unmissable.

**Error strings are lowercase, no trailing punctuation.** Errors get wrapped and chained: `failed to open config: open /etc/app.conf: permission denied` only reads right if every link in the chain starts lowercase and ends with no period.

```go
// Good.
return fmt.Errorf("open %s: %w", path, err)

// Bad. Capitalized, ends with period — breaks when wrapped.
return fmt.Errorf("Failed to open %s: %v.", path, err)
```

**Wrap with `%w` once** so `errors.Is` and `errors.As` work. Use `%v` only when you intentionally want to flatten — i.e., the inner error is an implementation detail not part of your contract.

**Wrapping is not branching.** `%w` only helps a caller who actually calls `errors.Is`/`errors.As` — it does nothing by itself. If *your own* function needs to treat one failure differently from the rest (not-found vs. everything else), check it with `errors.Is`/`errors.As` before you return; don't assume the branch happens further up the stack.

**Don't `_ = err`.** If you genuinely cannot act on an error, leave a comment explaining why. Silent discard is the default failure mode for `os.Setenv`, `f.Close()` on a read-only file, and similar cases.

**Indent the error branch, not the happy path.** Early return on failure; let success flow down the page.

**Don't panic in libraries.** Panic is for unrecoverable invariants, and even then prefer returning an error. `recover` only works inside a deferred function. See `references/interfaces-and-errors.md` for the rare cases where intra-package panic-as-cross-function-return is idiomatic.

## Concurrency

**"Don't communicate by sharing memory; share memory by communicating."** This is a guiding principle, not a rule. Channels model *ownership transfer* — a value passes from one goroutine's responsibility to another's. Mutexes model *guarded state* — many goroutines need to read or write the same data with a single owner. Both are idiomatic; pick by what the data represents.

**Every goroutine needs a documented exit path.** A goroutine without a planned termination is a leak waiting to happen. Four standard exits: (1) the work is finite and the function returns, (2) `<-ctx.Done()`, (3) an explicit close-channel signal, (4) a `sync.WaitGroup` the parent joins on. If you cannot say which one applies, do not start the goroutine.

**`context.Context` is the first parameter** of any function that does I/O, may block, or starts goroutines. Never store a `Context` in a struct — its lifetime is the call, not the object's. The parameter name is conventionally `ctx`.

```go
// Good.
func (c *Client) Fetch(ctx context.Context, id string) (*Doc, error)

// Bad. Context in struct; lifetime is now ambiguous.
type Client struct { ctx context.Context; /* ... */ }
```

**Prefer synchronous APIs.** A function that returns `(Result, error)` is composable, testable, and trivially wrappable in a goroutine by anyone who wants concurrency. A function that returns `<-chan Result` decides for the caller, leaks if the caller forgets to drain it, and complicates error reporting. Write the synchronous version first; let the caller add the goroutine — even a "streaming" API should be a thin wrapper over that synchronous core, not the primary shape.

```go
// Good: synchronous core; concurrency is the caller's choice, not baked into the API.
func (c *Client) FetchOne(ctx context.Context) (Item, error)

From TypeFox/agent-skills, MIT.

More from TypeFox/agent-skills