Commit 23ba54784a49fe13c665396c49a5ab3baa70ebd5

Parents: 0b0095eeadaf7645b731f277c06e3f19e63ea981

From: Robin Jarry <robin@jarry.cc>
Date: Thu Oct 26 00:03:43 2023 +0700

linters: avoid crash when analyzing function call
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>

Stats

contrib/linters.go +50/-19
worker/jmap/push.go +2/-0

Changeset

  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
diff --git a/contrib/linters.go b/contrib/linters.go
index 7876cab60d1b2193cb2d12dc22262940beed20bc..41655a67fb7c6bd85f61f4751c2d30d4242e16a9 100644
--- a/contrib/linters.go
+++ b/contrib/linters.go
@@ -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)
@@ -38,11 +56,14 @@ 				sel, ok := pass.TypesInfo.Selections[e]
 				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 {
@@ -56,22 +77,28 @@
 			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
@@ -86,6 +113,9 @@ 	}
 }
 
 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
@@ -121,6 +151,7 @@ // This must be implemented
 func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
 	return []*analysis.Analyzer{
 		PanicAnalyzer,
+		PanicIndirectAnalyzer,
 	}
 }
 
diff --git a/worker/jmap/push.go b/worker/jmap/push.go
index 34a90ca41fd3ae39d0911c81c11a390134257cfc..2f7b05e601b9235be38423ba48ba6a974f923f22 100644
--- a/worker/jmap/push.go
+++ b/worker/jmap/push.go
@@ -16,6 +16,8 @@ 	"git.sr.ht/~rockorager/go-jmap/mail/mailbox"
 )
 
 func (w *JMAPWorker) monitorChanges() {
+	defer log.PanicHandler()
+
 	events := push.EventSource{
 		Client:  w.client,
 		Handler: w.handleChange,