go.yaml

# Go Code Review Guidelines
# Comprehensive rules for security, error handling, performance, and concurrency

description: "Code review guidelines for Go applications"
globs:
  - "**/*.go"

rules:
  # ============================================================================
  # SECURITY RULES
  # ============================================================================

  - id: go-command-injection
    description: |
      Prevent command injection in os/exec usage. Never use shell expansion with user input. Use exec.Command with separate arguments instead of shell interpretation. Validate and sanitize all external input.
      BAD: exec.Command("sh", "-c", "ls " + userInput) or exec.Command("bash", "-c", fmt.Sprintf(...))
      GOOD: exec.Command("ls", sanitizedPath) or exec.Command("grep", "--", query, "file.txt")
    severity: high

  - id: go-path-traversal
    description: |
      Prevent path traversal in file operations. Validate file paths to prevent directory traversal attacks. Use filepath.Clean and verify the path is within allowed directories.
      BAD: filepath.Join(baseDir, userInput) without validation allows escape with ../
      GOOD: Use filepath.Clean, then verify strings.HasPrefix(cleanPath, filepath.Clean(baseDir) + string(os.PathSeparator))
    severity: high

  - id: go-unsafe-package
    description: |
      Avoid unsafe package unless absolutely necessary. The unsafe package bypasses Go's type safety. Only use it for low-level system programming or performance-critical code with extensive testing. Document why it's necessary.
      BAD: Type punning with unsafe.Pointer without justification.
      GOOD: If truly needed, document thoroughly with safety guarantees.
    severity: high

  - id: go-crypto-rand
    description: |
      Use crypto/rand for security-sensitive random generation. Use crypto/rand for tokens, keys, passwords, and security-sensitive values. math/rand is predictable and not cryptographically secure.
      BAD: import "math/rand"; rand.Read(token) is predictable.
      GOOD: import "crypto/rand"; rand.Read(token) with error handling.
    severity: high

  - id: go-sql-injection
    description: |
      Prevent SQL injection using parameterized queries. Use parameterized queries with placeholders. Never concatenate user input into SQL strings. Use prepared statements for repeated queries.
      BAD: fmt.Sprintf("SELECT * FROM users WHERE id = '%s'", userID)
      GOOD: db.Query("SELECT * FROM users WHERE id = $1", userID)
    severity: high

  # ============================================================================
  # ERROR HANDLING RULES
  # ============================================================================

  - id: go-error-checking
    description: |
      Always check returned errors. Never ignore errors with _. Handle or propagate every error. If an error truly can be ignored, document why explicitly.
      BAD: result, _ := doSomething() ignores error.
      GOOD: Check err != nil and return fmt.Errorf("doSomething failed: %w", err)
    severity: high

  - id: go-error-wrapping
    description: |
      Wrap errors with context using fmt.Errorf and %w. Wrap errors with context to create useful error chains. Use %w verb to allow errors.Is() and errors.As() to work. Include relevant context without exposing sensitive data.
      BAD: return err without context, or fmt.Errorf("failed: %v", err) which doesn't wrap.
      GOOD: return fmt.Errorf("failed to process user %s: %w", userID, err)
    severity: medium

  - id: go-sentinel-errors
    description: |
      Define and use sentinel errors correctly. Define sentinel errors as package-level variables with errors.New(). Use errors.Is() to check for sentinel errors, not == comparison. Use errors.As() for custom error types.
    severity: medium

  - id: go-error-in-defer
    description: |
      Handle errors in defer statements properly. Errors in defer can shadow returned errors. Use named returns or handle defer errors explicitly. Log deferred close errors at minimum.
      BAD: defer f.Close() ignores error.
      GOOD: Use named return and defer func() { if cerr := f.Close(); cerr != nil && err == nil { err = cerr } }()
    severity: medium

  # ============================================================================
  # PERFORMANCE RULES
  # ============================================================================

  - id: go-slice-preallocation
    description: |
      Pre-allocate slices when size is known. Pre-allocate slices with make() when the final size is known or can be estimated. This avoids multiple reallocations during append.
      BAD: var result []User; append in loop causes multiple reallocations.
      GOOD: result := make([]User, 0, len(userIDs)) for single allocation.
    severity: medium

  - id: go-strings-builder
    description: |
      Use strings.Builder for string concatenation. Use strings.Builder for building strings in loops. String concatenation with + creates new strings each time. Pre-grow the builder if size is known.
      BAD: result += s + "," in loop causes O(n²) allocations.
      GOOD: var builder strings.Builder; builder.WriteString(s); builder.WriteByte(',')
    severity: medium

  - id: go-goroutine-leak
    description: |
      Prevent goroutine leaks with proper lifecycle management. Ensure all goroutines can terminate. Use context for cancellation, close channels to signal completion, and use WaitGroups to track goroutine completion.
      BAD: Goroutine blocked forever on channel send if not read.
      GOOD: Use context.Done() select case and WaitGroup for proper lifecycle.
    severity: high

  - id: go-defer-in-loops
    description: |
      Avoid defer in loops - deferred calls accumulate. defer in loops accumulates until function returns, causing memory issues. Extract loop body to separate function or manage resources explicitly within the loop.
      BAD: defer f.Close() in loop keeps all files open until function returns.
      GOOD: Extract to processFile function with its own defer.
    severity: medium

  - id: go-sync-pool
    description: |
      Use sync.Pool for frequently allocated objects. Use sync.Pool to reuse frequently allocated objects, reducing GC pressure. Reset objects before returning to pool. Don't store pointers to pool objects long-term.
    severity: low

  # ============================================================================
  # BEST PRACTICES RULES
  # ============================================================================

  - id: go-interface-design
    description: |
      Accept interfaces, return concrete types. Define interfaces at point of use, not implementation. Keep interfaces small. Accept interfaces in function parameters, return concrete types.
      BAD: Large interface defined at implementation site.
      GOOD: Small interface at point of use, concrete return types.
    severity: medium

  - id: go-context-usage
    description: |
      Use context correctly for cancellation and values. Pass context as first parameter. Don't store context in structs. Use context values sparingly (request-scoped metadata only). Always respect context cancellation in long operations.
      BAD: Storing context in struct or passing as non-first parameter.
      GOOD: func process(ctx context.Context, data []byte) with select on ctx.Done().
    severity: medium

  - id: go-context-timeout
    description: |
      Set appropriate timeouts for external operations. Always use context with timeout/deadline for network calls, database queries, and external service calls. Propagate context through call chain.
    severity: medium

  # ============================================================================
  # CONCURRENCY RULES
  # ============================================================================

  - id: go-race-condition
    description: |
      Protect shared state from race conditions. Concurrent access to shared state requires synchronization. Use mutexes, channels, or atomic operations. Run tests with -race flag.
      BAD: func (c *Counter) Inc() { c.value++ } has data race.
      GOOD: Use sync.Mutex with Lock/Unlock or atomic.Int64.Add(1).
    severity: high

  - id: go-mutex-usage
    description: |
      Use mutexes correctly and prefer RWMutex for read-heavy workloads. Use sync.RWMutex when reads vastly outnumber writes. Always defer Unlock to prevent deadlocks. Lock for minimal duration. Never copy mutex.
      BAD: Full Lock for read operations.
      GOOD: RLock for reads, Lock for writes.
    severity: medium

  - id: go-channel-patterns
    description: |
      Use channels idiomatically for communication. Use channels for communication between goroutines, not for sharing state. Establish clear ownership - sender closes. Use buffered channels when needed to prevent blocking.
    severity: medium

  - id: go-channel-closing
    description: |
      Close channels correctly to signal completion. Only the sender should close a channel. Never close a nil channel. Never send on a closed channel. Use sync.Once for closing from multiple goroutines.
      BAD: Closing from receiver or double close panics.
      GOOD: Sender closes with defer close(ch), use sync.Once for safe multi-close.
    severity: high

  - id: go-select-default
    description: |
      Use select with default for non-blocking operations. Use select with default case for non-blocking channel operations. Without default, select blocks until a case is ready. Use default sparingly to avoid busy loops.
    severity: low

  - id: go-waitgroup-usage
    description: |
      Use WaitGroup correctly to synchronize goroutines. Call Add before launching goroutine, Done in defer. Don't copy WaitGroup. Use pointer in closures to avoid copy.
      BAD: wg.Add(1) inside goroutine causes race condition.
      GOOD: wg.Add(1) before go func(), defer wg.Done() inside.
    severity: medium

  - id: go-errgroup-usage
    description: |
      Use errgroup for concurrent operations with error handling. Use golang.org/x/sync/errgroup for concurrent operations that may fail. It handles WaitGroup, cancellation, and error collection.
    severity: low

  - id: go-atomic-operations
    description: |
      Use sync/atomic for low-level atomic operations. Use atomic operations for simple counters and flags when mutexes are overkill. Use atomic.Value for atomic pointer stores. Prefer atomic package types (atomic.Int64) over functions.
    severity: medium