range-val-address: fix false positive (#554)

range-val-address: fix false positive (closes #554)

Author: bernhardreisenberger

README.md

@@ -440,7 +440,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`function-result-limit`](./RULES_DESCRIPTIONS.md#function-result-limit) |  int | Specifies the maximum number of results a function can return    |    no    |  no   |
 | [`imports-blacklist`](./RULES_DESCRIPTIONS.md#imports-blacklist)   | []string | Disallows importing the specified packages                     |    no    |  no   |
 | [`range-val-in-closure`](./RULES_DESCRIPTIONS.md#range-val-in-closure)|  n/a   | Warns if range value is used in a closure dispatched as goroutine|    no    |  no   |
-| [`range-val-address`](./RULES_DESCRIPTIONS.md#range-val-address)|  n/a   | Warns if address of range value is used dangerously |    no    |  no   |
+| [`range-val-address`](./RULES_DESCRIPTIONS.md#range-val-address)|  n/a   | Warns if address of range value is used dangerously |    no    |  yes   |
 | [`waitgroup-by-value`](./RULES_DESCRIPTIONS.md#waitgroup-by-value)  |  n/a   | Warns on functions taking sync.WaitGroup as a by-value parameter |    no    |  no   |
 | [`atomic`](./RULES_DESCRIPTIONS.md#atomic)              |  n/a   | Check for common mistaken usages of the `sync/atomic` package    |    no    |  no   |
 | [`empty-lines`](./RULES_DESCRIPTIONS.md#empty-lines)   | n/a | Warns when there are heading or trailing newlines in a block              |    no    |  no   |

rule/range-val-address.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
+	"strings"
 
 	"github.com/mgechev/revive/lint"
 )
@@ -16,11 +17,13 @@ func (r *RangeValAddress) Apply(file *lint.File, _ lint.Arguments) []lint.Failur
 	var failures []lint.Failure
 
 	walker := rangeValAddress{
+		file: file,
 		onFailure: func(failure lint.Failure) {
 			failures = append(failures, failure)
 		},
 	}
 
+	file.Pkg.TypeCheck()
 	ast.Walk(walker, file.AST)
 
 	return failures
@@ -32,6 +35,7 @@ func (r *RangeValAddress) Name() string {
 }
 
 type rangeValAddress struct {
+	file      *lint.File
 	onFailure func(lint.Failure)
 }
 
@@ -46,17 +50,24 @@ func (w rangeValAddress) Visit(node ast.Node) ast.Visitor {
 		return w
 	}
 
+	valueIsStarExpr := false
+	if t := w.file.Pkg.TypeOf(value); t != nil {
+		valueIsStarExpr = strings.HasPrefix(t.String(), "*")
+	}
+
 	ast.Walk(rangeBodyVisitor{
-		valueID:   value.Obj,
-		onFailure: w.onFailure,
+		valueIsStarExpr: valueIsStarExpr,
+		valueID:         value.Obj,
+		onFailure:       w.onFailure,
 	}, n.Body)
 
 	return w
 }
 
 type rangeBodyVisitor struct {
-	valueID   *ast.Object
-	onFailure func(lint.Failure)
+	valueIsStarExpr bool
+	valueID         *ast.Object
+	onFailure       func(lint.Failure)
 }
 
 func (bw rangeBodyVisitor) Visit(node ast.Node) ast.Visitor {
@@ -112,6 +123,13 @@ func (bw rangeBodyVisitor) isAccessingRangeValueAddress(exp ast.Expr) bool {
 			return false
 		}
 		v, ok = s.X.(*ast.Ident)
+		if !ok {
+			return false
+		}
+
+		if bw.valueIsStarExpr { // check type of value
+			return false
+		}
 	}
 
 	return ok && v.Obj == bw.valueID

testdata/range-val-address.go

@@ -57,3 +57,84 @@ func rangeValAddress6() {
 		m = append(m, &value.id) // MATCH /suspicious assignment of 'value'. range-loop variables always have the same address/
 	}
 }
+
+func rangeValAddress7() {
+	type v struct {
+		id string
+	}
+	m := []*string{}
+
+	for _, value := range []v{{id: "A"}, {id: "B"}, {id: "C"}} {
+		m = append(m, &value.id) // MATCH /suspicious assignment of 'value'. range-loop variables always have the same address/
+	}
+}
+
+func rangeValAddress8() {
+	type v struct {
+		id string
+	}
+	m := []*string{}
+
+	mySlice := []*v{{id: "A"}, {id: "B"}, {id: "C"}}
+	for _, value := range mySlice {
+		m = append(m, &value.id)
+	}
+}
+
+func rangeValAddress9() {
+	type v struct {
+		id string
+	}
+	m := []*string{}
+
+	mySlice := map[string]*v{"a": {id: "A"}, "b": {id: "B"}, "c": {id: "C"}}
+	for _, value := range mySlice {
+		m = append(m, &value.id)
+	}
+}
+
+func rangeValAddress10() {
+	type v struct {
+		id string
+	}
+	m := []*string{}
+
+	for _, value := range map[string]*v{"a": {id: "A"}, "b": {id: "B"}, "c": {id: "C"}} {
+		m = append(m, &value.id)
+	}
+}
+
+func rangeValAddress11() {
+	type v struct {
+		id string
+	}
+	m := map[string]*string{}
+
+	for key, value := range map[string]*v{"a": {id: "A"}, "b": {id: "B"}, "c": {id: "C"}} {
+		m[key] = &value.id
+	}
+}
+
+func rangeValAddress12() {
+	type v struct {
+		id string
+	}
+	m := map[string]*string{}
+
+	for key, value := range map[string]v{"a": {id: "A"}, "b": {id: "B"}, "c": {id: "C"}} {
+		m[key] = &value.id // MATCH /suspicious assignment of 'value'. range-loop variables always have the same address/
+	}
+}
+
+func rangeValAddress13() {
+	type v struct {
+		id string
+	}
+	m := []*string{}
+
+	otherSlice := map[string]*v{"a": {id: "A"}, "b": {id: "B"}, "c": {id: "C"}}
+	mySlice := otherSlice
+	for _, value := range mySlice {
+		m = append(m, &value.id)
+	}
+}