Linter Patterns (opnDossier)#

Linter Patterns refers to the project-specific conventions, rules, and resolutions for static code analysis issues in the opnDossier project, a Go-based tool for generating meaningful output from OPNsense firewall configurations. The project uses golangci-lint as its comprehensive linting framework, enabling over 60 specialized linters to enforce code quality, security, style consistency, and Go best practices.

The linting infrastructure is a blocking quality gate in the CI/CD pipeline, requiring all code to pass just lint before pull request merge. Key linters include gocritic for advanced Go patterns, gosec for security scanning, revive for customizable Go rules, and specialized linters like mnd (magic number detection), goconst (repeated string extraction), and tparallel (parallel test validation).

The project follows strict conventions for suppressing linter warnings: //nolint directives must be placed on separate lines above the code they suppress (never inline) due to gofumpt formatting rules, and every suppression must include a justification comment explaining why it's safe. This documentation serves as a comprehensive reference for understanding, applying, and resolving common linter issues specific to opnDossier.

Linting Infrastructure#

golangci-lint Configuration#

The project's linting infrastructure is defined in .golangci.yml, which configures:

• 60+ enabled linters covering error handling, security, code quality, performance, style, complexity, and testing patterns (including forbidigo for pattern-based function bans)
• Run configuration: 8 concurrent workers, 30-minute timeout, tests included in linting
• Auto-fix enabled where possible to reduce manual correction burden
• Unlimited issue reporting (max-issues-per-linter: 0) for comprehensive feedback

Phased Adoption Strategy#

Several strict linters are deliberately disabled until after version 1.0, revealing a pragmatic approach to balancing code quality with release timelines:

• exhaustive, exhaustruct: Deferred post-1.0 - exhaustive switch/type checking
• funlen, gocognit, nestif: Deferred post-1.0 - function length and cognitive complexity limits
• prealloc: Disabled as potentially premature optimization
• gochecknoglobals: Disabled because it conflicts with Cobra CLI patterns

Development Workflow#

Developers run linting through the just task runner:

just lint # Run all linters locally
just check # Run pre-commit checks (includes linting)
just ci-check # Run comprehensive CI validation

Important: IDE diagnostics may differ from just lint output due to different configurations or versions. The authoritative source is always just lint, which matches the CI pipeline behavior.

Common Linter Patterns#

gocritic: emptyStringTest#

Issue: gocritic flags len(s) == 0 when checking if a string is empty.

Resolution: Use direct string comparison s == "" instead.

Documented in: AGENTS.md linter patterns table

Example:

// Incorrect - triggers gocritic
if len(name) == 0 {
    return errors.New("name cannot be empty")
}

// Correct
if name == "" {
    return errors.New("name cannot be empty")
}

Note: This rule applies to strings only. For slices, len(slice) == 0 remains the correct pattern, as demonstrated in cmd/shared_flags.go.

gosec G115: Integer Overflow#

Issue: gosec G115 flags unsafe integer conversions that could cause overflow (e.g., int to int32).

Resolution: Add //nolint:gosec on a separate line with a bounded value comment explaining why the conversion is safe.

Security context: Part of CWE-190 (Integer Overflow) mitigation in the project's security assurance program.

Example from cmd/validate.go:

// updateMaxExitCode atomically updates the max exit code if the new code is higher.
// Exit codes are small positive integers (0-127), so int32 conversion is safe.
func updateMaxExitCode(maxCode *atomic.Int32, newCode int) {
    // Exit codes are bounded [0, 127] by convention, so conversion is safe
    newCode32 := int32(newCode) //nolint:gosec // Exit codes are bounded [0, 127]
    for {
        current := maxCode.Load()
        if newCode32 <= current {
            return
        }
        if maxCode.CompareAndSwap(current, newCode32) {
            return
        }
    }
}

Pattern elements:

1. Function-level documentation explaining the bounds
2. Inline comment before the conversion
3. //nolint:gosec directive with specific bounded value justification

mnd: Magic Number Detection#

Issue: mnd flags unexplained numeric literals in code.

Resolution: Extract to named constants, preferably in internal/constants/constants.go.

Configuration: mnd ignores magic numbers in specific function calls like flag parsing, file operations, and Prometheus metrics.

Example from internal/constants/constants.go:

// InterfaceComplexityWeight is the complexity scoring weight per network interface.
InterfaceComplexityWeight = 5
// FirewallRuleComplexityWeight is the complexity scoring weight per firewall rule.
FirewallRuleComplexityWeight = 2
// UserComplexityWeight is the complexity scoring weight per user account.
UserComplexityWeight = 3

