Go Linting Gotchas#

Lead Section#

Go Linting Gotchas documents critical configuration pitfalls, nolint directive requirements, and formatter interaction issues specific to golangci-lint v2 in the CipherSwarmAgent project. This knowledge base article serves as a reference for avoiding common linting failures, understanding directive interactions, and managing the complexities of running multiple formatters alongside strict linter configurations.

The CipherSwarmAgent project maintains a comprehensive 71-linter configuration covering error handling, code quality, performance, and style enforcement. Key challenges arise from:

• Linter-specific directive syntax where suppressing one linter does NOT suppress related linters
• Formatter interactions with nolint comments, particularly golines (max-len 120)
• Mandatory explanation requirements enforced by gocritic's whyNoLint rule

These gotchas were discovered through extensive refactoring efforts that removed approximately 27 of 30 contextcheck directives by implementing proper context propagation patterns throughout the codebase.

Core Linting Configuration#

Understanding golangci-lint v2#

golangci-lint is a comprehensive linting aggregator for Go that runs multiple code analysis tools (called "linters") in parallel. Think of it as a quality control system that automatically inspects your Go code for potential bugs, security vulnerabilities, style inconsistencies, and performance issues. In the CipherSwarmAgent project, this system is configured to run 71 different linters simultaneously, each checking for specific types of problems.

The .golangci.yml configuration file defines which linters are enabled and how they behave. The configuration enables 8 parallel workers with a 30-minute timeout, allowing the system to check multiple files simultaneously for faster analysis. It also enables test file linting and uses readonly mode for module downloads, ensuring that even test code follows quality standards and that dependency downloads don't modify local state.

Enabled Linter Categories:

• Error Handling: errcheck, errname, errorlint, errchkjson, nilerr, nilnesserr, nilnil - These linters verify that all error return values are properly checked and handled. In Go, functions can return multiple values, and errors are typically returned as the last value. These linters ensure you don't accidentally ignore critical error conditions.

• Security: gosec (with G117 excluded) - The gosec linter specifically looks for common security vulnerabilities like SQL injection, command injection, weak cryptography, and path traversal attacks. G117 is disabled because it incorrectly flags the APIToken configuration field as a potential secret exposure.

• Code Quality: staticcheck, revive, gocritic, govet - These linters analyze code structure, naming conventions, and common mistakes. They catch issues like unused variables, inefficient code patterns, and violations of Go's standard naming conventions.

• Complexity: cyclop (max complexity 50, package average 10.0) - The cyclop linter measures code complexity by counting decision points (if statements, loops, switch cases). Functions with too many decision points become difficult to test and maintain. The configuration allows individual functions up to complexity 50, but requires the package average to stay below 10.0.

• Context Handling: contextcheck, bodyclose, durationcheck - Context is Go's mechanism for managing request lifecycles, deadlines, and cancellation signals. These linters ensure that context objects are properly propagated through function calls and that resources like HTTP response bodies are properly closed.

Notable Disabled Linters:

• gocyclo - Disabled because cyclop provides similar but more configurable complexity checking
• exhaustive, exhaustruct, funlen, gocognit, nestif - Temporarily disabled until the 1.0 release to manage technical debt in phases
• gochecknoglobals - Disabled because it conflicts with the Cobra command-line framework used by the project

Formatter Pipeline#

In addition to linters that analyze code for problems, the project uses formatters that automatically restructure code to match style guidelines. The configuration enables four formatters: golines, gofumpt, goimports, and gci:

• golines: Configured with max-len 120, this formatter breaks long lines into multiple shorter 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

The golines formatter is the source of several critical gotchas documented below, as it can inadvertently move or remove special linter suppression comments when reformatting code.

Compiler Directive Conflicts#

gocheckcompilerdirectives and //go#

Problem: //go:fix inline directives conflict with the gocheckcompilerdirectives linter.

Solution: Avoid adding //go:fix directives entirely. The CipherSwarmAgent codebase contains no instances of such directives, demonstrating successful adherence to this guideline.

Technical Context: The gocheckcompilerdirectives linter validates that compiler directives match the official Go toolchain's recognized set. //go:fix is not a standard Go compiler directive, causing validation failures.

Context Propagation Issues#

