CLAUDE.md

1# Go Coding Standards
2 
3## Project Requirements
4 
5- **Go Version**: 1.26
6- **Release Notes**: [https://go.dev/doc/go1.26](https://go.dev/doc/go1.26)
7 
8## Quick Reference
9 
10- Use `internal/errors` package (never standard `errors`)
11- Structured logging with `internal/logging`
12- Test with `-race` flag always
13- No magic numbers - use constants
14- Document all exports
15- **Zero linter tolerance** - fix all issues before commit
16 
17## Go 1.26 Features
18 
19### Enhanced new() Function
20 
21Use `new()` with expressions for pointer initialization:
22 
23```go
24// ✅ Go 1.26 - new() accepts expressions
25existingFirstSeen: new(time.Date(2025, 6, 15, 10, 0, 0, 0, time.UTC))
26 
27// ❌ Old pattern - wrapper functions unnecessary
28func ptr[T any](v T) *T { return &v }
29existingFirstSeen: ptr(time.Date(2025, 6, 15, 10, 0, 0, 0, time.UTC))
30```
31 
32### Green Tea GC (Default)
33 
34- **10-40% reduction in GC overhead** (enabled by default)
35- Additional 10% improvement on newer CPUs (Intel Ice Lake, AMD Zen 4+)
36- Opt-out: `GOEXPERIMENT=nogreenteagc` (will be removed in Go 1.27)
37 
38### Performance Improvements
39 
40- **30% faster cgo calls** - reduced baseline overhead
41- **Faster io.ReadAll()** - ~2x improvement with better allocation strategy
42- **JPEG decoder/encoder** - new, faster, more accurate implementation
43- **Reduced fmt allocations** for unformatted strings
44 
45### Modern Standard Library
46 
47- **strings.Cut()** - replaces `strings.Index` + slicing patterns:
48 
49```go
50  // ✅ Go 1.26 - cleaner and more efficient
51  if host, port, found := strings.Cut(rawURL, ":"); found {
52      return host
53  }
54 
55  // ❌ Old pattern
56  if colonIdx := strings.Index(rawURL, ":"); colonIdx != -1 {
57      return rawURL[:colonIdx]
58  }
59```
60 
61- **errors.AsType()** - type-safe version of `errors.As()`:
62 
63```go
64  // ✅ Go 1.26 - type-safe, ~3x faster
65  if pathErr, ok := errors.AsType[*fs.PathError](err); ok {
66      // Use pathErr
67  }
68 
69  // ❌ Old pattern - slower, requires pointer setup
70  var pathErr *fs.PathError
71  if errors.As(err, &pathErr) {
72      // Use pathErr
73  }
74```
75 
76- **filepath.IsLocal()** - comprehensive path validation (Go 1.20+):
77 
78  **IMPORTANT**: `filepath.IsLocal()` cleans paths internally! Use carefully based on context:
79 
80```go
81  // ✅ For FILE paths after cleaning - detects Windows reserved names, CVE-2023-45284/45283
82  cleanPath := filepath.Clean(userInput)
83  if !filepath.IsLocal(cleanPath) {
84      return errors.New("invalid path")
85  }
86 
87  // ✅ For URL paths - explicit check needed (IsLocal cleans "path/../etc" → "etc" → valid!)
88  if strings.Contains(urlPath, "..") {
89      return errors.New("traversal attempt")
90  }
91 
92  // ❌ Wrong - misses Windows reserved names
93  if strings.Contains(userPath, "..") {
94      return errors.New("invalid path")
95  }
96```
97 
98  **What filepath.IsLocal() detects**:
99  - Windows reserved names: `COM1`, `LPT1`, `NUL`, etc.
100  - Space-padded reserved names: `COM1` with trailing space (CVE-2023-45284)
101  - Windows `\??\` prefix attacks (CVE-2023-45283)
102  - Absolute paths and empty paths
103 
104  **Use cases**:
105  - File system operations AFTER cleaning
106  - Validating final destination paths
107  - NOT for detecting literal ".." in untrusted URLs
108 
109- **net.JoinHostPort()** - IPv6-safe host:port formatting:
110 
111```go
112  // ✅ Correct - handles IPv6 addresses with brackets
113  addr := net.JoinHostPort(host, strconv.Itoa(port))
114  // Result: "[2001:4860:4860::8888]:443" for IPv6
115 
116  // ❌ Wrong - breaks IPv6
117  addr := fmt.Sprintf("%s:%d", host, port)
118  // Result: "2001:4860:4860::8888:443" (invalid)
119```
120 
121- **reflect.Type.Fields()** - iterate struct fields (Go 1.26):
122 
123```go
124  // ✅ Go 1.26 - cleaner iteration
125  for field := range structType.Fields() {
126      // field is reflect.StructField
127  }
128 
129  // ❌ Old pattern
130  for i := range structType.NumField() {
131      field := structType.Field(i)
132  }
133```
134 
135- **bytes.Buffer.Peek()** - read without advancing position
136 
137### Preallocation Patterns
138 
139Preallocate slice capacity when size is known:
140 
141```go
142// ✅ Preallocate capacity
143fields := make([]logger.Field, 0, 5+len(extraFields))
144fields = append(fields, baseFields...)
145fields = append(fields, extraFields...)
146 
147// ❌ Multiple reallocations
148fields := []logger.Field{}
149fields = append(fields, baseFields...)
150fields = append(fields, extraFields...)
151```
152 
153### Security & Cryptography
154 
155- **Post-quantum ML-KEM** enabled by default in crypto/tls
156- **crypto/hpke** - Hybrid Public Key Encryption (RFC 9180)
157- **Secure randomness** - Random parameter in crypto functions now ignored (always secure)
158 
159## Import Rules
160 
161- **Use** `"github.com/tphakala/birdnet-go/internal/errors"` (never standard `"errors"`)
162- **Use** `internal/logging` for structured logging
163- Specify `.Component()` and `.Category()` for telemetry
164- Register new components in error package's `init()`
165 
166## Error Handling
167 
168- Wrap errors: `fmt.Errorf("operation failed: %w", err)`
169- Use sentinel errors: `var ErrNotFound = errors.New("not found")`
170- Log but continue on batch operation failures
171- Provide detailed context in messages
172 
173## Testing
174 
175- **Prefer `testing/synctest` over `time.Sleep()`** (Go 1.25)
176- `t.Parallel()` only for independent tests
177- **Use `t.TempDir()` for scratch space** (auto-cleanup)
178- **Use `t.ArtifactDir()` for test outputs to preserve** (Go 1.26+)
179- Test with `go test -race`
180- Table-driven tests with `t.Run()`
181- `b.ResetTimer()` after benchmark setup
182- Use `t.Attr()` for test metadata (Go 1.25)
183 
184### Test Directory Guidelines (Go 1.26)
185 
186- **`t.TempDir()`** - Temporary scratch space, auto-deleted after test
187- **`t.ArtifactDir()`** - Preserved test outputs (with `-artifacts` flag)
188- **Replace `os.MkdirTemp()` in tests** - use appropriate testing method
189 
190```go
191// ✅ Scratch space that should be cleaned up
192func TestProcess(t *testing.T) {
193    tmpDir := t.TempDir()  // auto-deleted
194    processFiles(tmpDir)
195}
196 
197// ✅ Test artifacts to preserve (logs, debug output)
198func TestAnalysis(t *testing.T) {
199    outDir := t.ArtifactDir()  // preserved with -artifacts flag
200    writeDebugLog(outDir + "/debug.log")
201}
202 
203// ❌ Manual temp dirs - no auto-cleanup
204tmpDir, _ := os.MkdirTemp("", "test_*")
205```
206 
207### Test Cleanup Best Practices
208 
209- **Use `t.Cleanup()` instead of `defer`** for test resource cleanup
210- `t.Cleanup()` runs after all defers, providing more predictable cleanup order
211- Particularly important for tests that restore global state
212- Example:
213 
214```go
215  func TestWithGlobalState(t *testing.T) {
216      // ❌ Wrong - defer may run at unpredictable times
217      originalValue := GetGlobalValue()
218      defer SetGlobalValue(originalValue)
219 
220      // ✅ Correct - cleanup runs after all test defers
221      originalValue := GetGlobalValue()
222      t.Cleanup(func() {
223          SetGlobalValue(originalValue)
224      })
225  }
226```
227 
228### Test Parallelization Guidelines
229 
230- Add `t.Parallel()` to **test functions** and **subtests** for speed
231- **NEVER parallelize tests that**:
232  - Mutate global state (e.g., `conftest.SetTestSettings()`)
233  - Share mutable data structures without synchronization
234  - Use shared map references without cloning
235- **Always clone shared test data** in subtests:
236 
237```go
238  import "maps"
239 
240  customSeasons := map[string]Season{...}
241  for _, tt := range tests {
242      t.Run(tt.name, func(t *testing.T) {
243          t.Parallel()
244          // Clone to avoid aliasing - prevents false positives
245          settings.Seasons = maps.Clone(customSeasons)
246      })
247  }
248```
249 
250### Test Helper File Naming
251 
252- Name test-only helper files with `_test.go` suffix
253- **Wrong**: `internal/foo/test_helpers.go` (included in production builds)
254- **Correct (package-local helpers)**: `internal/foo/test_helpers_test.go` (test-only)
255- **Correct (helpers shared across packages' tests)**: a dedicated testing-support subpackage, e.g. `internal/conf/conftest` or `internal/api/v2/apitest`. A `_test.go` file cannot be imported by other packages, so cross-package helpers must live in their own importable package that production code never imports.
256- This ensures helpers with `*testing.T` parameters (and the `testing` import) don't bloat production binaries
257 
258### Benchmark Best Practices
259 
260- Always call `b.ReportAllocs()` before `b.ResetTimer()` to track allocations
261- Use `b.Loop()` (Go 1.24+) for cleaner benchmark loops (optional)
262- Benchmark example:
263 
264```go
265  func BenchmarkValidation(b *testing.B) {
266      cfg := &Config{...}
267      b.ReportAllocs()
268      b.ResetTimer()
269      for i := 0; i < b.N; i++ {
270          _ = Validate(cfg)
271      }
272  }
273```
274 
275### Mock Generation with Mockery
276 
277**IMPORTANT**: Never manually write mocks. Use mockery for automated mock generation.
278 
279**Quick Start:**
280 
281```bash
282# Generate mocks for all interfaces
283go generate ./internal/datastore
284 
285# Or use mockery directly
286mockery --config .mockery.yaml
287```
288 
289**Using Generated Mocks in Tests:**
290 
291```go
292import (
293    "testing"
294    "github.com/stretchr/testify/mock"
295    "github.com/tphakala/birdnet-go/internal/datastore/mocks"
296)
297 
298func TestMyFunction(t *testing.T) {
299    // Create mock
300    mockDS := mocks.NewMockInterface(t)
301 
302    // Set expectations using .EXPECT() pattern
303    mockDS.EXPECT().
304        Save(mock.Anything, mock.Anything).
305        Return(nil).
306        Once()
307 
308    // Use the mock
309    err := myFunction(mockDS)
310 
311    // Assertions happen automatically
312}
313```
314 
315**Critical Rules:**
316 
317- **Conditional Mock Calls**: Use `.Maybe()` for methods called conditionally
318 
319```go
320// Method only called when NotificationSuppressionHours > 0
321mockDS.EXPECT().
322    GetActiveNotificationHistory(mock.AnythingOfType("time.Time")).
323    Return([]datastore.NotificationHistory{}, nil).
324    Maybe()  // Won't fail if not called
325```
326 
327- **Async Operations**: Use `.Maybe()` for methods called in goroutines
328 
329```go
330// Called asynchronously in RecordNotificationSent
331mockDS.EXPECT().
332    SaveNotificationHistory(mock.AnythingOfType("*datastore.NotificationHistory")).
333    Return(nil).
334    Maybe()  // Non-blocking operation
335```
336 
337- **Test Helpers**: Always use `t.Helper()` in setup functions
338 
339```go
340func createTestTracker(t *testing.T) *Tracker {
341    t.Helper()  // Stack traces point to caller, not this function
342    // ... setup
343}
344```
345 
346**Common Patterns:**
347 
348```go
349// Match any argument type
350mockDS.EXPECT().Get(mock.Anything).Return(note, nil)
351 
352// Match specific type
353mockDS.EXPECT().Save(mock.AnythingOfType("*datastore.Note")).Return(nil)
354 
355// Multiple calls
356mockDS.EXPECT().Get(mock.Anything).Return(note, nil).Times(3)
357 
358// Return different values on subsequent calls
359mockDS.EXPECT().Get("123").Return(note1, nil).Once()
360mockDS.EXPECT().Get("123").Return(note2, nil).Once()
361```
362 
363**When Interface Changes:**
364 
3651. Update the interface in `internal/datastore/interfaces.go`
3662. Run `go generate ./internal/datastore`
3673. Mocks automatically regenerate with all methods
3684. **Never** manually edit files in `internal/datastore/mocks/`
369 
370**Documentation:**
371 
372- Complete guide: `internal/datastore/mocks/README.md`
373- Configuration: `.mockery.yaml`
374- Migration guide and examples in README
375 
376## Go 1.25 Testing Features
377 
378See [Go 1.25 Release Notes](https://go.dev/doc/go1.25) for complete changelog.
379 
380### testing/synctest - Deterministic Concurrent Testing
381 
382Replace flaky sleep-based tests with deterministic scheduling:
383 
384```go
385// ❌ Old pattern - unreliable timing
386time.Sleep(100 * time.Millisecond)
387 
388// ✅ New pattern - deterministic
389import "testing/synctest"
390 
391func TestConcurrent(t *testing.T) {
392    synctest.Test(t, func() {
393        // Time moves instantly when all goroutines are blocked
394        // Perfect for testing timeouts, retries, rate limiting
395    })
396}
397```
398 
399### sync.WaitGroup.Go() - Cleaner Goroutines
400 
401```go
402// ❌ Old pattern
403var wg sync.WaitGroup
404wg.Add(1)
405go func() {
406    defer wg.Done()
407    // work
408}()
409 
410// ✅ New pattern - automatic Add/Done
411var wg sync.WaitGroup
412wg.Go(func() {
413    // work
414})
415```
416 
417### Test Output & Attributes
418 
419```go
420func TestAPI(t *testing.T) {
421    // Add test metadata
422    t.Attr("component", "api")
423    t.Attr("version", "v2")
424 
425    // Structured output
426    output := t.Output()
427    fmt.Fprintf(output, "Request: %v\n", req)
428}
429```
430 
431### runtime/trace.FlightRecorder - Production Diagnostics
432 
433Capture lightweight traces only when needed:
434 
435```go
436import "runtime/trace"
437 
438recorder := trace.NewFlightRecorder()
439defer recorder.Stop()
440 
441// Process audio/data
442if err != nil {
443    // Save trace only on error
444    recorder.WriteTo(errorLog)
445}
446```
447 
448### encoding/json/v2 (Experimental)
449 
450For performance-critical JSON operations:
451 
452```go
453import jsonv2 "encoding/json/v2"
454 
455// ~2x faster for API responses
456data, err := jsonv2.Marshal(response)
457```
458 
459## Benchmarks (Go 1.25)
460 
461- Use `b.Loop()` instead of manual `for i := 0; i < b.N; i++`
462- Use `b.TempDir()` instead of `os.MkdirTemp()`
463- Call `b.ReportAllocs()` to track memory allocations
464- Container-aware GOMAXPROCS adjusts CPU automatically
465 
466## Modern Go (1.25+)
467 
468- `any` not `interface{}`
469- `for i := range n` for loops
470- Pre-compile regex at package level
471- Store interfaces in `atomic.Value` directly
472- Use `os.Root` for filesystem sandboxing (<https://go.dev/blog/osroot>)
473- Use `sync.WaitGroup.Go()` for goroutines (<https://pkg.go.dev/sync#WaitGroup.Go>)
474- Use `testing/synctest` for concurrent tests (<https://go.dev/blog/synctest>)
475 
476## Standard Library First
477 
478- URLs: `url.Parse()`
479- IPs: `net.ParseIP()`, `ip.IsPrivate()`
480- Paths: `filepath.Join()`, `filepath.Clean()`
481- Never manual string parsing for these
482 
483## Common Patterns
484 
485- Safe type assertions: `if v, ok := x.(Type); ok { }`
486- Avoid circular dependencies in init
487- Accept interfaces, return concrete types
488- Copy data under read lock (RWMutex)
489- Chain contexts properly
490- Use dependency injection
491- Document all exports: `// TypeName does...`
492 
493## Dependency Injection for Testability
494 
495- **Pass dependencies as interfaces** through constructors or struct fields
496- **Avoid global state** - inject configuration, loggers, and clients
497- **Define minimal interfaces** close to where they're used
498- **Constructor pattern**: `NewService(deps...) *Service`
499- **Identify untestable code** - if you see direct instantiation of external dependencies, flag it
500- **Example pattern**:
501 
502```go
503  type Store interface {
504      Get(id string) (*Item, error)
505  }
506 
507  type Service struct {
508      store Store  // inject interface, not concrete type
509  }
510 
511  func NewService(store Store) *Service {
512      return &Service{store: store}
513  }
514```
515 
516- **Common injection targets**: databases, HTTP clients, file systems, time providers
517- **If you encounter code that would benefit from DI**, communicate it rather than leaving it untestable
518 
519## Security
520 
521- Validate all user input
522- Check path traversal, injection attacks
523- Validate UUIDs properly
524 
525## Goroutine Leak Detection
526 
527Add to tests that create services/goroutines:
528 
529```go
530defer goleak.VerifyNone(t,
531    goleak.IgnoreTopFunction("testing.(*T).Run"),
532    goleak.IgnoreTopFunction("runtime.gopark"),
533    goleak.IgnoreTopFunction("gopkg.in/natefinch/lumberjack%2ev2.(*Logger).millRun"),
534)
535```
536 
537- Always `defer service.Stop()` after creating services
538- Use local service instances, not global singletons
539- Use 500ms+ timeouts for async operations (CI reliability)
540 
541## Linter Compliance (Zero Tolerance)
542 
543### Active Linters & Common Fixes
544 
545| Linter           | Purpose                 | Common Fixes                                 |
546| ---------------- | ----------------------- | -------------------------------------------- |
547| **errorlint**    | Error handling          | Use `errors.Is()`, `errors.As()` not `==`    |
548| **errname**      | Error naming            | Prefix errors with `Err`: `var ErrNotFound`  |
549| **nilerr**       | Nil error returns       | Don't return nil error with non-nil value    |
550| **nilnil**       | Nil returns             | Avoid `return nil, nil` - return zero value  |
551| **bodyclose**    | HTTP bodies             | Always `defer resp.Body.Close()`             |
552| **ineffassign**  | Unused assignments      | Remove or use assigned values                |
553| **staticcheck**  | Static analysis         | Fix all SA\* warnings                        |
554| **gocritic**     | Style/performance       | Follow suggestions (rangeValCopy, etc.)      |
555| **gocognit**     | Complexity              | Split functions >50 complexity               |
556| **gocyclo**      | Cyclomatic complexity   | Refactor complex functions                   |
557| **dupl**         | Duplication             | Extract common code                          |
558| **misspell**     | Spelling                | Fix typos in comments/strings                |
559| **unconvert**    | Unnecessary conversions | Remove redundant type conversions            |
560| **wastedassign** | Wasted assignments      | Remove unused assignments                    |
561| **prealloc**     | Slice preallocation     | Use `make([]T, 0, cap)` when size known      |
562| **exhaustive**   | Switch exhaustiveness   | Handle all enum cases or add default         |
563| **testifylint**  | Testify usage           | Use `assert.Equal` not `assert.True(a == b)` |
564| **thelper**      | Test helpers            | Add `t.Helper()` to test functions           |
565| **fatcontext**   | Context usage           | Don't store context in structs               |
566| **iface**        | Interface pollution     | Accept interfaces, return structs            |
567 
568### Common Fixes by Category
569 
570#### Error Handling
571 
572```go
573// ❌ Wrong
574if err == io.EOF { }
575 
576// ✅ Correct
577if errors.Is(err, io.EOF) { }
578 
579// ❌ Wrong - nilerr
580if err != nil {
581    return nil, nil
582}
583 
584// ✅ Correct
585if err != nil {
586    return nil, err
587}
588```
589 
590#### Resource Management
591 
592```go
593// ❌ Wrong - bodyclose
594resp, _ := http.Get(url)
595 
596// ✅ Correct
597resp, err := http.Get(url)
598if err != nil {
599    return err
600}
601defer resp.Body.Close()
602```
603 
604#### Test Helpers
605 
606```go
607// ❌ Wrong - thelper
608func assertSomething(t *testing.T, val int) {
609    if val != 42 {
610        t.Errorf("expected 42")
611    }
612}
613 
614// ✅ Correct
615func assertSomething(t *testing.T, val int) {
616    t.Helper() // Add this
617    if val != 42 {
618        t.Errorf("expected 42")
619    }
620}
621```
622 
623#### Performance
624 
625```go
626// ❌ Wrong - prealloc
627var results []string
628for _, item := range items {
629    results = append(results, item)
630}
631 
632// ✅ Correct
633results := make([]string, 0, len(items))
634for _, item := range items {
635    results = append(results, item)
636}
637```
638 
639## Pre-Commit Checklist
640 
641- [ ] Run `golangci-lint run -v` - **MUST have zero errors**
642  - **Always run on full project** - never single files/packages (incomplete results)
643  - **Primary compilation validation** - don't run `go build` separately
644- [ ] Run `go test -race -v`
645- [ ] Check all linter categories above
646- [ ] No disabled linters with `//nolint` without justification
647- [ ] Document all exports
648- [ ] Handle all errors properly
649 
650## Linter Configuration Notes
651 
652- Config: `.golangci.yaml` (v2 format)
653- Complexity threshold: 50 (gocognit)
654- Disabled checks: commentFormatting, commentedOutCode (gocritic)
655- Exhaustive switches: `default` case marks as exhaustive
656- gosec disabled but configured for future use
657