[object Object]

← back to Cli Printing Press

fix(cli): add transitive reachability to dogfood dead function scanner (#183)

b436b081953eb7a92005249cd0554466a54824de · 2026-04-12 09:37:23 -0400 · Matt Van Horn

The dead function scanner was producing false positives for helper functions
that are only called by other helpers (not directly by command files). Adds
iterative fixed-point expansion within helpers.go so transitively reachable
functions are correctly marked as live.

Supersedes #110 (which bundled this fix with MarkFlagRequired changes that
already landed via #130).

🤖 Generated with Claude Opus 4.6 via [Claude Code](https://claude.com/claude-code) + Compound Engineering v2.56.1

Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Files touched

Diff

commit b436b081953eb7a92005249cd0554466a54824de
Author: Matt Van Horn <mvanhorn@users.noreply.github.com>
Date:   Sun Apr 12 09:37:23 2026 -0400

    fix(cli): add transitive reachability to dogfood dead function scanner (#183)
    
    The dead function scanner was producing false positives for helper functions
    that are only called by other helpers (not directly by command files). Adds
    iterative fixed-point expansion within helpers.go so transitively reachable
    functions are correctly marked as live.
    
    Supersedes #110 (which bundled this fix with MarkFlagRequired changes that
    already landed via #130).
    
    🤖 Generated with Claude Opus 4.6 via [Claude Code](https://claude.com/claude-code) + Compound Engineering v2.56.1
    
    Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
    Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 internal/pipeline/dogfood.go      |  86 +++++++++++++++++++++----
 internal/pipeline/dogfood_test.go | 130 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 202 insertions(+), 14 deletions(-)

diff --git a/internal/pipeline/dogfood.go b/internal/pipeline/dogfood.go
index d017c620..808ea4c3 100644
--- a/internal/pipeline/dogfood.go
+++ b/internal/pipeline/dogfood.go
@@ -649,37 +649,74 @@ func checkDeadFunctions(dir string) DeadCodeResult {
 		names[match[1]] = struct{}{}
 	}
 
-	// Include helpers.go in the search but strip function definition lines
-	// so that a function's own `func name(` line doesn't count as a call.
-	// This catches intra-file calls like bold() calling colorEnabled().
-	defLineRe := regexp.MustCompile(`(?m)^func\s+[A-Za-z_]\w*\s*\(.*$`)
-	helpersUsageOnly := defLineRe.ReplaceAllString(string(data), "")
-
+	// Collect external sources (everything except helpers.go) for liveness seeding.
 	files := listGoFiles(filepath.Join(dir, "internal", "cli"))
-	var otherSources []string
+	var externalSources []string
 	for _, file := range files {
 		if filepath.Base(file) == "helpers.go" {
-			otherSources = append(otherSources, helpersUsageOnly)
 			continue
 		}
 		content, err := os.ReadFile(file)
 		if err != nil {
 			continue
 		}
-		otherSources = append(otherSources, string(content))
+		externalSources = append(externalSources, string(content))
 	}
 
-	result := DeadCodeResult{Total: len(names)}
+	// Seed live set: functions called from external (non-helpers) files.
+	liveSet := make(map[string]bool)
 	for _, name := range sortedKeys(names) {
 		callRe := regexp.MustCompile(`\b` + regexp.QuoteMeta(name) + `\s*\(`)
-		used := false
-		for _, source := range otherSources {
+		for _, source := range externalSources {
 			if callRe.MatchString(source) {
-				used = true
+				liveSet[name] = true
 				break
 			}
 		}
-		if used {
+	}
+
+	// Build intra-helpers call map: for each helper function, which other
+	// helper functions does its body call?
+	helperBodies := extractFunctionBodies(string(data))
+	callsMap := make(map[string][]string)
+	for _, caller := range sortedKeys(names) {
+		body, ok := helperBodies[caller]
+		if !ok {
+			continue
+		}
+		for _, callee := range sortedKeys(names) {
+			if callee == caller {
+				continue
+			}
+			callRe := regexp.MustCompile(`\b` + regexp.QuoteMeta(callee) + `\s*\(`)
+			if callRe.MatchString(body) {
+				callsMap[caller] = append(callsMap[caller], callee)
+			}
+		}
+	}
+
+	// Iterative expansion: mark transitively reachable helpers as live.
+	for i := 0; i < 50; i++ {
+		changed := false
+		for _, fn := range sortedKeys(names) {
+			if !liveSet[fn] {
+				continue
+			}
+			for _, callee := range callsMap[fn] {
+				if !liveSet[callee] {
+					liveSet[callee] = true
+					changed = true
+				}
+			}
+		}
+		if !changed {
+			break
+		}
+	}
+
+	result := DeadCodeResult{Total: len(names)}
+	for _, name := range sortedKeys(names) {
+		if liveSet[name] {
 			continue
 		}
 		result.Dead++
@@ -688,6 +725,27 @@ func checkDeadFunctions(dir string) DeadCodeResult {
 	return result
 }
 
+// extractFunctionBodies returns a map from function name to its body text
+// (everything between its func line and the next top-level func line).
+func extractFunctionBodies(source string) map[string]string {
+	bodies := make(map[string]string)
+	funcLineRe := regexp.MustCompile(`(?m)^func\s+([A-Za-z_]\w*)\s*\(`)
+	locs := funcLineRe.FindAllStringIndex(source, -1)
+	nameMatches := funcLineRe.FindAllStringSubmatch(source, -1)
+	for i, match := range nameMatches {
+		name := match[1]
+		start := locs[i][1]
+		var end int
+		if i+1 < len(locs) {
+			end = locs[i+1][0]
+		} else {
+			end = len(source)
+		}
+		bodies[name] = source[start:end]
+	}
+	return bodies
+}
+
 func checkPipelineIntegrity(dir string) PipelineResult {
 	result := PipelineResult{
 		Detail: "sync/search/store files not found",
diff --git a/internal/pipeline/dogfood_test.go b/internal/pipeline/dogfood_test.go
index c221d8e1..a6704b85 100644
--- a/internal/pipeline/dogfood_test.go
+++ b/internal/pipeline/dogfood_test.go
@@ -741,6 +741,136 @@ func TestDeriveDogfoodVerdict_NovelFeatures(t *testing.T) {
 	assert.Equal(t, "PASS", deriveDogfoodVerdict(base, true))
 }
 
+func TestDeadFunctions_TransitiveReachability(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "helpers.go"), `package cli
+
+func funcA() {
+	funcB()
+}
+
+func funcB() {
+	// only called by funcA
+}
+
+func funcC() {
+	// never called by anything
+}
+`)
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "cmd.go"), `package cli
+
+func runCmd() {
+	funcA()
+}
+`)
+
+	result := checkDeadFunctions(dir)
+	assert.Equal(t, 3, result.Total)
+	assert.Equal(t, 1, result.Dead)
+	assert.Equal(t, []string{"funcC"}, result.Items)
+}
+
+func TestDeadFunctions_ChainOfThree(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "helpers.go"), `package cli
+
+func funcA() {
+	funcB()
+}
+
+func funcB() {
+	funcC()
+}
+
+func funcC() {
+	// end of chain
+}
+`)
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "cmd.go"), `package cli
+
+func runCmd() {
+	funcA()
+}
+`)
+
+	result := checkDeadFunctions(dir)
+	assert.Equal(t, 3, result.Total)
+	assert.Equal(t, 0, result.Dead)
+	assert.Empty(t, result.Items)
+}
+
+func TestDeadFunctions_GenuinelyDead(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "helpers.go"), `package cli
+
+func funcD() {
+	// defined but never called
+}
+`)
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "cmd.go"), `package cli
+
+func runCmd() {
+	// does not call funcD
+}
+`)
+
+	result := checkDeadFunctions(dir)
+	assert.Equal(t, 1, result.Total)
+	assert.Equal(t, 1, result.Dead)
+	assert.Equal(t, []string{"funcD"}, result.Items)
+}
+
+func TestDeadFlags_FrameworkFlags(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "root.go"), `package cli
+
+type rootFlags struct {
+	agent     bool
+	rateLimit int
+	noCache   bool
+	deadOnly  bool
+}
+
+func initFlags(flags *rootFlags) {
+	_ = &flags.agent
+	_ = &flags.rateLimit
+	_ = &flags.noCache
+	_ = &flags.deadOnly
+}
+
+func (f *rootFlags) newClient() {
+	client.New(cfg, f.rateLimit)
+}
+
+func execute(flags *rootFlags) {
+	if flags.agent {
+		enableAgent()
+	}
+}
+`)
+	writeTestFile(t, filepath.Join(dir, "internal", "cli", "export.go"), `package cli
+
+func runExport(flags *rootFlags) {
+	if flags.noCache {
+		skipCache()
+	}
+}
+`)
+
+	result := checkDeadFlags(dir)
+	assert.Equal(t, 4, result.Total)
+	assert.Equal(t, 1, result.Dead)
+	assert.Equal(t, []string{"deadOnly"}, result.Items)
+}
+
 func writeTestFile(t *testing.T, path string, content string) {
 	t.Helper()
 	require.NoError(t, os.WriteFile(path, []byte(content), 0o644))

← 7e640716 chore(main): release 2.1.0 (#181)  ·  back to Cli Printing Press  ·  fix(cli): cross-CLI retro findings - store, sync, scorer, Gr e2009bb1 →