Understanding Context in Go#

In Go, a context is an object that carries deadlines, cancellation signals, and request-scoped values across API boundaries. Think of it like a "job ticket" that follows work as it moves through different parts of your system. When you need to stop work (because a user cancelled a request, or a timeout expired), the context provides a clean way to signal that cancellation to all related operations.

contextcheck Linter#

Problem: The contextcheck linter flags functions that don't propagate context to downstream calls. This is usually correct — you should pass context along. However, two legitimate scenarios exist where context propagation is impossible or undesirable.

Solution: Use //nolint:contextcheck // reason only in these specific cases:

Scenario 1: Third-party APIs Without Context Support#

Some external libraries or APIs were written before Go's context package became standard (introduced in Go 1.7), or they simply don't accept context parameters. The hashcat library wrapper is one example:

//nolint:contextcheck // NewHashcatSession does not accept context
sess, err := hashcat.NewHashcatSession(strconv.FormatInt(attack.Id, 10), jobParams)

Source: lib/task/manager.go:162-163

In this case, the NewHashcatSession function signature cannot be changed (it's provided by an external library), so context propagation is technically impossible.

Scenario 2: Must-Complete Operations After Cancellation#

When a parent context is cancelled (for example, the user pressed Ctrl+C to stop the agent), some cleanup operations must still complete to maintain system consistency. For these operations, use context.Background() to create a fresh, uncancelled context:

//nolint:contextcheck // must-complete: prevents task starvation on server
taskMgr.AbandonTask(context.Background(), t)

Source: lib/agent/agent.go:352-353

Even though the main agent context has been cancelled, the task abandonment notification must reach the server. Otherwise, the server continues to believe the agent is working on that task, leading to task starvation (no other agent will pick it up).

Architectural Pattern: The project uses context.Background() for "must-complete" operations like cleanup, shutdown notifications, and task abandonment that should execute even after parent context cancellation. This pattern appears in approximately 3 locations across the codebase after a refactoring effort removed 27 of 30 contextcheck suppressions.

Documentation Comment Requirements#

revive Linter#

Problem 1: Group Comments Fail for Exported Constants

revive requires each exported constant to have its own // ConstName is... doc comment. A group comment alone doesn't satisfy it.

Compliant Example:

// MinStatusFields is the minimum number of elements required in hashcat status
// Progress and RecoveredHashes slices (current value and total).
const MinStatusFields = 2

Source: lib/display/display.go:17-19

Problem 2: Blank Lines Break Comment Association

A blank // line between a doc comment and a type/func declaration breaks the linter's comment association — keep them contiguous.

errorsastype Linter (Go 1.26+)#

Problem: The errorsastype linter suggests replacing the traditional errors.As(err, &target) pattern with the generic function errors.AsType[T]() introduced in Go 1.26+.

Guideline: Adopt this modern pattern when touching affected code, but don't refactor unrelated lines purely for this change. This allows gradual migration to the newer API without creating unnecessary churn in stable code.

Traditional pattern:

var apiErr *api.APIError
if errors.As(err, &apiErr) {
    // handle API error
}

Modern pattern (Go 1.26+):

if apiErr := errors.AsType[*api.APIError](err); apiErr != nil {
    // handle API error
}

Problem 3: Intentional Name Stutters

The project uses intentional name stutters to avoid conflicts with generated types:

type APIError struct { //nolint:revive // stutter is intentional — avoids conflict with generated Client type

Source: lib/api/errors.go:95

Linter Independence and Directive Stacking#

Critical Pattern: Multiple Linters, Multiple Directives#

The Problem: Each linter in golangci-lint operates independently. When you suppress one linter with //nolint:lintername, other linters still run and may flag the same line of code for different (but related) reasons. This is a common source of confusion because developers assume that suppressing one linter will suppress all warnings on that line.

//nolint:revive does NOT suppress staticcheck for the same issue — you must list all linters that need suppression.

Correct Syntax:

//nolint:revive,staticcheck // reason applying to both

Note the comma-separated list without spaces: revive,staticcheck.

Real-World Example: errcheck and gosec Interaction#

The most common linter combination requiring dual suppression is errcheck and gosec. When you intentionally ignore an error return value (using _ to discard it), two linters flag this:

• errcheck: Flags the ignored error return value
• gosec: Flags unhandled errors as security issue G104

Both must be suppressed together:

//nolint:errcheck,gosec // Error handler returns error for chaining; not needed here
cserrors.GetErrorHandler().Handle(ctx, err, opts)

Source: lib/task/errors.go:22-23

This pattern appears 7 times in task/errors.go alone, demonstrating how frequently both linters must be suppressed together for error handler calls that internally log errors.

Why This Happens#

The error handler Handle() function returns an error for chaining (allowing callers to decide whether to log, return, or ignore), but in cleanup paths the error has already been logged internally. The underscore assignment _ = cserrors.GetErrorHandler().Handle(...) tells the compiler "I'm aware this returns a value and I'm intentionally discarding it," but two separate linters both flag this pattern from different perspectives.

Mandatory Explanation Requirements#

gocritic whyNoLint Rule#

Problem: gocritic whyNoLint rule requires every //nolint: directive to include an explanation. Bare //nolint:linter directives fail CI.

Incorrect:

//nolint:errcheck
_ = someCall()

Correct:

//nolint:errcheck // Progress bar start failure not critical
_ = cpb.pool.Start()

Source: lib/progress/progress_tracking.go:54

Configuration Note: The .golangci.yml exempts golines from requiring explanations due to recurring formatter displacement issues:

nolintlint:
  allow-no-explanation: [funlen, gocognit, golines]

Common errcheck Patterns#

The codebase demonstrates three distinct errcheck suppression patterns:

1. Compile-Time Interface Assertions#

var _ error = (*ErrorObject)(nil) //nolint:errcheck // compile-time interface assertion

Source: lib/api/errors.go:19-20

2. Error Handling Delegation#

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)
    }
}()

