Add resetgen-analyzer static analysis tool #5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds resetgen-analyzer, a static analysis tool that detects missing Reset() calls before sync.Pool.Put() operations. The analyzer complements the existing resetgen code generator by providing runtime validation of proper pool usage patterns.
Key Changes:
- Adds static analyzer CLI and core detection logic for sync.Pool misuse patterns
- Implements variable tracking to match Reset() calls with Pool.Put() operations
- Includes comprehensive test suite with testdata covering various pool usage scenarios
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Adds golang.org/x/tools dependency for analysis framework |
go.sum |
Dependency checksums for new analyzer dependencies |
cmd/resetgen-analyzer/main.go |
CLI entry point using singlechecker for standalone/go vet integration |
cmd/resetgen-analyzer/analyzer/analyzer.go |
Core analysis logic tracking Reset() calls and sync.Pool.Put() operations |
cmd/resetgen-analyzer/analyzer/analyzer_test.go |
Test harness using analysistest framework |
cmd/resetgen-analyzer/analyzer/testdata/src/a/a.go |
Comprehensive test cases covering good/bad pool usage patterns |
README.md |
Documentation for both tools with installation, usage, and examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bufferPool.Put(w.buf) // want "sync.Pool.Put\\(\\) called without Reset\\(\\) on w" | ||
| } | ||
|
|
||
| // Generic pool wrapper that handles Reset internally - should NOT trigger |
There was a problem hiding this comment.
The comment "should NOT trigger" is misleading. The analyzer currently doesn't analyze the body of genericPool.Put() to see if it calls Reset() internally. The line at 112 doesn't trigger an error only because genericPool is not a sync.Pool type - it's a custom Pool[T] type. The analyzer only checks for direct sync.Pool.Put() calls. This should be clarified in the comment to avoid confusion about what the analyzer actually does versus what it's supposed to do.
| // Generic pool wrapper that handles Reset internally - should NOT trigger | |
| // Generic pool wrapper that handles Reset internally. | |
| // NOTE: The analyzer does NOT analyze the body of genericPool.Put() to see if it calls Reset(). | |
| // The line below does NOT trigger an error only because genericPool is not a sync.Pool type. | |
| // The analyzer only checks for direct sync.Pool.Put() calls. |
|
|
||
| buf.data = encodeJSON(buf.data, v) | ||
| _, err := dst.Write(buf.data) | ||
|
|
There was a problem hiding this comment.
This example has a bug: the buffer is returned to the pool at line 175 without calling buf.Reset() first. This violates the pattern that the resetgen-analyzer is designed to detect. The code should be:
buf.Reset()
bufPool.Put(buf)
Or alternatively, since this uses the generic Pool wrapper that calls Reset() internally (line 152), the comment should clarify that Reset() is handled by the wrapper's Put method.
| buf.Reset() |
| | Global pool | `bufferPool.Put(buf)` without `buf.Reset()` | | ||
| | Struct field pool | `s.pool.Put(buf)` without `buf.Reset()` | | ||
| | Wrapped variable | `pool.Put(w.buf)` without `w.buf.Reset()` | | ||
| | Pool wrapper | `p.p.Put(v)` without `v.Reset()` inside wrapper method | |
There was a problem hiding this comment.
The "Pool wrapper" pattern description is misleading. The current analyzer implementation doesn't actually analyze what happens inside wrapper methods. It only detects direct sync.Pool.Put() calls. So p.p.Put(v) is detected because p.p is a sync.Pool, not because it's inside a wrapper method. The analyzer would flag this even if Reset() was called elsewhere in the same method. Consider clarifying this documentation to accurately reflect what the analyzer does versus what it's intended to catch.
| | Pool wrapper | `p.p.Put(v)` without `v.Reset()` inside wrapper method | | |
| | Pool wrapper | `p.p.Put(v)` without `v.Reset()` (analyzer does not check if inside wrapper method or if Reset is called elsewhere) | |
1 participant
Summary
Adds
resetgen-analyzer- a static analyzer that detects missingReset()calls beforesync.Pool.Put().Features
sync.Pool.Put(x)without precedingx.Reset()p.p.Put(v)withoutv.Reset())go vet -vettoolInstallation
go install github.com/flaticols/resetgen/cmd/resetgen-analyzer@latest # or go get -tool github.com/flaticols/resetgen/cmd/resetgen-analyzerUsage
Files
cmd/resetgen-analyzer/main.go- CLI entry pointcmd/resetgen-analyzer/analyzer/- Core analysis logic with tests