Go Quality Review

Go Quality Review

Purpose

Audit Go code for structural quality, style conformance, and modern Go practices. This skill is the designated lint-tool runner — other vertical review skills do NOT run golangci-lint/staticcheck/go vet, avoiding duplicate execution.

This skill does NOT cover: security, concurrency, performance, error handling, test quality, or business logic — those belong to sibling vertical skills.

When To Use

• Reviewing code structure and readability
• Checking Go naming conventions and package organization
• Evaluating use of modern Go features
• Need to run golangci-lint / staticcheck / go vet

When NOT To Use

• Security vulnerabilities → go-security-review
• Concurrency safety → go-concurrency-review
• Error handling correctness → go-error-review
• Performance optimization → go-performance-review
• Business logic → go-logic-review

Mandatory Gates

1) Go Version Gate

Read go.mod for go directive. Do NOT recommend features above project version.

| Feature | Minimum Go | |---------|-----------| | Generics | 1.18 | | Typed atomics (atomic.Int64, atomic.Bool) | 1.19 | | slog, slices/maps packages, min/max builtins, sync.OnceValue/OnceFunc | 1.21 | | Range-over-func, enhanced loop variable semantics | 1.22 | | iter.Seq, unique package | 1.23 |

2) Anti-Example Suppression Gate

MUST quote specific code evidence. Category match alone insufficient.

Embedded anti-examples:

• "Should use generics" — when only one concrete type used throughout codebase. Generics add complexity without benefit for single-type usage.
• "Should use slog" — when project targets Go 50 lines)"** — when body is a straightforward table-driven switch, sequential pipeline with no nesting, or single select/case block. These are long but simple.
• "interface{} should be any" — pure cosmetic alias rename. Report only as part of broader modernization effort, never as standalone finding.
• "Should extract helper function" — when the code would only be called from one place and extraction reduces readability by splitting context.

3) Static Analysis Execution Protocol

Run lint tools in this priority order:

1. Check for .golangci.yml / .golangci.yaml — respect project config
2. golangci-lint run (config-aware)
3. staticcheck ./... (if golangci-lint unavailable)
4. go vet ./... (minimal fallback)

Report tool output in Execution Status. If no tools available, state Not available. Dedup rule: same location + same issue from multiple tools = report once.

4) Generated Code Exclusion Gate

Exclude: *.pb.go, *_gen.go, mock_*.go, *_string.go, *_enumer.go. Note excluded files in Execution Status.

Workflow

1. Define scope — files/diff under review. Apply Generated Code Exclusion Gate.
2. Check Go version from go.mod — gate all modern Go recommendations.
3. Run static analysis — follow execution protocol above. Record output.
4. Gather evidence — read changed files, identify quality/style patterns.
5. Load references — always load go-error-and-quality.md (quality sections); load go-modern-practices.md when modern Go features relevant.
6. Evaluate ALL 13 checklist items → cross-reference with lint output → suppress anti-examples → format.

Grep-Gated Execution Protocol

This skill uses mechanical grep pre-scanning to guarantee zero missed checklist items. 8 of 13 items are grep-gated; 5 are semantic-only.

Execution Order

1. Identify target files (from dispatch prompt, or write raw snippet to $TMPDIR/review_snippet.go)
2. Run grep for all grep-gated checklist items against target files
3. HIT → run semantic analysis to confirm or reject
4. MISS → auto-mark NOT FOUND, skip semantic analysis
5. For semantic-only items (items 1, 2, 5, 9, 13): full model reasoning — these require structural analysis
6. Report only FOUND items

Grep Audit Line

Include in Execution Status: Grep pre-scan: X/8 items hit, Z confirmed as findings (5 semantic-only)

Quality Checklist (12 Items)

All Medium severity unless marked (Low).

