Dead exported code in internal/ is invisible to `unused`; evaluate x/tools deadcode

[plombardi89]

Found during review of #668.

internal/net/netlink has 7 of 22 exported LinkManager methods with no production caller. make lint is clean, and unused is enabled. It cannot see them, and no setting changes that.

The evidence

Method	Production callers	Test callers
EnsureIPIPInterfaceWithRemote	0	1
EnsureGeneveInterfaceWithRemote	0	1
EnsureBridge	0	1
EnsureGeneveInterfaceWithCache	0	1
EnsureIPIPInterfaceWithCache	0	1
SetLinkNoARP	0	0
GetAddresses	0	0

SetLinkNoARP and GetAddresses have zero references anywhere in the tree.

The live tunnel paths are EnsureIPIPExternalInterface, EnsureGeneveInterface, EnsureVXLANInterface, EnsureWireGuardInterface and EnsureDummyInterface. The live CNI bridge path is Exists/EnsureMTUWithCache/EnsureBridgePortMTUs/EnsureBridgePodMTUs (mtu.go:19-55) — EnsureBridge is not part of it.

Why unused cannot catch this

Not a gap, a documented design decision. From honnef.co/go/tools@v0.8.0, unused/unused.go:48-62:

- packages use:
  - (1.1) exported named types
  - (1.2) exported functions (but not methods!)
- named types use:
  - (2.1) exported methods

The package uses LinkManager because it is an exported named type (1.1), and LinkManager uses every one of its exported methods (2.1). They are transitively live because the type is exported. That the package is internal/, and so provably has no external consumers, is not considered.

Three things follow:

• golangci-lint's unused settings (field-writes-are-uses, post-statements-are-reads, exported-fields-are-used, parameters-are-used, local-variables-are-used, generated-is-used) do not touch this.
• staticcheck's whole-program mode was removed; it survives only in source comments.
• This affects every exported identifier on an exported type across internal/, not just this one file.

Proposal: evaluate golang.org/x/tools/cmd/deadcode

Rapid Type Analysis from main, which is the analysis unused deliberately does not do. Already in the module graph as x/tools v0.49.0, and it has -json, -tags, -filter and -whylive.

The repo has a clean precedent in make vulncheck: run the analyser in JSON mode and let hack/cmd/vulncheck-gate own the verdict. go.mod:5-8 already carries tool directives for govulncheck and controller-gen, so deadcode would be a third, version-pinned alongside them.

Suggested first step: a report, not a gate. Add the tool directive and a make deadcode target, measure the findings across the tree, and only then decide whether a gate and allowlist are worth it. Building a gate around a findings count nobody has seen is a guess.

Caveats to carry into that work

• Roots are main packages only — 31 in this repo.
• Results are valid for a single GOOS/GOARCH/tags configuration. This tree has linux, !linux, cgo, !cgo, e2e, integrationtest and storageboundary, so one run will report !linux code as dead.
• It does not understand //go:linkname.
• Its own doc: "just because a function is reported as dead does not mean it is unconditionally safe to delete it" — a dead method may still be needed to satisfy an interface.

The trap worth recording

Without -test, code reachable only from tests reads as dead, which is exactly what surfaces these seven. With -test, five of them look live, because #668 added tests that call them.

Writing tests for dead code defeats the detection. That is an argument for deleting on evidence rather than covering, and a reason to prefer the no--test run for this particular question.

Scope

1. Add the tool directive and a make deadcode report target; measure.
2. Decide on a gate, based on the numbers.
3. Remove the 7 methods above (deliberately not done in net/netlink: a failed link lookup is not an absent interface #668, which stays scoped to error classification).

[plombardi89]

Worked through, split by component. Nine PRs, all independent of one another — the file sets are disjoint, so they can be reviewed and merged in any order and in parallel.

Tooling (step 1 of the scope above)

• deadcode: add the two analyzers that can find dead code under internal/ #673 — make deadcode: the x/tools/cmd/deadcode tool directive plus hack/cmd/deadcode-report, and a second analyzer, hack/cmd/unreferenced.

The second one is not what this issue asked for, and the reason is worth recording here. deadcode does not detect the seven LinkManager methods this issue is about:

$ go tool deadcode -whylive=...netlink.LinkManager.SetLinkNoARP ./...
deadcode: ...LinkManager.SetLinkNoARP is reachable only through reflection

x/tools/go/callgraph/rta/rta.go:281-287,433-437 — converting a value to an interface materializes its runtime type, and RTA then marks every exported method of that type reachable, because reflection could call any of them. That is soundness with respect to reflection, working as designed, and it is structurally the same blind spot as the unused rule quoted in the issue body, reached by a different route. *LinkManager is boxed somewhere, so all 22 of its exported methods read as live.

So the proposal in this issue, taken literally, would have shipped a tool that cannot see its own motivating example. hack/cmd/unreferenced resolves identifiers instead of tracing reachability, and is what reports all seven.

Deletions (step 3, and everything else the scan turned up)

• net: delete unreachable and unreferenced code in netlink, ebpf and metrics #675 — net: netlink, ebpf, metrics, config — contains the seven methods from this issue
• net: delete the superseded node-CIDR controller #676 — net: the superseded node-CIDR controller
• gantry: delete unused fakes, options and the unadopted log vocabulary #677 — gantry
• kube, forge: delete unused cluster and Azure helpers #678 — kube, forge
• agent: delete unreferenced helpers and unread constants #679 — agent
• metalman: delete unreferenced helpers #680 — metalman
• operator, orca, playpen: delete unreferenced helpers #681 — operator, orca, playpen
• hack: delete unreferenced dev-tooling helpers #682 — hack dev tooling

Numbers

	Before	After
Unreachable from any binary or test	80	1
Unreferenced declarations	120	23

3233 deletions. The residual 24 are deliberate keeps, each documented in a comment at the declaration: a String() that redacts a secret, the only code that detaches a TC filter, the functions that keep the override allowlist docs honest, three constants whose consumers are a shell script and a Python harness rather than Go, published CRD constants under api/, and schema re-export aliases.

Step 2, the gate

Still open, and now there are numbers to decide on. One finding from doing the removals should feed into it: removing a caller makes its callees dead in turn, and neither analyzer can report the second round until the first has landed. Re-running after all the deletions surfaced four more, one of which was NetlinkCache.HasLink — whose only callers were two of the seven methods above. A gate should expect to need more than one pass after a large deletion, and an allowlist sized from a single pass will be wrong.

Unobserved Prometheus metrics found during the sweep are tracked separately in #672, since deleting a registered-but-never-observed metric changes the scrape surface rather than just removing code.

I have left this issue open; it should close when the deletion PRs land, not when the tooling does.