Source: lib/zap/zap.go:88-91

3. Non-Critical Operations#

_ = cpb.pool.Start() //nolint:errcheck // Progress bar start failure not critical

Source: lib/progress/progress_tracking.go:54

gosec Security Linter Patterns#

G703: File Operations After Path Validation#

//nolint:gosec // G703 - validated by safePath
if _, err := os.Stat(wordList); os.IsNotExist(err) {
    return nil, fmt.Errorf("%w: %s", ErrWordlistNotOpened, wordList)
}

Source: lib/hashcat/params.go:240-242

Important Note on os.IsNotExist: os.IsNotExist guards can mask wrong-path bugs in cleanup code. When os.Remove returns "file not found," it may mean the path is wrong rather than the file being already cleaned. Always verify the resolved absolute path during debugging. See the hashcat session file cleanup solution for a case study where os.IsNotExist silently masked cleanup targeting CWD instead of the actual session directory.

G501/G401: MD5 for Checksums (Non-Cryptographic)#

"crypto/md5" //nolint:gosec // G501 - checksum verification

h := md5.New() //nolint:gosec // G401 - MD5 used for file integrity check, not security

Source: lib/downloader/downloader.go:6, 321

G702: Command Execution with Validated Paths#

return &Session{
    proc: exec.CommandContext( //nolint:gosec // G702 - binary path from internal config, not user input
        ctx,
        binaryPath,
        args...),

Source: lib/hashcat/session.go:107-110

Formatter Interaction Gotchas#

golines and nolint Comment Displacement#

The golines formatter (max-len 120) automatically breaks long lines into multiple shorter lines for readability. While this improves code formatting, it creates three critical problems with //nolint: directives that must be carefully managed.

Problem 1: Long Lines Split, Moving Comments#

When a line exceeds 120 characters and includes an inline //nolint: comment, golines splits the line and moves the comment. The comment ends up on a different line than the code it was meant to suppress, causing the linter to flag the original line.

Before golines:

someVeryLongFunctionCall(param1, param2, param3, param4) //nolint:errcheck // explanation here

After golines (broken):

someVeryLongFunctionCall(
    param1, param2, param3, param4) //nolint:errcheck // explanation here

The //nolint:errcheck is now on the closing parenthesis line, not the function call line, so it doesn't suppress the linter warning.

Solution: Keep nolint reasons short so the total line stays under 120 characters. If the line is inherently long (many parameters, long function names), use the workaround in Problem 2.

Problem 2: Multi-Line Expression Splitting#

When golines splits multi-line expressions, even short inline //nolint: comments get displaced. The formatter places the comment on the last line of the expression, which is often not the line being flagged by the linter.

Solution: Place nolint as a standalone comment on the line above:

//nolint:errcheck // LogAndSendError handles logging+sending internally
_ = cserrors.LogAndSendError(ctx, "Error closing zap file", cerr, api.SeverityCritical, task)

Source: lib/zap/zap.go:89-90

By placing the directive on its own line immediately before the flagged code, it remains associated with the correct statement regardless of how golines reformats the code.

Problem 3: Formatter Stripping nolint Comments#

In rare cases, //nolint:revive on APIError, Error_, and APIClient can be stripped by golines or other formatters. This appears to be a bug in the formatter interaction where comments on type declarations can be lost during reformatting.

Solution: Verify that nolint directives survive after running formatters. After running golangci-lint run --fix or just fmt, check these specific files:

• lib/api/errors.go for APIError and Error_ suppressions
• lib/api/interfaces.go for APIClient suppressions

If the directives are missing, manually re-add them and consider using the above-line placement strategy instead of inline comments.

Visual Diagram: golines Processing Flow#

CI Integration and Deployment Considerations#

just ci-check Behavior#

The CipherSwarmAgent project uses just (a command runner similar to make) for automation. The just ci-check command includes a go fix -diff dry-run whose output is informational only, not a failure.

What this means: When you run just ci-check in your development environment or CI pipeline, you may see output from go fix suggesting code improvements. These suggestions are informational warnings, not build-breaking errors. The build will succeed even if go fix reports potential improvements.

This design allows the team to gradually adopt go fix recommendations without blocking deployment, which is particularly important in secure lab environments where rapid iteration may be necessary.

Test Mocking Patterns#

httpmock URL Pattern Matching#

Problem: When mocking HTTP endpoints for OpenAPI-generated clients, URL patterns must match the actual API paths defined in the generated client code (client.gen.go), not the Go method names. This is a common source of test failures when the API path and method name differ.

Example Mismatch:

• Go method name: SetTaskAbandoned
• Actual API path: /tasks/{id}/abandon (not /tasks/{id}/set_abandoned)

Correct Pattern:

// Mock the actual API endpoint path from client.gen.go
abandonPattern := regexp.MustCompile(`^https?://[^/]+/api/v1/client/tasks/\d+/abandon$`)
httpmock.RegisterRegexpResponder("POST", abandonPattern, responder)

Debugging Tip: When httpmock tests fail with "no responder found," check the generated client.gen.go file to verify the exact path the client is using. OpenAPI code generators often transform method names differently than expected.

bodyclose Linter with httpmock#

Problem: Using httpmock.ResponderFromResponse with manually constructed *http.Response objects triggers the bodyclose linter warning. The linter detects that the response body is not explicitly closed in test code.

Incorrect (triggers bodyclose):

responder := httpmock.ResponderFromResponse(&http.Response{
    StatusCode: http.StatusOK,
    Header: http.Header{"Content-Type": []string{"application/json"}},
    Body: httpmock.NewRespBodyFromString(jsonData),
})

Correct (avoids bodyclose):

// Use NewJsonResponderOrPanic for JSON response mocks
responder := httpmock.NewJsonResponderOrPanic(http.StatusOK, responseObject)

// Or use NewStringResponder for simple text responses
responder := httpmock.NewStringResponder(http.StatusNoContent, "")

Why this happens: httpmock.NewJsonResponderOrPanic and NewStringResponder create responders that properly manage body lifecycle, satisfying the bodyclose linter's requirements. Manually constructed responses require explicit defer resp.Body.Close() calls that don't make sense in the responder context.

Cross-Platform Filesystem and Testing Gotchas#

DirEntry.Type() Unknown Type Handling#

Problem: Some filesystems return DirEntry.Type() == 0 (unknown type), causing type checks like entry.Type().IsRegular() to fail incorrectly. This is particularly common with networked filesystems or certain container environments.

Solution: Implement fallback to entry.Info() when Type() returns unknown:

func isRegularFile(entry fs.DirEntry) bool {
    if entry.Type() != 0 {
        return entry.Type().IsRegular()
    }
    // Fallback for filesystems that return unknown type
    info, err := entry.Info()
    if err != nil {
        return false
    }
    return info.Mode().IsRegular()
}

Source: lib/hashcat/session_dir.go

Technical Context: This pattern is critical when filtering directory entries to skip symlinks or special files. Without the fallback, legitimate regular files may be incorrectly skipped, causing cleanup operations or file discovery to fail silently. The isRegularFile() helper is used in hashcat session file cleanup to ensure only regular files are removed, preventing accidental symlink or directory deletion.

os.Symlink Windows Elevated Privileges#

Problem: os.Symlink requires elevated (administrator) privileges on Windows, causing tests that create symbolic links to fail in standard CI environments or developer workstations.

Solution: Skip symlink tests on Windows using runtime.GOOS guards:

func TestSymlinkBehavior(t *testing.T) {
    if runtime.GOOS == "windows" {
        t.Skip("symlink tests require elevated privileges on Windows")
    }
    // Test implementation
}

Alternative Approach: If symlink behavior is critical to test on Windows, configure your CI pipeline to run with elevated privileges or use Windows-specific test build constraints:

//go:build !windows

func TestSymlinkBehavior(t *testing.T) {
    // Test implementation
}

This build constraint automatically excludes the test on Windows builds, preventing CI failures without explicit skip logic.

Deployment Consideration: In air-gapped or secure lab environments running Windows, ensure documentation clearly states whether the application requires administrator privileges for any symlink-related functionality. For the CipherSwarmAgent project, symlink support is not required for core functionality — the isRegularFile() check explicitly rejects symlinks during cleanup operations.

Air-Gapped and Secure Environment Deployment#

When deploying in non-Internet-connected environments (secure labs, air-gapped networks):

1. Pre-download all linters: The .golangci.yml uses readonly mode for module downloads, which prevents golangci-lint from downloading dependencies during analysis. Ensure the golangci-lint binary and all 71 linter tools are installed before disconnecting from the network.

2. Cache formatter tools: The four formatters (golines, gofumpt, goimports, gci) are separate tools that must be available. Include them in your deployment package or ensure they're pre-installed in your build environment.

3. Local CI configuration: The 30-minute timeout and 8-worker concurrency settings are tuned for CI environments. On resource-constrained lab systems, consider adjusting these values in your local .golangci.yml override.

4. Offline documentation: Keep a local copy of this knowledge base article and the official golangci-lint documentation for reference when troubleshooting linting issues in disconnected environments.

Best Practices Summary#

1. Always include explanations for //nolint: directives (gocritic whyNoLint requirement)
2. List all linters when suppressing multiple issues on the same line
3. Place nolint above the line when golines would split multi-line expressions
4. Keep nolint reasons short to stay under 120-character limit
5. Verify nolint survival after running formatters on APIError, APIClient, Error_ types
6. Avoid //go:fix directives entirely (conflicts with gocheckcompilerdirectives)
7. Keep doc comments contiguous with declarations (no blank // lines)
8. Document each exported constant individually (revive requirement)
9. Use context.Background() for must-complete operations after parent context cancellation
10. Suppress contextcheck only when callee genuinely cannot accept context
11. Adopt errorsastype suggestions when modifying error handling code (Go 1.26+)
12. Match httpmock URL patterns to client.gen.go paths, not Go method names
13. Use httpmock helper functions (NewJsonResponderOrPanic, NewStringResponder) to avoid bodyclose warnings
14. Implement DirEntry.Type() fallback to entry.Info() for unknown type values (networked filesystems)
15. Skip os.Symlink tests on Windows using runtime.GOOS == "windows" guards (requires elevated privileges)

Relevant Code Files#

Related Topics#

• Context Propagation Architecture: Pattern documented in knowledge base showing how to properly propagate context throughout error handling chains
• Hashcat Session File Cleanup: Solution reference demonstrating os.IsNotExist masking wrong-path bugs and DirEntry.Type() unknown handling
• golangci-lint Configuration: Official documentation at https://golangci-lint.run/
• Go Compiler Directives: Official Go documentation on recognized compiler directives
• Code Formatting Pipeline: Multi-formatter setup with golines, gofumpt, goimports, gci