Compare commits

...

4 commits

Author SHA1 Message Date
e8053df661
Merge pull request #9 from MichaelUrman/contained
Some checks failed
Go / build (1.21, macos-latest) (push) Has been cancelled
Go / build (1.21, ubuntu-latest) (push) Has been cancelled
Go / build (1.21, windows-latest) (push) Has been cancelled
Go / build (1.22, macos-latest) (push) Has been cancelled
Go / build (1.22, ubuntu-latest) (push) Has been cancelled
Go / build (1.22, windows-latest) (push) Has been cancelled
golangci-lint / lint (1.21) (push) Has been cancelled
golangci-lint / lint (1.22) (push) Has been cancelled
Test and fix cases with nested contexts
2024-05-29 17:29:50 +02:00
Michael Urman
69e9ae12fc Explain getRootIdent/ObjectOf/Pos check 2024-05-29 10:26:10 -05:00
Michael Urman
0e13d068ad Fix array test
The prior construct probably shouldn't issue an error.
These, however, both should (and do).
2024-05-29 10:07:03 -05:00
Michael Urman
3e9e29f41c Test and fix cases with nested contexts
As long as the context is rooted in a non-pointer value that has a new
copy in the loop, it is as safe to copy that value as it is to copy the
context. So only report such cases when they are indirected and thus
shared.
2024-05-29 09:47:58 -05:00
2 changed files with 69 additions and 0 deletions

View file

@ -7,6 +7,7 @@ import (
"go/ast"
"go/printer"
"go/token"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
@ -54,6 +55,15 @@ func run(pass *analysis.Pass) (interface{}, error) {
break
}
// allow assignment to non-pointer children of values defined within the loop
if lhs := getRootIdent(pass, assignStmt.Lhs[0]); lhs != nil {
if obj := pass.TypesInfo.ObjectOf(lhs); obj != nil {
if obj.Pos() >= body.Pos() && obj.Pos() < body.End() {
continue // definition is within the loop
}
}
}
suggestedStmt := ast.AssignStmt{
Lhs: assignStmt.Lhs,
TokPos: assignStmt.TokPos,
@ -103,6 +113,24 @@ func getBody(node ast.Node) (*ast.BlockStmt, error) {
return nil, errUnknown
}
func getRootIdent(pass *analysis.Pass, node ast.Node) *ast.Ident {
for {
switch n := node.(type) {
case *ast.Ident:
return n
case *ast.IndexExpr:
node = n.X
case *ast.SelectorExpr:
if sel, ok := pass.TypesInfo.Selections[n]; ok && sel.Indirect() {
return nil // indirected (pointer) roots don't imply a (safe) copy
}
node = n.X
default:
return nil
}
}
}
// render returns the pretty-print of the given node
func render(fset *token.FileSet, x interface{}) (string, error) {
var buf bytes.Buffer

View file

@ -30,3 +30,44 @@ func example() {
func wrapContext(ctx context.Context) context.Context {
return context.WithoutCancel(ctx)
}
// storing contexts in a struct isn't recommended, but local copies of a non-pointer struct should act like local copies of a context.
func inStructs(ctx context.Context) {
for i := 0; i < 10; i++ {
c := struct{ Ctx context.Context }{ctx}
c.Ctx = context.WithValue(c.Ctx, "key", i)
c.Ctx = context.WithValue(c.Ctx, "other", "val")
}
for i := 0; i < 10; i++ {
c := []struct{ Ctx context.Context }{{ctx}}
c[0].Ctx = context.WithValue(c[0].Ctx, "key", i)
c[0].Ctx = context.WithValue(c[0].Ctx, "other", "val")
}
c := struct{ Ctx context.Context }{ctx}
for i := 0; i < 10; i++ {
c := c
c.Ctx = context.WithValue(c.Ctx, "key", i)
c.Ctx = context.WithValue(c.Ctx, "other", "val")
}
pc := &struct{ Ctx context.Context }{ctx}
for i := 0; i < 10; i++ {
c := pc
c.Ctx = context.WithValue(c.Ctx, "key", i) // want "nested context in loop"
c.Ctx = context.WithValue(c.Ctx, "other", "val")
}
r := []struct{ Ctx context.Context }{{ctx}}
for i := 0; i < 10; i++ {
r[0].Ctx = context.WithValue(r[0].Ctx, "key", i) // want "nested context in loop"
r[0].Ctx = context.WithValue(r[0].Ctx, "other", "val")
}
rp := []*struct{ Ctx context.Context }{{ctx}}
for i := 0; i < 10; i++ {
rp[0].Ctx = context.WithValue(rp[0].Ctx, "key", i) // want "nested context in loop"
rp[0].Ctx = context.WithValue(rp[0].Ctx, "other", "val")
}
}