Code Quality Tools

This document explains AWF’s code quality tooling strategy, covering all 17 linters, the formatter, and how to use them effectively.

Overview#

AWF uses golangci-lint v2 as a single aggregator for 17 specialized linters that enforce Go best practices, catch bugs early, and maintain consistent code style. The configuration follows late-2025 standards with strict cognitive complexity limits and modern error handling patterns.

Quality Philosophy:

• Correct - Tests prove functionality works
• Secure - No injection vulnerabilities, validated inputs
• Clear - Readable by unfamiliar developers in <5 minutes
• Minimal - No unnecessary abstractions or complexity

Linter Categories:

• Core quality (5 linters): Bug detection and correctness
• Readability (2 linters): Clear, maintainable code
• Error handling (1 linter): Proper error wrapping
• Security (1 linter): Vulnerability detection
• Architecture (1 linter): Dependency constraints
• Modern Go (7 linters): Late-2025 best practices

How It Works#

golangci-lint as Aggregator#

All linters run through a single command via golangci-lint v2:

make lint          # Run all 17 linters
make fix           # Auto-fix issues where possible
golangci-lint run  # Direct invocation

Why golangci-lint?

• Single binary with 100+ bundled linters
• Parallel execution for fast performance
• Unified configuration in .golangci.yml
• CI integration via golangci-lint-action@v9

CI Integration#

GitHub Actions runs quality checks on every push via .github/workflows/quality.yml:

Quality Gates:

1. Format check - make fmt must leave no changes (gofumpt)
2. Vet - go vet ./... must pass
3. Lint - make lint must pass (all 17 linters via golangci-lint)
4. Architecture lint - make lint-arch must pass (go-arch-lint) ← C051
5. Unit tests - make test-unit with -race detector
6. Integration tests - make test-integration with coverage ≥80%
7. Coverage check - Coverage must be ≥80%

CI fails if any gate fails, blocking merge. Architecture linting now catches dependency constraint violations before code review.

Old workflow (ci.yaml): Separate workflow with individual tool invocations New workflow (quality.yml): Consolidated quality checks including architecture enforcement

Makefile Targets#

Typical Workflow:

# 1. Make changes
vim internal/domain/workflow/workflow.go

# 2. Format code
make fmt

# 3. Check for issues
make lint

# 4. Fix auto-fixable issues
make fix

# 5. Check architecture constraints
make lint-arch

# 6. Run all quality checks (lint + fmt + vet + lint-arch + test)
make quality

# 7. Commit if all checks pass
git add .
git commit -m "feat(workflow): add validation"

Configuration Reference#

Core Quality Linters#

errcheck - Unhandled Errors#

Purpose: Detects unchecked errors that could cause silent failures.

Example violation:

file, _ := os.Open("config.yaml")  // ❌ Ignores error
defer file.Close()

Fix:

file, err := os.Open("config.yaml")  // ✅ Check error
if err != nil {
    return fmt.Errorf("open config: %w", err)
}
defer file.Close()

Configuration: Excludes common patterns:

• (io.Closer).Close - Deferred closes
• (net/http.ResponseWriter).Write - HTTP response writes
• (*zap.Logger).Sync - Logger flushes

govet - Official Go Analyzer#

Purpose: Official Go team static analyzer for suspicious constructs.

Example violation:

fmt.Printf("User %s has %d", user.Name)  // ❌ Missing argument

Fix:

fmt.Printf("User %s has %d points", user.Name, user.Points)  // ✅ Correct

Configuration: Enables all checks except fieldalignment (micro-optimization).

staticcheck - Comprehensive Analysis#

Purpose: Industry-standard static analyzer (includes gosimple, stylecheck).

Example violation:

if err := validate(); err != nil {
    return err  // ❌ Could return nil interface
}
return nil

Fix:

return validate()  // ✅ Direct return preserves nil correctness

ineffassign - Wasted Assignments#

Purpose: Detects assignments that are never read.

Example violation:

result := compute()  // ❌ Never used
result = compute2()
return result