| # | Item | Pattern | Grep Pattern | |---|------|---------|-------------| | 1 | Function too long | > 50 lines → extract helper (see anti-example for flat switch exception) | Semantic-Only (function length requires counting lines — no grep pattern) | | 2 | Excessive nesting | > 4 levels → early return pattern ("happy path left-aligned") | Semantic-Only (nesting depth requires counting indent levels) | | 3 | Naked return | In functions > 5 lines → explicit returns for clarity | return$\|return\s*$ (naked return — grep for return with nothing after) | | 4 | Mutable global variable | Mutable var at package level → const, getter, or functional options | ^var\s+\w (package-level var declarations — grep at file scope) | | 5 | Interface bloat | Interface > 3 methods or defined at implementation site → small interfaces at consumer site | Semantic-Only (interface bloat requires counting methods) | | 6 | Type assertion without ok | x.(T) without comma-ok → x, ok := x.(T) — panics on wrong type | \.\(\w (type assertion without comma-ok check) | | 7 | defer in loop | Defer accumulates until function return → extract to helper function | defer\s+ (compound: inside for loop — semantic confirmation) | | 8 | init() misuse | init() for non-registration logic → explicit initialization | func init\(\) | | 9 | Inconsistent receiver type | Mixed pointer/value receivers on same type → consistent choice | Semantic-Only (receiver consistency requires checking all methods on type) | | 10 | Modern Go alternatives | Outdated patterns when modern alternatives exist (version-gated): log → slog, atomic.AddInt64 → atomic.Int64, sort.Slice → slices.SortFunc | log\.\|atomic\.Add\|sort\.Slice (outdated patterns when modern alternatives exist) | | 11 | Generics vs interfaces | Wrong choice — type operations (containers, transforms) → generics; behavior contracts → interfaces | interface\s*\{\|interface\{\|any\b | | 12 | Naming / package structure (Low) | Stuttering (user.UserService), package utils/helpers/common, unexported type in exported return | \.User\w*Service\|\.User\w*Handler\|package\s+utils\|package\s+helpers\|package\s+common | | 13 | Missing context.Context on I/O function | Function performing DB / HTTP / cache / Redis I/O but signature lacks ctx context.Context as first parameter — queries cannot be cancelled on client disconnect or upstream timeout. Fix: add ctx context.Context first, then .WithContext(ctx) (GORM), http.NewRequestWithContext(ctx, ...), redis.*Cmd(ctx, ...) etc. | Semantic-Only (absence-of-pattern: requires reading both the signature and the body to identify I/O calls without ctx propagation) |

Severity Rubric

Medium — Maintainability/readability issue increasing cognitive load or bug risk.

Low — Style preference or minor inconsistency.

Evidence Rules

• For each finding: show current pattern vs idiomatic alternative
• For lint findings: include tool name and rule ID when available
• Merge rule: same issue at ≥3 locations → one finding with location list
• Dedup with lint: if lint flagged it, use lint output as evidence

Output Format

Findings

[Medium|Low] Short Title

• ID: QUAL-NNN
• Location: path:line
• Impact: Maintenance/readability consequence
• Evidence: Current pattern vs idiomatic alternative
• Recommendation: Specific refactoring suggestion
• Action: follow-up

Suppressed Items

[Suppressed] Short Title

• Reason: Anti-example matched + evidence cited

Execution Status

• Go version: X.Y
• Grep pre-scan: X/8 items hit, Z confirmed as findings (5 semantic-only)
• golangci-lint: PASS | FAIL | Not available
• staticcheck: PASS | FAIL | Covered by golangci-lint | Not available
• go vet: PASS | FAIL | Covered by golangci-lint | Not available
• Excluded (generated): list or None
• References loaded: list

Summary

1-2 lines. Count by severity + lint status.

Example Output

### Findings

#### [Medium] Function Exceeds 50 Lines with Deep Nesting
- **ID:** QUAL-001
- **Location:** `internal/service/order.go:45-120`
- **Impact:** 75-line function with 5 nesting levels — high cognitive load, hard to test individual branches
- **Evidence:** `ProcessOrder()` has nested if/for/if/switch/case structure. Unlike a flat switch (anti-example), this has complex branching.
- **Recommendation:** Extract inner switch into `classifyOrderType()` and validation into `validateOrderItems()`
- **Action:** follow-up

#### [Medium] Mutable Package-Level Variable
- **ID:** QUAL-002
- **Location:** `internal/config/defaults.go:8`
- **Impact:** `var DefaultTimeout = 30 * time.Second` — any package can mutate, non-deterministic in tests
- **Evidence:** Written at L8, read from 4 packages. golangci-lint: `gochecknoglobals`
- **Recommendation:** Change to `const` or getter: `func DefaultTimeout() time.Duration { return 30 * time.Second }`
- **Action:** follow-up

### Suppressed Items
#### [Suppressed] Long Function — Table-Driven Switch
- **Reason:** `routeRequest()` at router.go:30 is 60 lines but flat switch on HTTP method, no nesting. Anti-example: "straightforward table-driven switch"

### Execution Status
- Go version: 1.21
- golangci-lint: PASS (2 warnings reported above)
- staticcheck: Covered by golangci-lint config
- go vet: Covered by golangci-lint config
- Excluded (generated): None
- References loaded: go-error-and-quality.md, go-modern-practices.md

### Summary
2 Medium findings (function length, mutable global). Lint clean except reported items.

No-Finding Case

If no issues found: state No code quality findings identified. Still output Execution Status (lint results always reported).

Load References Selectively

| Reference | Load When | |-----------|-----------| | references/go-error-and-quality.md | Always (code quality sections) | | references/go-modern-practices.md | Code uses or could benefit from modern Go features | | references/go-review-anti-examples.md | Always |

Review Discipline

• Code quality, style, modern practices, and lint only — not security, concurrency, errors, performance, tests, or logic
• Designated lint runner — this skill runs golangci-lint; other skills reference its output
• Execute ALL 13 checklist items
• Version-gate all modern Go recommendations — check go.mod before recommending

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: johnqtcg
• Source: johnqtcg/awesome-skills
• License: MIT
• Homepage: https://johnqtcg.github.io/awesome-skills/

Install and usage instructions live in the source repository linked above.