fix: Improve SSA tracing for Alloc, nil Phi, and method values (#9)

* fix: Add Alloc tracing and nil Phi edge skipping (#8)

Improves SSA value tracing to handle more patterns correctly:

## Alloc Tracing
- Add `traceAlloc` to find values stored in local variables
- Fixes false positive for `log.Ctx(ctx).With()...Logger()` pattern
- Enables tracking through variable assignments in closures

## Nil Phi Edge Skipping
- Skip nil constant edges in Phi nodes during tracing
- Nil pointers can't have methods called on them, so they're irrelevant
- Fixes false positive for `var e *Event; if cond { e = ... }; if e != nil { e.Msg() }`

## Test Updates
- Remove outdated LIMITATION comments for now-working patterns
- Add test cases for nil Phi edge handling with mixed branches

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat: Add method value (bound method) detection

Detect zerolog terminators called via method values:

  e := logger.Info()
  msg := e.Msg        // MakeClosure with receiver in Bindings[0]
  msg("text")         // Now detected as terminator

Implementation:
- Check if call value is MakeClosure in checkTerminatorCall
- Extract receiver from MakeClosure.Bindings[0]
- Trace receiver to check for context

This was previously documented as a LIMITATION but is now fully supported.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* docs: Update KNOWN LIMITATIONS in evil_ssa.go

Remove fixed items:
- Method values (now supported)
- Phi node with nil (now handled)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: mpyw

internal/analyzer.go

@@ -192,6 +192,14 @@ func (c *checker) checkTerminatorCall(call *ssa.Call) {
 		return
 	}
 
+	// Check if this is a bound method call (method value)
+	// e.g., msg := e.Msg; msg("text")
+	// In this case, callee is the wrapper (*Event).Msg$bound and recv is nil
+	if mc, ok := call.Call.Value.(*ssa.MakeClosure); ok {
+		c.checkBoundMethodTerminator(call, mc, callee)
+		return
+	}
+
 	// Must be on zerolog.Event and return void (terminators: Msg, Msgf, MsgFunc, Send)
 	recv := call.Call.Signature().Recv()
 	if recv == nil || !isEvent(recv.Type()) || !returnsVoid(callee) {
@@ -206,6 +214,34 @@ func (c *checker) checkTerminatorCall(call *ssa.Call) {
 	c.report(call.Pos())
 }
 
+// checkBoundMethodTerminator checks if a bound method call (method value) is a terminator
+// without context. Bound methods are created when a method is extracted as a value:
+//
+//	msg := e.Msg    // Creates MakeClosure with receiver in Bindings[0]
+//	msg("text")     // Calls the bound method wrapper (*Event).Msg$bound
+func (c *checker) checkBoundMethodTerminator(call *ssa.Call, mc *ssa.MakeClosure, callee *ssa.Function) {
+	// Check if it returns void (terminators return void)
+	if !returnsVoid(callee) {
+		return
+	}
+
+	// Check if receiver (in Bindings[0]) is *zerolog.Event
+	if len(mc.Bindings) == 0 {
+		return
+	}
+	recvType := mc.Bindings[0].Type()
+	if !isEvent(recvType) {
+		return
+	}
+
+	// Trace the receiver to find if context was set
+	if c.eventChainHasCtx(mc.Bindings[0]) {
+		return
+	}
+
+	c.report(call.Pos())
+}
+
 // checkDirectLoggingCall checks for direct logging calls that bypass the Event chain
 // (Logger.Print, Logger.Printf, log.Print, log.Printf).
 // These calls cannot propagate context and should be reported.

internal/tracing.go

@@ -76,6 +76,11 @@ func (c *checker) traceValue(v ssa.Value, tracer tracer, visited map[ssa.Value]b
 
 	callee := call.Call.StaticCallee()
 	if callee == nil {
+		// Check if this is a call to a bound method (method value)
+		// e.g., msg := e.Msg; msg("text") - the receiver is in MakeClosure.Bindings
+		if mc, ok := call.Call.Value.(*ssa.MakeClosure); ok {
+			return c.traceBoundMethod(mc, visited, tracer)
+		}
 		return c.traceReceiver(call, visited, tracer)
 	}
 
@@ -114,6 +119,8 @@ func (c *checker) traceCommon(v ssa.Value, visited map[ssa.Value]bool, tracer tr
 		return c.traceValue(val.Tuple, tracer, visited)
 	case *ssa.UnOp:
 		return c.traceUnOp(val, visited, tracer)
+	case *ssa.Alloc:
+		return c.traceAlloc(val, visited, tracer)
 	case *ssa.ChangeType:
 		return c.traceValue(val.X, tracer, visited)
 	case *ssa.MakeInterface:
@@ -141,21 +148,35 @@ func (c *checker) traceCommon(v ssa.Value, visited map[ssa.Value]bool, tracer tr
 // =============================================================================
 
 // tracePhi handles SSA Phi nodes where multiple control flow paths merge.
-// For context tracking, ALL non-cyclic edges must have context set.
-// Cyclic edges (loop back-edges) are skipped as they depend on the
-// initial value (e.g., loops like: x := init; for { x = f(x) }).
+// For context tracking, ALL non-cyclic, non-nil edges must have context set.
+//
+// Skipped edges:
+//   - Cyclic edges (loop back-edges) depend on the initial value
+//   - Nil constant edges can never reach method calls (would panic)
+//
+// Example:
+//
+//	var e *zerolog.Event
+//	if cond { e = logger.Info().Ctx(ctx) }
+//	if e != nil { e.Msg("...") }  // Only non-nil edge matters
 func (c *checker) tracePhi(phi *ssa.Phi, visited map[ssa.Value]bool, tracer tracer) bool {
 	if len(phi.Edges) == 0 {
 		return false
 	}
 
-	hasNonCyclicEdge := false
+	hasValidEdge := false
 	for _, edge := range phi.Edges {
 		// Skip edges that would cycle back to this Phi
 		if edgeLeadsTo(edge, phi, visited) {
 			continue
 		}
-		hasNonCyclicEdge = true
+
+		// Skip nil constant edges - nil pointers can't have methods called on them
+		if isNilConst(edge) {
+			continue
+		}
+
+		hasValidEdge = true
 
 		// Clone visited for independent tracing of each branch
 		edgeVisited := make(map[ssa.Value]bool)
@@ -166,8 +187,20 @@ func (c *checker) tracePhi(phi *ssa.Phi, visited map[ssa.Value]bool, tracer trac
 		}
 	}
 
-	// If all edges are cyclic, we need at least one valid edge to check
-	return hasNonCyclicEdge
+	// Need at least one valid (non-cyclic, non-nil) edge to check
+	return hasValidEdge
+}
+
+// isNilConst checks if a value is a nil constant.
+// Nil pointers cannot have methods called on them, so they are safe to skip
+// when tracing Phi nodes (the nil path would panic before reaching the call).
+func isNilConst(v ssa.Value) bool {
+	c, ok := v.(*ssa.Const)
+	if !ok {
+		return false
+	}
+	// For nil pointer constants, Value is nil
+	return c.Value == nil
 }
 
 // edgeLeadsTo checks if tracing this edge would eventually lead back to target.
@@ -224,6 +257,21 @@ func (c *checker) traceUnOp(unop *ssa.UnOp, visited map[ssa.Value]bool, tracer t
 	return c.traceValue(unop.X, tracer, visited)
 }
 
+// traceAlloc handles SSA Alloc nodes (local variable allocation).
+// When a variable is used as a receiver (e.g., logger.Info()), we need to
+// find what value was stored into that variable.
+//
+// Example:
+//
+//	logger := log.Ctx(ctx).With().Logger()  // t0 = new Logger; *t0 = result
+//	logger.Info().Msg("test")               // uses t0 (address of logger)
+func (c *checker) traceAlloc(alloc *ssa.Alloc, visited map[ssa.Value]bool, tracer tracer) bool {
+	if stored := findStoredValue(alloc); stored != nil {
+		return c.traceValue(stored, tracer, visited)
+	}
+	return false
+}
+
 // traceFreeVar traces a FreeVar back to the value bound in MakeClosure.
 // FreeVars are variables captured from an enclosing function scope.
 func (c *checker) traceFreeVar(fv *ssa.FreeVar, visited map[ssa.Value]bool, tracer tracer) bool {
@@ -280,6 +328,22 @@ func (c *checker) traceReceiver(call *ssa.Call, visited map[ssa.Value]bool, trac
 	return false
 }
 
+// traceBoundMethod traces a bound method (method value) call.
+// When a method is extracted as a value (e.g., msg := e.Msg), SSA creates
+// a MakeClosure with the receiver in Bindings[0].
+//
+// Example:
+//
+//	e := logger.Info().Ctx(ctx)
+//	msg := e.Msg          // MakeClosure with Bindings[0] = e
+//	msg("text")           // Call to the closure
+func (c *checker) traceBoundMethod(mc *ssa.MakeClosure, visited map[ssa.Value]bool, tracer tracer) bool {
+	if len(mc.Bindings) > 0 {
+		return c.traceValue(mc.Bindings[0], tracer, visited)
+	}
+	return false
+}
+
 // =============================================================================
 // Store Tracking
 // =============================================================================

testdata/src/zerolog/evil.go

@@ -58,10 +58,7 @@ func evilClosureCapturingDerivedLogger(ctx context.Context, logger zerolog.Logge
 func evilClosureCapturingDerivedLoggerWithCtx(ctx context.Context, logger zerolog.Logger) {
 	derived := logger.With().Ctx(ctx).Logger()
 	doSomething := func() {
-		// NOTE: In test stubs, SSA optimizes away the receiver for method calls
-		// on zero-value structs with empty implementations. In production code
-		// with real zerolog, FreeVar tracing through MakeClosure bindings works.
-		derived.Info().Msg("captured derived with ctx") // want `zerolog call chain missing .Ctx\(ctx\)`
+		derived.Info().Msg("captured derived with ctx") // OK - ctx set via With().Ctx(ctx)
 	}
 	doSomething()
 }

testdata/src/zerolog/evil_logger.go

@@ -185,9 +185,7 @@ func badGoroutineWithDerivedLogger(ctx context.Context, logger zerolog.Logger) {
 func goodGoroutineWithDerivedLoggerCtx(ctx context.Context, logger zerolog.Logger) {
 	derived := logger.With().Ctx(ctx).Logger()
 	go func() {
-		// LIMITATION: FreeVar tracking through MakeClosure not working in test stubs
-		// In production, this would be OK since derived has ctx
-		derived.Info().Msg("goroutine derived with ctx") // want `zerolog call chain missing .Ctx\(ctx\)`
+		derived.Info().Msg("goroutine derived with ctx") // OK - ctx set via With().Ctx(ctx)
 	}()
 }
 

testdata/src/zerolog/evil_ssa.go

@@ -8,15 +8,13 @@
 //
 // False Negatives (should report but doesn't):
 //   - IIFE/Helper returns: Cross-function tracking not supported
-//   - Method values: `msg := e.Msg; msg("test")`
 //   - Deep FreeVar: Triple-nested closures
 //
 // False Positives (reports when shouldn't):
 //   - Channel send/receive: Can't trace through channels
 //   - sync.Pool: Can't trace through Get/Put
 //   - Embedded struct: `h.Msg()` where h embeds *Event
 //   - Closure-modified capture: Closure writes to outer var
-//   - Phi node with nil: `var e; if cond { e = ... }; e.Msg()`
 package zerolog
 
 import (
@@ -168,11 +166,10 @@ func badClosureCaptureDeep(ctx context.Context, logger zerolog.Logger) {
 	fn()
 }
 
-func limitationClosureCaptureWithCtx(ctx context.Context, logger zerolog.Logger) {
-	// LIMITATION: SSA doesn't track .Ctx() on captured variables used in closures
+func goodClosureCaptureWithCtx(ctx context.Context, logger zerolog.Logger) {
 	e := logger.Info().Ctx(ctx)
 	fn := func() {
-		e.Msg("captured with ctx") // want `zerolog call chain missing .Ctx\(ctx\)`
+		e.Msg("captured with ctx") // OK - ctx set via .Ctx(ctx)
 	}
 	fn()
 }
@@ -557,26 +554,31 @@ func badTripleNestedClosure(ctx context.Context, logger zerolog.Logger) {
 	}()
 }
 
-func limitationTripleNestedClosureWithCtx(ctx context.Context, logger zerolog.Logger) {
-	// LIMITATION: Deep FreeVar tracking through multiple closure levels
+func goodTripleNestedClosureWithCtx(ctx context.Context, logger zerolog.Logger) {
 	e := logger.Info().Ctx(ctx)
 	func() {
 		func() {
 			func() {
-				e.Msg("triple nested with ctx") // want `zerolog call chain missing .Ctx\(ctx\)`
+				e.Msg("triple nested with ctx") // OK - ctx set via .Ctx(ctx)
 			}()
 		}()
 	}()
 }
 
 // ===== METHOD VALUE =====
 
-// LIMITATION: Method values extract the function from the receiver, creating a different
-// SSA pattern. The analyzer cannot trace through method value extraction.
-func limitationMethodValue(ctx context.Context, logger zerolog.Logger) {
+// Method values extract the method from the receiver, creating a MakeClosure.
+// The analyzer traces through the closure bindings to find the receiver.
+func badMethodValue(ctx context.Context, logger zerolog.Logger) {
 	e := logger.Info()
 	msg := e.Msg
-	msg("method value") // LIMITATION: should report but doesn't due to method value extraction
+	msg("method value") // want `zerolog call chain missing .Ctx\(ctx\)`
+}
+
+func goodMethodValueWithCtx(ctx context.Context, logger zerolog.Logger) {
+	e := logger.Info().Ctx(ctx)
+	msg := e.Msg
+	msg("method value with ctx") // OK - ctx set via .Ctx(ctx)
 }
 
 // ===== STRUCT WITH MULTIPLE EVENT FIELDS =====
@@ -941,15 +943,42 @@ func badNilCheckBeforeUse(ctx context.Context, logger zerolog.Logger) {
 	}
 }
 
-// LIMITATION (false positive): Phi node sees potential nil from else branch.
-// Even with `if true`, SSA still models an else path where e is nil (zero value).
-func limitationNilCheckWithCtx(ctx context.Context, logger zerolog.Logger) {
+// Phi node with nil edge from uninitialized variable.
+// The nil edge is skipped because nil pointers can't have methods called on them.
+func goodNilCheckWithCtx(ctx context.Context, logger zerolog.Logger) {
 	var e *zerolog.Event
 	if true {
 		e = logger.Info().Ctx(ctx)
 	}
 	if e != nil {
-		e.Msg("nil checked with ctx") // want `zerolog call chain missing .Ctx\(ctx\)`
+		e.Msg("nil checked with ctx") // OK - nil edge skipped, ctx edge has context
+	}
+}
+
+// Phi node with nil + multiple non-nil edges.
+// All non-nil edges must have context.
+func badNilCheckMixedEdges(ctx context.Context, logger zerolog.Logger, cond1, cond2 bool) {
+	var e *zerolog.Event
+	if cond1 {
+		e = logger.Info().Ctx(ctx) // has ctx
+	} else if cond2 {
+		e = logger.Info() // no ctx
+	}
+	if e != nil {
+		e.Msg("mixed edges") // want `zerolog call chain missing .Ctx\(ctx\)`
+	}
+}
+
+// Phi node where all non-nil edges have context.
+func goodNilCheckAllEdgesWithCtx(ctx context.Context, logger zerolog.Logger, cond1, cond2 bool) {
+	var e *zerolog.Event
+	if cond1 {
+		e = logger.Info().Ctx(ctx)
+	} else if cond2 {
+		e = logger.Debug().Ctx(ctx)
+	}
+	if e != nil {
+		e.Msg("all edges with ctx") // OK - nil skipped, all non-nil have ctx
 	}
 }