Fix:

result := compute2()  // ✅ Remove unused assignment
return result

unused - Dead Code#

Purpose: Detects unused constants, variables, functions, types.

Example violation:

const maxRetries = 5  // ❌ Never referenced

Fix: Delete the unused constant.

Readability Linters#

misspell - Typo Detection#

Purpose: Catches typos in comments and strings using US English dictionary.

Example violation:

// Proces the workflow  // ❌ Typo: "Proces"

Fix:

// Process the workflow  // ✅ Correct spelling

Configuration: Allows British spellings “cancelled” and “cancelling” (used in API).

revive - Modern Linter#

Purpose: Replacement for deprecated golint with 47 configurable rules.

Example violation:

func (w *Workflow) GetID() string {}  // ❌ Stutters: Workflow.GetID

Fix:

func (w *Workflow) ID() string {}  // ✅ Concise

Error Handling Linters#

errorlint - Error Wrapping#

Purpose: Enforces proper %w wrapping for error chains and errors.Is/errors.As usage.

Example violation:

if err != nil {
    return fmt.Errorf("validation failed: %v", err)  // ❌ Use %w
}

Fix:

if err != nil {
    return fmt.Errorf("validation failed: %w", err)  // ✅ Wraps error
}

Security Linters#

gosec - Security Audit#

Purpose: Scans for common security vulnerabilities (SQL injection, path traversal, etc.).

Example violation:

cmd := exec.Command("sh", "-c", userInput)  // ❌ Command injection risk

Fix:

// Use ShellEscape() from pkg/interpolation
safeInput := interpolation.ShellEscape(userInput)
cmd := exec.Command("sh", "-c", safeInput)  // ✅ Escaped

Configuration: Excludes intentional patterns:

• G204 - Shell executor intentionally uses variable commands
• G304 - Workflow loader intentionally reads user-specified files

Architecture Linters#

depguard - Dependency Constraints (golangci-lint)#

Purpose: Enforces hexagonal architecture by preventing invalid imports in domain layer.

Example violation:

// internal/domain/workflow/workflow.go
import "github.com/spf13/cobra"  // ❌ Domain depends on CLI framework

Fix: Move CLI dependencies to internal/interfaces/cli/. Domain uses ports.Logger interface.

Blocked imports in domain:

• github.com/spf13/cobra - CLI framework
• go.uber.org/zap - Concrete logger
• github.com/fatih/color - UI components
• github.com/schollz/progressbar/v3 - UI components

go-arch-lint - Full Architecture Validation (C051)#

Purpose: AST-based architecture constraint enforcement using .go-arch-lint.yml configuration. Validates the complete hexagonal architecture dependency graph beyond what depguard can enforce.

Features:

• Component-to-package mapping verification
• Multi-layer dependency rules (domain, application, infrastructure, interfaces)
• Vendor dependency whitelisting per component
• Customizable depth scanning with deepScan mode

Usage:

make lint-arch       # Check all architecture constraints
make lint-arch-map   # View component-to-package mapping

Example constraint:

application:
  mayDependOn:
    - domain-workflow
    - domain-ports
    - domain-errors
  canUse:
    - go-stdlib
    - go-sync

The application layer can depend on domain components and stdlib only — no infrastructure or third-party vendor libraries.

Configuration: .go-arch-lint.yml at project root defines:

• 18 components across 4 layers (4 domain, 1 app, 11 infra, 2 interfaces)
• Dependency rules for each component
• Vendor allowlist
• Exclusions for test files

Benefits over depguard:

• Full dependency graph validation (not just domain layer)
• Component-aware rules that scale with architecture
• Infrastructure-application separation enforcement
• Prevents coupling violations across all layers
• Automatic CI integration via make quality target

Modern Go Quality Linters (Late-2025)#

gofumpt - Stricter Formatting#

Purpose: Stricter formatter than gofmt with deterministic rules (extra blank lines, import grouping).

Example violation:

import (
    "fmt"
    "github.com/awf-project/cli/internal/domain"
    "os"
)

