Go Code Review

Review Workflow

Follow this sequence to avoid false positives and catch version-specific issues:

1. 1. Check go.mod — Note the Go version. This determines which patterns apply (loop variable capture is only an issue pre-1.22, slog is available from 1.21, errors.Join from 1.20). Skip version-gated checks that don't apply.
2. Scan changed files — Read full functions, not just diffs. Many Go bugs hide in what surrounds the change.
3. Check each category — Work through the checklist below, loading references as needed.
4. Verify before reporting — Load beagle-go:review-verification-protocol before submitting findings.

Output Format

Report findings as:

CODEBLOCK0

Quick Reference

Review Checklist

Error Handling

• - [ ] All errors checked (no _ = err without justifying comment)
• [ ] Errors wrapped with context (fmt.Errorf("...: %w", err))
• [ ] errors.Is/errors.As used instead of string matching
• [ ] errors.Join used for aggregating multiple errors (Go 1.20+)
• [ ] Zero values returned alongside errors

Concurrency

• - [ ] No goroutine leaks (context cancellation or shutdown signal exists)
• [ ] Channels closed by sender only, exactly once
• [ ] Shared state protected by mutex or sync types
• [ ] WaitGroups used to wait for goroutine completion
• [ ] Context propagated through call chain
• [ ] Loop variable capture handled (pre-Go 1.22 codebases only)

Interfaces and Types

• - [ ] Interfaces defined by consumers, not producers
• [ ] Interface names follow -er convention
• [ ] Interfaces minimal (1-3 methods)
• [ ] Concrete types returned from constructors
• [ ] any preferred over interface{} (Go 1.18+)
• [ ] Generics used where appropriate instead of any or code generation

Resources and Lifecycle

• - [ ] Resources closed with defer immediately after creation
• [ ] HTTP response bodies always closed
• [ ] No defer in loops without closure wrapping
• [ ] init() functions avoided in favor of explicit initialization

Naming and Style

• - [ ] Exported names have doc comments
• [ ] No stuttering names (user.UserService → user.Service)
• [ ] No naked returns in functions > 5 lines
• [ ] Context passed as first parameter
• [ ] slog used over log for structured logging (Go 1.21+)

Severity Calibration

Critical (Block Merge)

• - Unchecked errors on I/O, network, or database operations
• Goroutine leaks (no shutdown path)
• Race conditions on shared state (concurrent map access without sync)
• Unbounded resource accumulation (defer in loop, unclosed connections)

Major (Should Fix)

• - Errors returned without context (bare return err)
• Missing WaitGroup for spawned goroutines
• INLINECODE20 for recoverable errors
• Context not propagated to downstream calls

Minor (Consider Fixing)

• - interface{} instead of any in Go 1.18+ codebases
• Missing doc comments on exports
• Stuttering names
• Slice not preallocated when size is known

Informational (Note Only)

• - Suggestions to add generics where code generation exists
• Refactoring ideas for interface design
• Performance optimizations without measured impact

When to Load References

• - Reviewing error return patterns → error-handling.md
• Reviewing goroutines, channels, or sync types → concurrency.md
• Reviewing type definitions, interfaces, or generics → interfaces.md
• General review (resources, naming, init, performance) → common-mistakes.md

Valid Patterns (Do NOT Flag)

These are acceptable Go patterns — reporting them wastes developer time:

• - _ = err with reason comment — Intentionally ignored errors with explanation
• Empty interface / any — For truly generic code or interop with untyped APIs
• Naked returns in short functions — Acceptable in functions < 5 lines with named returns
• Channel without close — When consumer stops via context cancellation, not channel close
• Mutex protecting struct fields — Even if accessed only via methods, this is correct encapsulation
• //nolint directives with reason — Acceptable when accompanied by explanation
• Defer in loop — When function scope cleanup is intentional (e.g., processing files in batches)
• Functional options pattern — type Option func(*T) with With* constructors is idiomatic
• sync.Pool for hot paths — Acceptable for reducing allocation pressure in performance-critical code
• context.Background() in main/tests — Valid root context for top-level calls
• select with default — Non-blocking channel operation, intentional pattern
• Short variable names in small scope — i, err, ctx, ok are idiomatic Go

Context-Sensitive Rules

Only flag these issues when the specific conditions apply:

Before Submitting Findings

Load and follow review-verification-protocol before reporting any issue.