Add deadcode CI check and remove unreachable functions#4974
Add deadcode CI check and remove unreachable functions#4974simonfaltum wants to merge 13 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.
Restore the test with //deadcode:allow to honor the original author's "park for later" intent. Fix check_deadcode.py to walk backward from the func line (stopping at a blank line) instead of forward from 5 lines above, which was aborting the scan at the blank line above the allow comment. Co-authored-by: Isaac
1 participant
Why
The CLI is not meant as a library and as such 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 walks backward from the reported func line, stopping at a blank line, so the allow comment is found whether it sits immediately above the function or above a doc-comment block.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 40 dead functions across 23 files found in the initial run. Preserved
DisabledTestNoDuplicatedAnnotationswith//deadcode:allowsince theDisabledprefix was a deliberate "park for later" marker from the original author.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.