Skip to content

Commit 17a95ef

Browse files
cschleidencschleiden
authored
Merge pull request #64 from cschleiden/analyzer-rules
Add remaining checks to analyzer
2 parents c4b8773 + 3b578ee commit 17a95ef

File tree

2 files changed

+189
-41
lines changed

2 files changed

+189
-41
lines changed

‎analyzer/analyzer.go‎

Lines changed: 119 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package analyzer
22

33
import (
4+
"fmt"
45
"go/ast"
56
"go/types"
67

@@ -19,58 +20,140 @@ var Analyzer = &analysis.Analyzer{
1920
func run(pass *analysis.Pass) (interface{}, error) {
2021
inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
2122

22-
nodeFilter := []ast.Node{(*ast.FuncDecl)(nil)}
23+
inspector.Nodes(nil, func(node ast.Node, push bool) bool {
24+
if !push {
25+
return false
26+
}
2327

24-
inspector.Preorder(nodeFilter, func(node ast.Node) {
25-
funcDecl := node.(*ast.FuncDecl)
28+
switch n := node.(type) {
29+
case *ast.FuncDecl:
30+
// Only check functions that look like workflows
31+
if !isWorkflow(n) {
32+
return false
33+
}
2634

27-
if !isWorkflow(funcDecl) {
28-
return
29-
}
35+
// Check return types
36+
if n.Type.Results == nil || len(n.Type.Results.List) == 0 {
37+
pass.Reportf(n.Pos(), "workflow `%v` doesn't return anything. needs to return at least `error`", n.Name.Name)
38+
} else {
39+
if len(n.Type.Results.List) > 2 {
40+
pass.Reportf(n.Pos(), "workflow `%v` returns more than two values", n.Name.Name)
41+
return true
42+
}
3043

31-
// Check return types
32-
if funcDecl.Type.Results == nil || len(funcDecl.Type.Results.List) == 0 {
33-
pass.Reportf(funcDecl.Pos(), "workflow %q doesn't return anything. needs to return at least `error`", funcDecl.Name.Name)
34-
} else {
35-
if len(funcDecl.Type.Results.List) > 2 {
36-
pass.Reportf(funcDecl.Pos(), "workflow %q returns more than two values", funcDecl.Name.Name)
37-
return
44+
lastResult := n.Type.Results.List[len(n.Type.Results.List)-1]
45+
if types.ExprString(lastResult.Type) != "error" {
46+
pass.Reportf(n.Pos(), "workflow `%v` doesn't return `error` as last return value", n.Name.Name)
47+
}
3848
}
3949

40-
lastResult := funcDecl.Type.Results.List[len(funcDecl.Type.Results.List)-1]
41-
if types.ExprString(lastResult.Type) != "error" {
42-
pass.Reportf(funcDecl.Pos(), "workflow %q doesn't return `error` as last return value", funcDecl.Name.Name)
50+
funcScope := pass.TypesInfo.Scopes[n.Type]
51+
if funcScope != nil {
52+
checkVarsInScope(pass, funcScope)
4353
}
44-
}
4554

46-
// Check for various errors in the workflow body
47-
for _, stmt := range funcDecl.Body.List {
48-
switch stmt := stmt.(type) {
49-
// Check for map iterations
50-
case *ast.RangeStmt:
51-
{
52-
t := pass.TypesInfo.TypeOf(stmt.X)
53-
if t == nil {
54-
continue
55-
}
56-
57-
if _, ok := t.(*types.Map); !ok {
58-
continue
59-
}
60-
61-
pass.Reportf(stmt.Pos(), "iterating over a map is not deterministic and not allowed in workflows")
55+
// Continue with the function's children
56+
return true
57+
58+
case *ast.RangeStmt:
59+
{
60+
t := pass.TypesInfo.TypeOf(n.X)
61+
if t == nil {
62+
break
63+
}
64+
65+
switch t.(type) {
66+
case *types.Map:
67+
pass.Reportf(n.Pos(), "iterating over a `map` is not deterministic and not allowed in workflows")
68+
69+
case *types.Chan:
70+
pass.Reportf(n.Pos(), "using native channels is not allowed in workflows, use `workflow.Channel` instead")
6271
}
6372

64-
// Check for `go` statements
65-
case *ast.GoStmt:
66-
pass.Reportf(stmt.Pos(), "use workflow.Go instead of `go` in workflows")
73+
// checkStatements(pass, n.Body.List)
74+
}
75+
76+
case *ast.SelectStmt:
77+
pass.Reportf(n.Pos(), "`select` statements are not allowed in workflows, use `workflow.Select` instead")
78+
79+
case *ast.GoStmt:
80+
pass.Reportf(n.Pos(), "use `workflow.Go` instead of `go` in workflows")
81+
82+
case *ast.CallExpr:
83+
var pkg *ast.Ident
84+
var id *ast.Ident
85+
switch fun := n.Fun.(type) {
86+
case *ast.SelectorExpr:
87+
pkg, _ = fun.X.(*ast.Ident)
88+
id = fun.Sel
89+
}
90+
91+
if pkg == nil || id == nil {
92+
break
93+
}
94+
95+
pkgInfo := pass.TypesInfo.Uses[pkg]
96+
pkgName, _ := pkgInfo.(*types.PkgName)
97+
if pkgName == nil {
98+
break
99+
}
100+
101+
switch pkgName.Imported().Path() {
102+
case "time":
103+
switch id.Name {
104+
case "Now":
105+
pass.Reportf(n.Pos(), "`time.Now` is not allowed in workflows, use `workflow.Now` instead")
106+
case "Sleep":
107+
pass.Reportf(n.Pos(), "`time.Sleep` is not allowed in workflows, use `workflow.Sleep` instead")
108+
}
67109
}
68110
}
111+
112+
// Continue with the children
113+
return true
69114
})
70115

71116
return nil, nil
72117
}
73118

119+
func checkVarsInScope(pass *analysis.Pass, scope *types.Scope) {
120+
for _, name := range scope.Names() {
121+
obj := scope.Lookup(name)
122+
switch t := obj.Type().(type) {
123+
case *types.Chan:
124+
pass.Reportf(obj.Pos(), "using native channels is not allowed in workflows, use `workflow.Channel` instead")
125+
126+
case *types.Named:
127+
checkNamed(pass, obj, t)
128+
129+
case *types.Pointer:
130+
if named, ok := t.Elem().(*types.Named); ok {
131+
checkNamed(pass, obj, named)
132+
}
133+
}
134+
}
135+
136+
for i := 0; i < scope.NumChildren(); i++ {
137+
checkVarsInScope(pass, scope.Child(i))
138+
}
139+
}
140+
141+
func checkNamed(pass *analysis.Pass, ref types.Object, named *types.Named) {
142+
if obj := named.Obj(); obj != nil {
143+
if pkg := obj.Pkg(); pkg != nil {
144+
fmt.Println(pkg.Path(), obj.Name(), obj.Id())
145+
146+
switch pkg.Path() {
147+
case "sync":
148+
if obj.Name() == "WaitGroup" {
149+
pass.Reportf(ref.Pos(), "using `sync.WaitGroup` is not allowed in workflows, use `workflow.WaitGroup` instead")
150+
}
151+
}
152+
}
153+
}
154+
155+
}
156+
74157
func isWorkflow(funcDecl *ast.FuncDecl) bool {
75158
params := funcDecl.Type.Params.List
76159

‎analyzer/testdata/src/p/p.go‎

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,45 @@
1+
//nolint
12
package p
23

34
// Work around module issues. The analyzer just looks for `workflow.Context` currently
45
import (
6+
"context"
57
workflow "context"
68
"fmt"
9+
"time"
10+
11+
"sync"
712
)
813

14+
var foo int = 42
15+
916
func wf(ctx workflow.Context) error {
17+
fmt.Println(foo)
18+
1019
return nil
1120
}
1221

1322
func wfWithResult(ctx workflow.Context) (string, error) {
1423
return "", nil
1524
}
1625

17-
func wfWithTooManyResults(ctx workflow.Context) (int, string, error) { // want "workflow \"wfWithTooManyResults\" returns more than two values"
26+
func wfWithTooManyResults(ctx workflow.Context) (int, string, error) { // want "workflow `wfWithTooManyResults` returns more than two values"
1827
return 42, "", nil
1928
}
2029

21-
func wfWrongOrder(ctx workflow.Context) (error, string) { // want "workflow \"wfWrongOrder\" doesn't return `error` as last return value"
30+
func wfWrongOrder(ctx workflow.Context) (error, string) { // want "workflow `wfWrongOrder` doesn't return `error` as last return value"
2231
return nil, ""
2332
}
2433

25-
func wfWithoutReturn(ctx workflow.Context) { // want "workflow \"wfWithoutReturn\" doesn't return anything. needs to return at least `error`"
34+
func wfWithoutReturn(ctx workflow.Context) { // want "workflow `wfWithoutReturn` doesn't return anything. needs to return at least `error`"
2635
}
2736

2837
func wfIteratingOverMap(ctx workflow.Context) error {
2938
x := make(map[string]string)
3039

3140
fmt.Println("log")
3241

33-
for _, v := range x { // want "iterating over a map is not deterministic and not allowed in workflows"
42+
for _, v := range x { // want "iterating over a `map` is not deterministic and not allowed in workflows"
3443
if v == "a" {
3544
return nil
3645
}
@@ -40,9 +49,65 @@ func wfIteratingOverMap(ctx workflow.Context) error {
4049
}
4150

4251
func wfUsingGoRoutine(ctx workflow.Context) error {
43-
go func() { // want "use workflow.Go instead of `go` in workflows"
52+
go func() { // want "use `workflow.Go` instead of `go` in workflows"
4453
fmt.Println("hello")
4554
}()
4655

4756
return nil
4857
}
58+
59+
func wfUsingSelect(ctx workflow.Context) error {
60+
select { // want "`select` statements are not allowed in workflows, use `workflow.Select` instead"
61+
}
62+
63+
return nil
64+
}
65+
66+
func wfChanRange(ctx workflow.Context) error {
67+
for range make(chan int, 0) { // want "using native channels is not allowed in workflows, use `workflow.Channel` instead"
68+
if ctx == nil {
69+
v := make(chan int, 0) // want "using native channels is not allowed in workflows, use `workflow.Channel` instead"
70+
<-v
71+
72+
for range make(chan int, 0) { // want "using native channels is not allowed in workflows, use `workflow.Channel` instead"
73+
}
74+
}
75+
}
76+
77+
return nil
78+
}
79+
80+
func wfVarUsage(ctx workflow.Context) error {
81+
var wg sync.WaitGroup // want "`sync.WaitGroup` is not allowed in workflows"
82+
wg.Wait()
83+
84+
wg2 := &sync.WaitGroup{} // want "`sync.WaitGroup` is not allowed in workflows"
85+
wg2.Wait()
86+
87+
return nil
88+
}
89+
90+
func wfChanFuncDecl(ctx workflow.Context) error {
91+
x := func() {
92+
select {} // want "`select` statements are not allowed in workflows, use `workflow.Select` instead"
93+
}
94+
95+
x()
96+
97+
return nil
98+
}
99+
100+
func wfFunctionUsage(ctx workflow.Context) error {
101+
time.Sleep(10 * time.Second) // want "`time.Sleep` is not allowed in workflows, use `workflow.Sleep` instead"
102+
fmt.Println(time.Now()) // want "`time.Now` is not allowed in workflows, use `workflow.Now` instead"
103+
104+
return nil
105+
}
106+
107+
func activity(ctx context.Context) error {
108+
go fmt.Println("test")
109+
110+
select {}
111+
112+
return nil
113+
}

0 commit comments

Comments
 (0)