Skip to content

Commit

Permalink
Merge branch 'support-slice'
Browse files Browse the repository at this point in the history
  • Loading branch information
kyoh86 committed Nov 16, 2022
2 parents e1c1ea0 + 15ddf40 commit 1a8d24d
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 31 deletions.
106 changes: 75 additions & 31 deletions looppointer.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,45 @@ func run(pass *analysis.Pass) (interface{}, error) {
(*ast.RangeStmt)(nil),
(*ast.ForStmt)(nil),
(*ast.UnaryExpr)(nil),
(*ast.SliceExpr)(nil),
}

inspect.WithStack(nodeFilter, func(n ast.Node, push bool, stack []ast.Node) bool {
id, insert, digg := search.Check(n, stack)
if id != nil && !noLinter.IgnoreNode(id, "looppointer") {
dMsg := fmt.Sprintf("taking a pointer for the loop variable %s", id.Name)
fMsg := fmt.Sprintf("loop variable %s should be pinned", id.Name)
var suggest []analysis.SuggestedFix
if insert != token.NoPos {
suggest = []analysis.SuggestedFix{{
Message: fMsg,
TextEdits: []analysis.TextEdit{{
Pos: insert,
End: insert,
NewText: []byte(fmt.Sprintf("%[1]s := %[1]s\n", id.Name)),
}},
}}
}
pass.Report(analysis.Diagnostic{
Pos: id.Pos(),
End: id.End(),
Message: dMsg,
Category: "looppointer",
SuggestedFixes: suggest,
})
inspect.WithStack(nodeFilter, func(n ast.Node, _ bool, stack []ast.Node) bool {
typ, id, insert, digg := search.Check(n, stack)
if typ == RefTypeNone {
return digg
}
if noLinter.IgnoreNode(id, "looppointer") {
return digg
}
var dMsg string
switch typ {
case RefTypePointer:
dMsg = fmt.Sprintf("taking a pointer for the loop variable %s", id.Name)
case RefTypeSlice:
dMsg = fmt.Sprintf("taking a ref for the slice from loop variable %s", id.Name)
default:
return digg
}
fMsg := fmt.Sprintf("loop variable %s should be pinned", id.Name)
var suggest []analysis.SuggestedFix
if insert != token.NoPos {
suggest = []analysis.SuggestedFix{{
Message: fMsg,
TextEdits: []analysis.TextEdit{{
Pos: insert,
End: insert,
NewText: []byte(fmt.Sprintf("%[1]s := %[1]s\n", id.Name)),
}},
}}
}
pass.Report(analysis.Diagnostic{
Pos: id.Pos(),
End: id.End(),
Message: dMsg,
Category: "looppointer",
SuggestedFixes: suggest,
})
return digg
})

Expand All @@ -74,16 +87,26 @@ type Searcher struct {
Stats map[token.Pos]struct{}
}

func (s *Searcher) Check(n ast.Node, stack []ast.Node) (*ast.Ident, token.Pos, bool) {
type RefType int

const (
RefTypeNone RefType = iota
RefTypePointer
RefTypeSlice
)

func (s *Searcher) Check(n ast.Node, stack []ast.Node) (RefType, *ast.Ident, token.Pos, bool) {
switch typed := n.(type) {
case *ast.RangeStmt:
s.parseRangeStmt(typed)
case *ast.ForStmt:
s.parseForStmt(typed)
case *ast.UnaryExpr:
return s.checkUnaryExpr(typed, stack)
case *ast.SliceExpr:
return s.checkSliceExpr(typed, stack)
}
return nil, token.NoPos, true
return RefTypeNone, nil, token.NoPos, true
}

func (s *Searcher) parseRangeStmt(n *ast.RangeStmt) {
Expand Down Expand Up @@ -129,29 +152,50 @@ func (s *Searcher) innermostLoop(stack []ast.Node) (ast.Node, token.Pos) {
return nil, token.NoPos
}

func (s *Searcher) checkUnaryExpr(n *ast.UnaryExpr, stack []ast.Node) (*ast.Ident, token.Pos, bool) {
func (s *Searcher) checkUnaryExpr(n *ast.UnaryExpr, stack []ast.Node) (RefType, *ast.Ident, token.Pos, bool) {
loop, insert := s.innermostLoop(stack)
if loop == nil {
return nil, token.NoPos, true
return RefTypeNone, nil, token.NoPos, true
}

if n.Op != token.AND {
return nil, token.NoPos, true
return RefTypeNone, nil, token.NoPos, true
}

// Get identity of the referred item
id := getIdentity(n.X)
if id == nil || id.Obj == nil {
return RefTypeNone, nil, token.NoPos, true
}

// If the identity is not the loop statement variable,
// it will not be reported.
if _, isStat := s.Stats[id.Obj.Pos()]; !isStat {
return RefTypeNone, nil, token.NoPos, true
}

return RefTypePointer, id, insert, false
}

func (s *Searcher) checkSliceExpr(n *ast.SliceExpr, stack []ast.Node) (RefType, *ast.Ident, token.Pos, bool) {
loop, insert := s.innermostLoop(stack)
if loop == nil {
return RefTypeNone, nil, token.NoPos, true
}

// Get identity of the referred item
id := getIdentity(n.X)
if id == nil || id.Obj == nil {
return nil, token.NoPos, true
return RefTypeNone, nil, token.NoPos, true
}

// If the identity is not the loop statement variable,
// it will not be reported.
if _, isStat := s.Stats[id.Obj.Pos()]; !isStat {
return nil, token.NoPos, true
return RefTypeNone, nil, token.NoPos, true
}

return id, insert, false
return RefTypeSlice, id, insert, false
}

// Get variable identity
Expand Down
1 change: 1 addition & 0 deletions looppointer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ func Test(t *testing.T) {
analysistest.Run(t, testdata, looppointer.Analyzer, "issue7")
analysistest.Run(t, testdata, looppointer.Analyzer, "nolint")
analysistest.Run(t, testdata, looppointer.Analyzer, "nested")
analysistest.Run(t, testdata, looppointer.Analyzer, "slice")
}
17 changes: 17 additions & 0 deletions testdata/src/slice/slice.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package main

import "fmt"

var a = [][3]int{
{1, 2, 3},
{4, 5, 6},
{7, 8, 9},
}

func main() {
var s [][]int
for _, e := range a {
s = append(s, e[:]) // want "taking a ref for the slice from loop variable e"
}
fmt.Println(s)
}

0 comments on commit 1a8d24d

Please sign in to comment.