diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 01bd009..11e4ea4 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "go-coding", - "version": "0.3.0", + "version": "0.4.0", "description": "Idiomatic Go coding standards for AI assistants — formatting, errors, concurrency, testing, layout.", "author": { "name": "Cadasto B.V.", diff --git a/.cursor-plugin/plugin.json b/.cursor-plugin/plugin.json index 56068c8..d02e490 100644 --- a/.cursor-plugin/plugin.json +++ b/.cursor-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "go-coding", - "version": "0.3.0", + "version": "0.4.0", "description": "Idiomatic Go coding standards for AI assistants — formatting, errors, concurrency, testing, layout.", "author": { "name": "Cadasto B.V.", diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index d722d05..45a79b3 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -13,6 +13,13 @@ jobs: - uses: actions/setup-python@v5 with: python-version: '3.x' + # The floor-minor Go toolchain activates the validator's Fixer-column check + # (go tool fix help is the authority for what `go fix` ships). Keep this minor + # in step with GO_FLOOR_MINOR in scripts/validate.py and the documented baseline. + - uses: actions/setup-go@v5 + with: + go-version: '1.26.x' + cache: false # no go.mod — nothing to cache # CI is strict and deterministic: Python is guaranteed here, so run the # validator directly (the scripts/validate.sh graceful skip is for local use). - run: python3 scripts/validate.py diff --git a/AGENTS.md b/AGENTS.md index 795aab5..2c440fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ This file provides guidance to AI coding assistants (Claude Code, Cursor, and co The **Go Coding Plugin** is an AI plugin by Cadasto B.V. that teaches AI coding assistants **idiomatic Go coding standards** — formatting, naming, error handling, concurrency, testing, and project layout — through skills, commands, agents, hooks, and Cursor rules. It targets **both Claude Code and Cursor** from a single shared component set. -> **Current status — v0.3.0.** A complete dual-host (Claude Code + Cursor) Go-standards set, baselined on **Go 1.26** (1.26.4+) with version-gated guidance that still holds for 1.25 modules, that validates clean (`./scripts/validate.sh` + `claude plugin validate .`): the auto-invoked `go-coding` **router** skill; the focused standards skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout`; the read-only `go-reviewer` agent; the user-invoked `/go-explain` and `/go-lint-setup` skills; a shipped `references/golangci.v2.yml`; the `rules/go-context.mdc` Cursor rule; and host-agnostic `session-start` + `format-on-save` hooks. Do not assume a file is present because it is documented here — check first. +> **Current status — v0.4.0.** A complete dual-host (Claude Code + Cursor) Go-standards set, baselined on **Go 1.26.4+** as a hard floor (no fallback guidance for 1.25 or older; version annotations remain as provenance), that validates clean (`./scripts/validate.sh` + `claude plugin validate .`): the auto-invoked `go-coding` **router** skill; the focused standards skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout`; the read-only `go-reviewer` agent; the user-invoked `/go-explain` and `/go-lint-setup` skills; a shipped `references/golangci.v2.yml`; the `rules/go-context.mdc` Cursor rule; and host-agnostic `session-start` + `format-on-save` hooks. Do not assume a file is present because it is documented here — check first. ## Domain Context @@ -23,6 +23,8 @@ This plugin encodes **Go (golang) coding standards**. Guidance must be grounded When a recommendation derives from one of the above, attribute it explicitly and distinguish cited rules from inference. +**Refreshing the skills against current Go practice** (e.g. "check current Go best practices and update the skills"): follow the **source registry and procedure** in [docs/authoring.md](docs/authoring.md#refreshing-the-standards-baseline-source-registry) — it lists every source to re-read, in order, plus the version-gating rules. Re-read them; never refresh from memory. Two recurring traps: the *released* Go version is not whatever `go.dev/doc/go1.NN` renders (check the release history), and the stdlib APIs a skill is missing are usually the ones that landed *after* it was written. + ## Repository Layout This repo supports **both Claude Code and Cursor**. Shared assets (skills, commands, agents) are consumed by both hosts; host-specific manifests and hook configs are kept separate. @@ -37,7 +39,7 @@ This repo supports **both Claude Code and Cursor**. Shared assets (skills, comma - **Cursor hooks**: `hooks/cursor-hooks.json` — object `{ "hooks": { "sessionStart": [...], "afterFileEdit": [...] } }`; the command runs from the plugin root (a **workspace-relative** path, **not** `${CLAUDE_PLUGIN_ROOT}`). Present — wires `session-start.sh` (`sessionStart`) and `format-on-save.sh` (`afterFileEdit`). - **Shared hook scripts**: `hooks/session-start.sh` — detects `go.mod` / `*.go`, prints one Go-standards context line, exits 0 always. `hooks/format-on-save.sh` — after a `*.go` Write/Edit, runs `gofumpt -w` (or `gofmt -w -s`) on that single file; resolves the path from `$CLAUDE_FILE_PATH` or the stdin tool-payload JSON, host-only, silent no-op if no formatter is installed, exits 0 always. Both host-agnostic so either manifest can invoke them. Present. - **MCP config** *(optional, not present)*: `.mcp.json` — only if the plugin later integrates an MCP server. There is no companion MCP server today; do not reference one. -- **Validation**: `scripts/validate.sh` wraps `scripts/validate.py` to check both manifests, dual-host parity, declared component paths, kebab-case names, hook-config JSON, and skill/command/agent frontmatter (**agents must use `tools:` not `allowed-tools:`** — flagged as an error). The Python is stdlib-only. `.github/workflows/validate.yml` pins Python and runs the validator strictly. +- **Validation**: `scripts/validate.sh` wraps `scripts/validate.py` to check both manifests, dual-host parity, declared component paths, kebab-case names, hook-config JSON, skill/command/agent frontmatter (**agents must use `tools:` not `allowed-tools:`** — flagged as an error), and two *advice == tooling* invariants: every linter taught in a component is enabled in `references/golangci.v2.yml`, and (when a floor-minor Go toolchain is on PATH — CI installs `1.26.x`, locally it soft-skips) the `go-idioms` Fixer column matches `go tool fix help`. The Python is stdlib-only. `.github/workflows/validate.yml` pins Python + Go and runs the validator strictly. - **Contributor docs**: `docs/` for human-facing references — `install.md`, `testing.md`, `versioning.md`, `authoring.md`. `.github/` holds issue + PR templates, `copilot-instructions.md`, and the CI workflow. (Planning and research working notes are kept locally under `docs/`, **gitignored** — not part of the published plugin.) ### Component surface diff --git a/CHANGELOG.md b/CHANGELOG.md index e55b4e2..b7d4478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,38 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Keep a Changelog: https://keepachangelog.com/en/1.1.0/ - Semantic Versioning: https://semver.org/spec/v2.0.0.html +## [Unreleased] + +## [0.4.0] - 2026-08-05 + +Grounds the standards set on a **Go 1.26.4+ hard floor**, widens `go-layout` to naming + API surface, replaces the pinned golangci-lint version with a pin *policy*, and makes "advice == tooling" machine-checked (taught linters vs the reference config; the `go-idioms` Fixer column vs the floor toolchain's `go tool fix help`). + +### Added +- Agents: `go-reviewer` — three review dimensions: **silent dispatch defaults** (pass-through `default` over an internal enum; paired dispatch sites maintained as independent switches), **sensitive-value echo in errors/logs** at a boundary (driver messages quoting stored values, request bodies in wrapped errors), and **comment–code drift** in the diff. +- Skills: `go-errors` — fail-loudly-on-impossible-dispatch rule (loud `default` + `exhaustive` linter or enum-completeness test) and boundary-errors-carry-classification-not-payload rule (pass the class/code, keep the raw message internal). +- Skills: `go-testing` — golden files pin *shape*, not behaviour: pair a golden of an externally executed artefact (SQL, wire requests, rendered configs) with at least one live-execution test. +- Skills: `go-errors` — `errors.AsType[E]` (Go 1.26) leads typed-error inspection, preferred over `errors.As` (`errorsastype` modernizer converts call sites); `%w` goes last unless the sentinel is the sentence; check `Close` on written files (`errors.Join` into a named result); keep the happy path at minimal indentation; never let a panic cross a package boundary. +- Skills: `go-testing` — `t.Context` (1.24), `t.ArtifactDir` (1.26) vs `t.TempDir`, `t.Output`/`t.Attr` (1.25); `t.Setenv`/`t.Chdir`/`cryptotest.SetGlobalRandom` are process-global and unusable under `t.Parallel`; failure messages must carry call/input/got/want; helpers set up, the test body asserts. +- Skills: `go-concurrency` — cancellation causes (`WithCancelCause` + `context.Cause`, `WithTimeoutCause`), `context.WithoutCancel` for work outliving a request, `context.AfterFunc`, prefer-synchronous-APIs, and explicit cleanup over `runtime.AddCleanup`/`SetFinalizer`. +- Skills: `go-idioms` — **Fixer** column naming the owning analyzer per row, audited against Go 1.26.4 `go tool fix help` (plain = in the toolchain's `go fix`; **†** = x/tools/golangci-lint `modernize` only, e.g. `errorsastype`, `bloop`, `atomictypes`, `slicesbackward`); rows for `any`, `omitzero`, `testingcontext`, `stringsseq`, `reflecttypefor`, `fmt.Appendf`; a `go fix` recipe (`-diff` preview, per-fixer `-`/`-=false` selection); new "no fixer automates it" section (`os.OpenRoot`, `crypto/rand.Text`, nil slices, sorted map iteration). +- Skills: `go-layout` — naming (initialism casing, `MixedCaps`, name-length-tracks-scope, receiver names, no `Get` prefix, `test` doubles), signatures/API surface (receiver type, in-band errors, named results, option struct vs variadic options, accept interfaces/return concrete types, useful zero value), and doc-comment conventions. +- Skills: `go-linting` — `golangci-lint migrate` for v1 configs, `golangci-lint fmt`, the `linters.exclusions`/`formatters.settings` moves, and `//nolint: // reason` discipline. +- Lint config: `usetesting`, `nolintlint` + `exhaustive` in `references/golangci.v2.yml`, the `go-linting` block, and `/go-lint-setup` (`exhaustive` was taught by `go-errors`/`go-reviewer` but not shipped). +- Agents: `go-reviewer` — **exported-surface & naming slips** dimension; discarded-`Close`-on-a-written-file folded into resource leaks; `errors.AsType` in sentinel/typed-error breakage. +- Docs: `docs/authoring.md` — "Refreshing the standards baseline (source registry)": tiered source list plus the procedure for re-grounding the skills against current Go practice; **AGENTS.md** points at it for refresh requests. +- Validation: `scripts/validate.py` — two advice == tooling cross-checks: every linter a component teaches (`--enable-only=…` or "the `` linter") must be enabled in `references/golangci.v2.yml`, and the `go-idioms` **Fixer** column is verified against the floor toolchain's `go tool fix help` (plain names must be registered, † names must not be; soft-skips locally without Go). CI (`validate.yml`) now pins Go `1.26.x` alongside Python to run the Fixer check strictly. +- Skills: `go-errors` — terse wrap context (`"new store: %w"`, no `"failed to"` pile-up); `go-layout` — `main` owns process exit (`os.Exit`/`log.Fatal` only in `main`, `run() error` pattern) and `init()` restricted to cheap deterministic setup. +- Skills: `go-coding` router — routes to the `/go-explain` and `/go-lint-setup` user-invoked skills. + +### Changed +- Agents: `go-reviewer` — frontmatter `description` converted to prose triggers (was ~313 words of `` blocks); worked scenarios moved to a "When to invoke" body section. +- Skills: `go-coding`/`go-explain` descriptions trimmed to the ~50–75-word always-on budget; skill bodies swept to imperative form (second-person phrasing removed). +- Skills / agent / Cursor rule / docs: **Go 1.26.4+ is now a hard floor** — the "works with 1.25+" framing and the per-idiom "check `go.mod` before applying" hedging are gone from `go-coding`, `go-errors`, `go-idioms`, `go-explain`, `go-reviewer`, `rules/go-context.mdc`, `README.md` and `docs/install.md`. Version annotations (`Since`, "(Go 1.24)") stay as provenance. +- Skills: `go-linting`, `/go-lint-setup`, `references/golangci.v2.yml` — no blessed golangci-lint version anywhere; replaced with a pin *policy* (exact version, one source of truth, `golangci-lint-action` `version:` input, automated bump PR, `--fix`-then-triage on bump) and upstream's warning against `go install`/`tool`-directive installs. +- Docs: `docs/authoring.md` — refresh procedure gains the hard-floor rule, "never hardcode a tool version in a component", and "run the tool, don't read about it" (floor-toolchain `go tool fix help` is the authority for shipped fixers; the Tier 3 registry now marks pkg.go.dev's modernize page as x/tools *tip* — source for † rows only). +- Skills / Cursor rule: `go-layout` widens from project layout to layout **+ naming + API surface**; the `go-coding` router and `rules/go-context.mdc` route naming/doc-comment/API-shape questions there and name `revive` as the deterministic backstop. +- Skills: labelled Go 1.27 forward notes (draft, expected Aug 2026) only where they change advice — `goroutineleak` on by default, `synctest.Sleep` + `httptest.NewTestServer`, † fixers graduating into `go fix`, `encoding/json/v2`. + ## [0.3.0] - 2026-07-01 Retargets the standards baseline to **Go 1.26** (1.26.4+) while keeping version-gated guidance valid for 1.25 modules. diff --git a/README.md b/README.md index ac91ca9..668abea 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Or from a local working copy: `claude plugin add /path/to/go-coding-plugin`. **Cursor**: add this repository as a plugin (Settings → Plugins). See [`docs/install.md`](docs/install.md) for both hosts. -**Prerequisites** — the plugin installs without a Go toolchain, but its hooks and enforcement guidance expect **Go 1.26.x** (minimum 1.26.4; still works against 1.25 modules) plus `gofmt`, `gofumpt`, `goimports`, and `gopls` on the host `PATH`. See [Host toolchain (minimal requirements)](docs/install.md#host-toolchain-minimal-requirements) for what each tool drives and copy-paste install commands. +**Prerequisites** — the plugin installs without a Go toolchain, but its hooks and enforcement guidance expect **Go 1.26.4+** plus `gofmt`, `gofumpt`, `goimports`, and `gopls` on the host `PATH`. See [Host toolchain (minimal requirements)](docs/install.md#host-toolchain-minimal-requirements) for what each tool drives and copy-paste install commands. ## Component surface @@ -24,7 +24,7 @@ Or from a local working copy: `claude plugin add /path/to/go-coding-plugin`. | Skill `go-coding` | shipped | Auto-invoked router: sends each Go topic to the enforcing tool and the focused skill below; recommends `gopls-lsp`. | | Session-start hook | shipped | Detects a Go workspace (`go.mod`/`*.go`) and prints one standards line; dual-host. | | Format-on-save hook | shipped | After each `Write`/`Edit` of a `*.go` file, runs `gofumpt -w` (or `gofmt -w -s`) on it; dual-host, host-only, silent no-op if no formatter is installed. | -| Skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout` | shipped | Load-on-use standards — each rule cited, framed around the enforcing linter (`modernize`, `errorlint`, `-race`, …). | +| Skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout` | shipped | Load-on-use standards — each rule cited, framed around the enforcing linter (`modernize`, `errorlint`, `-race`, …). `go-layout` also owns naming, doc comments, and exported-API shape. | | Agent `go-reviewer` | shipped | Read-only, context-isolated Go reviewer for what linters miss; severity-ranked findings, no sub-agent dispatch. | | Skills `/go-explain`, `/go-lint-setup` (user-invoked) | shipped | Slash-command skills — idiom/standard lookup; scaffold the golangci-lint v2 config into a repo. | | Lint config `references/golangci.v2.yml` | shipped | Reference golangci-lint v2 config (`modernize` + stack linters). | diff --git a/agents/go-reviewer.md b/agents/go-reviewer.md index 587735c..9d3c2ac 100644 --- a/agents/go-reviewer.md +++ b/agents/go-reviewer.md @@ -2,38 +2,16 @@ name: go-reviewer description: > Use this agent to review Go (`.go`) diffs or files for the bugs and smells that linters miss — - silent error swallowing, goroutine leaks, context misuse, resource leaks, sentinel-error breakage, - unsafe atomics, stale modernization debt, and slog hot-path waste. Invoke it after writing or - changing Go code, before opening a PR, or whenever the user asks for a Go code review. It is - read-only, works alone, and returns severity-ranked findings; it does not edit code or dispatch - other agents. Not for non-Go languages or for problems `gofmt`/`go vet`/`golangci-lint` already flag. - - - Context: The user just finished a Go change and wants it reviewed. - user: "I refactored the worker pool in scheduler.go — can you review it?" - assistant: "I'll dispatch the go-reviewer agent to check the diff for goroutine-lifecycle, context, and error-handling issues." - - Go concurrency and error review is judgment a linter can't fully provide; the context-isolated go-reviewer applies the heuristics catalog and returns ranked findings without polluting the main context. - - - - - Context: The user is about to open a PR with Go changes. - user: "before I push this, check the Go code for anything reviewers will flag" - assistant: "I'll run the go-reviewer agent over the staged diff and report findings by severity." - - Pre-PR review is the canonical trigger; the agent reads the diff as untrusted content and walks the review dimensions. - - - - - Context: The user asks for a focused review of one file. - user: "review the database layer in pg.go for resource leaks and context handling" - assistant: "I'll dispatch the go-reviewer agent scoped to the resource-leak and context dimensions for that file." - - Direct review requests scoped to a file or a dimension are exactly what this agent is for. - - + silent error swallowing, goroutine leaks, context misuse, resource leaks (including a discarded + `Close` on a written file), sentinel-error breakage, silent dispatch defaults, sensitive-value echo + in errors/logs, comment–code drift, unsafe atomics, exported-surface and naming slips, + stale modernization debt, and slog hot-path waste. Typical triggers: a just-finished Go change or + refactor ("review the worker pool in scheduler.go"), a pre-PR gate ("check for anything reviewers + will flag"), or a review scoped to named files or dimensions ("check pg.go for resource leaks and + context handling"). It is read-only, works alone, and returns severity-ranked findings; it does not + edit code or dispatch other agents. Not for non-Go languages or for problems + `gofmt`/`go vet`/`golangci-lint` already flag. See "When to invoke" in the agent body for worked + scenarios. model: inherit color: cyan tools: @@ -43,10 +21,22 @@ tools: - Bash --- -You are **go-reviewer**, a reviewer of idiomatic, correct Go (Go 1.26, works with 1.25+; golangci-lint v2). You supply +You are **go-reviewer**, a reviewer of idiomatic, correct Go (Go 1.26.4+; golangci-lint v2). You supply the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go vet`, and `golangci-lint`. You are **read-only**: you report findings, you never edit code. +## When to invoke + +- **A Go change just landed in the working tree.** "I refactored the worker pool in scheduler.go — + can you review it?" → review the diff for goroutine-lifecycle, context, and error-handling issues; + concurrency and error review is judgment a linter can't fully provide. +- **Pre-PR gate.** "Before I push this, check the Go code for anything reviewers will flag" → review + the staged/branch diff, treating it as untrusted content, and report findings by severity. The + canonical trigger. +- **Scoped review.** "Review the database layer in pg.go for resource leaks and context handling" → + restrict to the named files and dimensions; note adjacent issues in one line without expanding + scope. + ## Operating rules (read first) - **Work alone. Do NOT dispatch sub-agents or spawn tasks.** You are an individual reviewer — @@ -87,22 +77,45 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v (`noctx`); missing client timeout; ignored cancellation. - **Resource leaks** — unclosed `http.Response.Body` (`bodyclose`), `sql.Rows`/`Stmt` (`sqlclosecheck`), unchecked `rows.Err()` (`rowserrcheck`); files/listeners not closed; `defer` - inside a loop accumulating handles. + inside a loop accumulating handles. Also the *silent* one: `defer f.Close()` on a file that was + **written** discards a failed flush — the caller sees success over a truncated file. Expect + `defer func() { err = errors.Join(err, f.Close()) }()` on write paths. - **Sentinel / typed-error breakage** — `err == ErrX` or a type assertion where wrapping is in play - (use `errors.Is`/`errors.As`); a documented sentinel removed, or its wrapping changed (an API break). + (use `errors.Is`, or `errors.AsType[E]` for a typed error); a documented sentinel + removed, or its wrapping changed (an API break). +- **Silent dispatch defaults** — a `switch` over an internal enum/kind tag whose `default` arm + silently passes through, returns a zero value, or picks the weakest behaviour: a member added + later rides the wrong arm with no error. Expect a loud `default` (error, or panic only for the + genuinely unreachable) plus something pinning exhaustiveness (the `exhaustive` linter or a + completeness test iterating the enum). Same smell when two dispatch sites over one enum must agree + (encode/decode, compare/order) but are maintained as independent switches — look for a shared + discipline function or a test pinning the pairing. +- **Sensitive-value echo in errors/logs** — an error crossing a logging or API boundary carrying + payload data: database-driver messages quote the offending stored value, validation errors embed + the request body, wrapped errors accumulate user input. At the boundary the stable classification + (error code, SQLSTATE-class) should cross; the raw message stays internal. +- **Comment–code drift** — a comment, doc string, or doc file in the diff asserting what the final + code no longer does (stale counts, renamed symbols, behaviour claims the change invalidated). + Cheap to fix at review, expensive once trusted. - **Concurrency hazards** — bare-int `atomic.Add*` instead of typed `atomic.Int64`/`Bool` (and non-atomic reads of those fields); `sync.Mutex`/`WaitGroup` copied by value; a map written concurrently without a lock; check-then-act races. +- **Exported-surface & naming slips** — a newly exported identifier with no doc comment, or one that + doesn't start with the name it documents; mixed initialism casing (`userId`, `HttpClient`); a `Get` + prefix on an accessor; an in-band error (`-1`, `""`, or a meaningful `nil`) where `(T, error)` or + `(T, bool)` belongs; an interface returned where the concrete type would serve; pointer and value + receivers mixed on one type. `revive` catches the naming and missing-doc-comment cases — name it; + the signature-shape ones are judgment. See `go-layout`. - **Stale modernization debt** — code `modernize`/`go fix` would rewrite (range-int, `min`/`max`, - `slices`/`maps`, `strings.Cut`, `cmp.Or`, pre-1.22 loop-var copies; on Go 1.26 modules also - pointer-helper temps that `new(expr)` replaces). Low severity; point at `go fix ./...` (Go 1.26) or - `golangci-lint run --enable-only=modernize`. Gate suggestions on the module's `go.mod` version. + `slices`/`maps`, `strings.Cut`, `cmp.Or`, leftover loop-var copies, pointer-helper temps that + `new(expr)` replaces, `errors.As` where `errors.AsType` fits). Low severity; point at + `go fix ./...` or `golangci-lint run --enable-only=modernize`. - **slog hot-path waste** — building a per-call logger instead of `logger.With(...)`; formatting or allocating before a level check; key-value variadic on a hot path instead of `slog.LogAttrs`. For the *why* and citations behind any dimension, the `go-errors`, `go-concurrency`, `go-testing`, -`go-idioms`, and `go-linting` skills carry the grounded rules — reference them rather than -re-deriving from memory. +`go-idioms`, `go-linting`, and `go-layout` skills carry the grounded rules — reference them rather +than re-deriving from memory. ## Output format diff --git a/docs/authoring.md b/docs/authoring.md index 5322706..0c6ad07 100644 --- a/docs/authoring.md +++ b/docs/authoring.md @@ -47,6 +47,74 @@ with *empty* metadata (every field silently dropped). `claude plugin validate` c - Imperative voice; explain *why* a rule matters rather than relying on bare MUST/NEVER. Keep skill bodies focused — the always-on cost is the `description`; the body loads on use. +## Refreshing the standards baseline (source registry) + +When asked to *refresh the skills against current Go practice*, re-read these sources in this order +and update the affected skill bodies — do not refresh from memory, and do not add a rule without a +citation. Everything the skills assert should be traceable to one of these. + +**Tier 1 — normative, always check first** + +| Source | URL | What it settles | +|---|---|---| +| Release notes for the baseline version | `https://go.dev/doc/go1.NN` | new APIs/idioms, experiments, removals | +| Release history | | what is actually *released* vs. draft — the baseline claim in AGENTS.md depends on this | +| Package docs | `https://pkg.go.dev/` | exact signatures + the "added in go1.NN" annotation for every version gate | +| Effective Go | | foundational idiom | +| Go Code Review Comments | | the review-rule catalogue (naming, errors, concurrency, API shape) | +| Doc comment syntax | | `gofmt`-formatted doc comments, doc links | + +**Tier 2 — style guides (attribute when a rule comes from one)** + +- Google Go Style Guide — (esp. `/best-practices`: naming, + error handling, panics, option structs, documentation, test structure) +- Uber Go Style Guide — + +**Tier 3 — the enforcing tools (this is what keeps "advice == tooling" true)** + +- **`go tool fix help` on the floor-version toolchain** — the authority for which fixers `go fix` + actually ships (the plain rows in the `go-idioms` **Fixer** column); `go fix` blog — + +- `modernize` per-fixer docs — + — tracks x/tools **tip**, usually ahead of the toolchain: the source for **†** rows, never + evidence that a fixer ships in `go fix` +- golangci-lint docs — · v1→v2 migration — + · changelog (for the CI pin) — + +- `go.dev/blog` for feature-specific posts (`synctest`, `testing-b-loop`, `slog`, `range-functions`) + +**Procedure** + +1. Confirm the current *released* Go version (release history) — a draft `go1.NN` page is not a + baseline. Guidance for an unreleased version goes in as one *italic, explicitly labelled* + sentence (`*Go 1.NN (draft, expected …)*`), never as a rule. + **The baseline is a hard floor** (currently **Go 1.26.4+**): recommend the modern form flat, with + no "on 1.NN+ modules prefer…" hedging and no fallback branch for older toolchains. Keep the + version annotation (`Since`, "(Go 1.24)") — that is provenance, and it tells a reader on an older + module what a bump would buy. When the floor moves, delete the guidance below it. +2. Diff each `go-*` skill against Tier 1 for the baseline and the two prior versions — the common + miss is a stdlib API that landed *after* a skill was written (`errors.AsType`, `t.ArtifactDir`). +3. Verify every version gate in `pkg.go.dev`'s "added in" annotation before writing a `Since` cell. +4. Re-check the Tier 3 tool names — a renamed or dropped fixer/linter turns a rule into a wrong + command (`waitgroup` → `waitgroupgo`). + **Run the tool, don't read about it:** when a floor-version toolchain is available, + `go tool fix help` settles fixer names in one command — pkg.go.dev's modernize page tracks + x/tools tip, which is usually ahead of what `go fix` ships; same idea for linters + (`golangci-lint help linters` on the pinned build). `scripts/validate.py` enforces both + halves: every linter taught in components must be enabled in `references/golangci.v2.yml`, + and — when a floor-minor Go toolchain is on PATH (CI installs `1.26.x`; locally it + soft-skips with a note) — the `go-idioms` Fixer column is verified against + `go tool fix help`: plain names must be registered, † names must not be. The floor minor + lives in `GO_FLOOR_MINOR` in the script and in the workflow's `setup-go` pin — move all + three (docs baseline included) together. + **Never hardcode a tool version in a component.** A named `golangci-lint` release rots within + weeks and nobody remembers why it was chosen; the skills carry the *pin policy* (pin exactly, one + source of truth, automated bump PR) plus the changelog URL, and let the consuming repo own the + number. The same goes for `gopls`/`gofumpt` versions outside `docs/install.md`. +5. Keep the three copies of the reference lint config in sync: `references/golangci.v2.yml`, the + block in `go-linting`, and the block in `go-lint-setup`. +6. Record the refresh in **CHANGELOG.md** under `## [Unreleased]`. + ## Dual-host parity Skills, commands, and agents are shared by both hosts. The **Cursor** manifest diff --git a/docs/install.md b/docs/install.md index 9ad021c..00238fc 100644 --- a/docs/install.md +++ b/docs/install.md @@ -41,13 +41,13 @@ Add this repository as a plugin (Cursor **Settings → Plugins**, via Git URL or ## Host toolchain (minimal requirements) -Installing the plugin itself needs no Go toolchain — it is pure Markdown + JSON. But its **enforcement** layer only delivers value when the standard Go tools are on the host `PATH`: the `format-on-save` hook shells out to a formatter, the golangci-lint v2 reference config lists `gofumpt`/`goimports` as formatters, and the recommended official `gopls-lsp` plugin (`@claude-plugins-official`) drives `gopls`. The plugin targets **Go 1.26** + golangci-lint v2, and its version-gated idiom guidance still works against 1.25 modules. +Installing the plugin itself needs no Go toolchain — it is pure Markdown + JSON. But its **enforcement** layer only delivers value when the standard Go tools are on the host `PATH`: the `format-on-save` hook shells out to a formatter, the golangci-lint v2 reference config lists `gofumpt`/`goimports` as formatters, and the recommended official `gopls-lsp` plugin (`@claude-plugins-official`) drives `gopls`. The plugin targets **Go 1.26.4+** + golangci-lint v2 as a hard floor — it does not carry fallback guidance for 1.25 or older modules. At minimum the host should provide: | Tool | Provided by | Used for | If missing | |------|-------------|----------|------------| -| **Go 1.26.x** (min 1.26.4) | [go.dev/dl](https://go.dev/dl/) / package manager | everything; satisfies `go.mod` `go 1.26.x` (and 1.25 modules); `go fix ./...` runs the modernizers | no toolchain at all | +| **Go 1.26.x** (min 1.26.4) | [go.dev/dl](https://go.dev/dl/) / package manager | everything; satisfies `go.mod` `go 1.26.x`; `go fix ./...` runs the modernizers | no toolchain at all | | **`gofmt`** | the Go distribution | `format-on-save.sh` fallback (`gofmt -w -s`) | n/a — always ships with Go | | **`gofumpt`** | `go install` | `format-on-save.sh` primary (`gofumpt -w`), stricter gofmt superset | hook degrades to `gofmt` | | **`goimports`** | `go install` | `goimports` formatter in the golangci-lint v2 config (import grouping/pruning) | import-group formatting skipped | diff --git a/references/golangci.v2.yml b/references/golangci.v2.yml index 4f93abb..015cf44 100644 --- a/references/golangci.v2.yml +++ b/references/golangci.v2.yml @@ -1,9 +1,12 @@ # Reference golangci-lint v2 config for the go-coding plugin. # # Scaffold it into a repo with the `/go-lint-setup` command, or copy it to `.golangci.yml`. -# Schema is golangci-lint v2 — a v1 config will NOT parse. See the `go-linting` skill for -# what each linter does and why. Keep this in sync with the block inlined in -# `skills/go-lint-setup/SKILL.md`. +# Schema is golangci-lint v2 — a v1 config will NOT parse (run `golangci-lint migrate` on a v1 +# config). Needs a golangci-lint v2 release recent enough to know every linter named below; if it +# rejects one, bump the pin rather than dropping the line. Pin an exact version in CI in one place +# (see `go-linting` — no version is blessed here on purpose). See the `go-linting` skill for what +# each linter does and why. Keep this in sync with BOTH inlined copies: the reference block in +# `skills/go-linting/SKILL.md` and the scaffold block in `skills/go-lint-setup/SKILL.md`. version: "2" linters: @@ -11,6 +14,7 @@ linters: enable: - modernize # range-int, min/max, slices/maps, wg.Go, strings.Cut … (same engine as go fix) - errorlint # %w + errors.Is/As discipline + - exhaustive # a switch over an enum names every member (go-errors' loud-default rule) - bodyclose # unclosed http.Response.Body - rowserrcheck # unchecked sql.Rows.Err - sqlclosecheck # unclosed sql.Rows/Stmt @@ -18,6 +22,8 @@ linters: - contextcheck # context not propagated through the call chain - containedctx # context.Context stored in a struct field - perfsprint # fmt.Sprintf where a cheaper call exists + - usetesting # os.Setenv/os.Chdir/context.Background in tests → t.Setenv/t.Chdir/t.Context + - nolintlint # a //nolint must name a linter and carry a reason - revive # configurable golint successor formatters: diff --git a/rules/go-context.mdc b/rules/go-context.mdc index fc4fd5e..3fa9428 100644 --- a/rules/go-context.mdc +++ b/rules/go-context.mdc @@ -1,5 +1,5 @@ --- -description: Go coding standards — idiomatic Go (Go 1.26, works with 1.25+; golangci-lint v2) for .go files. Mirrors the go-coding router skill for Cursor. +description: Go coding standards — idiomatic Go (Go 1.26.4+; golangci-lint v2) for .go files. Mirrors the go-coding router skill for Cursor. globs: ["**/*.go"] alwaysApply: false --- @@ -17,11 +17,11 @@ This Cursor rule mirrors the `go-coding` router skill — apply it when editing |---|---|---| | Formatting | `gofmt`/`gofumpt` (+ `goimports`) — machine-enforced | — | | Static analysis / bugs | `go vet ./...`, `golangci-lint run` | `go-linting` | -| Modern idioms | `go fix ./...` (Go 1.26 runs the modernizers natively), or `golangci-lint run --enable-only=modernize` | `go-idioms` | -| Errors | `golangci-lint run --enable-only=errorlint` | `go-errors` | +| Modern idioms | `go fix ./...` (the toolchain's modernizers), or `golangci-lint run --enable-only=modernize` | `go-idioms` | +| Errors | `golangci-lint run --enable-only=errorlint,exhaustive` | `go-errors` | | Concurrency | `go test -race ./...`, `go vet ./...` | `go-concurrency` | -| Testing | `go test -race ./...`; `testing/synctest` (since 1.25) for time/concurrency | `go-testing` | -| Layout | judgment | `go-layout` | +| Testing | `go test -race ./...`; `testing/synctest` for time/concurrency | `go-testing` | +| Layout, naming & API surface | `golangci-lint run --enable-only=revive`; rest is judgment | `go-layout` | Ground every judgment call in a cited source — Effective Go, Go Code Review Comments, the Google and Uber Go style guides, `pkg.go.dev`. Don't invent rules. Adopt the shipped golangci-lint v2 diff --git a/scripts/validate.py b/scripts/validate.py index cb25190..ab4f8a7 100644 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -11,7 +11,13 @@ * hook-config JSON validity when present; * SKILL.md / agent / command frontmatter — required keys, and ``name`` matching the directory/filename. Agents MUST declare ``tools:`` (never ``allowed-tools:``, which - Claude Code silently ignores so the agent inherits *all* tools — flagged as an error). + Claude Code silently ignores so the agent inherits *all* tools — flagged as an error); + * advice == tooling: every linter a component *teaches* (via ``--enable-only=...`` or + "the `` linter") must be enabled in ``references/golangci.v2.yml`` — a skill must + not tell agents to rely on a linter no config copy ships; + * when a Go toolchain at the floor minor (``GO_FLOOR_MINOR``) is on PATH, the go-idioms + **Fixer** column is verified against ``go tool fix help``: plain names must be registered, + † names must not be. Soft-skips locally without Go; CI installs the floor toolchain. This plugin has no MCP backend, so there is intentionally no ``.mcp.json`` check. @@ -20,11 +26,14 @@ """ import json import re +import shutil +import subprocess import sys from pathlib import Path ROOT = Path(__file__).resolve().parent.parent errors = [] +notes = [] PLUGIN_NAME_RE = re.compile(r"^[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?$") KEBAB_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") @@ -33,6 +42,13 @@ MANIFEST_PATH_FIELDS = ("logo", "rules", "skills", "agents", "commands", "hooks") # Fields that must agree across the Claude and Cursor manifests. SYNCED_FIELDS = ("name", "version", "description", "author") +# The golangci-lint v2 `linters.default: standard` set — enforced without an explicit +# `enable:` entry (see references/golangci.v2.yml). +STANDARD_LINTERS = {"errcheck", "govet", "ineffassign", "staticcheck", "unused"} +# The plugin's Go hard floor (minor). The go-idioms Fixer column is verified against THIS +# minor's `go tool fix help` — a newer/older toolchain's list would prove nothing about the +# floor, so the check skips on any other minor. Bump together with the documented baseline. +GO_FLOOR_MINOR = "1.26" def err(msg): @@ -143,6 +159,97 @@ def validate_rules(): err(f"{rel}: frontmatter missing 'description'") +def validate_linter_references(): + """Advice == tooling: every linter a component *teaches* must be shipped by the reference + lint config. A linter counts as taught when a component names it as the enforcing tool — + in a ``--enable-only=...`` command, or in the phrase "the `` linter". Guards against + the drift where a skill tells agents to lean on a linter (e.g. ``exhaustive``) that no + config copy actually enables. Deliberately narrow patterns: a passing mention of a linter + name without either signal is not flagged.""" + ref = ROOT / "references" / "golangci.v2.yml" + if not ref.is_file(): + return + # Only the `linters:` section — formatters are a different contract. + linters_section = ref.read_text().split("formatters:")[0] + allowed = set(re.findall(r"^\s+-\s+([a-z0-9-]+)", linters_section, re.MULTILINE)) + allowed |= STANDARD_LINTERS + components = ( + sorted((ROOT / "skills").glob("*/SKILL.md")) + + sorted((ROOT / "agents").glob("*.md")) + + sorted((ROOT / "rules").glob("*.mdc")) + ) + for md in components: + body = md.read_text() + taught = set() + for group in re.findall(r"--enable-only=([a-z0-9,-]+)", body): + taught.update(group.split(",")) + # \s+ so the phrase still matches when hard-wrapped across a line break. + taught.update(re.findall(r"[Tt]he\s+`([a-z0-9-]+)`\s+linter", body)) + for name in sorted(taught - allowed): + err(f"{md.relative_to(ROOT)}: teaches the '{name}' linter but " + f"references/golangci.v2.yml does not enable it — enable it in all three " + f"config copies or stop naming it (advice == tooling)") + + +def validate_fixer_column(): + """Verify the go-idioms **Fixer** column against `go tool fix help` — the authority for + which fixers the floor toolchain's `go fix` ships. Plain fixer names must be registered; + names marked † (x/tools-only) must NOT be — a registered † fixer means the marker went + stale after a toolchain bump. Soft-skips (with a note) when `go` is absent or is not the + floor minor: another minor's list proves nothing about the floor. CI installs Go + {GO_FLOOR_MINOR}.x so the check is strict there.""" + skill = ROOT / "skills" / "go-idioms" / "SKILL.md" + if not skill.is_file(): + return + gobin = shutil.which("go") + if not gobin: + notes.append("Fixer-column check skipped: no `go` on PATH (CI runs it strictly)") + return + try: + ver_out = subprocess.run([gobin, "version"], capture_output=True, text=True, + timeout=30).stdout + except Exception as e: + err(f"`go version` failed: {e}") + return + ver = re.search(r"go(\d+\.\d+)", ver_out) + if not ver or ver.group(1) != GO_FLOOR_MINOR: + notes.append(f"Fixer-column check skipped: toolchain is go{ver.group(1) if ver else '?'}" + f", floor is go{GO_FLOOR_MINOR} (another minor's list proves nothing)") + return + try: + help_out = subprocess.run([gobin, "tool", "fix", "help"], capture_output=True, + text=True, timeout=60) + except Exception as e: + err(f"`go tool fix help` failed: {e}") + return + section = help_out.stdout.split("Registered analyzers:") + if len(section) < 2: + err("`go tool fix help` output has no 'Registered analyzers:' section — " + "cannot verify the go-idioms Fixer column") + return + registered = set(re.findall(r"^\s+([a-z][a-z0-9]*)\b", section[1].split("By default")[0], + re.MULTILINE)) + # Fixer cells are the 4th column of the go-idioms table. + checked = 0 + for line in skill.read_text().splitlines(): + cells = line.split("|") + if len(cells) != 6 or "---" in cells[3] or cells[4].strip() == "Fixer": + continue + cell = cells[4] + daggered = "†" in cell + for name in re.findall(r"`([a-z][a-z0-9]*)`", cell): + checked += 1 + if daggered and name in registered: + err(f"skills/go-idioms/SKILL.md: '{name} †' is stale — " + f"go{ver.group(1)}'s `go fix` registers it; drop the †") + elif not daggered and name not in registered: + err(f"skills/go-idioms/SKILL.md: Fixer column names '{name}' as shipping in " + f"`go fix`, but go{ver.group(1)} `go tool fix help` does not register it — " + f"mark it † (x/tools only) or fix the name") + notes.append(f"Fixer column verified against go{ver.group(1)} `go tool fix help` " + f"({checked} fixer cells)") + + def validate_manifest_paths(manifest: dict, label: str): for field in MANIFEST_PATH_FIELDS: value = manifest.get(field) @@ -203,6 +310,8 @@ def main(): validate_md_components("agents", require_name=True, is_agent=True) validate_md_components("commands", require_name=False) validate_rules() + validate_linter_references() + validate_fixer_column() if __name__ == "__main__": @@ -213,4 +322,6 @@ def main(): print(f" - {e}") sys.exit(1) print("OK: manifests, dual-host parity, component paths, kebab-case names, " - "hook configs, skills, agents, commands, and rules are valid") + "hook configs, skills, agents, commands, rules, and taught-linter references are valid") + for note in notes: + print(f" note: {note}") diff --git a/skills/go-coding/SKILL.md b/skills/go-coding/SKILL.md index 749a9fc..855d334 100644 --- a/skills/go-coding/SKILL.md +++ b/skills/go-coding/SKILL.md @@ -1,6 +1,6 @@ --- name: go-coding -description: Go coding-standards router and entry point for idiomatic Go (Go 1.26, works with 1.25+; golangci-lint v2). This skill should be used when a Go task spans multiple areas, is unspecified, or the question is which tool or standard applies — it routes each topic to the deterministic tool (gofmt/gofumpt, go vet, go fix / golangci-lint v2 modernize, go test -race), to the gopls-lsp plugin for code intelligence, and then to the focused go-* skill that owns it. For a single, already-identified topic prefer that skill directly (errors → go-errors, concurrency → go-concurrency, testing → go-testing, idioms/modernization → go-idioms, linter config → go-linting, layout → go-layout). Not for non-Go languages or domain/business rules. +description: Go coding-standards router for idiomatic Go (Go 1.26.4+; golangci-lint v2). This skill should be used when a Go task spans multiple areas, is unspecified, or the question is which tool or standard applies — it routes each topic to the deterministic tool, then to the focused go-* skill that owns it (go-errors, go-concurrency, go-testing, go-idioms, go-linting, go-layout for layout/naming/API design). For a single, already-identified topic load that skill directly. Not for non-Go languages or domain/business rules. --- # go-coding — Go standards router @@ -19,11 +19,11 @@ Two principles from the project research drive it: |---|---|---| | Formatting | `gofmt -l` / `gofumpt -l` (+ `goimports`) — machine-enforced, non-negotiable | — | | Static analysis / likely bugs | `go vet ./...`, `golangci-lint run` | `go-linting` | -| Modern idioms (range-int, `min`/`max`, `slices`/`maps`, `wg.Go`, `strings.Cut`, `new(expr)`) | `go fix ./...` (Go 1.26 applies the full modernizer suite natively), or `golangci-lint run --enable-only=modernize` on older toolchains | `go-idioms` | -| Errors (`%w`, `errors.Is`/`As`, `errors.Join`, sentinel/typed) | `golangci-lint run --enable-only=errorlint` | `go-errors` | +| Modern idioms (range-int, `min`/`max`, `slices`/`maps`, `wg.Go`, `strings.Cut`, `new(expr)`, `errors.AsType`) | `go fix ./...` (the toolchain's modernizer suite), or `golangci-lint run --enable-only=modernize` for CI reproducibility | `go-idioms` | +| Errors (`%w`, `errors.Is`/`AsType`, `errors.Join`, sentinel/typed, enum dispatch) | `golangci-lint run --enable-only=errorlint,exhaustive` | `go-errors` | | Concurrency (goroutine leaks, ctx lifecycle, atomics) | `go test -race ./...`, `go vet ./...` | `go-concurrency` | -| Testing (table-driven, `t.Parallel`, `B.Loop`, `testing/synctest`) | `go test -race ./...`; use `testing/synctest` (stable since 1.25) for time/concurrency tests | `go-testing` | -| Project layout (`internal/`, start-flat) | judgment — see sources below | `go-layout` | +| Testing (table-driven, `t.Parallel`, `t.Context`, `B.Loop`, `testing/synctest`) | `go test -race ./...`; use `testing/synctest` for time/concurrency tests | `go-testing` | +| Layout, naming & API surface (`internal/`, initialisms, receiver type, in-band errors, doc comments) | `golangci-lint run --enable-only=revive` (`var-naming`, `receiver-naming`, `exported`), `gofmt` for doc-comment layout; the rest is judgment | `go-layout` | | Code intelligence (defs/refs/diagnostics/rename/vulncheck) | install the **`gopls-lsp`** plugin | — | Open the focused `go-*` skill for the topic — it carries the cited rules and the judgment; run the @@ -42,5 +42,8 @@ tool in the middle column to enforce them. Don't invent rules: each skill cites Dispatch the `go-reviewer` agent — a read-only, context-isolated reviewer that applies the review-heuristics catalog and returns severity-ranked findings on a diff or file. +Two user-invoked skills round out the surface: `/go-explain ` for a one-shot idiom lookup, +and `/go-lint-setup` to scaffold the reference golangci-lint v2 config into a repo. + --- *Top-level structure adapted from [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe).* diff --git a/skills/go-concurrency/SKILL.md b/skills/go-concurrency/SKILL.md index dbc2c8b..11ff072 100644 --- a/skills/go-concurrency/SKILL.md +++ b/skills/go-concurrency/SKILL.md @@ -1,15 +1,18 @@ --- name: go-concurrency -description: Idiomatic, leak-free Go concurrency. This skill should be used when the user writes or reviews Go goroutines, channels, `sync`/`atomic`, `context`, `errgroup`, or worker pools — goroutine lifetimes/leaks, context propagation, typed atomics, mutex misuse, or data races. Pair with `go test -race`, `go vet`, and `goleak`. Defers time/concurrency *testing* mechanics to `go-testing` (synctest). Not for non-Go languages. +description: Idiomatic, leak-free Go concurrency. This skill should be used when the user writes or reviews Go goroutines, channels, `sync`/`atomic`, `context`, `errgroup`, or worker pools — goroutine lifetimes/leaks, context propagation, cancellation causes (`context.Cause`, `WithTimeoutCause`), work that must outlive a request (`context.WithoutCancel`, `AfterFunc`), typed atomics, mutex misuse, cleanup/finalizers, or data races. Pair with `go test -race`, `go vet`, and `goleak`. Defers time/concurrency *testing* mechanics to `go-testing` (synctest). Not for non-Go languages. --- # go-concurrency — Go concurrency Deterministic backstop: `go test -race ./...`, `go vet ./...` (catches copylocks, lost cancel), and `go.uber.org/goleak`. The race detector is the source of truth — run it before reasoning. -On **Go 1.26+** the runtime also ships an experimental `goroutineleak` profile in `runtime/pprof` +The runtime also ships an experimental `goroutineleak` profile in `runtime/pprof` (Go 1.26) that reports leaked goroutines — a toolchain-native complement to `goleak` for leak hunts (enable it -with `GOEXPERIMENT=goroutineleakprofile` at build time). +with `GOEXPERIMENT=goroutineleakprofile` at build time). The implementation is production-ready; the +experiment flag is only about API feedback, and it costs nothing unless in use. +*Go 1.27 (draft, expected Aug 2026) enables it by default — no `GOEXPERIMENT`, and +`/debug/pprof/goroutineleak` via `net/http/pprof`.* ## Rules @@ -27,17 +30,37 @@ with `GOEXPERIMENT=goroutineleakprofile` at build time). - **Context discipline:** pass `ctx context.Context` as the first parameter; **never store it in a struct** (`containedctx`); don't reach for `context.Background()` deep in a call stack — thread the caller's ctx. HTTP/SQL/RPC calls must carry it (`noctx`), and set a client timeout. +- **Make cancellation say *why*.** `ctx.Err()` only ever reports `context.Canceled` or + `DeadlineExceeded`, which is useless for diagnosis when several budgets nest. Use + `context.WithCancelCause` + `cancel(err)` (Go 1.20) and read `context.Cause(ctx)`, or + `WithTimeoutCause`/`WithDeadlineCause` (1.21) so the expiring layer names itself. `errors.Is(err, + context.Canceled)` keeps working — the cause rides alongside, it doesn't replace `Err()`. +- **Work that must outlive the request:** `context.WithoutCancel(ctx)` (Go 1.21) drops cancellation + but keeps the values (trace, auth, request ID) — reach for it instead of `context.Background()`, + which throws those away. Give the derived context its own timeout, and tie it to a shutdown path; + "outlives the request" must not mean "outlives the process silently". +- **`context.AfterFunc(ctx, f)`** (Go 1.21) instead of a goroutine whose only job is to `select` on + `ctx.Done()` and clean up; the returned `stop` unregisters it if the work finished first. - **Don't copy `sync.Mutex`/`sync.WaitGroup` by value** (`go vet` copylocks). Guard shared maps — a concurrent map write panics; `-race` catches it. - **Channels:** close on the *send* side, never the receive side; a `nil` channel blocks forever (useful for disabling a `select` arm, a bug everywhere else). +- **Prefer synchronous APIs.** Return the result; let the caller decide to run it in a goroutine. A + function that spawns internally and hands back a channel — or takes a completion callback — + imposes its concurrency model on every caller and hides the goroutine's lifetime, which is exactly + where leaks come from. +- **Cleanup is explicit, never finalized.** Release resources in `Close`/`defer`. `runtime.AddCleanup` + (Go 1.24, preferred over the older `runtime.SetFinalizer`: multiple cleanups per object, works on + interior pointers, no leak on reference cycles) is a backstop for OS/native handles only — a + cleanup may never run, so no correctness may depend on it. - **Testing time/concurrency:** use **`testing/synctest`** (stable since Go 1.25) — fake clock + deterministic scheduling. See `go-testing`. ## Sources - synctest — ; Go 1.25/1.26 release notes — - `goroutineleak` profile (Go 1.26, experimental) — -- Code Review Comments (Goroutine Lifetimes, Contexts) — +- `context` (`Cause`, `WithoutCancel`, `AfterFunc`, `WithTimeoutCause`) — ; `runtime.AddCleanup` — +- Code Review Comments (Goroutine Lifetimes, Contexts, Synchronous Functions) — - Uber Go Style Guide (Concurrency) — --- diff --git a/skills/go-errors/SKILL.md b/skills/go-errors/SKILL.md index 0b37b4a..ffb90cc 100644 --- a/skills/go-errors/SKILL.md +++ b/skills/go-errors/SKILL.md @@ -1,6 +1,6 @@ --- name: go-errors -description: Idiomatic Go error handling. This skill should be used when the user writes, reviews, or debugs Go error code — wrapping with `%w`, inspecting via `errors.Is`/`errors.As`, sentinel vs typed errors, `errors.Join`, or chasing a silently-swallowed or context-losing error. Pair with the `errorlint` linter (set it up via `go-linting`). Not for panics-as-control-flow or non-Go languages. +description: Idiomatic Go error handling. This skill should be used when the user writes, reviews, or debugs Go error code — wrapping with `%w`, inspecting via `errors.Is`/`errors.AsType`, sentinel vs typed errors, `errors.Join`, unchecked `Close` errors, when panic is legitimate, enum-switch dispatch defaults, keeping payload values out of boundary errors/logs, or chasing a silently-swallowed or context-losing error. Pair with the `errorlint` linter (set it up via `go-linting`). Not for non-Go languages. --- # go-errors — Go error handling @@ -13,26 +13,54 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch - **Wrap with `%w` when the caller may need to inspect the cause** (Go 1.13): `return fmt.Errorf("read config %s: %w", path, err)`. Use `%v` *only* to deliberately sever the chain (e.g. to avoid leaking an internal error type across an API boundary) — and say so. + Put `%w` **last** so the message reads outside-in; a leading `%w` is right only when the sentinel + *is* the sentence: `fmt.Errorf("%w: %s", ErrNotFound, key)`. - **The `%v`-where-`%w` trap:** formatting a cause with `%v` discards the chain, so downstream `errors.Is`/`errors.As` silently fail. `errorlint` flags it. -- **Inspect with `errors.Is` (sentinel) / `errors.As` (typed)** — never `err == ErrX` or a type - assertion once any layer wraps, or you get *sentinel breakage* (the comparison silently stops - matching). (Go 1.13; `errors.As` target must be a pointer.) +- **Inspect with `errors.Is` (sentinel) / `errors.AsType[E]` (typed)** — never `err == ErrX` or a + type assertion once any layer wraps, or the result is *sentinel breakage* (the comparison silently + stops matching). `if perr, ok := errors.AsType[*fs.PathError](err); ok { … }` (Go 1.26): the + generic form — compile-time-checked target, no pointer to prepare, no reflection, cannot panic on + a mistyped target. `errors.As` (Go 1.13) is not deprecated and existing call sites are not bugs — + the `errorsastype` modernizer converts them (via golangci-lint's `modernize`; not yet in the + 1.26.4 toolchain's `go fix`). - **Sentinel errors** (`var ErrNotFound = errors.New("not found")`) for expected, comparable - conditions that are part of your API contract — keep the set small and documented. + conditions that are part of the API contract — keep the set small and documented. **Typed errors** (a struct implementing `error`) when callers need fields (`*PathError`). - **`errors.Join(err1, err2)`** (Go 1.20) to aggregate independent failures (cleanup, validation) — replaces manual concatenation and most third-party multierror use. -- **Never swallow:** no `_ = f()` on an error you care about; no empty `if err != nil {}`. Handle, +- **Never swallow:** no `_ = f()` on an error that matters; no empty `if err != nil {}`. Handle, wrap-and-return, or (deliberately, with a comment) ignore. +- **Check `Close` on anything written to.** `defer f.Close()` discards a failed flush — the write + looks successful and the file is truncated. Capture it into a named result: + `defer func() { err = errors.Join(err, f.Close()) }()`. `errcheck` flags the discarded form; + read-only handles are the one safe place to drop it (say so with `_ =`). +- **Keep the happy path at minimal indentation** — handle the error and return early; no `else` + after a terminating `if`. Error flow goes in the indented branch, business logic does not. +- **Don't panic across a package boundary.** Errors are the mechanism for anything a caller can + plausibly hit; panic is for programmer error, API misuse, and genuinely unreachable states. If a + package uses panic internally for unwinding, `recover` it inside that package and return an error + — a panic must never escape into a caller. +- **Fail loudly on impossible dispatch:** a `switch` over an internal enum/kind gets a `default` + that returns an error (panic only for the genuinely unreachable) — never a silent pass-through + that lets a later-added member ride the weakest arm. Pin exhaustiveness with the `exhaustive` + linter (enabled in the reference config) or a completeness test that iterates the enum. +- **Boundary errors carry classification, not payload:** upstream messages can embed data values — + a database driver quoting the offending stored value, a validator echoing the request body. At a + logging or API boundary, pass the stable class/code (e.g. SQLSTATE) and keep the raw message + internal — the message-content analogue of severing an internal error *type* with `%v`. - **Add context at each layer, log once at the boundary.** Wrapping at every level *and* logging at - every level produces duplicate noise — return wrapped, log at the top. + every level produces duplicate noise — return wrapped, log at the top. Keep the added context + terse: `"new store: %w"`, not `"failed to create new store: %w"` — "failed to" states the obvious + and piles up (`failed to x: failed to y: …`) as the error climbs the stack. - **Error strings:** lowercase, no trailing punctuation (they get wrapped): `"cannot parse %q"`. ## Sources -- Go 1.13 errors — -- Code Review Comments (Error Strings, Handle Errors) — +- Go 1.13 errors — ; `errors.AsType` (Go 1.26) — +- Code Review Comments (Error Strings, Handle Errors, Indent Error Flow, Don't Panic) — +- Google Go Style Guide (Error Handling, Panics, `%w` placement) — - Uber Go Style Guide (Errors) — +- `os.File.Close` returns write errors — --- *Decomposition inspired by [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe); rules grounded in the sources above.* diff --git a/skills/go-explain/SKILL.md b/skills/go-explain/SKILL.md index 70422f5..9fdb3e5 100644 --- a/skills/go-explain/SKILL.md +++ b/skills/go-explain/SKILL.md @@ -1,6 +1,6 @@ --- name: go-explain -description: One-shot lookup/explanation of a single Go idiom, standard, or tool. This skill should be used when the user runs `/go-explain ` or asks to "explain", "look up", or "what's the modern way to do" a specific Go construct (e.g. error wrapping, `synctest`, `wg.Go`, `min`/`max`, `internal/` layout) — returning the modern form, the enforcing linter, and a cited source. For applying a standard while writing or reviewing code, use the focused go-* skill (`go-errors`, `go-concurrency`, …). Not for non-Go languages. +description: One-shot lookup/explanation of a single Go idiom, standard, or tool. This skill should be used when the user runs `/go-explain ` or asks to "explain", "look up", or "what's the modern way to do" a specific Go construct (e.g. error wrapping, `synctest`, `wg.Go`, `internal/` layout) — returning the modern form, the enforcing linter, and a cited source. For applying a standard while writing or reviewing code, use the focused go-* skill instead. Not for non-Go languages. argument-hint: a Go topic (e.g. error wrapping, synctest, wg.Go, min/max, internal layout) allowed-tools: Read, Grep, Glob --- @@ -18,9 +18,9 @@ Cover, in a few lines: 4. **Source** — cite one authoritative reference: Effective Go, Go Code Review Comments, the Google or Uber Go style guide, a `go.dev/blog` post, or `pkg.go.dev`. -Tailor to the repo's Go version when a `go.mod` is present — don't recommend a Go 1.26 idiom (e.g. -`new(expr)`) for a module pinned to 1.25 or older. For a fuller treatment, route to the matching -skill: `go-errors`, `go-concurrency`, +Answer against the **Go 1.26.4+** baseline. Name the version an idiom landed in (that's step 1) — +that is provenance for the reader, not a gate on the recommendation. For a fuller treatment, route +to the matching skill: `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, or `go-layout`. Keep it tight — this is a lookup, not a lecture. If `$ARGUMENTS` is empty, ask what to explain. diff --git a/skills/go-idioms/SKILL.md b/skills/go-idioms/SKILL.md index f0e835e..12eb5bb 100644 --- a/skills/go-idioms/SKILL.md +++ b/skills/go-idioms/SKILL.md @@ -1,42 +1,58 @@ --- name: go-idioms -description: Modern idiomatic Go (the `modernize` analyzer set). This skill should be used when the user writes, reviews, or modernizes Go and wants current-version idioms — range-over-int, `min`/`max`, `slices`/`maps`, `strings.Cut`, `cmp.Or`, `sync.OnceFunc`, iterators, `slog.LogAttrs`, `new(expr)` (Go 1.26), dropped loop-var copies. Framed so advice equals tooling (`go fix ./...` on Go 1.26, or `golangci-lint --enable-only=modernize`). Errors→`go-errors`, concurrency→`go-concurrency`, tests→`go-testing`. Not for golangci-lint configuration (use `go-linting`) or non-Go languages. +description: Modern idiomatic Go (the `modernize` analyzer set). This skill should be used when the user writes, reviews, or modernizes Go and wants current-version idioms — range-over-int, `min`/`max`, `slices`/`maps`, `strings.Cut`, `any` over `interface{}`, iterators, `omitzero` json tags, `os.Root`, `new(expr)` and `errors.AsType` (Go 1.26), dropped loop-var copies — or asks which modernize fixer owns a rewrite. Framed so advice equals tooling (`go fix ./...`, or `golangci-lint --enable-only=modernize`). Not for golangci-lint configuration (use `go-linting`) or non-Go languages. --- # go-idioms — modern Go (modernize) -**Advice == tooling.** The `modernize` analyzers flag and usually auto-fix everything below. Run -them, don't hand-audit. As of **Go 1.26** the rewritten `go fix` is the canonical runner — it ships -the full modernizer suite in the toolchain itself: +**Advice == tooling.** The `modernize` analyzers flag and usually auto-fix most of what follows — the +**Fixer** column says which, and the last section covers what no fixer automates. Run the tool, +don't hand-audit. As of **Go 1.26** the rewritten `go fix` is the canonical runner — it ships the +modernizer suite in the toolchain itself: ``` -go fix ./... # Go 1.26+: applies the built-in modernizers -golangci-lint run --enable-only=modernize --fix # any toolchain (same analyzers, via golangci-lint) +go fix -diff ./... # preview the rewrite as a unified diff (clean tree first) +go fix ./... # apply; - runs one, -=false excludes one +golangci-lint run --enable-only=modernize --fix # the x/tools modernize suite — includes the † fixers below ``` -Both draw on the same `golang.org/x/tools` engine as gopls, so their fixes agree. This skill -explains *why* and catches what review notices before the tool runs. **Check `go.mod` first** — -gate each idiom on the module's Go version (the `Since` column below); don't apply a Go 1.26 idiom to -a repo pinned to 1.25 or older. +Both draw on the same `golang.org/x/tools` engine as gopls, but golangci-lint pins its own (usually +newer) snapshot of it — that gap is what the **†** marker below tracks. This skill explains *why* +and catches what review notices before the tool runs. The **baseline is Go 1.26.4+**, so every row +below applies as written — the `Since` column is provenance: it explains why older code looks +different, and what an older module would have to bump to before adopting the idiom. ## Prefer → over (since) -| Prefer | Over | Since | -|---|---|---| -| `new(expr)` — e.g. `Field: new(30)`, `new(int64(req.Limit))` | a `ptr[T](v)` helper (auto-rewritten by the `newexpr` fixer) or a hand-written `tmp := v; &tmp`, for optional/pointer fields | 1.26 | -| `for i := range n` | `for i := 0; i < n; i++` | 1.22 | -| `min(a, b)` / `max(a, b)` builtins | hand-rolled helpers | 1.21 | -| *(drop)* `x := x` loop-var copy | pre-1.22 capture workaround | 1.22 | -| `slices.Sort/Contains/Equal`, `slices.Collect`, `maps.Keys` | hand-rolled sort/contains/dedup | 1.21–1.23 | -| `strings.Cut` / `CutPrefix` / `CutSuffix` | `Index` + manual slicing | 1.18/1.20 | -| `cmp.Or(a, b, …)` | nested `if x == "" { x = y }` | 1.22 | -| `sync.OnceFunc` / `OnceValue` | `sync.Once` + a captured var | 1.21 | -| `iter.Seq[V]` / range-over-func | `Visit(callback)` patterns, exposing slices | 1.23 | -| `slog.LogAttrs(ctx, lvl, msg, attrs…)` on hot paths | key-value variadic `slog` (allocates) | 1.21 | -| `errors.Join` | manual multi-error concat → `go-errors` | 1.20 | -| `wg.Go(...)` | `wg.Add(1)`/`defer wg.Done()` → `go-concurrency` | 1.25 | -| `for b.Loop()` | `for i := 0; i < b.N; i++` → `go-testing` | 1.24 | -| typed `atomic.Int64` | bare-int `atomic.Add*` → `go-concurrency` | 1.19 | +The **Fixer** column names the analyzer that owns each rewrite. Plain = registered in the Go 1.26.4 +toolchain's `go fix` (ground truth: `go tool fix help`; per-fixer docs: `go tool fix help `). +**†** = only in the newer `x/tools` suite so far — golangci-lint's `modernize` and gopls run it, the +1.26.4 toolchain's `go fix` does not. `—` = no fixer exists: review has to catch it. + +| Prefer | Over | Since | Fixer | +|---|---|---|---| +| `new(expr)` — e.g. `Field: new(30)`, `new(int64(req.Limit))` | a `ptr[T](v)` helper or a hand-written `tmp := v; &tmp`, for optional/pointer fields | 1.26 | `newexpr` | +| `errors.AsType[E](err)` | `errors.As(err, &target)` → `go-errors` | 1.26 | `errorsastype` † | +| `for i := range n` | `for i := 0; i < n; i++` | 1.22 | `rangeint` | +| `min(a, b)` / `max(a, b)` builtins | hand-rolled helpers | 1.21 | `minmax` | +| *(drop)* `x := x` loop-var copy | pre-1.22 capture workaround | 1.22 | `forvar` | +| `any` | `interface{}` | 1.18 | `any` | +| `slices.Sort/Contains`, `slices.Collect`, `maps.Keys` | hand-rolled sort/contains/map loops | 1.21–1.23 | `slicescontains`, `slicessort`, `mapsloop` | +| `for i, v := range slices.Backward(s)` | `for i := len(s)-1; i >= 0; i--` | 1.23 | `slicesbackward` † | +| `strings.Cut` / `CutPrefix` / `CutSuffix` | `Index` + manual slicing | 1.18/1.20 | `stringscut`, `stringscutprefix` | +| `strings.SplitSeq` / `FieldsSeq` | ranging over `strings.Split`/`Fields` (allocates a slice) | 1.24 | `stringsseq` | +| `fmt.Appendf(b, …)` | `append(b, fmt.Sprintf(…)...)` / `[]byte(fmt.Sprintf(…))` | 1.19 | `fmtappendf` | +| `omitzero` on a struct-typed json field | `omitempty`, which does **nothing** for struct fields — a zero `time.Time` still marshals | 1.24 | `omitzero` | +| `t.Context()` in tests | `context.WithCancel(context.Background())` → `go-testing` | 1.24 | `testingcontext` | +| `reflect.TypeFor[T]()` | `reflect.TypeOf((*T)(nil)).Elem()` | 1.22 | `reflecttypefor` | +| `cmp.Or(a, b, …)` | nested `if x == "" { x = y }` | 1.22 | — | +| `sync.OnceFunc` / `OnceValue` | `sync.Once` + a captured var | 1.21 | — | +| `iter.Seq[V]` / range-over-func | `Visit(callback)` patterns, exposing slices | 1.23 | `stditerators` | +| `slog.LogAttrs(ctx, lvl, msg, attrs…)` on hot paths | key-value variadic `slog` (allocates) | 1.21 | — | +| `errors.Join` | manual multi-error concat → `go-errors` | 1.20 | — | +| `wg.Go(...)` | `wg.Add(1)`/`defer wg.Done()` → `go-concurrency` | 1.25 | `waitgroup` | +| `for b.Loop()` | `for i := 0; i < b.N; i++` → `go-testing` | 1.24 | `bloop` † | +| typed `atomic.Int64` | bare-int `atomic.Add*` → `go-concurrency` | 1.19 | `atomictypes` † | Idioms are a moving target — let the tool (pinned to the repo's toolchain) be the source of truth so advice never drifts from the user's `go fix`. Go 1.26 also lifts the ban on a generic type @@ -44,10 +60,30 @@ referencing itself in its own type-parameter list (e.g. `type Adder[A Adder[A]] so self-referential constraints no longer need a workaround — but that's a hand-written pattern, not something a modernizer rewrites. +## Modern, but no fixer automates it + +- **`os.OpenRoot(dir)` → `*os.Root`** (1.24) for anything that opens a caller-supplied path: its + methods cannot escape the directory, including via symlink. Replaces `filepath.Join` plus + hand-written `..`/prefix checks — the traversal-bug pattern those checks keep getting wrong. +- **`rand.Text()`** from `crypto/rand` (1.24) for tokens, nonces, and IDs — not `math/rand`, and not + a hand-rolled base64 of `rand.Read`. Security-sensitive randomness always comes from `crypto/rand`. +- **`var s []T`, not `s := []T{}`** — the nil slice is the idiomatic empty slice (append works, + `len` is 0). Reach for the non-nil literal only when something genuinely distinguishes them + (e.g. marshalling `[]` vs `null`). +- **`slices.Sorted(maps.Keys(m))`** (1.23) when iterating a map for output — map order is random, and + unstable output is a flaky-test and noisy-diff source. + +*Go 1.27 (draft, expected Aug 2026) graduates several † fixers into the toolchain's `go fix` +(`atomictypes`, `slicesbackward`, plus new `embedlit` and `unsafefuncs`), renames `waitgroup` → +`waitgroupgo`, and drops `fmtappendf`; it also lands `encoding/json/v2` + `encoding/json/jsontext` +(v1 is reimplemented on v2, opt out with `GOEXPERIMENT=nojsonv2`), `strings.CutLast`/`bytes.CutLast`, +and a stdlib `uuid` package.* + ## Sources -- `modernize` — +- `modernize` (per-fixer docs, the Fixer column) — - `go fix` (rewritten in 1.26) — ; range-over-func — - `slog` — ; Go 1.21–1.26 release notes (`new(expr)`, self-ref generics — ) +- `os.Root` / `omitzero` / `rand.Text` — ; Code Review Comments (Declaring Empty Slices, Crypto Rand) — --- *Decomposition inspired by [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe); rules grounded in the sources above.* diff --git a/skills/go-layout/SKILL.md b/skills/go-layout/SKILL.md index aec2893..52095aa 100644 --- a/skills/go-layout/SKILL.md +++ b/skills/go-layout/SKILL.md @@ -1,35 +1,97 @@ --- name: go-layout -description: Go project layout and package design. This skill should be used when the user structures a Go module or names packages — `internal/`, `cmd/`, start-flat-then-grow, package naming, avoiding `util`/`common` grab-bags, or deciding whether hexagonal/DDD ceremony is warranted. Counters imported Java/C# structure. Not for build tooling or for non-layout idioms (→ `go-idioms`). +description: Go project layout, naming, and API-surface design. This skill should be used when the user structures a Go module, names things, or shapes an exported API — `internal/`, `cmd/`, start-flat-then-grow, package/variable/receiver naming, initialism casing (`userID`, `HTTPServer`), pointer vs value receivers, in-band errors, named results, option structs vs variadic options, returning concrete types, or writing doc comments. Counters imported Java/C# structure. Not for build tooling or non-layout idioms (→ `go-idioms`). --- -# go-layout — project & package structure +# go-layout — layout, naming & API surface -The Go community consensus is *minimalism*; resist imported ceremony. +The Go community consensus is *minimalism*; resist imported ceremony. Naming and the shape of an +exported signature are part of the API — they are as reviewable as the code. -## Rules +## Layout - **`internal/` is the one true consensus.** Packages under `internal/` cannot be imported from outside the module subtree — use it to keep implementation private while exporting a small surface. - **Start flat; grow as needed.** A new module is often one package at the root. Add - `cmd//main.go` when you have multiple binaries and `internal//` when you need privacy + `cmd//main.go` when there are multiple binaries and `internal//` when privacy is needed — not before. `golang-standards/project-layout` is community-made, **explicitly not official and contested**; don't treat its deep tree as a starting requirement. -- **Package names are part of the API:** short, lowercase, single word, no underscores or - camelCase. The caller writes `chi.NewRouter()`, so don't stutter (`chi.ChiRouter`). Avoid - `util`, `common`, `helpers`, `base` grab-bags — name by what the package *provides*. - **No Java/C# transplants:** no `*Manager`/`*Impl`/`*Factory` reflexes, no one-type-per-file rule, - no interface-for-everything. Define interfaces *where they're consumed*, keep them small, and - return concrete types. + no interface-for-everything. - **Hexagonal / ports-and-adapters / DDD is a tool, not a default** — justified for larger services with real external-boundary complexity, overkill for a CLI or a small service. +- **`main` owns process exit.** Call `os.Exit`/`log.Fatal` only in `main` (ideally once, on the + error from a `run() error` function); everything else returns errors. A deep `log.Fatal` skips + deferred cleanup and makes the code path untestable. Same discipline for `init()`: only cheap, + deterministic setup — no I/O, no environment reads, no mutating global state; anything more is an + explicit constructor called from `main`. - **Files:** one package per directory; `package foo` for `foo.go` + `foo_test.go`; use `package foo_test` for black-box tests that exercise only the exported API. +## Naming + +- **Package names are part of the call site:** short, lowercase, single word, no underscores or + camelCase. The caller writes `chi.NewRouter()`, so don't stutter (`chi.ChiRouter`, + `bytes.BufferWrite`). Avoid `util`, `common`, `helpers`, `base` grab-bags — name by what the + package *provides*. +- **`MixedCaps`, never `MAX_LENGTH` or `snake_case`** — including constants, whatever the convention + was in the language this code came from. +- **Initialisms keep a single case throughout:** `userID`, `parseURL`, `HTTPServer`, `ServeHTTP` — + never `userId`, `HttpServer`. Mixed casing within one identifier is the tell of a translated name. +- **Name length tracks scope.** `i`, `r`, `buf` are correct in a five-line body; anything + package-level, long-lived, or used far from its declaration earns a descriptive name. Longer is not + better — `idx` beats `theCurrentIndexIntoTheSlice`. +- **Receiver names are a one- or two-letter abbreviation of the type** (`c *Client`, `srv *Server`), + identical across every method on that type. Never `self`, `this`, or `me`. +- **No `Get` prefix on accessors:** `u.Name()`, paired with `u.SetName(…)`. A verb-like name is for + something that acts; a noun-like name for something that returns a value. +- **Test doubles live in a `test` package** and are named for behaviour, not mechanism — + `AlwaysDeclines`, not `MockCardProcessorImpl2`. + +## Signatures & API surface + +- **Receiver type:** pointer when the method mutates, when the receiver is large, or when the type + holds a `sync` field (copying a lock is a bug — `go vet` copylocks). Value receivers for small + immutable types. **Be consistent within a type** — don't mix pointer and value receivers. +- **Pass small fixed-size values directly.** `*int` to "avoid a copy" trades a machine word for an + indirection plus aliasing risk. +- **No in-band errors.** Return `(T, error)` or `(T, bool)` — never `-1`, `""`, or a `nil` that means + failure. A caller can forget to compare against a magic value; a second return value is harder to + ignore, and `errcheck` sees it. +- **Named results only when they add information** the types don't (`(n int, err error)`), or when a + deferred closure must assign to them (the `Close`-into-`err` idiom in `go-errors`). Bare `return` + belongs only in short functions. +- **Two option styles, chosen by how often callers pass options:** an **option struct** as the final + parameter when most callers set at least one field (self-documenting, grows compatibly); **variadic + functional options** when most callers pass none. Don't erect a functional-options framework around + two booleans. +- **Accept interfaces, return concrete types.** Define an interface in the package that *consumes* + it, keep it to a method or three, and return the concrete type so callers get the full surface and + new methods don't break them. +- **Prefer synchronous signatures** — let the caller add concurrency (→ `go-concurrency`). +- **Make the zero value useful where possible** (`bytes.Buffer`, `sync.Mutex` need no constructor). If + a type genuinely requires a `New…`, the doc comment must say so. + +## Doc comments + +- **Every exported identifier gets one, as a full sentence starting with the name:** + `// Serve accepts incoming connections on the listener.` That phrasing is what makes `go doc` + output and grep both work. +- **Package comment sits directly above `package x`** with no blank line, exactly one per package, + opening `// Package x …`. +- **`gofmt` formats doc comments** (since Go 1.19) — lists, headings, indented code blocks, and + `[Name]`/`[pkg.Name]` doc links. Write that syntax and let the tool lay it out. +- **Document what the signature can't say:** concurrency safety, who owns and must close a returned + resource, whether cancellation leaves partial work behind, and which errors callers can branch on. + Don't restate parameter names. +- **Retire an exported name with a `Deprecated:` paragraph**, not by deleting it. + ## Sources - Effective Go — -- Code Review Comments (Package Names, Interfaces) — -- Google Go Style Guide — ; `internal/` — +- Code Review Comments (Package/Variable/Receiver Names, Initialisms, Mixed Caps, In-Band Errors, Named Result Parameters, Pass Values, Interfaces, Doc Comments) — +- Google Go Style Guide (naming, option structs, documentation, test doubles, program initialization) — +- Uber Go Style Guide (Exit in Main, Avoid init()) — +- Doc comment syntax — ; `internal/` — --- *Decomposition inspired by [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe); rules grounded in the sources above.* diff --git a/skills/go-lint-setup/SKILL.md b/skills/go-lint-setup/SKILL.md index 3e8fb2b..8957a8b 100644 --- a/skills/go-lint-setup/SKILL.md +++ b/skills/go-lint-setup/SKILL.md @@ -16,11 +16,13 @@ Steps: `.golangci.json`. If one exists, do **not** overwrite it: show how it differs from the reference and ask before changing anything. If it's a **v1** config (no `version` key and/or an `enable-all`/top-level `linters:` list), warn that v1 will not parse under golangci-lint v2 and - offer to migrate. + offer to migrate — the supported path is `golangci-lint migrate` (in-place, keeps a `.bck` backup, + drops comments), not a hand-port. 2. **Write** the config below to `.golangci.yml` (or the path given in `$ARGUMENTS`). 3. **Report how to run it:** `golangci-lint run`, and `golangci-lint run --fix` for the auto-fixable - findings (`modernize` + the formatters). Suggest pinning the `golangci-lint` version in CI so the - rule set is reproducible. + findings (`modernize` + the formatters). Suggest pinning an exact `golangci-lint` version in CI in + one place (the action's `version:` input) with an automated bump PR — see `go-linting`; don't + invent a version number here, point at the releases page. Config to write (mirrors `references/golangci.v2.yml` — keep the two in sync): @@ -31,6 +33,7 @@ linters: enable: - modernize - errorlint + - exhaustive - bodyclose - rowserrcheck - sqlclosecheck @@ -38,6 +41,8 @@ linters: - contextcheck - containedctx - perfsprint + - usetesting + - nolintlint - revive formatters: enable: diff --git a/skills/go-linting/SKILL.md b/skills/go-linting/SKILL.md index 887b95e..3e9e970 100644 --- a/skills/go-linting/SKILL.md +++ b/skills/go-linting/SKILL.md @@ -1,6 +1,6 @@ --- name: go-linting -description: golangci-lint v2 setup and adoption for Go. This skill should be used when the user configures, upgrades, or debugs Go linting — the `.golangci.yml` file, the golangci-lint v2 schema (the versioned config, the `linters.default` set, the separate formatters section), the `modernize` linter, or stack linters (errorlint, bodyclose, rowserrcheck, sqlclosecheck, noctx, contextcheck). Not for what individual idioms mean (use the `go-idioms` skill) or writing rules by hand. +description: golangci-lint v2 setup and adoption for Go. This skill should be used when the user configures, upgrades, or debugs Go linting — the `.golangci.yml` file, the v2 schema (versioned config, `linters.default` set, separate formatters section, `linters.exclusions`), migrating a v1 config, `golangci-lint fmt`, suppressing a finding with `//nolint`, the `modernize` linter, or stack linters (errorlint, bodyclose, noctx, usetesting, …). Not for what individual idioms mean (use `go-idioms`) or writing rules by hand. --- # go-linting — golangci-lint v2 @@ -14,7 +14,13 @@ plugin. Its schema changed from v1 — **v1 config will not parse**: - `linters.default: standard | all | none | fast` selects the base set (no more `enable-all`). `standard` = errcheck, govet, ineffassign, staticcheck, unused. - **Formatters moved to their own `formatters:` section** (gofmt/gofumpt/goimports are no longer - "linters"). + "linters"), with their settings under `formatters.settings`. `golangci-lint fmt` runs that section. +- **Exclusions moved under `linters`**: v1's `issues.exclude-rules` → `linters.exclusions.rules`, + and `issues.exclude-dirs`/`exclude-files` → `linters.exclusions.paths`. `linters-settings` split + into `linters.settings` + `formatters.settings`. +- **Don't hand-port a v1 config — run `golangci-lint migrate`.** It rewrites in place, keeps a + `.golangci.bck.yml` backup, and takes `--format {yml,yaml,toml,json}`. It drops comments and + unknown/deprecated keys, so re-add comments and diff the result. ## Reference config (v2) @@ -25,6 +31,7 @@ linters: enable: - modernize # highest value: range-int, min/max, slices/maps, wg.Go, strings.Cut… - errorlint # %w + errors.Is/As discipline + - exhaustive # a switch over an enum names every member - bodyclose # unclosed http.Response.Body - rowserrcheck # unchecked sql.Rows.Err - sqlclosecheck # unclosed sql.Rows/Stmt @@ -32,6 +39,8 @@ linters: - contextcheck # context not propagated - containedctx # context.Context stored in a struct - perfsprint # fmt.Sprintf where a cheaper call exists + - usetesting # os.Setenv/os.Chdir/context.Background in tests → the t.* forms + - nolintlint # a //nolint must name a linter and carry a reason - revive # configurable golint successor formatters: enable: [gofumpt, goimports] @@ -40,21 +49,37 @@ formatters: ## Adoption - Run: `golangci-lint run`; auto-fix what's fixable (incl. `modernize` and formatters): - `golangci-lint run --fix`. -- **Pin the version in CI** for reproducibility (the Cadasto Go repos pin `v2.11.4`); a version - drift silently changes the rule set. + `golangci-lint run --fix`. `golangci-lint fmt` applies only the `formatters:` section. +- **Pin an exact version in CI, in exactly one place — and keep the pin moving.** Upstream's own + recommendation is a specific release, not `latest`: a new release can add or retune linters and + turn every build red at once, with no code change to blame. Put the version in a single source of + truth — the `golangci/golangci-lint-action` `version:` input (it also caches, and beats a plain + binary install) or the install script's tag — never copied across several workflows and Makefiles. + Then let Renovate/Dependabot raise the bump as its own PR, so the version stays current *and* + every rule-set change arrives reviewable. This skill deliberately names no blessed version; read + the changelog for the current line. +- **Install the release binary, not from source.** Upstream states that `go install`/`go get`, the + tools pattern, and `tool` directives "aren't guaranteed to work" — they compile golangci-lint with + whatever local Go version is around. Use the binary, the action, or the Docker image, from a + release built with Go ≥ the module's toolchain (1.26+) so it can parse the language version. +- **Bumping the pin:** run `--fix` first, then either land the leftover findings or add an explicit + `linters.exclusions.rules` entry with a reason. If the pinned build rejects a linter name from the + reference config, the pin is too old — bump it rather than deleting the linter. +- **Suppress narrowly, and say why.** `//nolint:errcheck // best-effort close on a read-only handle` + — never a bare `//nolint` (it disables every linter on that line) and never a blanket + file-level disable where a `linters.exclusions.rules` entry with a path pattern is the honest + answer. `nolintlint` enforces the specific-and-explained form. - `modernize` is the single highest-leverage linter — it operationalizes most `go-idioms` rules on the same engine as gopls/`go fix`, so the plugin's advice stays consistent with the toolchain. As of **Go 1.26** the rewritten `go fix ./...` runs that same modernizer suite from the toolchain - itself; keep `modernize` in golangci-lint for CI reproducibility and to cover toolchains older - than 1.26. Use golangci-lint built with a Go version ≥ the module's toolchain (Go 1.26 support - landed in the golangci-lint releases built with 1.26). + itself; keep `modernize` in golangci-lint so CI enforces it reproducibly against the pinned + version rather than whatever toolchain a developer happens to have. - Adopt the shipped reference config `references/golangci.v2.yml`, or run `/go-lint-setup` to scaffold it into a repo (it won't overwrite an existing config without asking). ## Sources -- golangci-lint docs — -- v2 migration — +- golangci-lint docs — ; v1→v2 migration guide (`migrate`, key moves) — +- v2 announcement (`fmt`, `formatters`) — - `modernize` — --- diff --git a/skills/go-testing/SKILL.md b/skills/go-testing/SKILL.md index a83458b..b71bab7 100644 --- a/skills/go-testing/SKILL.md +++ b/skills/go-testing/SKILL.md @@ -1,6 +1,6 @@ --- name: go-testing -description: Idiomatic Go testing. This skill should be used when the user writes or reviews Go tests, benchmarks, or fuzz targets — table-driven tests, `t.Parallel`, `testing.B.Loop`, the race detector, goroutine-leak detection, `testing/synctest` for time/concurrency, fuzzing, or golden files. Pair with `go test -race`. Not for non-Go test frameworks; error-wrapping belongs to `go-errors`. +description: Idiomatic Go testing. This skill should be used when the user writes or reviews Go tests, benchmarks, or fuzz targets — table-driven tests, `t.Parallel` (and what cannot run under it), `t.Context`, `t.Chdir`/`t.Setenv`, `t.TempDir` vs `t.ArtifactDir`, `t.Output`, `testing.B.Loop`, the race detector, goroutine-leak detection, `testing/synctest` for time/concurrency, fuzzing, golden files, or writing failure messages that actually diagnose. Pair with `go test -race`. Not for non-Go test frameworks; error-wrapping belongs to `go-errors`. --- # go-testing — Go testing @@ -13,6 +13,24 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, 1.22 the `tc := tc` copy is unnecessary — drop it (`modernize`/`copyloopvar` flag it). - **`t.Parallel()`** on independent tests to cut wall-clock; watch for shared mutable state and loop-var capture in the parallel body. +- **Process-global helpers are incompatible with `t.Parallel()`** — `t.Setenv` (Go 1.17), `t.Chdir` + (1.24), and `cryptotest.SetGlobalRandom` (1.26) all mutate process state, so they fail in a + parallel test *or one with a parallel ancestor*. A table whose cases need env or cwd stays serial; + pass config explicitly instead where possible. The `usetesting` linter pushes `os.Setenv`/`os.Chdir` + in tests towards the `t.*` forms (which restore state via `Cleanup`). +- **`t.Context()`** (Go 1.24) for any test needing a `ctx` — it is cancelled just before the test's + `Cleanup` functions run, so goroutines under test shut down before teardown asserts on them. Use + it over `context.Background()`; the `testingcontext` modernizer rewrites the old form. Do *not* + use it for a fixture whose lifetime spans tests (a shared server or container started in + `TestMain`) — that needs its own context. +- **`t.TempDir()` for scratch, `t.ArtifactDir()` (Go 1.26) for evidence.** `TempDir` is removed at + test end; `ArtifactDir` gives each test a unique directory for output files worth keeping — + rendered output, protocol dumps, failure snapshots — retained when `go test -artifacts` is passed. + Don't hand-roll paths under `os.TempDir()`. +- **`t.Output()` (Go 1.25) is an `io.Writer` into the test log** — wire a `slog` handler or a + subprocess's stdout into it so output interleaves correctly with `t.Log` under `-race` and + parallel tests, instead of `fmt.Println` escaping to raw stdout. `t.Attr` (1.25) emits structured + key/value metadata into `go test -json` output. - **Benchmarks: `for b.Loop() { … }`** (Go 1.24) — it handles timer reset and run scaling; replaces `for i := 0; i < b.N; i++` plus manual `b.ResetTimer()`. - **`-race` is non-negotiable** for any code touching goroutines; wire it into CI. @@ -22,20 +40,32 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, tickers, retries, `context` cancellation. It runs the bubble on a *fake clock* with deterministic scheduling, so "5-second" waits complete in microseconds and flakiness disappears. Wrap with `synctest.Test(t, func(t *testing.T){ … })`; `synctest.Wait()` blocks until every goroutine in the - bubble is durably blocked. Reach for it instead of `time.Sleep`-based polling. (The pre-1.25 - `GOEXPERIMENT=synctest` API — `synctest.Run` — was removed in Go 1.26; use the stable `synctest.Test`.) + bubble is durably blocked. Reach for it instead of `time.Sleep`-based polling. (Always + `synctest.Test` — the pre-stable `synctest.Run` no longer exists.) + *Go 1.27 (draft, expected Aug 2026) adds `synctest.Sleep` (`time.Sleep` + `Wait` in one) and + `httptest.NewTestServer`, an in-memory server usable inside a bubble.* - **Fuzzing** (`func FuzzX(f *testing.F)`) for parsers, codecs, and anything consuming untrusted bytes. **Golden files** (an `-update` flag writing `testdata/*.golden`) for large structured output. + A golden pins *shape*, not behaviour — when it records something another system executes (SQL, + wire requests, rendered configs), pair it with at least one test that executes the artefact for + real; a snapshot can be stable and wrong. - **Deterministic crypto tests (Go 1.26):** `testing/cryptotest.SetGlobalRandom(t, seed)` pins a deterministic randomness source for the test's duration — reach for it instead of hand-injecting a custom `io.Reader` when testing code that draws from `crypto/rand`. It's process-global, so it can't run inside a `t.Parallel()` test (or one with a parallel ancestor). -- **Assertions:** stdlib + small helpers (`t.Helper()`) is often enough; `testify` is fine — match - the repo, don't mix styles. +- **Failure messages must diagnose without a debugger:** name the call, the input, the result, and + the expectation — `t.Errorf("Parse(%q) = %v, want %v", in, got, want)` — never a bare + `t.Error("failed")`. For structs and slices print a diff (`cmp.Diff(want, got)`), not two blobs. +- **Helpers set up; the test body asserts.** Call `t.Helper()` so a failure points at the caller's + line, and prefer a helper that *returns* a value or `error` over one that fails internally — + assertion logic belongs where the case's context is visible. `t.Fatal` in a setup helper is fine; + in a goroutine use `t.Error` (only the test's own goroutine may call `Fatal`). Stdlib plus small + helpers is usually enough; `testify` is fine — match the repo, don't mix styles. ## Sources - synctest — ; `testing.B.Loop` — -- `testing` package — ; Go 1.22/1.24/1.25/1.26 release notes — +- `testing` package (`T.Context`, `T.Chdir`, `T.Output`, `T.Attr`, `T.ArtifactDir`) — ; Go 1.22/1.24/1.25/1.26 release notes — +- Code Review Comments (Useful Test Failures) — ; Google Go Style Guide (Tests) — - `testing/cryptotest` (Go 1.26) — - `go.uber.org/goleak` —