Effective Go

---
name: effective-go
description: Analyzes and refactors Go code using Effective Go principles. Use when refactoring Go code, identifying anti-patterns, improving Go idioms, or applying Go best practices.
allowed-tools: Read, Grep, Glob, Bash, Edit, Write
---

# Effective Go Skill

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

## When to Use This Skill

- Refactoring Go code for idiomatic patterns
- Identifying Go-specific anti-patterns
- Improving code formatting and style
- Applying proper error handling patterns
- Optimizing concurrency patterns (goroutines, channels)
- Implementing proper naming conventions
- Analyzing interface and method design
- Reviewing receiver types (pointer vs value)

## Prerequisites

**CRITICAL**: Always load the Effective Go principles first:

```bash
Read: plugins/effective-go/resources/effective-go-principles.json
```

This JSON 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:**
```bash
# Check if code is gofmt'd
gofmt -l . | grep -v "vendor/"

# Find exported names that might be wrong
grep -r "^func [a-z]" --include="*.go"
grep -r "^type [a-z]" --include="*.go"

# Find potential goroutine issues
grep -r "go func" --include="*.go"
grep -r "go " --include="*.go" | grep -v "go func"

# Find error handling
grep -r "err :=" --include="*.go"
grep -r "if err" --include="*.go"

# Find panic usage
grep -r "panic(" --include="*.go"

# Find interfaces
grep -r "type.*interface" --include="*.go"
```

#### 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

**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
- [ ] 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
```go
// 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
```go
// 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
```go
// 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
```go
// 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

		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
```go
// 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