Go Linter Patterns And Common Fixes#
Go Linter Patterns And Common Fixes is a comprehensive guide to understanding and resolving common linting issues encountered in Go codebases, with specific patterns and solutions drawn from the CipherSwarmAgent project. This article documents the practical application of golangci-lint v2, a comprehensive linting aggregator that runs 71 different code analysis tools in parallel, covering error handling, security vulnerabilities, code quality, and style enforcement. The guide addresses frequently encountered issues across multiple linter categories including gocritic (code quality), gosec (security), revive (style), errcheck (error handling), contextcheck (context propagation), mnd (magic numbers), minmax (Go 1.21+ builtins), tparallel (test parallelization), and modernize (language updates).
A critical aspect of working with golangci-lint is understanding the strict //nolint: directive requirements enforced by the whyNoLint rule: every suppression must include an explanation, and linters operate independently (suppressing one does not affect others). The CipherSwarmAgent project employs a four-stage formatter pipeline (golines, gofumpt, goimports, gci) that can interact problematically with nolint directives, particularly when golines' 120-character line limit splits long lines and displaces inline comments.
This knowledge base serves as a definitive reference for engineers working in secure, non-Internet-connected environments such as cybersecurity labs where hashcat and similar tools are deployed. It provides concrete code examples showing both incorrect patterns that trigger linter warnings and the correct fixes, along with the rationale behind each rule. The patterns documented here reflect real-world solutions to maintaining code quality while meeting strict security and reliability requirements.
Configuration#
golangci-lint Setup#
The CipherSwarmAgent project configures golangci-lint v2 with 8 parallel workers and a 30-minute timeout for comprehensive codebase analysis. The configuration enables test file linting and uses readonly mode for module downloads to ensure deterministic builds in isolated environments. Key configuration decisions include excluding gosec rule G117, setting cyclop maximum complexity to 50 with a package average of 10.0, and allowing no-explanation nolint directives specifically for funlen, gocognit, and golines due to their frequent false positives.
Formatter Pipeline#
The project uses a four-stage formatter pipeline that processes code in sequence:
- golines: Configured with max-len 120 to break long lines for readability
- gofumpt: A stricter version of Go's standard gofmt formatter with additional style rules
- goimports: Automatically adds missing imports and removes unused ones
- gci: Controls the ordering of import statements
nolint Directive Standards#
Mandatory Requirements#
Every //nolint: directive must include an explanation enforced by gocritic's whyNoLint rule. This ensures that future maintainers understand why a particular linting rule was suppressed. Additionally, linters operate independently — suppressing revive does not automatically suppress staticcheck for the same issue. All linters that need suppression must be explicitly listed in comma-separated format.
Incorrect (missing explanation):
//nolint:errcheck
_ = someCall()
Correct:
//nolint:errcheck // Progress bar start failure not critical
_ = cpb.pool.Start()
Incorrect (incomplete suppression):
//nolint:revive // only suppresses revive, not staticcheck
type APIClient interface { }
Correct (multiple linters):
//nolint:errcheck,gosec // Error handler returns error for chaining; not needed here
cserrors.GetErrorHandler().Handle(ctx, err, opts)
Placement Rules and Decision Tree#
The placement of nolint directives depends on line length and formatter behavior:
Key placement considerations:
- Place nolint on separate line above when golines would split multi-line expressions
- Keep explanations short (under 120 characters total including code)
- Verify survival after formatting — formatters can strip nolint on APIError, APIClient, Error_
Separate Line Example (lib/zap/zap.go:89-90):
//nolint:errcheck // LogAndSendError handles logging+sending internally
_ = cserrors.LogAndSendError(ctx, "Error closing zap file", cerr, api.SeverityCritical, task)
Inline Example (lib/progress/progress_tracking.go:54):
_ = cpb.pool.Start() //nolint:errcheck // Progress bar start failure not critical
Common Linter Patterns and Fixes#
gocritic (Code Quality)#
The gocritic linter enforces code quality and idiomatic Go patterns. Key rules include:
emptyStringTest: Use s == "" instead of len(s) == 0 for direct string empty checks. This pattern is more readable and expresses intent clearly.
// Incorrect
if len(crackedHash.Plaintext) == 0 {
return errors.New("received empty cracked hash")
}
// Correct
if strings.TrimSpace(crackedHash.Plaintext) == "" {
return errors.New("received empty cracked hash")
}
Additional examples throughout the codebase: lib/config/config.go, lib/hashcat/params.go, lib/agent/agent.go
whyNoLint: Requires explanations on all nolint directives (see nolint Directive Standards section).
commentFormatting: A blank // line between a doc comment and a declaration breaks the linter's comment association — keep doc comments contiguous with their declarations without blank comment lines.
revive (Style Enforcement)#
The revive linter enforces Go style guidelines and naming conventions.
exported: Each exported constant requires its own individual doc comment in the format // ConstName is.... Group comments alone don't satisfy this requirement.
// Correct: individual doc comment
// MinStatusFields is the minimum number of elements required in hashcat status
// Progress and RecoveredHashes slices (current value and total).
const MinStatusFields = 2
stuttering: Revive flags type names that stutter with their package name (e.g., api.APIClient). However, intentional stutters may be necessary to avoid conflicts with code-generated types:
// lib/api/interfaces.go:85
type APIClient interface { //nolint:revive // stutter is intentional — avoids conflict with generated Client type
Tasks() TasksClient
Attacks() AttacksClient
}
// lib/api/errors.go:95
type APIError struct { //nolint:revive // stutter is intentional — avoids conflict with generated Client type
StatusCode int
Message string
}
errcheck (Error Handling)#
The errcheck linter ensures all error return values are checked. Three legitimate suppression patterns exist:
1. Compile-Time Interface Assertions (lib/api/errors.go:19-20):
var _ error = (*ErrorObject)(nil) //nolint:errcheck // compile-time interface assertion
2. Error Handling Delegation (lib/zap/zap.go:88-91):
defer func() {
if cerr := outFile.Close(); cerr != nil {
//nolint:errcheck // LogAndSendError handles logging+sending internally
_ = cserrors.LogAndSendError(ctx, "Error closing zap file", cerr, api.SeverityCritical, task)
}
}()
3. Non-Critical Operations (lib/progress/progress_tracking.go:54):
_ = cpb.pool.Start() //nolint:errcheck // Progress bar start failure not critical
Multiple Error Wrapping (Go 1.20+)#
Go 1.20+ supports wrapping multiple errors using fmt.Errorf("%w: %w", sentinel, inner), preserving both errors in the errors.Is/errors.As chain. This is critical for maintaining error context while allowing callers to check for specific sentinel errors.
// Incorrect: breaks error chain
return nil, fmt.Errorf("%w: %s", errCacheCorrupt, err.Error())
// Correct: preserves both errors
return nil, fmt.Errorf("%w: %w", errCacheCorrupt, err)
Pattern: Never use %s with .Error() when wrapping errors — it converts the error to a string and breaks the chain, preventing errors.Is() and errors.As() from working on the wrapped error.
gosec (Security Scanner)#
The gosec linter identifies security vulnerabilities in Go code. The CipherSwarmAgent project standardizes gosec suppressions with the pattern //nolint:gosec // GXX - reason, always including the specific rule ID for traceability.
G115: Integer Overflow Conversion#
Explicit bounds checking before type conversion prevents integer overflow vulnerabilities:
// lib/cracker/cracker.go:114-123
if pidValue < 0 || pidValue > math.MaxInt32 {
agentstate.Logger.Error("PID value out of int32 range", "pid", pidValue)
return true
}
pidRunning, err := process.PidExistsWithContext(
context.Background(),
int32(pidValue), //nolint:gosec // G115 - bounds checked above
)
Pattern: Validate value range before conversion, then reference the validation in the nolint comment.
G703: File Operations After Path Validation#
Path traversal protection through validation before file operations (lib/hashcat/params.go:240-242):
//nolint:gosec // G703 - validated by safePath
if _, err := os.Stat(wordList); os.IsNotExist(err) {
return nil, fmt.Errorf("%w: %s", ErrWordlistNotOpened, wordList)
}
Pattern: Use helper functions to validate paths before file operations, then document the validation method.
G501/G401: MD5 for Non-Cryptographic Checksums#
MD5 usage for file integrity verification (not cryptographic security):
"crypto/md5" //nolint:gosec // G501 - checksum verification
h := md5.New() //nolint:gosec // G401 - MD5 used for file integrity check, not security
Pattern: Clearly state that MD5 is used for checksum/integrity verification, not cryptographic purposes.
G702: Command Execution with Validated Paths#
Command execution with internally-configured binary paths:
return &Session{
proc: exec.CommandContext( //nolint:gosec // G702 - binary path from internal config, not user input
ctx,
binaryPath,
args...),
Pattern: Document that the command path comes from internal configuration, not user input.
contextcheck (Context Propagation)#
The contextcheck linter flags functions that don't propagate context to downstream calls. Two legitimate scenarios exist where context propagation is impossible or undesirable:
1. Third-Party APIs Without Context Support (lib/task/manager.go:162-163):
//nolint:contextcheck // NewHashcatSession does not accept context
sess, err := hashcat.NewHashcatSession(strconv.FormatInt(attack.Id, 10), jobParams)
When calling libraries or external APIs that don't accept context parameters, propagation is impossible.
2. Must-Complete Operations After Cancellation (lib/agent/agent.go:352-353):
//nolint:contextcheck // must-complete: prevents task starvation on server
taskMgr.AbandonTask(context.Background(), t)
Certain cleanup operations must complete even when the parent context is cancelled. Use context.Background() and document the must-complete requirement.
mnd (Magic Number Detector)#
The mnd linter identifies unnamed numeric constants that should be extracted for clarity and maintainability. Extract magic numbers to named constants with descriptive documentation:
// Incorrect: magic numbers in code
if len(charsets) > 4 {
return errors.New("too many charsets")
}
args := make([]string, 0, 32)
// Correct: named constants
const (
maxCharsets = 4 // Maximum custom charsets allowed by hashcat
defaultArgsCapacity = 32 // Default slice capacity for command arguments
maskArgsCapacity = 6 // Expected capacity for mask-specific arguments
)
if len(charsets) > maxCharsets {
return errors.New("too many charsets")
}
args := make([]string, 0, defaultArgsCapacity)
Additional examples: MinStatusFields = 2, channelBufferSize = 5, epsilon = 1e-4, maxBenchmarkRetries = 10
minmax (Manual Comparisons)#
Go 1.21+ introduced built-in min() and max() functions. Use these builtins instead of manual comparisons:
// Incorrect: manual min comparison
multiplier := consecutiveFailures
if maxBackoffMultiplier < consecutiveFailures {
multiplier = maxBackoffMultiplier
}
// Correct: use min() builtin (Go 1.21+)
func calculateHeartbeatBackoff(
baseInterval time.Duration,
consecutiveFailures int,
maxBackoffMultiplier int,
) time.Duration {
// Cap the multiplier to prevent overflow
multiplier := min(consecutiveFailures, maxBackoffMultiplier)
// Exponential backoff: baseInterval * 2^multiplier
return baseInterval * time.Duration(1<<multiplier)
}
The builtin functions are more concise and clearly express intent.
tparallel (Test Parallelization)#
The tparallel linter ensures proper test parallelization setup. When subtests call t.Parallel(), the parent test must also call it:
// Correct pattern
func TestErrorObject_Error(t *testing.T) {
t.Parallel() // Parent test calls t.Parallel()
tests := []struct {
name string
err ErrorObject
expected string
}{
// test cases...
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel() // Subtests also call t.Parallel()
assert.Equal(t, tt.expected, tt.err.Error())
})
}
}
This pattern enables concurrent execution of test cases within a test suite, significantly reducing test execution time.
Linter Auto-Fix Behavior#
The CipherSwarmAgent .golangci.yml configuration has fix: true enabled, which automatically transforms manual error assertions into testify-specific helpers:
// Before linter auto-fix
import "errors"
assert.True(t, errors.Is(err, sentinel))
// After linter auto-fix
assert.ErrorIs(t, err, sentinel) // errors import automatically removed
Pattern: Don't manually add errors imports for testify-only assertions. The linter will transform verbose patterns and remove unused imports automatically.
Mock Session Limitations#
The hashcat.NewTestSession function creates sessions with proc=nil, meaning calling Start() on these sessions will panic. Tests requiring actual benchmark execution (like runBenchmarkTask or UpdateBenchmarks) cannot be unit tested without interface refactoring.
Pattern: Mock at higher abstraction levels or use integration tests when hashcat session execution is required. Unit tests should focus on logic that doesn't require process execution.
bodyclose (HTTP Response Body Management)#
The bodyclose linter ensures all HTTP response bodies are properly closed to prevent resource leaks. In test code using httpmock, manually constructed *http.Response passed to httpmock.ResponderFromResponse triggers bodyclose warnings:
// Incorrect: triggers bodyclose lint
responder := httpmock.ResponderFromResponse(&http.Response{
StatusCode: http.StatusOK,
Header: http.Header{"Content-Type": []string{"application/json"}},
Body: httpmock.NewRespBodyFromString(jsonData),
})
// Correct: use NewJsonResponderOrPanic for JSON responses
responder := httpmock.NewJsonResponderOrPanic(http.StatusOK, responseObject)
Pattern: Use httpmock.NewJsonResponderOrPanic for JSON mock responses. For non-JSON responses, use httpmock.NewStringResponder or other helper constructors that handle body lifecycle correctly.
httpmock URL Pattern Matching#
When mocking HTTP endpoints with httpmock, URL patterns must match the generated client paths in client.gen.go, not the Go method names. OpenAPI code generation uses URL paths from the API specification, which may differ from the derived method names.
// Incorrect: assuming method name matches URL path
// Method: client.Tasks().SetTaskAbandoned(taskID)
abandonPattern := regexp.MustCompile(`/tasks/\d+/set_abandoned$`)
// Correct: check client.gen.go for actual URL
// Method: client.Tasks().SetTaskAbandoned(taskID)
// Actual URL from OpenAPI spec: POST /tasks/{id}/abandon
abandonPattern := regexp.MustCompile(`/tasks/\d+/abandon$`)
Pattern: Always verify the actual URL path in client.gen.go when registering httpmock responders. Mismatched patterns result in "no responder found" errors that are difficult to diagnose.
errorsastype (Go 1.26+ Error Handling)#
The errorsastype linter suggests replacing errors.As(err, &target) with the generic errors.AsType[T]() function introduced in Go 1.26+:
// Before (Go 1.20+)
var apiErr *api.APIError
if errors.As(err, &apiErr) {
// handle apiErr
}
// After (Go 1.26+)
if apiErr := errors.AsType[*api.APIError](err); apiErr != nil {
// handle apiErr
}
Pattern: Adopt this pattern when modifying code that already uses errors.As(). Don't refactor unrelated lines solely for this change.
modernize (Language Feature Updates)#
The modernize linter identifies patterns that can be updated to use modern Go language features. One key pattern is selective use of omitempty on JSON struct tags:
// Modern pattern: remove omitempty from struct-typed and slice fields
type Params struct {
AttackMode int64 `json:"attack_mode"` // No omitempty - required field
HashType int64 `json:"hash_type"` // No omitempty - required field
HashFile string `json:"hash_file"` // No omitempty - required field
Mask string `json:"mask,omitempty"` // Has omitempty - optional field
MaskCustomCharsets []string `json:"mask_custom_charsets"` // No omitempty on slice
// ...
}
Rule: Remove omitempty from struct-typed and slice fields. In modern Go, zero-value slices serialize correctly as empty arrays, and struct fields serialize as objects even when empty.
Formatter Interaction Gotchas#
golines and nolint Comment Displacement#
The golines formatter enforces a 120-character line limit by automatically splitting long lines. This creates three critical problems with //nolint: directives:
Problem 1: Long Lines Split, Moving Inline Comments#
When a line exceeds 120 characters and includes an inline //nolint: comment, golines splits the line and moves the comment, causing the linter to flag the original line.
// Before golines
result := someVeryLongFunctionCall(arg1, arg2, arg3, arg4) //nolint:errcheck // reason here
// After golines splits it
result := someVeryLongFunctionCall(
arg1, arg2, arg3, arg4) //nolint:errcheck // reason here (now on wrong line!)
Solution: Keep nolint explanations short so the total line stays under 120 characters.
Problem 2: Multi-Line Expression Splitting#
When golines splits multi-line expressions, even short inline //nolint: comments get displaced.
// Incorrect: inline comment will be displaced
_ = cserrors.LogAndSendError(ctx, "Error closing zap file", cerr, api.SeverityCritical, task) //nolint:errcheck // reason
// Correct: separate line above
//nolint:errcheck // LogAndSendError handles logging+sending internally
_ = cserrors.LogAndSendError(ctx, "Error closing zap file", cerr, api.SeverityCritical, task)
Solution: Place nolint directives on a separate line above multi-line expressions.
Problem 3: Formatter Stripping nolint Comments#
Solution: After running golangci-lint run --fix, verify that nolint directives survive on these specific types. Manually re-add them if necessary.
Compiler Directive Conflicts#
//go:fix directives conflict with the gocheckcompilerdirectives linter because //go:fix is not a standard Go compiler directive.
Solution: Avoid adding //go:fix directives entirely. Use standard Go directives like //go:inline, //go:noinline, //go:build, etc.
IDE and Editor Post-Save Refactoring#
IDE and editor post-save hooks (goimports, golines, LSP actions) may refactor code beyond the original edit. Common issues include:
- Return type changes: Editor may infer different return types when modifying function bodies
- Removing unused sentinels: When changing error handling, sentinel errors may be removed if no longer referenced
- Import reordering: gci and goimports can restructure imports, affecting line numbers
Solution: Always verify the file state after saves before making dependent edits. Review the entire diff after significant changes, especially when editing error handling with sentinel values.
Best Practices Summary#
- Always include explanations for nolint directives — enforced by gocritic's whyNoLint rule
- List all linters when suppressing multiple issues — linters operate independently
- Place nolint above line for multi-line expressions — prevents golines displacement
- Keep explanations short (under 120 chars total) — avoids line-splitting issues
- Verify nolint survival after formatting — especially on APIError, APIClient, Error_
- Avoid
//go:fixdirectives — conflicts with gocheckcompilerdirectives - Keep doc comments contiguous with declarations — no blank
//lines between them - Document each exported constant individually — revive requirement
- Use
context.Background()for must-complete operations — when parent context is cancelled - Suppress contextcheck only when callee cannot accept context — third-party APIs or must-complete operations
- Use
s == ""instead oflen(s) == 0— gocritic emptyStringTest preference - Extract magic numbers to named constants — improves maintainability
- Use
min()/max()builtins (Go 1.21+) — clearer than manual comparisons - Include specific gosec rule IDs in nolint comments — e.g.,
// G115 - bounds checked above - Remove
omitemptyfrom struct-typed and slice JSON fields — modern Go serialization - Use
fmt.Errorf("%w: %w", sentinel, inner)for multiple error wrapping — preserves error chains (Go 1.20+) - Don't add
errorsimports for testify assertions — linter auto-fixes toassert.ErrorIsand removes unused imports - Mock at higher levels when testing hashcat session logic —
NewTestSessioncreatesproc=nilsessions that panic onStart() - Use
httpmock.NewJsonResponderOrPanicfor JSON mock responses — avoids bodyclose lint warnings - Verify httpmock URL patterns match
client.gen.gopaths — OpenAPI-generated paths differ from Go method names - Adopt
errors.AsType[T]()when touching error handling code (Go 1.26+) — don't refactor unrelated lines - Verify file state after IDE/editor saves — post-save hooks may refactor beyond the original edit
Working in Air-Gapped Environments#
For deployment in secure, non-Internet-connected environments (common in cybersecurity labs using hashcat):
- Readonly module downloads: The configuration uses readonly mode to ensure deterministic builds without network access
- Vendor dependencies: Consider vendoring all Go modules before deployment to isolated networks
- IDE diagnostics vs. authoritative linting: IDE-based linting suggestions (often marked with ★) are informational only —
golangci-lint runis the authoritative source - Pre-commit validation: Run
golangci-lint run --fixbefore committing to catch and auto-fix issues - CI/CD integration: Configure golangci-lint in your build pipeline with the same configuration used locally
IDE vs. Command Line Linting#
Important distinction for deployment teams: IDE linting integrations (VS Code, GoLand, etc.) may show different warnings than golangci-lint. The command-line output from golangci-lint run is authoritative. IDE diagnostics serve as helpful suggestions during development but should not be treated as errors. Always validate with golangci-lint run before finalizing code.
See Also#
- golangci-lint Official Documentation — Complete reference for golangci-lint configuration and linters
- CipherSwarmAgent GOTCHAS.md — Centralized documentation of linting gotchas and edge cases
- CipherSwarmAgent AGENTS.md — AI agent development guidelines including gosec nolint standards
- Static Analysis in Go — Related techniques for code quality enforcement
- Go Testing Patterns — Broader context for tparallel and test organization
- Context Propagation in Go — Deep dive into Go context patterns and best practices
Relevant Code Files#
| File | Purpose | Key Patterns |
|---|---|---|
| .golangci.yml | golangci-lint v2 configuration | 71 enabled linters, formatter pipeline, nolintlint config |
| GOTCHAS.md | Linting gotchas documentation | nolint requirements, formatter interactions, doc comment rules |
| AGENTS.md | AI development guidelines | gosec nolint style standards (GXX format) |
| lib/api/errors.go | API error types | revive stutters, compile-time interface assertions |
| lib/task/errors.go | Task error handling | errcheck,gosec combined directives (7 instances) |
| lib/cracker/cracker.go | Hash cracker process management | gosec G115 integer overflow with bounds checking |
| lib/hashcat/params.go | Hashcat parameter building | mnd constants, gosec G703 path validation, modernize omitempty |
| lib/agent/agent.go | Agent lifecycle management | minmax builtins, contextcheck must-complete operations |
| lib/display/display.go | Status display constants | mnd constants with doc comments |
| lib/zap/zap.go | Zap file handling | Separate line nolint for error delegation |
| lib/progress/progress_tracking.go | Progress bar management | Inline nolint for non-critical operations |
| lib/api/errors_test.go | API error tests | tparallel parent+subtest pattern |