mirror of
https://github.com/Crocmagnon/fatcontext.git
synced 2024-11-25 01:18:09 +01:00
Compare commits
4 commits
0d2c4019d4
...
4410b65005
Author | SHA1 | Date | |
---|---|---|---|
|
4410b65005 | ||
90afb8d3fa | |||
021f1b4ed6 | |||
|
be0aa70f23 |
4 changed files with 92 additions and 15 deletions
2
.github/workflows/golangci-lint.yml
vendored
2
.github/workflows/golangci-lint.yml
vendored
|
@ -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:
|
||||||
|
|
|
@ -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
|
||||||
|
|
|
@ -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) {
|
||||||
|
|
48
testdata/src/example.go
vendored
48
testdata/src/example.go
vendored
|
@ -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
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue