Skip to content

Commit

Permalink
Fix false positive on if statement
Browse files Browse the repository at this point in the history
  • Loading branch information
ykadowak committed Dec 1, 2023
1 parent 903c0a7 commit cda2ef5
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 6 deletions.
40 changes: 38 additions & 2 deletions testdata/src/a/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,32 @@ func positives() {
Int("n", 1),
)

// conditional
// conditional1
logger2 := log.Info() // want "must be dispatched by Msg or Send method"
if err != nil {
logger2 = log.Error() // want "must be dispatched by Msg or Send method"
}
logger2.Str("foo", "bar")

// conditional2
loggerCond2 := log.Info().Str("a", "b") // want "must be dispatched by Msg or Send method"
if err != nil {
loggerCond2 = loggerCond2.Str("c", "d")
}
loggerCond2.Str("foo", "bar")

// conditional3
loggerCond3 := log.Info().Str("a", "b") // want "must be dispatched by Msg or Send method"
if err != nil {
loggerCond3 = loggerCond3.Str("c", "d")
}
if err != nil {
loggerCond3 = loggerCond3.Str("e", "f")
}
loggerCond3.Str("foo", "bar")

// defer patterns
defer log.Info() // want "must be dispatched by Msg or Send method"
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)
Expand Down Expand Up @@ -92,6 +109,25 @@ func negatives() {
}
logger2.Send()

// conditional2
loggerCond2 := log.Info().Str("a", "b")
if err != nil {
loggerCond2 = loggerCond2.Str("c", "d")
}
loggerCond2.Str("foo", "bar")
loggerCond2.Send()

// conditional3
loggerCond3 := log.Info().Str("a", "b")
if err != nil {
loggerCond3 = loggerCond3.Str("c", "d")
}
if err != nil {
loggerCond3 = loggerCond3.Str("e", "f")
}
loggerCond3.Str("foo", "bar")
loggerCond3.Send()

// dispatch variation
log.Info().Msgf("")
log.Info().MsgFunc(func() string { return "foo" })
Expand Down
32 changes: 28 additions & 4 deletions zerologlint.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,37 @@ func inspect(cd callDefer, set map[posser]struct{}) {
}
for _, arg := range c.Args {
if isZerologEvent(arg) {
val := getRootSsaValue(arg)
// if there's branch, remove both ways from the set
if phi, ok := val.(*ssa.Phi); ok {
// If there's branch, track both ways
// this is for the case like:
// logger := log.Info()
// if err != nil {
// logger = log.Error()
// }
// logger.Send()
//
// Similar case like below goes to the same root but that doesn't
// have any side effect.
// logger := log.Info()
// if err != nil {
// logger = logger.Str("a", "b")
// }
// logger.Send()
if phi, ok := arg.(*ssa.Phi); ok {
for _, edge := range phi.Edges {
delete(set, edge)
// FIXME: this is scary. Need to set a limit to the loop
for {
val := getRootSsaValue(edge)
if v, ok := val.(*ssa.Phi); ok {
edge = v.Edges[0]
continue
} else {
delete(set, val)
break
}
}
}
} else {
val := getRootSsaValue(arg)
delete(set, val)
}
}
Expand Down

0 comments on commit cda2ef5

Please sign in to comment.