Repository navigation
Dead exported code in internal/ is invisible to unused; evaluate x/tools deadcode #670
Description
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
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
unusedsettings (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
mainpackages only — 31 in this repo. - Results are valid for a single GOOS/GOARCH/tags configuration. This tree has
linux,!linux,cgo,!cgo,e2e,integrationtestandstorageboundary, so one run will report!linuxcode 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
- Add the tool directive and a
make deadcodereport target; measure. - Decide on a gate, based on the numbers.
- 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).
Activity
plombardi89 commentedon Aug 26, 2026
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)
make deadcode: thex/tools/cmd/deadcodetool directive plushack/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.
deadcodedoes not detect the sevenLinkManagermethods this issue is about: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 theunusedrule quoted in the issue body, reached by a different route.*LinkManageris 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/unreferencedresolves identifiers instead of tracing reachability, and is what reports all seven.Deletions (step 3, and everything else the scan turned up)
net: netlink, ebpf, metrics, config — contains the seven methods from this issuenet: the superseded node-CIDR controllergantrykube,forgeagentmetalmanoperator,orca,playpenhackdev toolingNumbers
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 underapi/, 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.
notice: record golang.org/x/tools as a direct dependency