src

sa2001.go (3172B)

---

 package sa2001
 
 import (
 	"go/ast"
 	"go/types"
 
 	"honnef.co/go/tools/analysis/code"
 	"honnef.co/go/tools/analysis/lint"
 	"honnef.co/go/tools/analysis/report"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 )
 
 var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
 	Analyzer: &analysis.Analyzer{
 		Name:     "SA2001",
 		Run:      run,
 		Requires: []*analysis.Analyzer{inspect.Analyzer},
 	},
 	Doc: &lint.RawDocumentation{
 		Title: `Empty critical section, did you mean to defer the unlock?`,
 		Text: `Empty critical sections of the kind
 
     mu.Lock()
     mu.Unlock()
 
 are very often a typo, and the following was intended instead:
 
     mu.Lock()
     defer mu.Unlock()
 
 Do note that sometimes empty critical sections can be useful, as a
 form of signaling to wait on another goroutine. Many times, there are
 simpler ways of achieving the same effect. When that isn't the case,
 the code should be amply commented to avoid confusion. Combining such
 comments with a \'//lint:ignore\' directive can be used to suppress this
 rare false positive.`,
 		Since:    "2017.1",
 		Severity: lint.SeverityWarning,
 		MergeIf:  lint.MergeIfAny,
 	},
 })
 
 var Analyzer = SCAnalyzer.Analyzer
 
 func run(pass *analysis.Pass) (any, error) {
 	if pass.Pkg.Path() == "sync_test" {
 		// exception for the sync package's tests
 		return nil, nil
 	}
 
 	// Initially it might seem like this check would be easier to
 	// implement using IR. After all, we're only checking for two
 	// consecutive method calls. In reality, however, there may be any
 	// number of other instructions between the lock and unlock, while
 	// still constituting an empty critical section. For example,
 	// given `m.x().Lock(); m.x().Unlock()`, there will be a call to
 	// x(). In the AST-based approach, this has a tiny potential for a
 	// false positive (the second call to x might be doing work that
 	// is protected by the mutex). In an IR-based approach, however,
 	// it would miss a lot of real bugs.
 
 	mutexParams := func(s ast.Stmt) (x ast.Expr, funcName string, ok bool) {
 		expr, ok := s.(*ast.ExprStmt)
 		if !ok {
 			return nil, "", false
 		}
 		call, ok := ast.Unparen(expr.X).(*ast.CallExpr)
 		if !ok {
 			return nil, "", false
 		}
 		sel, ok := call.Fun.(*ast.SelectorExpr)
 		if !ok {
 			return nil, "", false
 		}
 
 		fn, ok := pass.TypesInfo.ObjectOf(sel.Sel).(*types.Func)
 		if !ok {
 			return nil, "", false
 		}
 		sig := fn.Type().(*types.Signature)
 		if sig.Params().Len() != 0 || sig.Results().Len() != 0 {
 			return nil, "", false
 		}
 
 		return sel.X, fn.Name(), true
 	}
 
 	fn := func(node ast.Node) {
 		block := node.(*ast.BlockStmt)
 		if len(block.List) < 2 {
 			return
 		}
 		for i := range block.List[:len(block.List)-1] {
 			sel1, method1, ok1 := mutexParams(block.List[i])
 			sel2, method2, ok2 := mutexParams(block.List[i+1])
 
 			if !ok1 || !ok2 || report.Render(pass, sel1) != report.Render(pass, sel2) {
 				continue
 			}
 			if (method1 == "Lock" && method2 == "Unlock") ||
 				(method1 == "RLock" && method2 == "RUnlock") {
 				report.Report(pass, block.List[i+1], "empty critical section")
 			}
 		}
 	}
 	code.Preorder(pass, fn, (*ast.BlockStmt)(nil))
 	return nil, nil
 }