Check all branches for end in defer (#31)

* Check all branches for end in defer

* Refactor the check for end calls in defer

Author: parsaaes

spancheck.go

@@ -23,6 +23,12 @@ const (
 	spanOpenCensus                    // from go.opencensus.io/trace
 )
 
+const (
+	selNameEnd         = "End"
+	selNameSetStatus   = "SetStatus"
+	selNameRecordError = "RecordError"
+)
+
 // SpanTypes is a list of all span types by name.
 var SpanTypes = map[string]spanType{
 	"opentelemetry": spanOpenTelemetry,
@@ -183,23 +189,23 @@ func runFunc(pass *analysis.Pass, node ast.Node, config *Config) {
 	for _, sv := range spanVars {
 		if config.endCheckEnabled {
 			// Check if there's no End to the span.
-			if ret := getMissingSpanCalls(pass, g, sv, "End", func(_ *analysis.Pass, ret *ast.ReturnStmt) *ast.ReturnStmt { return ret }, nil, config.startSpanMatchers); ret != nil {
+			if ret := getMissingSpanCalls(pass, g, sv, selNameEnd, func(_ *analysis.Pass, ret *ast.ReturnStmt) *ast.ReturnStmt { return ret }, nil, config.startSpanMatchers); ret != nil {
 				pass.ReportRangef(sv.stmt, "%s.End is not called on all paths, possible memory leak", sv.vr.Name())
 				pass.ReportRangef(ret, "return can be reached without calling %s.End", sv.vr.Name())
 			}
 		}
 
 		if config.setStatusEnabled {
 			// Check if there's no SetStatus to the span setting an error.
-			if ret := getMissingSpanCalls(pass, g, sv, "SetStatus", getErrorReturn, config.ignoreChecksSignatures, config.startSpanMatchers); ret != nil {
+			if ret := getMissingSpanCalls(pass, g, sv, selNameSetStatus, getErrorReturn, config.ignoreChecksSignatures, config.startSpanMatchers); ret != nil {
 				pass.ReportRangef(sv.stmt, "%s.SetStatus is not called on all paths", sv.vr.Name())
 				pass.ReportRangef(ret, "return can be reached without calling %s.SetStatus", sv.vr.Name())
 			}
 		}
 
 		if config.recordErrorEnabled && sv.spanType == spanOpenTelemetry { // RecordError only exists in OpenTelemetry
 			// Check if there's no RecordError to the span setting an error.
-			if ret := getMissingSpanCalls(pass, g, sv, "RecordError", getErrorReturn, config.ignoreChecksSignatures, config.startSpanMatchers); ret != nil {
+			if ret := getMissingSpanCalls(pass, g, sv, selNameRecordError, getErrorReturn, config.ignoreChecksSignatures, config.startSpanMatchers); ret != nil {
 				pass.ReportRangef(sv.stmt, "%s.RecordError is not called on all paths", sv.vr.Name())
 				pass.ReportRangef(ret, "return can be reached without calling %s.RecordError", sv.vr.Name())
 			}
@@ -216,7 +222,7 @@ func isSpanStart(info *types.Info, n ast.Node, startSpanMatchers []spanStartMatc
 
 	fnSig := info.ObjectOf(sel.Sel).String()
 
-	// Check if the function is a span start function
+	// Check if the function is a span start function.
 	for _, matcher := range startSpanMatchers {
 		if matcher.signature.MatchString(fnSig) {
 			return matcher.spanType, true
@@ -399,6 +405,26 @@ func usesCall(
 				}
 
 				if g := cfgs.FuncLit(f); g != nil && len(g.Blocks) > 0 {
+					if selName == selNameEnd {
+						// Check if all returning blocks call end.
+						for _, b := range g.Blocks {
+							if b.Return() != nil && !usesCall(
+								pass,
+								b.Nodes,
+								sv,
+								selName,
+								ignoreCheckSig,
+								startSpanMatchers,
+								depth+1,
+							) {
+								return false
+							}
+						}
+
+						found = true
+						return false
+					}
+
 					for _, b := range g.Blocks {
 						if usesCall(
 							pass,

testdata/enableall/enable_all.go

@@ -5,10 +5,11 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/jjti/go-spancheck/testdata/enableall/util"
 	"go.opencensus.io/trace"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/codes"
+
+	"github.com/jjti/go-spancheck/testdata/enableall/util"
 )
 
 type testError struct{}
@@ -266,15 +267,15 @@ func _() (err error) {
 }
 
 func _() (err error) {
-	_, span := otel.Tracer("foo").Start(context.Background(), "bar") // want "span.SetStatus is not called on all paths"
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar") // want "span.SetStatus is not called on all paths" "span.End is not called on all paths, possible memory leak"
 	defer func() {
 		if true {
 			span.End()
 		}
 		span.RecordError(err)
 	}()
 
-	return errors.New("test") // want "return can be reached without calling span.SetStatus"
+	return errors.New("test") // want "return can be reached without calling span.SetStatus" "return can be reached without calling span.End"
 }
 
 func _() (err error) {
@@ -287,3 +288,26 @@ func _() (err error) {
 
 	return errors.New("test")
 }
+
+func _() (err error) {
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar") // want "span.End is not called on all paths, possible memory leak"
+	defer func() {
+		if err != nil {
+			span.RecordError(err)
+			span.SetStatus(codes.Error, "test")
+			span.End()
+		}
+	}()
+
+	return errors.New("test") // want "return can be reached without calling span.End"
+}
+
+func _() (err error) {
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar") // want "span.End is not called on all paths, possible memory leak"
+	defer func() {
+		span.RecordError(err)
+		span.SetStatus(codes.Error, "test")
+	}()
+
+	return errors.New("test") // want "return can be reached without calling span.End"
+}