Skip to content

Commit e7f8bd0

Browse files
committed
use fact
1 parent 30ee69c commit e7f8bd0

File tree

4 files changed

+90
-185
lines changed

4 files changed

+90
-185
lines changed

contextcheck.go

+83-65
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"go/types"
77
"strconv"
88
"strings"
9-
"sync"
109

1110
"github.com/gostaticanalysis/analysisutil"
1211
"golang.org/x/tools/go/analysis"
@@ -23,6 +22,7 @@ func NewAnalyzer() *analysis.Analyzer {
2322
Requires: []*analysis.Analyzer{
2423
buildssa.Analyzer,
2524
},
25+
FactTypes: []analysis.Fact{(*ctxFact)(nil)},
2626
}
2727
}
2828

@@ -39,27 +39,38 @@ const (
3939
CtxInOut = CtxIn | CtxOut
4040
)
4141

42-
var (
43-
checkedMap = make(map[string]bool)
44-
checkedMapLock sync.RWMutex
45-
c *collector
46-
)
42+
type resInfo struct {
43+
Valid bool
44+
Funcs []string
45+
}
46+
47+
type ctxFact map[string]resInfo
48+
49+
func (*ctxFact) String() string { return "ctxCheck" }
50+
func (*ctxFact) AFact() {}
4751

4852
type runner struct {
4953
pass *analysis.Pass
5054
ctxTyp *types.Named
5155
ctxPTyp *types.Pointer
5256
cmpPath string
5357
skipFile map[*ast.File]bool
58+
59+
currentFact ctxFact
5460
}
5561

5662
func NewRun(pkgs []*packages.Package) func(pass *analysis.Pass) (interface{}, error) {
57-
c = newCollector(pkgs)
63+
m := make(map[string]bool)
64+
for _, pkg := range pkgs {
65+
m[strings.Split(pkg.PkgPath, "/")[0]] = true
66+
}
5867
return func(pass *analysis.Pass) (interface{}, error) {
59-
defer c.DecUse(pass)
68+
// skip different repo
69+
if !m[strings.Split(pass.Pkg.Path(), "/")[0]] {
70+
return nil, nil
71+
}
6072

61-
r := new(runner)
62-
r.run(pass)
73+
new(runner).run(pass)
6374
return nil, nil
6475
}
6576
}
@@ -90,21 +101,33 @@ func (r *runner) run(pass *analysis.Pass) {
90101
}
91102

92103
r.skipFile = make(map[*ast.File]bool)
104+
r.currentFact = make(ctxFact)
93105

106+
var tmpFuncs []*ssa.Function
94107
for _, f := range funcs {
95108
// skip checked function
96109
key := f.RelString(nil)
97-
_, ok := getValue(key)
98-
if ok {
110+
if _, ok := r.currentFact[key]; ok {
99111
continue
100112
}
101113

102114
if !r.checkIsEntry(f, f.Pos()) {
115+
// record the result of nomal function
116+
checkingMap := make(map[string]bool)
117+
checkingMap[key] = true
118+
r.setFact(key, r.checkFuncWithoutCtx(f, checkingMap), f.Name())
103119
continue
104120
}
105121

122+
tmpFuncs = append(tmpFuncs, f)
123+
}
124+
125+
for _, f := range tmpFuncs {
106126
r.checkFuncWithCtx(f)
107-
setValue(key, true)
127+
}
128+
129+
if len(r.currentFact) > 0 {
130+
pass.ExportPackageFact(&r.currentFact)
108131
}
109132
}
110133

@@ -269,16 +292,6 @@ func (r *runner) collectCtxRef(f *ssa.Function) (refMap map[ssa.Instruction]bool
269292
return
270293
}
271294

272-
func (r *runner) buildPkg(f *ssa.Function) (ff *ssa.Function) {
273-
if f.Blocks != nil {
274-
ff = f
275-
return
276-
}
277-
278-
ff = c.GetFunction(f)
279-
return
280-
}
281-
282295
func (r *runner) checkIsSameRepo(s string) bool {
283296
return strings.HasPrefix(s, r.cmpPath+"/")
284297
}
@@ -313,31 +326,10 @@ func (r *runner) checkFuncWithCtx(f *ssa.Function) {
313326
}
314327

