Update LINTING.md with current baseline of 34 issues (bd-45)

This commit is contained in:
Steve Yegge
2025-10-27 18:42:56 -07:00
parent 8289778ac0
commit aa38f68670
+22 -46
View File
@@ -4,13 +4,13 @@ This document explains our approach to `golangci-lint` warnings in this codebase
## Current Status ## 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. Running `golangci-lint run ./...` currently reports **34 issues** as of Oct 27, 2025. 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. **Historical note**: The count was ~200 before extensive cleanup in October 2025. The remaining issues represent the acceptable baseline that doesn't warrant fixing.
## Issue Breakdown ## Issue Breakdown
### errcheck (159 issues) ### errcheck (24 issues)
**Pattern**: Unchecked errors from `defer` cleanup operations **Pattern**: Unchecked errors from `defer` cleanup operations
**Status**: Intentional and idiomatic **Status**: Intentional and idiomatic
@@ -29,59 +29,35 @@ defer os.RemoveAll(tmpDir) // in tests
Fixing these would add noise without improving code quality. The critical cleanup operations (where errors matter) are already checked explicitly. Fixing these would add noise without improving code quality. The critical cleanup operations (where errors matter) are already checked explicitly.
### revive (21 issues) ### gosec (10 issues)
**Pattern 1**: Unused parameters in Cobra command handlers (18 issues) **Pattern 1**: G204 - Subprocess launched with variable (3 issues)
**Status**: Required by interface **Status**: Intentional - launching editor and git commands with user-specified paths
Examples: Examples:
```go - Launching `$EDITOR` for issue editing
Run: func(cmd *cobra.Command, args []string) { - Executing git commands
// cmd or args may not be used in every handler - Running bd daemon binary
}
```
**Rationale**: Cobra requires this exact function signature. Renaming to `_` would make the code less clear when parameters *are* used. **Pattern 2**: G304 - File inclusion via variable (3 issues)
**Status**: Intended feature - user-specified file paths for import/export
**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: All file paths are either:
- User-provided CLI arguments (expected for import/export commands) - User-provided CLI arguments (expected for import/export commands)
- Test fixtures in controlled test environments - Test fixtures in controlled test environments
- Validated paths with security checks (e.g., markdown.go uses validateMarkdownPath) - Validated paths with security checks
**Pattern 3**: G301 - Directory permissions (2 issues) **Pattern 3**: G301/G302/G306 - File permissions (3 issues)
**Status**: Acceptable - 0755 is reasonable for database directories **Status**: Acceptable for user-facing database files
### gocyclo (1 issue) - G301: 0755 for database directories (allows other users to read)
- G302: 0644 for JSONL files (version controlled, needs to be readable)
- G306: 0644 for new JSONL files (consistency with existing files)
**Pattern**: High cyclomatic complexity in `TestExportImport` (31) **Pattern 4**: G115 - Integer overflow conversion (1 issue)
**Status**: Acceptable **Status**: False positive - bounded by max retry count
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. The exponential backoff calculation is bounded by a small retry counter, making overflow impossible in practice.
### 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 ## golangci-lint Configuration Challenges
@@ -94,10 +70,10 @@ This appears to be a known limitation of golangci-lint's configuration system.
## Recommendation ## Recommendation
**For contributors**: Don't be alarmed by the lint warnings. The code quality is high. **For contributors**: Don't be alarmed by the 34 lint warnings. The code quality is high.
**For code review**: Focus on: **For code review**: Focus on:
- New issues introduced by changes (not the baseline ~200) - New issues introduced by changes (not the baseline 34)
- Actual logic errors - Actual logic errors
- Missing error checks on critical operations (file writes, database commits) - Missing error checks on critical operations (file writes, database commits)
- Security concerns beyond gosec's false positives - Security concerns beyond gosec's false positives