// MaxPort is the maximum valid TCP/UDP port number.
MaxPort = 65535
// MaxIPv4Subnet is the maximum IPv4 subnet prefix length.
MaxIPv4Subnet = 32
// MinMTU is the minimum valid MTU (RFC 791 minimum for IPv4).
MinMTU = 68

Additional examples:

• internal/plugins/stig/stig.go: MaxDHCPInterfaces = 2
• internal/sanitizer/mapper.go: defaultRedactedDomain = "example.com"

minmax: Manual Comparisons#

Issue: minmax flags manual min/max implementations when Go 1.21+ builtins are available.

Resolution: Use min() and max() builtin functions.

Example from cmd/convert.go:

// Use a semaphore to limit concurrent file operations
maxConcurrent := max(runtime.NumCPU(), 1)
sem := make(chan struct{}, maxConcurrent)

Example from cmd/help.go (Levenshtein distance):

matrix[i][j] = min(
    matrix[i-1][j]+1, // deletion
    matrix[i][j-1]+1, // insertion
    matrix[i-1][j-1]+cost, // substitution
)

Additional usage: internal/progress/bar.go, internal/diff/security/scorer.go

goconst: Repeated Strings#

Issue: goconst detects strings used multiple times that should be extracted as constants.

Resolution: Extract to package-level constants with descriptive names.

Example from internal/converter/enrichment.go:

// redactedValue is the placeholder for sensitive fields in exported output.
const redactedValue = "[REDACTED]"

// serviceNameSNMP is the display name for the SNMP service in statistics.
const serviceNameSNMP = "SNMP Daemon"

Example from internal/sanitizer/rules.go:

// asciiLowercaseDelta is the offset between uppercase and lowercase ASCII letters.
const asciiLowercaseDelta = 32

3-occurrence threshold: goconst flags string literals appearing 3+ times in the same package. Tests that hand-write a hostname or other fixture-derived string already present in fixture generators like generateSmallConfig/generateLargeConfig add a fourth occurrence and fail lint.

• Wrong: expected := "small-config" in a new test
• Right: expected := smallConfig.System.Hostname — sources from the fixture and stays in sync if the fixture changes
• Avoid extracting a package-level constant unless you're going to refactor the fixture builders to use it too — partial extraction keeps the lint warning and adds drift surface

tparallel: Parallel Test Issues#

Issue: tparallel flags improper use of t.Parallel() in subtests when the parent test doesn't also call t.Parallel().

Resolution: When subtests use t.Parallel(), the parent test must also call t.Parallel().

Documented in: AGENTS.md linter patterns table

Example pattern:

// Correct: parent test calls t.Parallel()
func TestFeature(t *testing.T) {
    t.Parallel() // Must be here if subtests use t.Parallel()

    t.Run("subtest1", func(t *testing.T) {
        t.Parallel()
        // test logic
    })
}

Note: This guidance does NOT apply to cmd/ package tests, where t.Parallel() is forbidden entirely by forbidigo (see below).

testifylint: go-require Rule#

Issue: require.NoError / require.NotNil and other require.* calls invoke t.FailNow, which is only valid on the test's main goroutine. The testifylint linter (go-require rule) flags any require.* inside a goroutine body.

Impact: golangci-lint run will fail at commit time even when the test passes at runtime.

Resolution: Collect per-goroutine outcomes into shared slices, then assert with require.* from the main test goroutine after wg.Wait().

Canonical example: TestCoreProcessor_Process_ResultIsolation in internal/processor/processor_test.go — processErrs and reports slices are populated inside goroutines, asserted in a separate post-wg.Wait() loop.

Pattern:

func TestConcurrentOperation(t *testing.T) {
    var (
        mu sync.Mutex
        results []string
        errors []error
    )

    var wg sync.WaitGroup
    for i := 0; i < 10; i++ {
        wg.Add(1)
        go func(id int) {
            defer wg.Done()

            result, err := doWork(id)

            mu.Lock()
            if err != nil {
                errors = append(errors, err)
            } else {
                results = append(results, result)
            }
            mu.Unlock()

            // Use assert.* inside goroutines for soft checks
            assert.NotEmpty(t, result)
        }(i)
    }

    wg.Wait()

    // Use require.* on the main goroutine for halt-the-test checks
    require.Empty(t, errors)
    require.Len(t, results, 10)
}

Note: assert.* is fine inside goroutines — it doesn't call FailNow. Use assert for soft checks during the goroutine, require for halt-the-test checks after the join.

