Skip to content

Commit

Permalink
linters: avoid crash when analyzing function call
Browse files Browse the repository at this point in the history
When encountering a statement such as:

	go functionName()

The identifier is not a local symbol but should be looked up in the
current package. Do not consider that all these statements will refer to
local variables.

The only way to do this is to run a second analyzer that depends on the
first one. Store all unresolved methods and functions into
a indirectCalls struct. Reuse that result in the second analyzer to
resolve the function declarations and check their bodies for the
defer log.PanicHandler() statement.

Fix one newly reported issues.

Signed-off-by: Robin Jarry <robin@jarry.cc>
Reviewed-by: Tim Culverhouse <tim@timculverhouse.com>
  • Loading branch information
rjarry committed Nov 2, 2023
1 parent 0b0095e commit 23ba547
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 19 deletions.
69 changes: 50 additions & 19 deletions contrib/linters.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,36 @@ import (
"go/ast"
"go/token"
"go/types"
"reflect"

"golang.org/x/tools/go/analysis"
)

type indirectCalls struct {
methods map[token.Pos]string
functions map[string]token.Pos
}

var PanicAnalyzer = &analysis.Analyzer{
Name: "panic",
Doc: "finds goroutines that do not initialize the panic handler",
Run: runPanic,
Name: "panic",
Doc: "finds goroutines that do not initialize the panic handler",
Run: runPanic,
ResultType: reflect.TypeOf(&indirectCalls{}),
}

var PanicIndirectAnalyzer = &analysis.Analyzer{
Name: "panicindirect",
Doc: "finds functions called as goroutines that do not initialize the panic handler",
Run: runPanicIndirect,
Requires: []*analysis.Analyzer{PanicAnalyzer},
}

func runPanic(pass *analysis.Pass) (interface{}, error) {
methods := make(map[token.Pos]string)
var calls indirectCalls

calls.methods = make(map[token.Pos]string)
calls.functions = make(map[string]token.Pos)

for _, file := range pass.Files {
ast.Inspect(file, func(n ast.Node) bool {
g, ok := n.(*ast.GoStmt)
Expand All @@ -38,11 +56,14 @@ func runPanic(pass *analysis.Pass) (interface{}, error) {
if ok {
f, ok := sel.Obj().(*types.Func)
if ok {
methods[f.Pos()] = f.Name()
calls.methods[f.Pos()] = f.Name()
}
}
case *ast.Ident:
block = inlineFuncBody(e)
if block == nil {
calls.functions[e.Name] = e.NamePos
}
}

if block == nil {
Expand All @@ -56,22 +77,28 @@ func runPanic(pass *analysis.Pass) (interface{}, error) {
return true
})
}

return &calls, nil
}

func runPanicIndirect(pass *analysis.Pass) (interface{}, error) {
calls := pass.ResultOf[PanicAnalyzer].(*indirectCalls)

for _, file := range pass.Files {
ast.Inspect(file, func(n ast.Node) bool {
f, ok := n.(*ast.FuncDecl)
if !ok {
return false
}
_, found := methods[f.Name.Pos()]
if !found {
return false
}
delete(methods, f.Name.Pos())
if !isPanicHandlerInstall(f.Body.List[0]) {
pass.Report(panicDiag(f.Body.Pos()))
for _, decl := range file.Decls {
if f, ok := decl.(*ast.FuncDecl); ok {
if _, ok := calls.methods[f.Name.Pos()]; ok {
delete(calls.methods, f.Name.Pos())
} else if _, ok := calls.functions[f.Name.Name]; ok {
delete(calls.functions, f.Name.Name)
} else {
continue
}
if !isPanicHandlerInstall(f.Body.List[0]) {
pass.Report(panicDiag(f.Body.Pos()))
}
}
return false
})
}
}

return nil, nil
Expand All @@ -86,6 +113,9 @@ func panicDiag(pos token.Pos) analysis.Diagnostic {
}

func inlineFuncBody(s *ast.Ident) *ast.BlockStmt {
if s.Obj == nil || s.Obj.Decl == nil {
return nil
}
d, ok := s.Obj.Decl.(*ast.AssignStmt)
if !ok {
return nil
Expand Down Expand Up @@ -121,6 +151,7 @@ type analyzerPlugin struct{}
func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
return []*analysis.Analyzer{
PanicAnalyzer,
PanicIndirectAnalyzer,
}
}

Expand Down
2 changes: 2 additions & 0 deletions worker/jmap/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
)

func (w *JMAPWorker) monitorChanges() {
defer log.PanicHandler()

events := push.EventSource{
Client: w.client,
Handler: w.handleChange,
Expand Down

0 comments on commit 23ba547

Please sign in to comment.