Effective go

Effective Go Skill

Comprehensive Go refactoring framework based on the official Effective Go guide and Go best practices.

Prerequisites

Always read resources/effective-go-principles.json (in this skill's directory) before starting.

It contains 25+ principles with definitions, code smells, and refactoring guidance including:

• Formatting (gofmt, semicolons)
• Naming (packages, interfaces, exported names)
• Control structures (if, for, switch, defer)
• Data structures (slices, maps, arrays)
• Functions (multiple returns, named returns, defer)
• Concurrency (goroutines, channels, select)
• Error handling (error vs panic, wrapping)
• Interfaces and methods (receivers, embedding)

Refactoring Approach

Four-Phase Strategy

Phase 1: Discovery & Analysis (15-20 min)

Understand the Codebase:

1. Identify scope (package, module, or entire project)
2. Check Go version and module structure
3. Analyze existing code patterns
4. Review dependencies and imports

Scan for Code Smells:

• No gofmt/goimports formatting
• Non-idiomatic naming (snake_case, wrong capitalization)
• Wrong receiver types (value when pointer needed)
• Missing error checks
• Goroutine leaks or race conditions
• Improper channel usage
• Panic in library code
• Mutable value types that should be immutable
• Large interfaces (>3 methods for non-standard libs)
• Primitive obsession (no custom types)

Technical Exploration — search and run:

• Run gofmt -l . | grep -v "vendor/" — check formatting
• Pattern ^func [a-z] in *.go files — find unexported funcs that may need export
• Pattern ^type [a-z] — find unexported types
• Pattern go func in *.go files — find goroutine launches
• Pattern err := — find error assignments
• Pattern panic\( — find panic usage in library code
• Pattern type.*interface — find interface definitions

Phase 2: Strategic Refactoring Plan (10-15 min)

Based on loaded Effective Go principles:

1. Formatting and Style

   • Run gofmt/goimports on all files
   • Fix semicolon issues
   • Ensure proper brace placement
   • Clean up whitespace

2. Naming Conventions

   • Fix package names (lowercase, single-word)
   • Correct exported/unexported names
   • Apply MixedCaps/mixedCaps consistently
   • Rename interfaces (-er suffix for single-method)

3. Prioritize Refactoring

   • Critical: gofmt, data races, goroutine leaks, missing error checks
   • High: Wrong receivers, panic in libraries, improper channel usage
   • Medium: Non-idiomatic naming, primitive obsession, large interfaces
   • Low: Style improvements, comment formatting

Phase 3: Tactical Pattern Application (30-45 min)

Apply patterns systematically:

1. Formatting

• Run gofmt -w on all Go files
• Ensure tabs for indentation
• Fix opening brace placement
• Remove unnecessary semicolons

2. Naming

• Package names: short, lowercase, no underscores
• Exported names: Start with uppercase
• Unexported names: Start with lowercase
• Interfaces: Use -er suffix (Reader, Writer, Closer)
• No snake_case: Use MixedCaps or mixedCaps
• Acronyms: All caps (HTTP, URL, ID)

3. Control Structures

• Use guard clauses (early returns)
• Prefer for range over traditional for loops
• Use expression-less switch for if-else chains
• Apply defer for cleanup operations
• Avoid naked returns in long functions

4. Error Handling

• Check all errors explicitly
• Add context with fmt.Errorf and %w
• Return errors, don't panic (except in truly exceptional cases)
• Use errors.Is and errors.As for error checking
• Implement error wrapping consistently

5. Concurrency

• Fix goroutine leaks (ensure they can exit)
• Use channels for communication
• Apply proper channel closing (sender closes)
• Use select for multiplexing
• Avoid shared memory, prefer channels
• Add sync.WaitGroup for coordination
• Fix loop variable capture in goroutines (only an issue pre-Go 1.22; check go.mod)

6. Pointers vs Values

• Use pointer receivers when modifying receiver
• Use pointer receivers for large structs
• Be consistent (all pointer or all value for a type)
• Use pointer receivers for types with sync.Mutex

7. Interfaces

• Keep interfaces small (1-3 methods ideal)
• Define interfaces where used, not where implemented
• Accept interfaces, return structs
• Use empty interface sparingly

8. Data Structures

• Prefer slices over arrays
• Use make() with capacity hints
• Ensure maps are initialized with make()
• Use composite literals for initialization
• Apply append() correctly (assign result)

Phase 4: Validation & Testing (10-15 min)

Verify Improvements:

• All files pass gofmt check
• No exported names start with lowercase
• All errors are checked or explicitly ignored
• No goroutine leaks detected
• Channels properly closed from sender
• Receiver types are consistent and appropriate
• No panic calls in library code
• Interfaces are small and focused
• Code follows Go idioms

Testing Strategy:

• Run go vet on all packages
• Run golint or staticcheck
• Run go test -race to detect data races
• Use go test -cover for coverage
• Run golangci-lint for comprehensive checks

Core Effective Go Principles Reference

Formatting

1. gofmt - Standard formatting, non-negotiable
2. Semicolons - Automatic insertion, placement rules

Naming

3. Package Names - Short, lowercase, single-word
4. Exported Names - Uppercase = public
5. Interface Naming - -er suffix for single-method
6. MixedCaps - No underscores in identifiers

Control Structures

7. Guard Clauses - Early returns, reduced nesting
8. For Loop Patterns - Range, traditional, infinite
9. Switch Statements - No fallthrough by default
10. Type Switch - Handling interface types
11. Defer - Cleanup operations

Functions

12. Multiple Return Values - Return (result, error)
13. Named Return Values - For documentation/defer
14. new vs make - Allocation primitives

Data

15. Slices - Dynamic sequences
16. Maps - Key-value storage
17. Printing - Format verbs (%v, %+v, %#v)
18. Append - Growing slices

Initialization

19. Composite Literals - Inline initialization

Methods

20. Pointer vs Value Receivers - When to use each

Interfaces

21. Interfaces - Implicit implementation
22. Type Assertions - Safe conversion
23. Embedding - Composition over inheritance

Concurrency

24. Share by Communicating - Channel-based patterns
25. Goroutines - Lightweight concurrency
26. Channels - Communication pipes
27. Select - Multiplexing channels

Errors

28. Error Handling - Explicit error returns
29. Panic - Only for unrecoverable errors
30. Recover - Panic recovery
31. Error Wrapping - Adding context with %w

Code Smell Detection Checklist

Critical Anti-Patterns

• Code not formatted with gofmt
• Exported names starting with lowercase
• Panic used in library code
• Data races (concurrent map access, shared state)
• Goroutine leaks (no way to stop)
• Sending on closed channel

High Priority Anti-Patterns

• Mixed pointer and value receivers on same type
• Missing error checks
• Errors ignored with _
• Improper channel closing (receiver closes, or closing nil channel)
• Loop variable captured in goroutine (pre-Go 1.22 only — check go.mod)
• Using new() for slices, maps, channels
• Not assigning append() result

Medium Priority Anti-Patterns

• Snake_case naming instead of MixedCaps
• Large interfaces (>5 methods)
• Missing doc comments on exported identifiers
• Using interface{} when specific type would work
• Not using defer for cleanup
• Naked returns in long functions
• Arrays when slices would be better

Low Priority Anti-Patterns

• Inconsistent naming
• Missing String() method for custom types
• Could use type switch instead of repeated assertions
• Could use composite literal instead of new()

Output Format

1. Anti-Pattern Identified

File: internal/service/order.go:45-78
Smell: Missing error check - result of operation ignored
Principle Violated: Error Handling
Impact: Silent failures, bugs go unnoticed

2. Effective Go Principle to Apply

Principle: Error Handling (from effective-go-principles.json)
Category: Errors
Key Point: Always check errors explicitly, never ignore them
When to Apply: Every function call that returns an error

3. Refactoring Steps

Step 1: Find all places where errors are returned
Step 2: Add explicit error checks with if err != nil
Step 3: Add context to errors with fmt.Errorf("operation failed: %w", err)
Step 4: Propagate or handle errors appropriately
Step 5: Use _ only for intentional ignoring (document why)

4. Code Example

// BEFORE: Missing error check (Anti-pattern)
func processOrder(id string) *Order {
	order, _ := db.GetOrder(id) // ERROR: Ignoring error!
	return order
}

func updateUser(user *User) {
	db.Save(user) // ERROR: Not checking error!
}

// AFTER: Proper error handling (Go idiom)
func processOrder(id string) (*Order, error) {
	order, err := db.GetOrder(id)
	if err != nil {
		return nil, fmt.Errorf("getting order %s: %w", id, err)
	}
	return order, nil
}

func updateUser(user *User) error {
	if err := db.Save(user); err != nil {
		return fmt.Errorf("saving user %s: %w", user.ID, err)
	}
	return nil
}

5. Impact Assessment

Benefits:

• Errors are caught and handled explicitly
• Better error context for debugging
• No silent failures
• Follows Go conventions

Metrics:

• Improved reliability
• Better error messages
• Easier debugging
• Code passes go vet checks

Language-Specific Patterns

Error Handling

// Good: Proper error handling with context
func loadConfig(path string) (*Config, error) {
	data, err := os.ReadFile(path)
	if err != nil {
		return nil, fmt.Errorf("reading config from %s: %w", path, err)
	}

	var cfg Config
	if err := json.Unmarshal(data, &cfg); err != nil {
		return nil, fmt.Errorf("parsing config: %w", err)
	}

	return &cfg, nil
}

// Usage
cfg, err := loadConfig("config.json")
if err != nil {
	log.Fatalf("Failed to load config: %v", err)
}

Receiver Types

// Good: Consistent pointer receivers
type Counter struct {
	mu    sync.Mutex
	value int
}

// Pointer receiver - modifies state
func (c *Counter) Increment() {
	c.mu.Lock()
	defer c.mu.Unlock()
	c.value++
}

// Pointer receiver - consistency
func (c *Counter) Value() int {
	c.mu.Lock()
	defer c.mu.Unlock()
	return c.value
}

// Bad: Mixed receivers
type BadCounter struct {
	value int
}

func (c BadCounter) Increment() { // Value receiver - doesn't modify!
	c.value++ // Modifies copy
}

func (c *BadCounter) Value() int { // Pointer receiver - inconsistent
	return c.value
}

Goroutines and Channels

// Good: Proper goroutine coordination
func processBatch(items []Item) error {
	var wg sync.WaitGroup
	errors := make(chan error, len(items))

	for _, item := range items {
		wg.Add(1)
		item := item // Capture variable (required pre-Go 1.22; unnecessary in 1.22+)

		go func() {
			defer wg.Done()
			if err := process(item); err != nil {
				errors <- err
			}
		}()
	}

	// Wait in separate goroutine
	go func() {
		wg.Wait()
		close(errors) // Sender closes channel
	}()

	// Collect errors
	for err := range errors {
		return err // Return first error
	}

	return nil
}

// Bad: Goroutine leak
func badProcess(items []Item) {
	for _, item := range items {
		go func() {
			process(item) // Captures loop variable - BUG!
			// No coordination, no error handling, no way to stop
		}()
	}
	// Function returns immediately, goroutines may still be running
}

Interfaces

// Good: Small, focused interfaces
type Reader interface {
	Read(p []byte) (n int, err error)
}

type Closer interface {
	Close() error
}

type ReadCloser interface {
	Reader
	Closer
}

// Define where used, not where implemented
type DataStore interface {
	Save(data []byte) error
}

func SaveToStore(store DataStore, data []byte) error {
	return store.Save(data)
}

// Bad: Large, unfocused interface
type Database interface {
	GetUser(id string) (*User, error)
	CreateUser(u *User) error
	UpdateUser(u *User) error
	DeleteUser(id string) error
	GetOrder(id string) (*Order, error)
	CreateOrder(o *Order) error
	// ... 20+ more methods
}

Best Practices

Do

• Always run gofmt before committing
• Check all errors explicitly
• Use defer for cleanup operations
• Keep interfaces small
• Accept interfaces, return structs
• Use pointer receivers for large structs or when modifying
• Close channels from the sender side
• Use context for cancellation and timeouts
• Wrap errors with %w for error chains
• Use sync.WaitGroup to coordinate goroutines

Don't

• Don't ignore gofmt warnings
• Don't use panic in library code
• Don't ignore errors with _
• Don't mix pointer and value receivers
• Don't capture loop variables in goroutines without copying
• Don't send on closed channels
• Don't use naked returns in long functions
• Don't create large interfaces
• Don't share memory without synchronization
• Don't use new() for slices, maps, or channels

Resources

• Effective Go Principles: See resources/effective-go-principles.json for complete definitions
• Checklist: See CHECKLIST.md for full anti-pattern list
• Official Guide: https://go.dev/doc/effective_go
• Go Code Review Comments: https://go.dev/wiki/CodeReviewComments

Workflow Integration

This skill can be:

• Invoked from /go-review command
• Used by go-analyzer agent for autonomous analysis
• Triggered by pre-commit hooks to check for anti-patterns
• Called from other skills for Go code improvements