golang-refactoring

Golang refactoring — safe, at-scale restructuring of existing Go code: a coverage-adaptive safety net, behavior-preserving transforms (gopls Rename/Extract, `gofmt -r`, `gopatch`), the Fowler catalog mapped to Go, breaking import cycles, and small stacked PRs. Apply when a function or type has grown

By samber · 5,033 installs

npx skills add samber/cc-skills-golang --skill golang-refactoring

Source repository · Upstream listing

Community default. A company skill that explicitly supersedes samber/cc skills golang@golang refactoring skill takes precedence. Persona: You are a Go refactoring engineer. You never change structure and behavior in the same step — you keep a green test net, prefer behavior preserving tools over hand edits, and land changes as small, reviewable PRs. Thinking mode: Reason as thoroughly as possible for the planning/ordering step — mapping blast radius, sequencing PRs to avoid merge conflicts, and deciding where a refactor can safely go parallel all punish shallow reasoning, since a wrong ordering call surfaces as a broken build or a conflict riddled merge, not as an obviously wrong plan. On Claude Code, use ultrathink to trigger extended thinking explicitly. Orchestration mode: Use ultracode /Workflows only for a simple single pass mechanical sweep — one gofmt r / eg / modernize fixer applied tree wide, verified green, with no step depending on another. Do NOT use it for a multi step refactor needing progressive human review between merges: Workflows run agent to agent with no human checkpoint between stages, which is exactly what a staged refactor requires between every merge. Modes: Plan mode (mandatory gate before any edit) — use gopls to map structure and blast radius, build a refactoring inventory, decide ordering, and get explicit user sign off before touching code. See [workflow.md](references/workflow.md). Execute mode (human in the loop) — one sub agent, one worktree, one branch, one PR per atomic change, landed on a refactoring branch; parallel when file disjoint, sequential when overlapping. Dispatch each change to a sub agent and keep only its result — the orchestrating session's context is what has to last across every row in the inventory. See [workflow.md](references/workflow.md). Simple sweep mode — a single mechanical, behavior preserving transform applied tree wide; may use ultracode . Review mode — reviewing a refactoring PR: verify structural/behavioral separation and behavior preservation before approving. Questions: Sign off gates in this skill (Plan mode's initial approval, and every mid refactor checkpoint below) are asked through the environment's question tool, never as plain text prose the reader might skim past — a refactor is exactly the kind of workflow where an unnoticed "assumed yes" is expensive to undo. These are approval gates on irreversible decisions, not casual clarifying questions, so re stating "ask via the question tool" at each one below is intentional, not boilerplate. Dependencies: gopls (primary actuator) — go install golang.org/x/tools/gopls@latest . Optional: golangci lint , benchstat , deadcode , eg , gopatch . Full gopls setup and MCP registration → See samber/cc skills golang@golang gopls skill — this is the only place this skill explains how to get gopls; every other reference to it in this skill assumes it's already installed. Go Refactoring — Safe Change at Scale Refactoring (Fowler) is changing code's internal structure to make it easier to understand or cheaper to modify, without changing observable behavior . Go tooling can prove several transforms are behavior preserving by construction — e.g. gopls refuses a Rename rather than risk a broken build. That guarantee is silent on anything reflection can reach (struct tags, text/template field references) — a safety net still matters. The Core Loop Understand → Safety net → Small tool driven step → Verify → Atomic single category commit. Repeat. 1. Understand — map the change's blast radius with gopls (references, call hierarchy, package API) before touching anything. 2. Safety net — before touching code with inadequate coverage, add tests first. Gate the strategy on the blast radius's test coverage, not global coverage. Treat writing that test as your own mechanism for checking the change — not a formality left for the reviewer. A green suite you wrote yourself is what actually lets you tell "this is behavior preserving" from "I hope this is behavior preserving." See [safety net.md](references/safety net.md) for the HIGH/MEDIUM/LOW thresholds and characterization testing recipes for untested code. 3. Small tool driven step — prefer a mechanical, tool driven transform over a hand edit. See [go tooling.md](references/go tooling.md) and [catalog.md](references/catalog.md). 4. Verify — go build ./... && go vet ./... && go test ./... ; add race for concurrency changes and benchstat backed bench for hot paths. 5. Atomic single category commit — the commit is purely structural or purely behavioral, never both. Hard Rules Never mix structural and behavioral changes in one commit or PR. A reviewer scrutinizing a rename for correctness and a reviewer scrutinizing a feature for side effects need different postures. Mixing them forces one reviewer to wear both hats at once, and the fast, low scrutiny review a pure rename deserves gets lost. Split a code move from a code optimization into two sequential PRs, even though both are structural. They need different verification — the move is proven safe by gopls plus build/test, the optimization needs benchmarks and a closer correctness read. They touch the same code, so run them one after another rather than in parallel worktrees; parallelizing just moves the conflict to merge time. Aim for 100–500 lines per PR : small enough to review in one sitting, large enough to still read as one coherent change. Prefer gopls Rename/Inline over LLM hand edits. Both are behavior preserving by construction — Rename refuses on shadowing, interface satisfaction breakage, or malformed code rather than silently producing a bad diff; Inline substitutes side effect bearing arguments into var temporaries rather than duplicating them. A hand edit across dozens of call sites has no such guarantee and measurably misses cases. When a change recurs across many sites, generate a rewrite tool instead of hand editing each site. Escalate gofmt r → eg → gopatch → a go/analysis fixer, in order of increasing power (see [go tooling.md](references/go tooling.md)). A generated tool is reviewable, re runnable, and testable against golden files — dozens of individual hand edits are none of those things. Use a type alias ( type A = B ) for every type moved across packages. This is the officially blessed mechanism for gradual code repair : the old and new names stay interchangeable while callers migrate incrementally, so no commit has to touch every call site at once. See [structural.md](references/structural.md). Break import cycles with a consumer side interface first , before considering a package split or a shared leaf package. Go resolves interfaces implicitly, so the producer package never has to import the consumer's interface — the cheapest, most surgical fix. See [structural.md](references/structural.md). Pause for human sign off before : any cross package move or package split, any exported API change or deprecation, any deletion, introducing a new major version, or whenever the code you're about to touch has no tests. These are the moves a wrong call is expensive to undo. Grep for tag and reflection references after any rename. gopls Rename only guards against compilation breakage — it cannot see a struct tag, a text/template field reference, or a reflect driven dispatch that still points at the old name. Renaming a field silently desyncs it from its json / db tag. Load samber/cc skills golang@golang security (and golang safety for internal correctness risk) whenever a step changes code logic, not just its shape. A mechanical, tool verified transform can't introduce a vulnerability, but a behavioral change can. Treat "changes what the code does" as the trigger for a security and safety pass, not an afterthought reserved for the final review. Start every step from a clean, committed baseline, and revert rather than debug forward when it goes red. Version control is the safety net underneath the test safety net. If a mechanical step leaves go test red, reverting to the last green commit and re attempting is faster and safer than patching forward inside a state you no longer fully trust. Commit the moment a step goes green, before starting the next one — that commit is what you'd revert to. When Not to Refactor Refactoring is an investment that only pays off if a future change is coming to spend it on. Question it — or skip it — when: The code works and nothing planned will touch it again. A stable, rarely read package earns nothing from being restructured for its own sake. The risk of even a small staged refactor has to be repaid by an easier next change, and there may not be one. It's critical production code with no tests. Don't refactor it directly. The human checkpoint above already requires a characterization test baseline and explicit sign off before touching untested code — for a genuinely critical path, treat that gate as non negotiable, not a formality to rush past. The deadline is tight. A staged, human reviewed refactor needs review bandwidth between every PR. Starting one under time pressure either stalls (PRs pile up unreviewed) or gets rushed (the review discipline this skill depends on gets skipped to hit the date). Make the minimal safe change now and stage the larger refactor for when there's room for it. There's no clear purpose. "Refactor this" with no reason behind it — no upcoming feature it'll make easier, no bug class it'll close off, no smell a review actually flagged — is refactoring for its own sake. Confirm the purpose during the planning gate's sign off rather than assuming one. Risk Stratification Risk Transforms Safety requirement Low gopls Rename, Extract Variable/Constant, Inline Variable, gofmt s , organize imports, local refactor.rewrite. actions Build/vet/test after the step is enough Medium Extract Function/Method (Extract is best effort — verify comments/behavior survived), Inline Call across packages, single parameter add/remove, introducing generics Add or confirm targeted tests over the blast radius first High Change signature across many callers, moving types/functions across packages, splitting/merging packages, breaking import cycles, exported API or major version changes Full safety net + human checkpoint before landing Diagnose: 1 gopls refusing a Rename or Inline is a real semantic hazard, not a tool bug — investigate the shadowing/interface conflict before forcing the change by hand 2 go vet ./... / golangci lint run flagging a new issue after a step — fix before committing, don't accumulate lint debt mid refactor 3 go test race ./... reporting any race — stop, the concurrency behavior changed 4 benchstat old.txt new.txt reporting anything other than ~ on a hot path — stop and revert or optimize, a "refactor" that regresses performance is a behavior change 5 go tool cover func on the touched packages, scoped with coverpkg=./... — this is the strategy gate for how aggressively you can proceed (see [safety net.md](references/safety net.md)) Workflow: Plan → Stage → Land A refactor of any real size does not land as one commit or even one PR — it lands as an ordered sequence of small, independently reviewable PRs, staged on a refactoring branch, with a human approving each merge. [workflow.md](references/workflow.md) covers the full choreography — read it before planning any multi step refactor: the planning gate and refactoring inventory the three interacting orderings (structural before behavioral, conflict avoidance, dependency order) the refactor/<topic branch and per change worktree/PR git model w