Fix (gofumpt adds grouping):

import (
    "fmt"
    "os"

    "github.com/awf-project/cli/internal/domain"
)

Configuration: Runs automatically via make fmt. No manual configuration needed.

gocognit - Cognitive Complexity#

Purpose: Measures how difficult a function is to understand (nesting, conditionals, loops).

Threshold: 15 (strict - default is 30)

Example violation:

func ProcessWorkflow(w *Workflow) error {
    for _, step := range w.Steps {
        if step.Type == "parallel" {
            for _, branch := range step.Branches {
                if branch.Condition != "" {
                    if EvalCondition(branch.Condition) {
                        // ... nested logic continues
                    }
                }
            }
        } else if step.Type == "loop" {
            // ... more nesting
        }
    }
    return nil
}
// Cognitive complexity: 18 (exceeds 15)

Fix: Extract helper functions:

func ProcessWorkflow(w *Workflow) error {
    for _, step := range w.Steps {
        if err := processStep(step); err != nil {
            return err
        }
    }
    return nil
}

func processStep(step Step) error {
    switch step.Type {
    case "parallel":
        return processParallelStep(step)
    case "loop":
        return processLoopStep(step)
    default:
        return processSimpleStep(step)
    }
}

gocritic - Advanced Static Analysis#

Purpose: 100+ checks for bugs, style issues, and performance problems.

Enabled check categories:

• diagnostic - Bug detection (nil dereference, type assertions)
• style - Idiomatic Go (range loop optimizations, assignment ops)
• performance - Efficiency improvements (unnecessary allocations)

Example violation:

if _, err := validate(); err != nil {  // ❌ Nested if
    return err
}

Fix:

if _, err := validate(); err != nil {
    return err  // ✅ gocritic prefers this style
}

exhaustive - Enum Switch Exhaustiveness#

Purpose: Ensures switch statements on enums handle all cases.

Example violation:

type Status int
const (
    StatusPending Status = iota
    StatusRunning
    StatusCompleted
    StatusFailed
)

func Handle(status Status) {
    switch status {
    case StatusPending:
        // ...
    case StatusRunning:
        // ...
    // ❌ Missing StatusCompleted, StatusFailed
    }
}

Fix:

func Handle(status Status) {
    switch status {
    case StatusPending:
        // ...
    case StatusRunning:
        // ...
    case StatusCompleted:
        // ...
    case StatusFailed:
        // ...
    // ✅ All cases handled
    }
}

Configuration: default-signifies-exhaustive: true - Allows default: to satisfy exhaustiveness.

noctx - HTTP Context Requirements#

Purpose: Ensures HTTP requests include context for cancellation/timeouts.

Example violation:

req, _ := http.NewRequest("GET", url, nil)  // ❌ No context
resp, err := client.Do(req)

Fix:

req, _ := http.NewRequestWithContext(ctx, "GET", url, nil)  // ✅ Has context
resp, err := client.Do(req)

prealloc - Slice Capacity Optimization#

Purpose: Detects slices that could be preallocated with known capacity.

Example violation:

var results []Result  // ❌ Will grow multiple times
for _, item := range items {
    results = append(results, Process(item))
}

Fix:

results := make([]Result, 0, len(items))  // ✅ Preallocated capacity
for _, item := range items {
    results = append(results, Process(item))
}

wrapcheck - Error Wrapping at Boundaries#

Purpose: Enforces error wrapping when crossing package boundaries.

Example violation:

// internal/interfaces/cli/run.go
result, err := workflowService.Execute(ctx, workflow)
if err != nil {
    return err  // ❌ Error crosses boundary unwrapped
}

Fix:

result, err := workflowService.Execute(ctx, workflow)
if err != nil {
    return fmt.Errorf("execute workflow: %w", err)  // ✅ Wrapped with context
}

Configuration: ignorePackageGlobs: ["**/internal/**"] - Allows unwrapped errors within internal packages.

Common Issues#

“Cognitive complexity 18 of func exceeds max of 15”#

