Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add rule datarace #683

Merged
merged 1 commit into from Apr 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
}