forbidigo: Forbidden Function Calls#

Purpose: Type-aware pattern matching to forbid specific function calls that are unsafe in certain contexts.

Configuration: Uses analyze-types: true for type-aware matching across different variable names.

Current rule: Bans testing.T.Parallel() calls in cmd/ package tests.

Rationale: The cmd/ package uses package-level CLI flag globals (sharedDeviceType, sharedAuditMode, outputFile, format, force, etc.). Any test that calls t.Parallel() risks a data race with sibling tests that mutate those globals. The race detector reports collateral races in unrelated utilities, so the policy is a blanket ban in cmd/. See CI Pipeline Gotchas §1.1 for detailed rationale.

Scoping: The linter is restricted to cmd/ via a path-except: 'cmd/.*\.go$' exclusion rule, so the ban only applies within cmd/ tests. Tests elsewhere in the codebase can still use t.Parallel() according to tparallel guidance.

Pattern matching: Uses ^testing\.T\.Parallel$ pattern to catch t.Parallel() regardless of variable name (t, tt, tc, etc.).

Example from cmd/audit_handler_test.go:

func TestValidAuditModes(t *testing.T) {
    // Do NOT use t.Parallel() — cmd package uses package-level flag globals.
    // See GOTCHAS §1.1.
    completions, directive := ValidAuditModes(nil, nil, "")
    // test logic
}

revive: redefines-builtin#

Issue: revive flags local variables or parameters that shadow Go builtin identifiers (e.g., len, copy, log).

Resolution: Rename the shadowing identifier or use import aliases for conflicting packages.

Documented in: AGENTS.md linter patterns table

Example from cmd/root.go:

import (
    "github.com/EvilBit-Labs/opnDossier/internal/logging"
    charmLog "github.com/charmbracelet/log" // Renamed to avoid shadowing standard library
    "github.com/spf13/cobra"
)

Pattern: The internal package is named logging (not log) to prevent conflicts from the start.

revive: stutters#

Issue: revive flags package-level identifiers that stutter by repeating the package name (e.g., compliance.CompliancePlugin).

Resolution: Drop the redundant prefix (e.g., use compliance.Plugin instead).

Documented in: AGENTS.md linter patterns table

Example from internal/compliance/interfaces.go:

// Plugin defines the interface that all compliance plugins must implement.
type Plugin interface { // Correct: compliance.Plugin (not CompliancePlugin)
    Name() string
    Description() string
    Run(ctx context.Context, device *common.Device) ([]Control, error)
}

Additional examples:

• internal/plugins/firewall/firewall.go: firewall.Plugin
• internal/plugins/sans/sans.go: sans.Plugin
• internal/plugins/stig/stig.go: stig.Plugin

modernize: Omitempty on Struct Fields#

Issue: modernize flags omitempty in JSON tags on struct-typed fields (not primitives), as it has no effect.

Resolution: Remove omitempty from JSON tags on struct-typed fields.

Documented in: AGENTS.md linter patterns table

Example:

// Incorrect
type Config struct {
    Server ServerConfig `json:"server,omitempty"` // omitempty has no effect on structs
}

// Correct
type Config struct {
    Server ServerConfig `json:"server"`
}

strings.EqualFold: Case-Insensitive Comparison#

Pattern: Use strings.EqualFold(a, b) for case-insensitive string comparisons instead of converting both strings to lowercase.

Documented in: AGENTS.md Go patterns reference

Example from internal/config/validation.go:

func isValidEnum(value string, validOptions []string) bool {
    for _, opt := range validOptions {
        if strings.EqualFold(opt, value) {
            return true
        }
    }
    return false
}

Additional examples:

• cmd/shared_flags.go: Device type validation
• internal/processor/example.go: Admin user detection
• internal/model/opnsense/converter.go: XML boolean parsing

nolint Directive Rules#

Critical Placement Rule#

//nolint directives MUST be placed on a separate line above the code they suppress, never inline. This is a strict project convention because gofumpt strips inline nolint comments during formatting.

Example from build_test.go:

// Correct: separate line
//nolint:gosec // This is test code, the binary path is controlled by the test
cmd := exec.CommandContext(ctx, "go", "build", "-o", s.binaryPath, ".")

// Incorrect: inline (will be stripped by gofumpt)
cmd := exec.CommandContext(ctx, "go", "build", "-o", s.binaryPath, ".") //nolint:gosec

Justification Requirement#

Every //nolint directive must include a comment explaining why the suppression is safe. The codebase demonstrates 50+ examples of properly justified suppressions.

