Add deadcode CI check and remove unreachable functions#4974
Add deadcode CI check and remove unreachable functions#4974simonfaltum wants to merge 11 commits intomainfrom
Conversation
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Approval status: pending
|
- check_deadcode.py: fail if deadcode itself exits non-zero with no stdout (e.g. package errors), instead of silently reporting success - render.go: update panic message to reference RenderIterator (the previously referenced RenderIteratorWithTemplate was removed) - data_sources.go.tmpl: remove NewDataSources from the codegen template so it won't be reintroduced on next generate Co-authored-by: Isaac
Generated files (bundle/internal/tf/schema/) should be excluded from the deadcode check rather than modifying codegen templates. This avoids circular issues where the check and the generator fight over dead code in generated files. Also adds returncode checking to check_deadcode.py so it fails properly when deadcode itself errors out. Co-authored-by: Isaac
…ion scanning Revert the hand-edit to bundle/internal/tf/schema/data_sources.go since that directory is already excluded from deadcode. The codegen template still emits NewDataSources(), so the removal would be undone on the next regeneration. Fix the inline suppression logic in check_deadcode.py to scan a window of up to 5 lines above the reported function line (stopping at a blank line). This handles the common case where a doc comment sits between the allow comment and the func keyword. Update the panic message in RenderWithTemplate to be more descriptive.
1 participant
Why
The CLI is its own application, not a library. Nobody imports it to use random functions. Any function not reachable from
main()is dead code. The existingunusedlinter skips exported functions by default (it assumes external consumers might use them), leaving a gap.deadcodefromgolang.org/x/toolsdoes whole-program reachability analysis and catches these.Changes
Added
deadcodeas a CI check viamake checks. A Python wrapper script (tools/check_deadcode.py) runsdeadcode -test ./...and supports two suppression mechanisms:libs/gorules/, which contains lint rule definitions loaded by golangci-lint's ruleguard engine, not through Go's call graph).//deadcode:allow <reason>) above a function, matching the//nolint:pattern.The wrapper is needed because raw
deadcodehas no suppression mechanism. It reports every unreachable function with no way to exclude known false positives (code loaded via reflection, plugin systems, or code generators). Without the wrapper, the only options would be to either accept noisy output that developers learn to ignore, or not run the check at all. The wrapper keeps the check strict (zero tolerance, CI fails on any finding) while giving developers two escape hatches for legitimate exceptions.Both mechanisms are documented in the script itself.
Removed 41 dead functions across 24 files found in the initial run.
Test plan
make deadcodepasses clean ("No dead code found.")make checkspasses (includes deadcode)make lintfullpasses (0 issues, no unused imports)go build ./...passesThis pull request was AI-assisted by Isaac.