Remove golangci-lint ireturn Directives

Claude Code·Opus 4.8[1m]·Unknown author·3mo ago·17min·1 Checkpoint·14 file changes·+20/-16·27.1K tokens

what would it take to get rid of all //nolint:ireturn directives? would it be a big refactor? i really hate them because gofmt sometimes strips them and golangci-lint wants them back, and they fight.

3mo ago·29.7s

Bottom line

Not a refactor at all — it's a config decision. The 7 directives can't (and shouldn't) be refactored away by changing return types; the right move is either disable ireturn (kills the fight permanently) or convert the 7 inline directives into allow-list entries (keeps the linter for new code). Both are small config edits.

Why "refactor to concrete types" is the wrong path

I looked at all 7. None are accidental — they're either framework-forced or deliberate abstraction boundaries:

  • activity_tui.go → tea.Msg — forced by bubbletea's Cmd signature.
  • uiform.go → huh.Theme — forced, it's an interface in huh v2.
  • attach.go, open.go, manual_commit.go (×2) → store capabilities (CommittedStore/TemporaryStore) — intentional abstraction boundaries.
  • manual_commit_condensation.go → summarize.Generator — intentional provider abstraction.

Changing these to concrete returns is impossible for the first two and actively harms the design for the rest. So refactoring is off the table.

The actual fight you're hitting

buildSummaryGenerator is the tell. summarize.Generator is already in the allow-list (.golangci.yaml:118), so ireturn shouldn't fire — yet the directive exists. Its own comment says it: ireturn fires in some linter versions, and when it doesn't, nolintlint (you have require-explanation + require-specific on) flags the directive as unused. Version-dependent whack-a-mole. That's the loop you hate.

Two clean options

Option A — disable ireturn (smallest, ends it for good)

  • Remove - ireturn (.golangci.yaml:36)
  • Delete the ireturn: settings block (lines 110–135, ~26 lines)
  • Delete the 2 path exclusions (lines 171–176)
  • Delete all 7 inline directives
  • Net: ~35 fewer lines of config, zero directives, nothing for gofmt/nolintlint to fight over. Cost: no nudge toward concrete returns on new code.

Option B — keep the linter, kill the inline directives

  • Add ~3 type entries to the existing allow list: ...checkpoint.CommittedStore, ...checkpoint.TemporaryStore, charm.land/bubbletea/v2.Msg, plus a path exclusion for uiform.go (huh.Theme).
  • Delete all 7 inline directives (the summarize one is already covered).
  • Net: a few config lines added, all inline directives gone, linter still guards new code. This matches the pattern the repo already uses (25 allow entries + 2 path exclusions).

My recommendation: Option B if you still want the discipline on new code, Option A if you (like most people) think ireturn is more noise than signal. Either is ~10 minutes and I can verify with mise run lint after.

Want me to do one of them?

yep implement option B and entire trail create with the rationale.

3mo ago·4m

Done. Bottom line below.

Done

Option B implemented — all 7 //nolint:ireturn directives gone, linter still enabled.

  • Added 4 type entries to the allow list in .golangci.yaml: bubbletea/v2.Msg, huh/v2.Theme, checkpoint.CommittedStore, checkpoint.TemporaryStore. (summarize.Generator was already allowed — its directive was redundant, the root of the version-dependent fight.)
  • Removed inline directives from: activity_tui.go, attach.go, checkpoint/open.go, uiform/uiform.go, manual_commit.go (×2), manual_commit_condensation.go.
  • mise run lint → 0 issues; grep confirms zero nolint:ireturn remain.

Trail created for branch warm-noodling-iverson (ID 019ef2ff-…e833a), titled "Remove all //nolint:ireturn directives (Option B)" with the full rationale in the body.

Two things to flag:

  • trail create pushed the branch to origin — that's inherent to the command (it can't create a trail for a branch the remote doesn't have), not a git push I chose to run. Your code changes are still uncommitted, so only the pre-existing commits went up.
  • I haven't committed the ireturn changes — leaving that to you per your usual flow. Say the word and I'll commit.

commit and push

3mo ago·47.9s

Committed and pushed.

  • Commit 83c4c3b787 — chore(lint): drop all //nolint:ireturn directives via allow-list (7 files).
  • Pushed to origin/warm-noodling-iverson (7dc26c0900..83c4c3b787).

The trail for this branch already carries the full rationale.