Problem: Function has too much nesting/branching.

Solution:

1. Extract nested blocks into helper functions
2. Replace nested if/else with early returns
3. Use switch statements instead of chained if/else if

Example:

// Before (complexity 18)
func Validate(w *Workflow) error {
    if w.Name == "" {
        return errors.New("name required")
    } else {
        if len(w.States) == 0 {
            return errors.New("states required")
        } else {
            for _, state := range w.States {
                if state.Type == "step" {
                    if state.Command == "" {
                        return errors.New("command required")
                    }
                }
            }
        }
    }
    return nil
}

// After (complexity 8)
func Validate(w *Workflow) error {
    if w.Name == "" {
        return errors.New("name required")
    }
    if len(w.States) == 0 {
        return errors.New("states required")
    }
    return validateStates(w.States)
}

func validateStates(states []State) error {
    for _, state := range states {
        if err := validateState(state); err != nil {
            return err
        }
    }
    return nil
}

“Missing cases in switch of type”#

Problem: Switch on enum doesn’t handle all cases.

Solution:

1. Add missing cases explicitly
2. Add default: case if intentionally ignoring some values

Example:

// Before
switch status {
case StatusRunning:
    return "running"
}

// After (explicit)
switch status {
case StatusRunning:
    return "running"
case StatusCompleted:
    return "completed"
case StatusFailed:
    return "failed"
default:
    return "unknown"
}

“Error return value of fmt.Errorf should be wrapped with %w”#

Problem: Using %v instead of %w breaks error chains.

Solution: Always use %w for errors:

// Before
return fmt.Errorf("failed: %v", err)

// After
return fmt.Errorf("failed: %w", err)

“Subprocess launched with variable (G204)”#

Problem: gosec detects potential command injection.

Solution:

1. If intentional (like AWF’s shell executor), add //nolint:gosec // G204 intentional
2. Otherwise, use ShellEscape() from pkg/interpolation

“File inclusion via variable (G304)”#

Problem: gosec detects potential path traversal.

Solution:

1. Validate file paths against allowed directories
2. Use filepath.Clean() to normalize paths
3. Add //nolint:gosec // G304 validated if checks are in place

“Append result not assigned to the same slice”#

Problem: Forgetting that append returns new slice.

Solution:

// Before
append(items, newItem)  // ❌ Result discarded

// After
items = append(items, newItem)  // ✅ Assigned

Best Practices#

When to Use //nolint#

Use //nolint directives sparingly and only when:

1. Linter is wrong - False positive that can’t be fixed
2. Intentional violation - Pattern required by design (e.g., shell executor using variable commands)
3. External constraint - Third-party API forces non-idiomatic code

Format:

//nolint:lintername // Reason for suppression
cmd := exec.Command("sh", "-c", userCmd)  // G204 intentional - shell executor

Good reasons:

• G204 intentional - shell executor design
• G304 validated - path checked against allowlist
• gocognit - Cobra command setup legitimately complex

Bad reasons:

• TODO fix later - Fix now or file issue
• linter is annoying - Linter is usually right
• No comment - Always explain

Rejected Linters Rationale#

Some popular linters are intentionally not enabled because they conflict with CLI tool patterns:

funlen - Function Length#

Why rejected: CLI command handlers are legitimately long due to:

• Cobra command setup (flags, descriptions, examples)
• Flag parsing and validation
• Business logic invocation
• Output formatting

Example: internal/interfaces/cli/run.go RunCmd is 150 lines but clear and readable. Breaking it into artificial helpers would reduce clarity.

gochecknoglobals - Global Variables#

Why rejected: CLI tools require package-level variables for:

• Logger instances (var logger *zap.Logger)
• Configuration paths (var configPath string)
• Cobra root command (var rootCmd *cobra.Command)

Pattern: Globals acceptable in cmd/ and internal/interfaces/cli/. Domain layer must not use globals.

wsl - Whitespace Linter#

Why rejected: Too opinionated and controversial. Team prefers gofumpt’s deterministic rules over wsl’s subjective whitespace requirements.

