Compare commits

..

4 commits

Author SHA1 Message Date
Gabriel Augendre
4410b65005
doc: update linter description
Some checks are pending
Go / build (1.22, macos-latest) (push) Waiting to run
Go / build (1.22, ubuntu-latest) (push) Waiting to run
Go / build (1.22, windows-latest) (push) Waiting to run
Go / build (1.23, macos-latest) (push) Waiting to run
Go / build (1.23, ubuntu-latest) (push) Waiting to run
Go / build (1.23, windows-latest) (push) Waiting to run
golangci-lint / lint (1.22) (push) Waiting to run
golangci-lint / lint (1.23) (push) Waiting to run
2024-08-26 10:30:12 +02:00
90afb8d3fa update readme 2024-08-26 07:47:30 +02:00
021f1b4ed6 cd: use go1.23 for golangci-lint 2024-08-26 07:46:50 +02:00
Venkatesh Kotwade
be0aa70f23
Detect nested contexts in function literals (#18)
* feat: Add detection for nested contexts in function literals
* feat: Improve detection of nested contexts in function literals
* refactor: Update getReportMessage function to handle unsupported nested context types
* use node instead of block
* refactor: use multi case
* added one more case
* feat: also added support for multiple contexts
2024-08-26 07:46:02 +02:00
4 changed files with 92 additions and 15 deletions

View file

@ -16,7 +16,7 @@ jobs:
golangci: golangci:
strategy: strategy:
matrix: matrix:
go: ['1.21', '1.22'] go: ['1.22', '1.23']
name: lint name: lint
runs-on: ubuntu-latest runs-on: ubuntu-latest
steps: steps:

View file

@ -1,6 +1,6 @@
# fatcontext # fatcontext
`fatcontext` is a Go linter which detects potential fat contexts in loops. `fatcontext` is a Go linter which detects potential fat contexts in loops or function literals.
They can lead to performance issues, as documented here: https://gabnotes.org/fat-contexts/ They can lead to performance issues, as documented here: https://gabnotes.org/fat-contexts/
## Installation / usage ## Installation / usage

View file

@ -7,6 +7,7 @@ import (
"go/ast" "go/ast"
"go/printer" "go/printer"
"go/token" "go/token"
"go/types"
"golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect" "golang.org/x/tools/go/analysis/passes/inspect"
@ -15,7 +16,7 @@ import (
var Analyzer = &analysis.Analyzer{ var Analyzer = &analysis.Analyzer{
Name: "fatcontext", Name: "fatcontext",
Doc: "detects nested contexts in loops", Doc: "detects nested contexts in loops and function literals",
Run: run, Run: run,
Requires: []*analysis.Analyzer{inspect.Analyzer}, Requires: []*analysis.Analyzer{inspect.Analyzer},
} }
@ -28,6 +29,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
nodeFilter := []ast.Node{ nodeFilter := []ast.Node{
(*ast.ForStmt)(nil), (*ast.ForStmt)(nil),
(*ast.RangeStmt)(nil), (*ast.RangeStmt)(nil),
(*ast.FuncLit)(nil),
} }
inspctr.Preorder(nodeFilter, func(node ast.Node) { inspctr.Preorder(nodeFilter, func(node ast.Node) {
@ -36,7 +38,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
return return
} }
assignStmt := findNestedContext(pass, body, body.List) assignStmt := findNestedContext(pass, node, body.List)
if assignStmt == nil { if assignStmt == nil {
return return
} }
@ -65,15 +67,25 @@ func run(pass *analysis.Pass) (interface{}, error) {
pass.Report(analysis.Diagnostic{ pass.Report(analysis.Diagnostic{
Pos: assignStmt.Pos(), Pos: assignStmt.Pos(),
Message: "nested context in loop", Message: getReportMessage(node),
SuggestedFixes: fixes, SuggestedFixes: fixes,
}) })
}) })
return nil, nil return nil, nil
} }
func getReportMessage(node ast.Node) string {
switch node.(type) {
case *ast.ForStmt, *ast.RangeStmt:
return "nested context in loop"
case *ast.FuncLit:
return "nested context in function literal"
default:
return "unsupported nested context type"
}
}
func getBody(node ast.Node) (*ast.BlockStmt, error) { func getBody(node ast.Node) (*ast.BlockStmt, error) {
forStmt, ok := node.(*ast.ForStmt) forStmt, ok := node.(*ast.ForStmt)
if ok { if ok {
@ -85,49 +97,54 @@ func getBody(node ast.Node) (*ast.BlockStmt, error) {
return rangeStmt.Body, nil return rangeStmt.Body, nil
} }
funcLit, ok := node.(*ast.FuncLit)
if ok {
return funcLit.Body, nil
}
return nil, errUnknown return nil, errUnknown
} }
func findNestedContext(pass *analysis.Pass, block *ast.BlockStmt, stmts []ast.Stmt) *ast.AssignStmt { func findNestedContext(pass *analysis.Pass, node ast.Node, stmts []ast.Stmt) *ast.AssignStmt {
for _, stmt := range stmts { for _, stmt := range stmts {
// Recurse if necessary // Recurse if necessary
if inner, ok := stmt.(*ast.BlockStmt); ok { if inner, ok := stmt.(*ast.BlockStmt); ok {
found := findNestedContext(pass, inner, inner.List) found := findNestedContext(pass, node, inner.List)
if found != nil { if found != nil {
return found return found
} }
} }
if inner, ok := stmt.(*ast.IfStmt); ok { if inner, ok := stmt.(*ast.IfStmt); ok {
found := findNestedContext(pass, inner.Body, inner.Body.List) found := findNestedContext(pass, node, inner.Body.List)
if found != nil { if found != nil {
return found return found
} }
} }
if inner, ok := stmt.(*ast.SwitchStmt); ok { if inner, ok := stmt.(*ast.SwitchStmt); ok {
found := findNestedContext(pass, inner.Body, inner.Body.List) found := findNestedContext(pass, node, inner.Body.List)
if found != nil { if found != nil {
return found return found
} }
} }
if inner, ok := stmt.(*ast.CaseClause); ok { if inner, ok := stmt.(*ast.CaseClause); ok {
found := findNestedContext(pass, block, inner.Body) found := findNestedContext(pass, node, inner.Body)
if found != nil { if found != nil {
return found return found
} }
} }
if inner, ok := stmt.(*ast.SelectStmt); ok { if inner, ok := stmt.(*ast.SelectStmt); ok {
found := findNestedContext(pass, inner.Body, inner.Body.List) found := findNestedContext(pass, node, inner.Body.List)
if found != nil { if found != nil {
return found return found
} }
} }
if inner, ok := stmt.(*ast.CommClause); ok { if inner, ok := stmt.(*ast.CommClause); ok {
found := findNestedContext(pass, block, inner.Body) found := findNestedContext(pass, node, inner.Body)
if found != nil { if found != nil {
return found return found
} }
@ -149,13 +166,13 @@ func findNestedContext(pass *analysis.Pass, block *ast.BlockStmt, stmts []ast.St
} }
if assignStmt.Tok == token.DEFINE { if assignStmt.Tok == token.DEFINE {
break continue
} }
// allow assignment to non-pointer children of values defined within the loop // allow assignment to non-pointer children of values defined within the loop
if lhs := getRootIdent(pass, assignStmt.Lhs[0]); lhs != nil { if lhs := getRootIdent(pass, assignStmt.Lhs[0]); lhs != nil {
if obj := pass.TypesInfo.ObjectOf(lhs); obj != nil { if obj := pass.TypesInfo.ObjectOf(lhs); obj != nil {
if obj.Pos() >= block.Pos() && obj.Pos() < block.End() { if checkObjectScopeWithinNode(obj.Parent(), node) {
continue // definition is within the loop continue // definition is within the loop
} }
} }
@ -167,6 +184,18 @@ func findNestedContext(pass *analysis.Pass, block *ast.BlockStmt, stmts []ast.St
return nil return nil
} }
func checkObjectScopeWithinNode(scope *types.Scope, node ast.Node) bool {
if scope == nil {
return false
}
if scope.Pos() >= node.Pos() && scope.End() <= node.End() {
return true
}
return false
}
func getRootIdent(pass *analysis.Pass, node ast.Node) *ast.Ident { func getRootIdent(pass *analysis.Pass, node ast.Node) *ast.Ident {
for { for {
switch n := node.(type) { switch n := node.(type) {

View file

@ -59,6 +59,26 @@ func example() {
break break
} }
// detects contexts wrapped in function literals (this is risky as function literals can be called multiple times)
_ = func() {
ctx = wrapContext(ctx) // want "nested context in function literal"
}
// this is fine because the context is created in the loop
for {
if ctx := context.Background(); doSomething() != nil {
ctx = wrapContext(ctx)
}
}
for {
ctx2 := context.Background()
ctx = wrapContext(ctx) // want "nested context in loop"
if doSomething() != nil {
ctx2 = wrapContext(ctx2)
}
}
} }
func wrapContext(ctx context.Context) context.Context { func wrapContext(ctx context.Context) context.Context {
@ -180,3 +200,31 @@ func inVariousNestedBlocks(ctx context.Context) {
break break
} }
} }
// this middleware could run on every request, bloating the request parameter level context and causing a memory leak
func badMiddleware(ctx context.Context) func() error {
return func() error {
ctx = wrapContext(ctx) // want "nested context in function literal"
return doSomethingWithCtx(ctx)
}
}
// this middleware is fine, as it doesn't modify the context of parent function
func okMiddleware(ctx context.Context) func() error {
return func() error {
ctx := wrapContext(ctx)
return doSomethingWithCtx(ctx)
}
}
// this middleware is fine, as it only modifies the context passed to it
func okMiddleware2(ctx context.Context) func(ctx context.Context) error {
return func(ctx context.Context) error {
ctx = wrapContext(ctx)
return doSomethingWithCtx(ctx)
}
}
func doSomethingWithCtx(ctx context.Context) error {
return nil
}