Skip to content

Commit

Permalink
add rule datarace (mgechev#683)
Browse files Browse the repository at this point in the history
Signed-off-by: subham sarkar <subham@deepsource.io>
  • Loading branch information
chavacava authored and subham-deepsource committed Aug 5, 2022
1 parent f04ddd3 commit 318a09e
Show file tree
Hide file tree
Showing 6 changed files with 191 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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

Expand Down
6 changes: 6 additions & 0 deletions RULES_DESCRIPTIONS.md
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions config/config.go
Expand Up @@ -85,6 +85,7 @@ var allRules = append([]lint.Rule{
&rule.BannedCharsRule{},
&rule.OptimizeOperandsOrderRule{},
&rule.UseAnyRule{},
&rule.DataRaceRule{},
}, defaultRules...)

var allFormatters = []lint.Formatter{
Expand Down
142 changes: 142 additions & 0 deletions 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
}
11 changes: 11 additions & 0 deletions 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{})
}
30 changes: 30 additions & 0 deletions 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)
}

0 comments on commit 318a09e

Please sign in to comment.