Cognitive Complexity Guidelines#

Target: Keep functions under complexity 15

Strategies:

1. Extract helpers: Move nested blocks to separate functions
2. Early returns: Reduce nesting with guard clauses
3. Table-driven logic: Replace nested switches with maps
4. Strategy pattern: Replace conditional chains with interfaces

Complexity Budget:

• 0-5: Simple functions (getters, formatters)
• 6-10: Standard business logic
• 11-15: Complex coordination (acceptable with clear structure)
• 16+: Refactor required (unless //nolint:gocognit justified)

Error Wrapping Strategy#

Rule: Wrap errors at every layer boundary with context:

// Domain layer (internal/domain/workflow)
func (s *Service) Validate(w *Workflow) error {
    if err := validateStates(w.States); err != nil {
        return fmt.Errorf("validate states: %w", err)
    }
    return nil
}

// Application layer (internal/application)
func (s *WorkflowService) Execute(ctx context.Context, w *Workflow) error {
    if err := s.validator.Validate(w); err != nil {
        return fmt.Errorf("validate workflow: %w", err)
    }
    return nil
}

// Interface layer (internal/interfaces/cli)
func RunCmd(cmd *cobra.Command, args []string) error {
    if err := service.Execute(ctx, workflow); err != nil {
        return fmt.Errorf("execute workflow %s: %w", workflow.Name, err)
    }
    return nil
}

Result: Error messages show full context:

execute workflow deploy: validate workflow: validate states: state "step1" missing command

Package Documentation (C056)#

All major packages include doc.go files providing discoverable documentation via go doc:

When to add package documentation:

• Creating a new package in domain, application, infrastructure, or interfaces layers
• Modifying exported APIs in existing documented packages

Documentation checklist:

• doc.go file created with // Package <name> comment
• Package purpose documented (1-2 sentences)
• Architecture role explained (which layer, what concern)
• Key types listed with brief descriptions
• At least one usage example provided
• go doc ./package produces readable output
• Internal package comments removed (single source of truth: doc.go)

Example minimal doc.go (concise style for simple packages):

// Package executor provides shell command execution.
//
// The ShellExecutor implements ports.CommandExecutor, enabling AWF to invoke
// arbitrary shell commands with environment context and process group management.
//
// Usage:
//
//	executor := executor.NewShellExecutor(logger)
//	result, err := executor.Execute(ctx, cmd)
//
// See [ports.CommandExecutor] for the interface definition.
package executor

For documentation depth guidelines, see Package Documentation Guide.

Pre-Commit Checklist#

Before committing, run:

make quality  # Runs lint + fmt + vet + test

If make lint reports issues:

make fix      # Auto-fix issues
make lint     # Verify remaining issues

For remaining issues:

1. Read error message carefully
2. Consult “Common Issues” section above
3. Fix manually or add justified //nolint
4. Verify fix: make lint

If adding a new package:

1. Create doc.go with package documentation
2. Run go doc ./<package> to verify output
3. Update related packages’ “See also” references if applicable

CI Pipeline Integration#

GitHub Actions runs quality checks via two workflows:

# .github/workflows/ci.yaml - Lint, test, build
- name: Lint
  uses: golangci/golangci-lint-action@v9
  with:
    version: latest

# .github/workflows/quality.yml - Architecture enforcement
- name: Architecture lint
  run: |
    go install github.com/fe3dback/go-arch-lint@latest
    go-arch-lint check --project-path . --arch-file .go-arch-lint.yml

Local == CI: If make quality passes locally, CI will pass.

Formatter Integration#

Always run formatter before linting:

make fmt   # Format with gofumpt
make lint  # Check for issues

Why? Some lint issues are auto-fixed by formatting (import order, blank lines).

References#

• golangci-lint documentation
• go-arch-lint repository
• gofumpt repository
• Effective Go
• Uber Go Style Guide
• AWF hexagonal architecture: docs/architecture/hexagonal.md
• Contributing guidelines: CONTRIBUTING.md