-
Notifications
You must be signed in to change notification settings - Fork 0
Add resetgen-analyzer static analysis tool #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,20 @@ | |||||
|
|
||||||
| Generate allocation-free `Reset()` methods for your structs. Perfect for `sync.Pool` usage. | ||||||
|
|
||||||
| ## Installation | ||||||
| ## Tools | ||||||
|
|
||||||
| This project provides two tools: | ||||||
|
|
||||||
| | Tool | Description | | ||||||
| |------|-------------| | ||||||
| | `resetgen` | Code generator — creates `Reset()` methods from struct tags | | ||||||
| | `resetgen-analyzer` | Static analyzer — detects missing `Reset()` calls before `sync.Pool.Put()` | | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## resetgen (Code Generator) | ||||||
|
|
||||||
| ### Installation | ||||||
|
|
||||||
| ```bash | ||||||
| go install github.com/flaticols/resetgen@latest | ||||||
|
|
@@ -14,7 +27,7 @@ Or add as a tool dependency (Go 1.24+): | |||||
| go get -tool github.com/flaticols/resetgen | ||||||
| ``` | ||||||
|
|
||||||
| ## Usage | ||||||
| ### Usage | ||||||
|
|
||||||
| Add `reset` tags to your struct fields and run the generator: | ||||||
|
|
||||||
|
|
@@ -66,29 +79,172 @@ func (s *Request) Reset() { | |||||
| > [!TIP] | ||||||
| > Structs without any `reset` tags are automatically ignored. You can have pooled and regular structs in the same file. | ||||||
|
|
||||||
| ## Example | ||||||
| ## Resetter Interface | ||||||
|
|
||||||
| Define a common interface for pooled objects: | ||||||
|
|
||||||
| ```go | ||||||
| type Resetter interface { | ||||||
| Reset() | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| All generated `Reset()` methods satisfy this interface, enabling generic pool helpers. | ||||||
|
|
||||||
| ## sync.Pool Examples | ||||||
|
|
||||||
| ### Example 1: HTTP Request Pool | ||||||
|
|
||||||
| ```go | ||||||
| //go:generate resetgen | ||||||
|
|
||||||
| package server | ||||||
|
|
||||||
| type Request struct { | ||||||
| Path string `reset:""` | ||||||
| Method string `reset:"GET"` | ||||||
| Headers map[string]string `reset:""` | ||||||
| Body []byte `reset:""` | ||||||
| UserID int `reset:""` | ||||||
| } | ||||||
|
|
||||||
| var requestPool = sync.Pool{ | ||||||
| New: func() any { return new(Request) }, | ||||||
| } | ||||||
|
|
||||||
| func HandleRequest(path, method string, body []byte) { | ||||||
| req := requestPool.Get().(*Request) | ||||||
|
|
||||||
| req.Path = path | ||||||
| req.Method = method | ||||||
| req.Body = append(req.Body, body...) | ||||||
|
|
||||||
| process(req) | ||||||
|
|
||||||
| req.Reset() | ||||||
| requestPool.Put(req) | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| ### Example 2: Generic Pool with Resetter | ||||||
|
|
||||||
| ```go | ||||||
| //go:generate resetgen | ||||||
|
|
||||||
| package pool | ||||||
|
|
||||||
| type Resetter interface { | ||||||
| Reset() | ||||||
| } | ||||||
|
|
||||||
| // Pool is a generic, allocation-free pool for any Resetter | ||||||
| type Pool[T Resetter] struct { | ||||||
| p sync.Pool | ||||||
| } | ||||||
|
|
||||||
| func NewPool[T Resetter](newFn func() T) *Pool[T] { | ||||||
| return &Pool[T]{ | ||||||
| p: sync.Pool{New: func() any { return newFn() }}, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| func (p *Pool[T]) Get() T { return p.p.Get().(T) } | ||||||
| func (p *Pool[T]) Put(v T) { v.Reset(); p.p.Put(v) } | ||||||
| ``` | ||||||
|
|
||||||
| Usage with a buffer: | ||||||
|
|
||||||
| ```go | ||||||
| //go:generate resetgen | ||||||
|
|
||||||
| package encoding | ||||||
|
|
||||||
| type Buffer struct { | ||||||
| Data []byte `reset:""` | ||||||
| Headers map[string]string `reset:""` | ||||||
| Status int `reset:"200"` | ||||||
| err error // no tag = unchanged | ||||||
| data []byte `reset:""` | ||||||
| } | ||||||
|
|
||||||
| // Config has no reset tags — ignored | ||||||
| type Config struct { | ||||||
| Timeout int | ||||||
| Debug bool | ||||||
| var bufPool = pool.NewPool(func() *Buffer { return new(Buffer) }) | ||||||
|
|
||||||
| // MarshalTo writes encoded data directly to dst — zero allocations | ||||||
| func MarshalTo(dst io.Writer, v any) error { | ||||||
| buf := bufPool.Get() | ||||||
|
|
||||||
| buf.data = encodeJSON(buf.data, v) | ||||||
| _, err := dst.Write(buf.data) | ||||||
|
|
||||||
| bufPool.Put(buf) | ||||||
| return err | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| > [!NOTE] | ||||||
| > Only `Buffer` gets a `Reset()` method. `Config` is ignored. | ||||||
| > Both examples avoid defer closures and return values that reference pooled memory. | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## resetgen-analyzer (Static Analyzer) | ||||||
|
|
||||||
| Detects when `sync.Pool.Put()` is called without a preceding `Reset()` call. | ||||||
|
|
||||||
| ### Installation | ||||||
|
|
||||||
| ```bash | ||||||
| go install github.com/flaticols/resetgen/cmd/resetgen-analyzer@latest | ||||||
| ``` | ||||||
|
|
||||||
| Or add as a tool dependency (Go 1.24+): | ||||||
|
|
||||||
| ```bash | ||||||
| go get -tool github.com/flaticols/resetgen/cmd/resetgen-analyzer | ||||||
| ``` | ||||||
|
|
||||||
| ### Usage | ||||||
|
|
||||||
| Run standalone: | ||||||
|
|
||||||
| ```bash | ||||||
| resetgen-analyzer ./... | ||||||
| ``` | ||||||
|
|
||||||
| Run with `go vet`: | ||||||
|
|
||||||
| ```bash | ||||||
| go vet -vettool=$(which resetgen-analyzer) ./... | ||||||
| ``` | ||||||
|
|
||||||
| Add to your CI pipeline or Makefile: | ||||||
|
|
||||||
| ```makefile | ||||||
| .PHONY: lint | ||||||
| lint: | ||||||
| go vet -vettool=$(which resetgen-analyzer) ./... | ||||||
| ``` | ||||||
|
|
||||||
| ### What it detects | ||||||
|
|
||||||
| ```go | ||||||
| func BadUsage() { | ||||||
| buf := bufferPool.Get().(*Buffer) | ||||||
| buf.data = append(buf.data, "hello"...) | ||||||
| bufferPool.Put(buf) // ERROR: sync.Pool.Put() called without Reset() on buf | ||||||
| } | ||||||
|
|
||||||
| func GoodUsage() { | ||||||
| buf := bufferPool.Get().(*Buffer) | ||||||
| buf.data = append(buf.data, "hello"...) | ||||||
| buf.Reset() | ||||||
| bufferPool.Put(buf) // OK | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| ### Detected patterns | ||||||
|
|
||||||
| | Pattern | Example | | ||||||
| |---------|---------| | ||||||
| | 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 | | ||||||
|
||||||
| | 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) | |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| // Package analyzer provides a static analyzer that detects sync.Pool usage | ||
| // without calling Reset() before Put(). | ||
| package analyzer | ||
|
|
||
| import ( | ||
| "go/ast" | ||
| "go/types" | ||
|
|
||
| "golang.org/x/tools/go/analysis" | ||
| "golang.org/x/tools/go/analysis/passes/inspect" | ||
| "golang.org/x/tools/go/ast/inspector" | ||
| ) | ||
|
|
||
| var Analyzer = &analysis.Analyzer{ | ||
| Name: "resetcheck", | ||
| Doc: "checks that Reset() is called before sync.Pool.Put()", | ||
| Requires: []*analysis.Analyzer{inspect.Analyzer}, | ||
| Run: run, | ||
| } | ||
|
|
||
| func run(pass *analysis.Pass) (any, error) { | ||
| insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) | ||
|
|
||
| // Analyze each function separately | ||
| nodeFilter := []ast.Node{ | ||
| (*ast.FuncDecl)(nil), | ||
| (*ast.FuncLit)(nil), | ||
| } | ||
|
|
||
| insp.Preorder(nodeFilter, func(n ast.Node) { | ||
| var body *ast.BlockStmt | ||
| switch fn := n.(type) { | ||
| case *ast.FuncDecl: | ||
| if fn.Body == nil { | ||
| return | ||
| } | ||
| body = fn.Body | ||
| case *ast.FuncLit: | ||
| body = fn.Body | ||
| } | ||
|
|
||
| analyzeFunction(pass, body) | ||
| }) | ||
|
|
||
| return nil, nil | ||
| } | ||
|
|
||
| func analyzeFunction(pass *analysis.Pass, body *ast.BlockStmt) { | ||
| // Track variables that had Reset() called on them | ||
| resetCalled := make(map[string]bool) | ||
|
|
||
| // Walk statements in order | ||
| ast.Inspect(body, func(n ast.Node) bool { | ||
| stmt, ok := n.(*ast.ExprStmt) | ||
| if !ok { | ||
| return true | ||
| } | ||
|
|
||
| call, ok := stmt.X.(*ast.CallExpr) | ||
| if !ok { | ||
| return true | ||
| } | ||
|
|
||
| sel, ok := call.Fun.(*ast.SelectorExpr) | ||
| if !ok { | ||
| return true | ||
| } | ||
|
|
||
| // Check for x.Reset() calls - track any variable that had Reset called | ||
| if sel.Sel.Name == "Reset" && len(call.Args) == 0 { | ||
| varName := extractVarName(sel.X) | ||
| if varName != "" { | ||
| resetCalled[varName] = true | ||
| } | ||
| } | ||
|
|
||
| // Check for sync.Pool.Put(x) calls | ||
| if sel.Sel.Name == "Put" && isSyncPoolMethod(sel, pass.TypesInfo) { | ||
| if len(call.Args) == 1 { | ||
| varName := extractVarName(call.Args[0]) | ||
| if varName != "" && !resetCalled[varName] { | ||
| pass.Reportf(call.Pos(), "sync.Pool.Put() called without Reset() on %s", varName) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return true | ||
| }) | ||
| } | ||
flaticols marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // extractVarName gets the variable name from an expression | ||
| // Handles: x, s.x, s.field.x | ||
| func extractVarName(expr ast.Expr) string { | ||
| switch e := expr.(type) { | ||
| case *ast.Ident: | ||
| return e.Name | ||
| case *ast.SelectorExpr: | ||
| // For s.field, we still track by the root identifier | ||
| return extractVarName(e.X) | ||
| case *ast.StarExpr: | ||
| return extractVarName(e.X) | ||
| } | ||
| return "" | ||
| } | ||
flaticols marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // isSyncPoolMethod checks if sel is a method on sync.Pool | ||
| func isSyncPoolMethod(sel *ast.SelectorExpr, info *types.Info) bool { | ||
| tv, ok := info.Types[sel.X] | ||
| if !ok { | ||
| return false | ||
| } | ||
|
|
||
| t := tv.Type | ||
| if ptr, isPtr := t.(*types.Pointer); isPtr { | ||
| t = ptr.Elem() | ||
| } | ||
|
|
||
| named, isNamed := t.(*types.Named) | ||
| if !isNamed { | ||
| return false | ||
| } | ||
|
|
||
| obj := named.Obj() | ||
| if obj == nil { | ||
| return false | ||
| } | ||
|
|
||
| pkg := obj.Pkg() | ||
| if pkg == nil { | ||
| return false | ||
| } | ||
|
|
||
| return pkg.Path() == "sync" && obj.Name() == "Pool" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package analyzer_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/flaticols/resetgen/cmd/resetgen-analyzer/analyzer" | ||
| "golang.org/x/tools/go/analysis/analysistest" | ||
| ) | ||
|
|
||
| func TestAnalyzer(t *testing.T) { | ||
| testdata := analysistest.TestData() | ||
| analysistest.Run(t, testdata, analyzer.Analyzer, "a") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
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.