Example from cmd/root.go:

var (
    cfgFile string //nolint:gochecknoglobals // CLI config file path
    cfg *config.Config //nolint:gochecknoglobals // Application configuration (internal)
    logger *logging.Logger //nolint:gochecknoglobals // Application logger (internal)
)

Common Suppression Patterns#

Cobra flag globals (cmd/shared_flags.go):

var (
    sharedDeviceType string //nolint:gochecknoglobals // Force device type (bypasses auto-detection)
    sharedSections []string //nolint:gochecknoglobals // Sections to include
    sharedTheme string //nolint:gochecknoglobals // Theme for rendering
)

Rationale: Disabled because gochecknoglobals conflicts with Cobra CLI command patterns

Test code security exceptions (cmd/config_init_test.go):

//nolint:gosec // Test file permissions are fine for testing
err := os.WriteFile(outputPath, []byte("existing content"), 0o644)

Validated file paths (internal/testing/modeltest/completeness.go):

//nolint:gosec // filePath is validated by validateFilePath
data, err := os.ReadFile(filePath)

Multiple suppressions (cmd/audit_handler.go):

//nolint:errcheck,gosec // Build writes to bytes.Buffer which cannot fail
md.Build()

Intentionally similar test structures (internal/schema/security_test.go):

//nolint:dupl // Source/Destination IsAny tests are structurally similar by design
func TestSource_IsAny(t *testing.T) {

Linter-Specific Configuration#

gocritic Settings#

Lines 110-125 of .golangci.yml:

• Enabled tags: diagnostic, experimental, opinionated, performance, style
• Disabled checks: hugeParam, rangeExprCopy, rangeValCopy (performance trade-offs)
• captLocal.paramsOnly: false - Check all local variables for captured state, not just parameters
• underef.skipRecvDeref: false - Don't skip receiver dereference checks

gosec Security Exclusions#

Lines 179-182 of .golangci.yml:

• G301 excluded: "Man pages require 755 permissions for public access"
• G304 excluded: "Loading user-specified templates is expected CLI behavior"

These exclusions make explicit security trade-offs to support CLI-specific behaviors.

revive Custom Rules#

Lines 98-100 of .golangci.yml:

• Custom exclusion: Suppresses "var-naming: avoid meaningless package names" for internal/model/common/ path

mnd Ignored Contexts#

Lines 191-204 of .golangci.yml - Magic numbers are allowed in:

• Flag parsing: flag.Arg, flag.Duration.*, etc.
• File operations: os.Chmod, os.Mkdir.*, os.OpenFile, os.WriteFile
• Prometheus metrics: ExponentialBuckets, LinearBuckets
• Error handling: args.Error

govet Configuration#

Lines 183-187 of .golangci.yml:

• fieldalignment disabled: "Known issue with the govet tool"
• shadow disabled: "Temporarily to focus on other issues first"

Strict Policies#

nakedret Lines 206-207:

• max-func-lines: 1 - Effectively bans naked returns

sloglint Lines 215-217:

• no-global: all - Prevents global logger usage
• context: scope - Enforces context-aware logging

Code Formatting Integration#

gofumpt and Inline Comments#

The project uses gofumpt for stricter formatting than standard gofmt. A critical consequence: gofumpt strips inline //nolint directives, which is why separate-line placement is mandatory.

Configuration Lines 239-241:

gofumpt:
  extra-rules: true
  module-path: "github.com/EvilBit-Labs/opnDossier"

Additional Formatters#

Lines 233-246 of .golangci.yml:

• golines: Max line length 120 characters
• goimports: Import organization and cleanup
• gci: Import grouping and ordering

Relevant Code Files#

Related Topics#

Go Code Quality Standards#

• golangci-lint: Industry-standard meta-linter aggregating 70+ Go linters
• gofmt/gofumpt: Go code formatting tools (gofumpt provides stricter rules)
• staticcheck: Advanced static analysis for Go

Security Linting#

• gosec: Go Security Checker for common security issues
• CWE-190: Integer Overflow vulnerability class
• Security assurance programs: Integrating security scanning into CI/CD

Cobra CLI Patterns#

• Global state management: Handling package-level variables in CLI applications
• Cobra commands: Command-line interface framework for Go
• Flag binding: Connecting CLI flags to application configuration

Plugin Architecture#

• Interface design: Defining clean contracts for extensibility
• Package naming conventions: Avoiding stuttering and builtin shadowing
• Compliance plugins: Domain-specific validation implementations (firewall, SANS, STIG)