github.com/amarpal/go-tools@v0.0.0-20240422043104-40142f59f616/staticcheck/sa2001/sa2001.go (about) 1 package sa2001 2 3 import ( 4 "go/ast" 5 "go/types" 6 7 "github.com/amarpal/go-tools/analysis/code" 8 "github.com/amarpal/go-tools/analysis/lint" 9 "github.com/amarpal/go-tools/analysis/report" 10 "github.com/amarpal/go-tools/go/ast/astutil" 11 12 "golang.org/x/tools/go/analysis" 13 "golang.org/x/tools/go/analysis/passes/inspect" 14 ) 15 16 var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{ 17 Analyzer: &analysis.Analyzer{ 18 Name: "SA2001", 19 Run: run, 20 Requires: []*analysis.Analyzer{inspect.Analyzer}, 21 }, 22 Doc: &lint.Documentation{ 23 Title: `Empty critical section, did you mean to defer the unlock?`, 24 Text: `Empty critical sections of the kind 25 26 mu.Lock() 27 mu.Unlock() 28 29 are very often a typo, and the following was intended instead: 30 31 mu.Lock() 32 defer mu.Unlock() 33 34 Do note that sometimes empty critical sections can be useful, as a 35 form of signaling to wait on another goroutine. Many times, there are 36 simpler ways of achieving the same effect. When that isn't the case, 37 the code should be amply commented to avoid confusion. Combining such 38 comments with a \'//lint:ignore\' directive can be used to suppress this 39 rare false positive.`, 40 Since: "2017.1", 41 Severity: lint.SeverityWarning, 42 MergeIf: lint.MergeIfAny, 43 }, 44 }) 45 46 var Analyzer = SCAnalyzer.Analyzer 47 48 func run(pass *analysis.Pass) (interface{}, error) { 49 if pass.Pkg.Path() == "sync_test" { 50 // exception for the sync package's tests 51 return nil, nil 52 } 53 54 // Initially it might seem like this check would be easier to 55 // implement using IR. After all, we're only checking for two 56 // consecutive method calls. In reality, however, there may be any 57 // number of other instructions between the lock and unlock, while 58 // still constituting an empty critical section. For example, 59 // given `m.x().Lock(); m.x().Unlock()`, there will be a call to 60 // x(). In the AST-based approach, this has a tiny potential for a 61 // false positive (the second call to x might be doing work that 62 // is protected by the mutex). In an IR-based approach, however, 63 // it would miss a lot of real bugs. 64 65 mutexParams := func(s ast.Stmt) (x ast.Expr, funcName string, ok bool) { 66 expr, ok := s.(*ast.ExprStmt) 67 if !ok { 68 return nil, "", false 69 } 70 call, ok := astutil.Unparen(expr.X).(*ast.CallExpr) 71 if !ok { 72 return nil, "", false 73 } 74 sel, ok := call.Fun.(*ast.SelectorExpr) 75 if !ok { 76 return nil, "", false 77 } 78 79 fn, ok := pass.TypesInfo.ObjectOf(sel.Sel).(*types.Func) 80 if !ok { 81 return nil, "", false 82 } 83 sig := fn.Type().(*types.Signature) 84 if sig.Params().Len() != 0 || sig.Results().Len() != 0 { 85 return nil, "", false 86 } 87 88 return sel.X, fn.Name(), true 89 } 90 91 fn := func(node ast.Node) { 92 block := node.(*ast.BlockStmt) 93 if len(block.List) < 2 { 94 return 95 } 96 for i := range block.List[:len(block.List)-1] { 97 sel1, method1, ok1 := mutexParams(block.List[i]) 98 sel2, method2, ok2 := mutexParams(block.List[i+1]) 99 100 if !ok1 || !ok2 || report.Render(pass, sel1) != report.Render(pass, sel2) { 101 continue 102 } 103 if (method1 == "Lock" && method2 == "Unlock") || 104 (method1 == "RLock" && method2 == "RUnlock") { 105 report.Report(pass, block.List[i+1], "empty critical section") 106 } 107 } 108 } 109 code.Preorder(pass, fn, (*ast.BlockStmt)(nil)) 110 return nil, nil 111 }