beads/LINTING.md

Linting Policy

This document explains our approach to golangci-lint warnings in this codebase.

Current Status

Running golangci-lint run ./... currently reports ~200 "issues". However, these are not actual code quality problems - they are false positives or intentional patterns that reflect idiomatic Go practice.

Note: The count increased from ~100 to ~200 between Oct 12-14, 2025, due to significant test coverage additions for collision resolution (1100+ lines) and auto-flush features (300+ lines). All new warnings follow the same idiomatic patterns documented below.

Issue Breakdown

errcheck (159 issues)

Pattern: Unchecked errors from defer cleanup operations Status: Intentional and idiomatic

Examples:

defer rows.Close()
defer tx.Rollback()
defer os.RemoveAll(tmpDir)  // in tests

Rationale: In Go, it's standard practice to ignore errors from deferred cleanup operations:

• rows.Close() - closing already-consumed result sets rarely errors
• tx.Rollback() - rollback on defer is a safety net; if commit succeeded, rollback is a no-op
• Test cleanup - errors during test cleanup don't affect test outcomes

Fixing these would add noise without improving code quality. The critical cleanup operations (where errors matter) are already checked explicitly.

revive (21 issues)

Pattern 1: Unused parameters in Cobra command handlers (18 issues) Status: Required by interface

Examples:

Run: func(cmd *cobra.Command, args []string) {
    // cmd or args may not be used in every handler
}

Rationale: Cobra requires this exact function signature. Renaming to _ would make the code less clear when parameters are used.

Pattern 2: Package naming (3 issues)

• package types - Clear and appropriate for a types package
• SQLiteStorage - Intentional; sqlite.Storage would be confusing with the interface
• Blank import comment - Required for database driver registration

gosec (19 issues)

Pattern 1: G201 - SQL string formatting (6 issues) Status: False positive - all SQL is validated

All dynamic SQL construction uses:

• Validated field names via allowlist (see allowedUpdateFields in sqlite.go:197)
• Parameterized queries for all values
• Safe string building for clauses like ORDER BY and LIMIT

Pattern 2: G304 - File inclusion via variable (11 issues) Status: Intended feature - user-specified file paths for import/export/test fixtures

All file paths are either:

• User-provided CLI arguments (expected for import/export commands)
• Test fixtures in controlled test environments
• Validated paths with security checks (e.g., markdown.go uses validateMarkdownPath)

Pattern 3: G301 - Directory permissions (2 issues) Status: Acceptable - 0755 is reasonable for database directories

gocyclo (1 issue)

Pattern: High cyclomatic complexity in TestExportImport (31) Status: Acceptable

This comprehensive integration test covers multiple scenarios (export, import, filters, updates). The complexity comes from thorough test coverage, not production code. Splitting would reduce readability.

goconst (2 issues)

Pattern: Repeated string constants in tests Status: Acceptable

Repeated test strings like "test-user" and file paths appear multiple times. Extracting these to constants would not improve test readability or maintainability.

golangci-lint Configuration Challenges

We've attempted to configure .golangci.yml to exclude these false positives, but golangci-lint's exclusion mechanisms have proven challenging:

• exclude-functions works for some errcheck patterns
• exclude patterns with regex don't match as expected
• exclude-rules with text matching doesn't work reliably

This appears to be a known limitation of golangci-lint's configuration system.

Recommendation

For contributors: Don't be alarmed by the lint warnings. The code quality is high.

For code review: Focus on:

• New issues introduced by changes (not the baseline ~200)
• Actual logic errors
• Missing error checks on critical operations (file writes, database commits)
• Security concerns beyond gosec's false positives

For CI/CD: The current GitHub Actions workflow runs linting but doesn't fail on these known issues. We may add --issues-exit-code=0 or configure the workflow to check for regressions only.

Future Work

Potential approaches to reduce noise:

1. Disable specific linters (errcheck, revive) if the signal-to-noise ratio doesn't improve
2. Use //nolint directives sparingly for clear false positives
3. Investigate alternative linters with better exclusion support
4. Contribute to golangci-lint to improve exclusion mechanisms

Summary

These "issues" are not technical debt - they represent intentional, idiomatic Go code. The codebase maintains high quality through:

• Comprehensive test coverage (>80%)
• Careful error handling where it matters
• Security validation of user input
• Clear documentation

Don't let the linter count distract from the actual code quality.