Ignore when ranging over a slice of pointers (#6)

Author: kyoh86

exportloopref.go

@@ -3,6 +3,7 @@ package exportloopref
 import (
 	"go/ast"
 	"go/token"
+	"go/types"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
@@ -29,6 +30,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	search := &Searcher{
 		Stats: map[token.Pos]struct{}{},
 		Vars:  map[token.Pos]map[token.Pos]struct{}{},
+		Types: pass.TypesInfo.Types,
 	}
 
 	nodeFilter := []ast.Node{
@@ -51,10 +53,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
 }
 
 type Searcher struct {
-	// statement variables
+	// Statement variables : map to collect positions that
+	// variables are declared like below.
+	//  - for <KEY>, <VALUE> := range ...
+	//  - var <X> int
+	//  - D := ...
 	Stats map[token.Pos]struct{}
-	// internal variables
-	Vars map[token.Pos]map[token.Pos]struct{}
+	// Internal variables maps loop-position, decl-location to ignore
+	// safe pointers for variable which declared in the loop.
+	Vars  map[token.Pos]map[token.Pos]struct{}
+	Types map[ast.Expr]types.TypeAndValue
 }
 
 func (s *Searcher) Check(n ast.Node, stack []ast.Node) (*ast.Ident, bool) {
@@ -67,6 +75,7 @@ func (s *Searcher) Check(n ast.Node, stack []ast.Node) (*ast.Ident, bool) {
 		s.parseDeclStmt(typed, stack)
 	case *ast.AssignStmt:
 		s.parseAssignStmt(typed, stack)
+
 	case *ast.UnaryExpr:
 		return s.checkUnaryExpr(typed, stack)
 	}
@@ -162,7 +171,7 @@ func (s *Searcher) checkUnaryExpr(n *ast.UnaryExpr, stack []ast.Node) (*ast.Iden
 	}
 
 	// Get identity of the referred item
-	id := getIdentity(n.X)
+	id := s.getIdentity(n.X)
 	if id == nil {
 		return nil, true
 	}
@@ -245,20 +254,27 @@ func (s *Searcher) isVar(loop ast.Node, expr ast.Expr) bool {
 }
 
 // Get variable identity
-func getIdentity(expr ast.Expr) *ast.Ident {
+func (s *Searcher) getIdentity(expr ast.Expr) *ast.Ident {
 	switch typed := expr.(type) {
 	case *ast.SelectorExpr:
 		// Get parent identity; i.e. `a` of the `a.b`.
 		parent, ok := typed.X.(*ast.Ident)
 		if !ok {
 			return nil
 		}
+
 		// parent is a package name identity
 		if parent.Obj == nil {
 			return nil
 		}
+
+		// Ignore if the parent is pointer ref (fix for #2)
+		if _, ok := s.Types[parent].Type.(*types.Pointer); ok {
+			return nil
+		}
+
 		// NOTE: If that is descendants member like `a.b.c`,
-		//       typed.X will be `*ast.SelectorExpr`.
+		//       typed.X will be `*ast.SelectorExpr` `a.b`.
 		return parent
 
 	case *ast.Ident:

exportloopref_test.go

@@ -7,10 +7,32 @@ import (
 	"golang.org/x/tools/go/analysis/analysistest"
 )
 
-func Test(t *testing.T) {
+func TestSimple(t *testing.T) {
 	testdata := analysistest.TestData()
 	analysistest.Run(t, testdata, exportloopref.Analyzer, "simple")
+}
+
+func TestStruct(t *testing.T) {
+	testdata := analysistest.TestData()
 	analysistest.Run(t, testdata, exportloopref.Analyzer, "struct")
+}
+
+func TestComplex(t *testing.T) {
+	testdata := analysistest.TestData()
 	analysistest.Run(t, testdata, exportloopref.Analyzer, "complex")
+}
+
+func TestFixed(t *testing.T) {
+	testdata := analysistest.TestData()
 	analysistest.Run(t, testdata, exportloopref.Analyzer, "fixed")
 }
+
+func TestPslice(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, exportloopref.Analyzer, "pslice")
+}
+
+func TestIssue2(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, exportloopref.Analyzer, "issue2")
+}

testdata/src/issue2/issue2.go

@@ -0,0 +1,33 @@
+package main
+
+// Issue #2 (https://github.com/kyoh86/exportloopref/issues/2
+// This is valid code because n takes new pointer values, so &n.name points correctly to different names.
+
+import (
+	"fmt"
+)
+
+type Node struct {
+	name string
+}
+
+type Nodes []*Node
+
+type Graph struct {
+	nodes Nodes
+}
+
+func main() {
+	var graph Graph
+	var s *string
+
+	graph.nodes = Nodes{&Node{"hello"}, &Node{"world"}, &Node{"foo"}, &Node{"bar"}, &Node{"baz"}}
+
+	for i, n := range graph.nodes {
+		if i == 2 {
+			s = &n.name // here
+		}
+	}
+
+	fmt.Println(*s)
+}

testdata/src/pslice/pslice.go

@@ -0,0 +1,25 @@
+package main
+
+func main() {
+	var pslice []*int
+	var ppslice []**int
+
+	println("loop expecting 10, 11, 12, 13")
+	for _, p := range []int{10, 11, 12, 13} {
+		p := p
+		pslice = append(pslice, &p) // not a diagnostic (fixed p)
+	}
+
+	for _, p := range pslice {
+		ppslice = append(ppslice, &p) // want "exporting a pointer for the loop variable p"
+	}
+
+	println(`ppslice expecting "10, 11, 12, 13" but "13, 13, 13, 13"`)
+	for _, p := range ppslice {
+		printp(*p)
+	}
+}
+
+func printp(p *int) {
+	println(*p)
+}