add rule datarace (#683)

README.md

@@ -482,6 +482,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`banned-characters`](./RULES_DESCRIPTIONS.md#banned-characters)          |  n/a   |  Checks banned characters in identifiers |    no    |  no   |
 | [`optimize-operands-order`](./RULES_DESCRIPTIONS.md#optimize-operands-order)          |  n/a   |  Checks inefficient conditional expressions |    no    |  no   |
 | [`use-any`](./RULES_DESCRIPTIONS.md#use-any)          |  n/a   |  Proposes to replace `interface{}` with its alias `any` |    no    |  no   |
+| [`datarace`](./RULES_DESCRIPTIONS.md#datarace)          |  n/a   |  Spots potential dataraces |    no    |  no   |
 
 ## Configurable rules
 

RULES_DESCRIPTIONS.md

@@ -19,6 +19,7 @@ List of all available rules.
   - [context-as-argument](#context-as-argument)
   - [context-keys-type](#context-keys-type)
   - [cyclomatic](#cyclomatic)
+  - [datarace](#datarace)
   - [deep-exit](#deep-exit)
   - [defer](#defer)
   - [dot-imports](#dot-imports)
@@ -217,6 +218,11 @@ Example:
 [rule.cyclomatic]
   arguments =[3]
 ```
+## datarace
+
+_Description_: This rule spots potential dataraces caused by go-routines capturing (by-reference) particular identifiers of the function from which go-routines are created. The rule is able to spot two of such cases: go-routines capturing named return values, and capturing `for-range` values.
+
+_Configuration_: N/A
 
 ## deep-exit
 

config/config.go

@@ -85,6 +85,7 @@ var allRules = append([]lint.Rule{
 	&rule.BannedCharsRule{},
 	&rule.OptimizeOperandsOrderRule{},
 	&rule.UseAnyRule{},
+	&rule.DataRaceRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

rule/datarace.go

@@ -0,0 +1,142 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// DataRaceRule lints assignments to value method-receivers.
+type DataRaceRule struct{}
+
+// Apply applies the rule to given file.
+func (*DataRaceRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+	w := lintDataRaces{onFailure: onFailure}
+
+	ast.Walk(w, file.AST)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*DataRaceRule) Name() string {
+	return "datarace"
+}
+
+type lintDataRaces struct {
+	onFailure func(failure lint.Failure)
+}
+
+func (w lintDataRaces) Visit(n ast.Node) ast.Visitor {
+	node, ok := n.(*ast.FuncDecl)
+	if !ok {
+		return w // not function declaration
+	}
+	if node.Body == nil {
+		return nil // empty body
+	}
+
+	results := node.Type.Results
+
+	returnIDs := map[*ast.Object]struct{}{}
+	if results != nil {
+		returnIDs = w.ExtractReturnIDs(results.List)
+	}
+	fl := &lintFunctionForDataRaces{onFailure: w.onFailure, returnIDs: returnIDs, rangeIDs: map[*ast.Object]struct{}{}}
+	ast.Walk(fl, node.Body)
+
+	return nil
+}
+
+func (w lintDataRaces) ExtractReturnIDs(fields []*ast.Field) map[*ast.Object]struct{} {
+	r := map[*ast.Object]struct{}{}
+	for _, f := range fields {
+		for _, id := range f.Names {
+			r[id.Obj] = struct{}{}
+		}
+	}
+
+	return r
+}
+
+type lintFunctionForDataRaces struct {
+	_         struct{}
+	onFailure func(failure lint.Failure)
+	returnIDs map[*ast.Object]struct{}
+	rangeIDs  map[*ast.Object]struct{}
+}
+
+func (w lintFunctionForDataRaces) Visit(node ast.Node) ast.Visitor {
+	switch n := node.(type) {
+	case *ast.RangeStmt:
+		if n.Body == nil {
+			return nil
+		}
+
+		getIds := func(exprs ...ast.Expr) []*ast.Ident {
+			r := []*ast.Ident{}
+			for _, expr := range exprs {
+				if id, ok := expr.(*ast.Ident); ok {
+					r = append(r, id)
+				}
+			}
+			return r
+		}
+
+		ids := getIds(n.Key, n.Value)
+		for _, id := range ids {
+			w.rangeIDs[id.Obj] = struct{}{}
+		}
+
+		ast.Walk(w, n.Body)
+
+		for _, id := range ids {
+			delete(w.rangeIDs, id.Obj)
+		}
+
+		return nil // do not visit the body of the range, it has been already visited
+	case *ast.GoStmt:
+		f := n.Call.Fun
+		funcLit, ok := f.(*ast.FuncLit)
+		if !ok {
+			return nil
+		}
+		selectIDs := func(n ast.Node) bool {
+			_, ok := n.(*ast.Ident)
+			return ok
+		}
+
+		ids := pick(funcLit.Body, selectIDs, nil)
+		for _, id := range ids {
+			id := id.(*ast.Ident)
+			_, isRangeID := w.rangeIDs[id.Obj]
+			_, isReturnID := w.returnIDs[id.Obj]
+
+			switch {
+			case isRangeID:
+				w.onFailure(lint.Failure{
+					Confidence: 1,
+					Node:       id,
+					Category:   "logic",
+					Failure:    fmt.Sprintf("datarace: range value %s is captured (by-reference) in goroutine", id.Name),
+				})
+			case isReturnID:
+				w.onFailure(lint.Failure{
+					Confidence: 0.8,
+					Node:       id,
+					Category:   "logic",
+					Failure:    fmt.Sprintf("potential datarace: return value %s is captured (by-reference) in goroutine", id.Name),
+				})
+			}
+		}
+
+		return nil
+	}
+
+	return w
+}

test/datarace_test.go

@@ -0,0 +1,11 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestDatarace(t *testing.T) {
+	testRule(t, "datarace", &rule.DataRaceRule{})
+}

testdata/datarace.go

@@ -0,0 +1,30 @@
+package fixtures
+
+func datarace() (r int, c char) {
+	for _, p := range []int{1, 2} {
+		go func() {
+			print(r) // MATCH /potential datarace: return value r is captured (by-reference) in goroutine/
+			print(p) // MATCH /datarace: range value p is captured (by-reference) in goroutine/
+		}()
+		for i, p1 := range []int{1, 2} {
+			a := p1
+			go func() {
+				print(r)  // MATCH /potential datarace: return value r is captured (by-reference) in goroutine/
+				print(p)  // MATCH /datarace: range value p is captured (by-reference) in goroutine/
+				print(p1) // MATCH /datarace: range value p1 is captured (by-reference) in goroutine/
+				print(a)
+				print(i) // MATCH /datarace: range value i is captured (by-reference) in goroutine/
+			}()
+			print(i)
+			print(p)
+			go func() {
+				_ = c // MATCH /potential datarace: return value c is captured (by-reference) in goroutine/
+			}()
+		}
+		print(p1)
+	}
+	go func() {
+		print(r) // MATCH /potential datarace: return value r is captured (by-reference) in goroutine/
+	}()
+	print(r)
+}