315328
key := ff.RelString(nil)
316-
valid, ok := getValue(key)
329+
res, ok := r.getValue(key, ff)
317330
if ok {
318-
if !valid {
319-
r.pass.Reportf(instr.Pos(), "Function `%s` should pass the context parameter", ff.Name())
320-
}
321-
continue
322-
}
323-
324-
// check is thunk or bound
325-
if strings.HasSuffix(key, "$thunk") || strings.HasSuffix(key, "$bound") {
326-
continue
327-
}
328-
329-
// if ff has no ctx, start deep traversal check
330-
if !r.checkIsEntry(ff, instr.Pos()) {
331-
if ff = r.buildPkg(ff); ff == nil {
332-
continue
333-
}
334-
335-
checkingMap := make(map[string]bool)
336-
checkingMap[key] = true
337-
valid := r.checkFuncWithoutCtx(ff, checkingMap)
338-
setValue(key, valid)
339-
if !valid {
340-
r.pass.Reportf(instr.Pos(), "Function `%s` should pass the context parameter", ff.Name())
331+
if !res.Valid {
332+
r.pass.Reportf(instr.Pos(), "Function `%s` should pass the context parameter", strings.Join(reverse(res.Funcs), "->"))
341333
}
342334
}
343335
}
@@ -346,6 +338,7 @@ func (r *runner) checkFuncWithCtx(f *ssa.Function) {
346338

347339
func (r *runner) checkFuncWithoutCtx(f *ssa.Function, checkingMap map[string]bool) (ret bool) {
348340
ret = true
341+
orgKey := f.RelString(nil)
349342
for _, b := range f.Blocks {
350343
for _, instr := range b.Instrs {
351344
tp, ok := r.getCtxType(instr)
@@ -362,7 +355,6 @@ func (r *runner) checkFuncWithoutCtx(f *ssa.Function, checkingMap map[string]boo
362355
if tp&CtxInField == 0 {
363356
ret = false
364357
}
365-
continue
366358
}
367359

368360
ff := r.getFunction(instr)
@@ -371,11 +363,13 @@ func (r *runner) checkFuncWithoutCtx(f *ssa.Function, checkingMap map[string]boo
371363
}
372364

373365
key := ff.RelString(nil)
374-
valid, ok := getValue(key)
366+
res, ok := r.getValue(key, ff)
375367
if ok {
376-
if !valid {
368+
if !res.Valid {
377369
ret = false
378-
r.pass.Reportf(instr.Pos(), "Function `%s` should pass the context parameter", ff.Name())
370+
371+
// save the call link
372+
r.setFact(orgKey, res.Valid, res.Funcs...)
379373
}
380374
continue
381375
}
@@ -386,21 +380,21 @@ func (r *runner) checkFuncWithoutCtx(f *ssa.Function, checkingMap map[string]boo
386380
}
387381

388382
if !r.checkIsEntry(ff, instr.Pos()) {
389-
// handler ring call
390-
if checkingMap[key] {
383+
// cannot get info from fact, skip
384+
if ff.Blocks == nil {
391385
continue
392386
}
393-
checkingMap[key] = true
394387

395-
if ff = r.buildPkg(ff); ff == nil {
388+
// handler ring call
389+
if checkingMap[key] {
396390
continue
397391
}
392+
checkingMap[key] = true
398393

399394
valid := r.checkFuncWithoutCtx(ff, checkingMap)
400-
setValue(key, valid)
395+
r.setFact(orgKey, valid, ff.Name())
401396
if !valid {
402397
ret = false
403-
r.pass.Reportf(instr.Pos(), "Function `%s` should pass the context parameter", ff.Name())
404398
}
405399
}
406400
}
@@ -501,15 +495,39 @@ func (r *runner) isCtxType(tp types.Type) bool {
501495
return types.Identical(tp, r.ctxTyp) || types.Identical(tp, r.ctxPTyp)
502496
}
503497

504-
func getValue(key string) (valid, ok bool) {
505-
checkedMapLock.RLock()
506-
valid, ok = checkedMap[key]
507-
checkedMapLock.RUnlock()
498+
func (r *runner) getValue(key string, f *ssa.Function) (res resInfo, ok bool) {
499+
res, ok = r.currentFact[key]
500+
if ok {
501+
return
502+
}
503+
504+
if f.Pkg == nil {
505+
return
506+
}
507+
508+
var fact ctxFact
509+
if r.pass.ImportPackageFact(f.Pkg.Pkg, &fact) {
510+
res, ok = fact[key]
511+
}
508512
return
509513
}
510514

511-
func setValue(key string, valid bool) {
512-
checkedMapLock.Lock()
513-
checkedMap[key] = valid
514-
checkedMapLock.Unlock()
515+
func (r *runner) setFact(key string, valid bool, funcs ...string) {
516+
r.currentFact[key] = resInfo{
517+
Valid: valid,
518+
Funcs: append(r.currentFact[key].Funcs, funcs...),
519+
}
520+
}
521+
522+
func reverse(arr1 []string) (arr2 []string) {
523+
l := len(arr1)
524+
if l == 0 {
525+
return
526+
}
527+
arr2 = make([]string, l)
528+
for i := 0; i <= l/2; i++ {
529+
arr2[i] = arr1[l-1-i]
530+
arr2[l-1-i] = arr1[i]
531+
}
532+
return
515533
}

contextcheck_test.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ import (
66

77
"github.com/sylvia7788/contextcheck"
88
"golang.org/x/tools/go/analysis/analysistest"
9+
"golang.org/x/tools/go/packages"
910
)
1011

1112
func Test(t *testing.T) {
1213
log.SetFlags(log.Lshortfile)
1314
testdata := analysistest.TestData()
14-
analysistest.Run(t, testdata, contextcheck.NewAnalyzer(), "a")
15+
analyzer := contextcheck.NewAnalyzer()
16+
analyzer.Run = contextcheck.NewRun([]*packages.Package{{PkgPath: "a"}})
17+
analysistest.Run(t, testdata, analyzer, "a")
1518
}

dep.go

-116
This file was deleted.

0 commit comments

Comments
 (0)