go-refactoring

Files (2)

• SKILL.md
• bulk-rewrites.md · references/

SKILL.md (first lines)

# Go Refactoring Workflow

A refactor changes structure, never behavior. The definition of done is
mechanical: same tests pass before and after every step, and every step
is small enough to revert alone.

Detailed reference material, loaded on demand:

- `references/bulk-rewrites.md` — `gofmt -r`, `//go:fix inline`, `eg`,
  `gopatch` and custom analyzers, with the verification to run after.

Read a reference file only when the summary below is not enough.

## 0. Risk, Checkpoints, and When Not To

| Risk | Transformations | Before you start |
|---|---|---|
| Low | gopls rename, extract/inline variable, organize imports | Green build and tests |
| Medium | Extract function or method, inline a call across packages, add/remove a parameter | Tests that cover every changed call site |
| High | Move types across packages, split or merge packages, break cycles, change exported API | Full safety net, and the developer's sign-off |

Stop for the developer's approval before any cross-package move, exported
API change, deletion, or change to code with no tests. Question the
refactor itself when the code is stable and nothing planned touches it,
when it is critical and untested, or when no concrete next change gets
easier.

## 1. The Loop

1. **Baseline:** `go build ./... && go test ./...` must be green before
   touching anything. If tests are missing around the target code,
   write characterization tests FIRST — they pin current behavior,
   even if that behavior looks wrong.
2. **One transformation** from the catalog below.
3. **Verify:** build + tests + `go vet ./...`.
4. **Commit.** Never mix a refactor commit with a behavior change —
   reviewers can skim a `refactor:` commit; they must scrutinize a
   mixed one.
5. Repeat.

If step 3 fails and the fix isn't obvious in a minute, revert the step
rather than debugging a half-applied transformation.

## 2. Renames — Let Tools Do Them

```bash
# Preferred: gopls (understands types, interfaces, embedding)
gopls rename -w internal/service/user.go:#offset newName

# Module path or package import path changes:
# update go.mod, then rewrite imports mechanically
find . -name '*.go' -exec sed -i 's|github.com/acme/old|github.com/acme/new|g' {} +
go build ./...   # the compiler is the reviewer
```

After a rename, grep for the old name in struct tags, templates, SQL and
anything reflection reads — gopls guards compilation, not those. Renaming
a field silently desyncs it from its `json`/`db` tag.

Never rename an exported identifier of a published library without a
deprecation cycle: add the new name, mark the old one
`// Deprecated: use NewName.`, delete in the next major version.

## 3. Extract Package

Moving code out of a god package, in compiler-checked steps:

1. Create the new package; **move one type and its methods** (`gopls`
   or cut/paste), leaving everything else.
2. In the old package, add type aliases so nothing breaks:
   `type User = user.User` (aliases, `=`, not definitions).
3. Build. Migrate importers to the new path in batches; build each batch.
4. Delete the aliases when no importer remains.

This keeps every commit green with an arbitrarily large caller base.

## 4. Break a Circular Dependency

Packages `a → b` and `b → a` won't compile; near-cycles show up as
god packages. Three escapes, in order of preference:

1. **Extract the shared core:** both `a` and `b` actually depend on a
   type — move it to a third package `c` that imports nothing.
2. **Invert with an interface:** if `store` calls back into `service`,
   define the callback interface IN `store` (consumer side) and let
   `service` implement it. The arrow flips at compile time.
3. **Merge:** if two packages can't be described without each other,
   they were one package all along.

## 5. Change a Function Signature Safely

For exported functions with many callers:

```go
// Step 1 — add the new form alongside the old
func (s *Service) ProcessCtx(ctx context.Context, id string) error { ... }

// Step 2 — old form delegates; mark deprecated
// Deprecated: use ProcessCtx.
func (s *Service) Process(id string) error {
    return s.ProcessCtx(context.Background(), id)
}

// Step 3 — migrate callers batch by batch, building each batch
// Step 4 — delete the old form (same module) or keep until next major (library)
```

On Go 1.26+, add `//go:fix inline` above the deprecated wrapper and step 3
becomes `go fix ./...` — for your module now, and for every downstream
module when it next runs `go fix`.

Inside a single module, prefer atomic signature changes when the
compiler can find every caller for you: change it, then chase the
build errors — that's the compiler enumerating your TODO list.

## 6. Strangler Migration for Subsystems

Replacing a subsystem (old store, legacy client) too big for one PR:

From eduardo-sl/go-agent-skills, MIT.

More from eduardo-sl/go-agent-skills