From 4438f689ad15086050351f9df29d10795f4c7941 Mon Sep 17 00:00:00 2001
From: Hugo Wetterberg <hugo@wetterberg.nu>
Date: Tue, 31 Mar 2026 08:22:55 +0200
Subject: [PATCH] detect Close calls in helper functions that receive resp.Body

When resp.Body is passed as an argument to a helper function that calls
Close on that parameter, bodyclose currently reports a false positive
(see #30).

This happens because isBodyProperlyHandled only checks direct referrers
of the body value for Close calls, but does not follow the body into
function arguments.

This change adds two methods:

- closedViaHelper: checks if the body (or an interface-converted form of
  it) is passed to a function that closes it.
- argClosedInCallee: resolves the static callee, maps the argument to
  the corresponding parameter, and checks if Close is called on that
  parameter's referrers.

This covers common patterns like:

    defer safeClose("label", resp.Body)

where safeClose calls body.Close() internally, as well as patterns
where the body is first converted to io.Closer or io.ReadCloser before
being passed to the helper.

Test cases added for:
- helper taking io.ReadCloser that closes it (should pass)
- helper taking io.Closer that closes it (should pass)
- helper taking io.ReadCloser that does NOT close it (should fail)
---
 passes/bodyclose/bodyclose.go                 | 70 +++++++++++++++++++
 .../testdata/src/a/close_by_helper.go         | 52 ++++++++++++++
 2 files changed, 122 insertions(+)
 create mode 100644 passes/bodyclose/testdata/src/a/close_by_helper.go

diff --git a/passes/bodyclose/bodyclose.go b/passes/bodyclose/bodyclose.go
index 4a86252..01dcbdc 100644
--- a/passes/bodyclose/bodyclose.go
+++ b/passes/bodyclose/bodyclose.go
@@ -321,12 +321,82 @@ func (r *runner) isBodyProperlyHandled(bOp *ssa.UnOp) bool {
 			// Close found and consumption checking enabled - check consumption
 			return r.hasConsumptionForBody(bOp)
 		}
+
+		if r.closedViaHelper(ccall, bOp) {
+			if !r.checkConsumption {
+				return true
+			}
+			return r.hasConsumptionForBody(bOp)
+		}
 	}
 
 	// No close call found
 	return false
 }
 
+// closedViaHelper checks if body is passed as an argument to a function
+// (directly or via ChangeInterface) that calls Close on that parameter.
+func (r *runner) closedViaHelper(instr ssa.Instruction, body *ssa.UnOp) bool {
+	if r.argClosedInCallee(instr, body) {
+		return true
+	}
+	// Check if body is converted to an interface (io.Closer, io.ReadCloser)
+	// and that value is passed to a callee that closes it.
+	for _, ref := range *body.Referrers() {
+		ci, ok := ref.(*ssa.ChangeInterface)
+		if !ok {
+			continue
+		}
+		if ci.Referrers() == nil {
+			continue
+		}
+		for _, ciRef := range *ci.Referrers() {
+			if r.argClosedInCallee(ciRef, ci) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// argClosedInCallee checks if instr is a Call or Defer where val is one of
+// the arguments and the callee calls Close on the matching parameter.
+func (r *runner) argClosedInCallee(instr ssa.Instruction, val ssa.Value) bool {
+	var call ssa.CallCommon
+	switch c := instr.(type) {
+	case *ssa.Call:
+		call = c.Call
+	case *ssa.Defer:
+		call = c.Call
+	default:
+		return false
+	}
+	f, ok := call.Value.(*ssa.Function)
+	if !ok || f.Blocks == nil {
+		return false
+	}
+	argIdx := -1
+	for i, arg := range call.Args {
+		if arg == val {
+			argIdx = i
+			break
+		}
+	}
+	if argIdx < 0 || argIdx >= len(f.Params) {
+		return false
+	}
+	param := f.Params[argIdx]
+	if param.Referrers() == nil {
+		return false
+	}
+	for _, ref := range *param.Referrers() {
+		if r.isCloseCall(ref) {
+			return true
+		}
+	}
+	return false
+}
+
 // hasConsumptionForBody searches the function for consumption calls that use the specific response body
 func (r *runner) hasConsumptionForBody(bodyOp *ssa.UnOp) bool {
 	fn := bodyOp.Block().Parent()
diff --git a/passes/bodyclose/testdata/src/a/close_by_helper.go b/passes/bodyclose/testdata/src/a/close_by_helper.go
new file mode 100644
index 0000000..aa02e25
--- /dev/null
+++ b/passes/bodyclose/testdata/src/a/close_by_helper.go
@@ -0,0 +1,52 @@
+package a
+
+import (
+	"io"
+	"net/http"
+)
+
+// closeByHelperOK passes resp.Body to a helper that closes it.
+func closeByHelperOK() {
+	resp, err := http.Get("http://example.com/") // OK
+	if err != nil {
+		return
+	}
+
+	defer safeClose("response body", resp.Body)
+}
+
+// closeByHelperDeferOK uses a helper that takes an io.Closer.
+func closeByHelperDeferOK() {
+	resp, err := http.Get("http://example.com/") // OK
+	if err != nil {
+		return
+	}
+
+	defer safeCloseCloser(resp.Body)
+}
+
+// closeByHelperNotClosed passes resp.Body to a function that does NOT close it.
+func closeByHelperNotClosed() {
+	resp, err := http.Get("http://example.com/") // want "response body must be closed"
+	if err != nil {
+		return
+	}
+
+	defer consumeBody(resp.Body)
+}
+
+func safeClose(label string, body io.ReadCloser) {
+	err := body.Close()
+	if err != nil {
+		panic(err)
+	}
+	_ = label
+}
+
+func safeCloseCloser(c io.Closer) {
+	_ = c.Close()
+}
+
+func consumeBody(body io.ReadCloser) {
+	_, _ = io.ReadAll(body)
+}