fix false positive with defer (#4)

Author: ykadowak

testdata/src/a/a.go

@@ -33,6 +33,19 @@ func bad() {
 		logger2 = log.Error() // want "must be dispatched by Msg or Send method"
 	}
 	logger2.Str("foo", "bar")
+
+	// defer patterns
+	defer log.Info()      // want "must be dispatched by Msg or Send method"
+
+	logger3 := log.Error() // want "must be dispatched by Msg or Send method"
+	defer logger3.Err(err).Str("foo", "bar").Int("foo", 1)
+
+	defer log.Info(). // want "must be dispatched by Msg or Send method"
+				Str("foo", "bar").
+				Dict("dict", zerolog.Dict().
+					Str("bar", "baz").
+					Int("n", 1),
+		)
 }
 
 func ok() {
@@ -68,4 +81,17 @@ func ok() {
 	// dispatch variation
 	log.Info().Msgf("")
 	log.Info().MsgFunc(func() string { return "foo" })
+
+	// defer patterns
+	defer log.Info().Msg("")
+
+	logger3 := log.Info()
+	defer logger3.Msg("")
+
+	defer log.Info().
+		Str("foo", "bar").
+		Dict("dict", zerolog.Dict().
+			Str("bar", "baz").
+			Int("n", 1),
+		).Send()
 }

zerologlint.go

@@ -1,6 +1,7 @@
 package zerologlint
 
 import (
+	"go/token"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
@@ -20,54 +21,33 @@ var Analyzer = &analysis.Analyzer{
 	},
 }
 
+type posser interface {
+	Pos() token.Pos
+}
+
+// posser is an interface just to hold both ssa.Call and ssa.Defer in our set
+type callDefer interface {
+	Common() *ssa.CallCommon
+	Pos() token.Pos
+}
+
 func run(pass *analysis.Pass) (interface{}, error) {
 	srcFuncs := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs
 
-	// This map holds all the ssa block that is a zerolog.Event type instance
+	// This set holds all the ssa block that is a zerolog.Event type instance
 	// that should be dispatched.
 	// Everytime the zerolog.Event is dispatched with Msg() or Send(),
-	// deletes that block from this map.
+	// deletes that block from this set.
 	// At the end, check if the set is empty, or report the not dispatched block.
-	set := make(map[ssa.Value]struct{})
+	set := make(map[posser]struct{})
 
 	for _, sf := range srcFuncs {
 		for _, b := range sf.Blocks {
 			for _, instr := range b.Instrs {
 				if c, ok := instr.(*ssa.Call); ok {
-					v := c.Value()
-					// check if it's in github.com/rs/zerolog/log since there's some
-					// functions in github.com/rs/zerolog that returns zerolog.Event
-					// which should not be included
-					if isInLogPkg(v) {
-						if isZerologEvent(v) {
-							// check if this is a new instance of zerolog.Event like logger := log.Error()
-							// which should be dispatched afterwards at some point
-							if len(v.Call.Args) == 0 {
-								set[v] = struct{}{}
-							}
-							continue
-						}
-					}
-
-					// if the call does not return zerolog.Event,
-					// check if the base is zerolog.Event.
-					// if so, check if the StaticCallee is Send() or Msg().
-					// if so, remove the arg[0] from the set.
-					for _, arg := range v.Call.Args {
-						if isZerologEvent(arg) {
-							if isDispatchMethod(v) {
-								val := getRootSsaValue(arg)
-								// if there's branch, remove both ways from the set
-								if phi, ok := val.(*ssa.Phi); ok {
-									for _, edge := range phi.Edges {
-										delete(set, edge)
-									}
-								} else {
-									delete(set, val)
-								}
-							}
-						}
-					}
+					inspect(c, &set)
+				} else if c, ok := instr.(*ssa.Defer); ok {
+					inspect(c, &set)
 				}
 			}
 		}
@@ -80,8 +60,46 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
-func isInLogPkg(c *ssa.Call) bool {
-	switch v := c.Call.Value.(type) {
+func inspect(cd callDefer, set *map[posser]struct{}) {
+	c := cd.Common()
+
+	// check if it's in github.com/rs/zerolog/log since there's some
+	// functions in github.com/rs/zerolog that returns zerolog.Event
+	// which should not be included
+	if isInLogPkg(*c) {
+		if isZerologEvent(c.Value) {
+			// check if this is a new instance of zerolog.Event like logger := log.Error()
+			// which should be dispatched afterwards at some point
+			if len(c.Args) == 0 {
+				(*set)[cd] = struct{}{}
+			}
+			return
+		}
+	}
+
+	// if the call does not return zerolog.Event,
+	// check if the base is zerolog.Event.
+	// if so, check if the StaticCallee is Send() or Msg().
+	// if so, remove the arg[0] from the set.
+	for _, arg := range c.Args {
+		if isZerologEvent(arg) {
+			if isDispatchMethod(*c) {
+				val := getRootSsaValue(arg)
+				// if there's branch, remove both ways from the set
+				if phi, ok := val.(*ssa.Phi); ok {
+					for _, edge := range phi.Edges {
+						delete(*set, edge)
+					}
+				} else {
+					delete(*set, val)
+				}
+			}
+		}
+	}
+}
+
+func isInLogPkg(c ssa.CallCommon) bool {
+	switch v := c.Value.(type) {
 	case ssa.Member:
 		p := v.Package()
 		if p == nil {
@@ -98,8 +116,8 @@ func isZerologEvent(v ssa.Value) bool {
 	return strings.HasSuffix(ts, "github.com/rs/zerolog.Event")
 }
 
-func isDispatchMethod(c *ssa.Call) bool {
-	m := c.Common().StaticCallee().Name()
+func isDispatchMethod(c ssa.CallCommon) bool {
+	m := c.StaticCallee().Name()
 	if m == "Send" || m == "Msg" || m == "Msgf" || m == "MsgFunc" {
 		return true
 	}