fix(health): stop flagging reachable code as unreachable in the dataflow CFG#760
Merged
Conversation
…low CFG Two defects made CFG.reachable_ids() flag plainly reachable lines, tripping a tree-wide sweep on 106 functions with zero real positives. 1. Comment nodes entered the CFG as statements. Tree-sitter emits a trailing comment as a named sibling after its statement, so a return followed by a same-line comment put a comment node after the terminator and _process_seq spawned a bogus unreachable block at the return line. _body_stmts now drops comment nodes, the same idiom nloc counting uses. 2. A finally body was only reachable through the normal join, so a try whose body and handlers all terminate (return / raise) had its finally flagged unreachable even though it runs on every path. The builder now adds an edge from the protected-region entry to the finally entry, mirroring the existing handler approximation. Also renames the misleading local in derive_facts that held the reachable set under the name unreachable. No shipped consumer reads reachable_ids() yet; statement-level dead code will build on it, so it gets fixture coverage for both shapes first.
|
✅ Health: 7.7 (unchanged) 📋 At a glance Files & modules (2)
🩹 Review priority (files here with the most recent bug-fix history — defects cluster, so review these first)
🔎 More signals (2)🔥 Hotspots touched (3)
🔗 Hidden coupling (1 file)
📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-10 15:29 UTC |
swati510
approved these changes
Jul 10, 2026
swati510
approved these changes
Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CFG.reachable_ids()flagged plainly reachable lines as unreachable. A tree-wide sweep overpackages/reported 106 functions with unreachable lines; a hand-checked stratified sample was 0 for 5 real. Repro examples: the trailingreturn Falseintool_search_symbols.py::_symbol_kind_for_request_kind, the guarded barereturninwebhooks.py::_verify_gitlab_token, and the mid-chain return ingraph-toolbar.tsx::scopeOverlaysToViewMode(the defect is language-independent).No shipped consumer reads this method yet, but statement-level dead code is planned to build directly on it, so it needs to be right first.
Root causes (two)
Comment nodes entered the CFG as statements. Tree-sitter emits a trailing comment as a named sibling after its statement, so
return x # noteputs a comment node after the terminator._process_seqtreated it as a statement and spawned anunreachableblock whose start line is the return line itself. Every one of the five sampled false positives was this shape. Fixed by dropping comment nodes in_body_stmts, the single choke point that produces statement sequences (same substring idiom nloc counting uses; coverscomment/line_comment/block_comment/doc_commentacross grammars).finallybodies were unreachable when the try body and all handlers terminate. The finally entry was only wired from the normal fall-through paths, so atry: ... return/except: return/finally: cleanup()had its cleanup flagged, even though finally runs on every path. Hand-checking the 10 functions that survived fix 1 surfaced this: all 10 were finally bodies. Fixed with an edge from the protected-region entry to the finally entry, mirroring the existing body-to-handler approximation.Also renames the misleading local in
derive_factsthat held the reachable set under the nameunreachable.Tests
New fixtures in
tests/unit/health/test_dataflow_cfg.py: the five reported shapes (guarded bare return, trailing return after an if-chain, post-guard body, if-guarded return mid-function, TS mid-return chain, each with trailing comments), a standalone comment after a return, the two finally shapes (with and without handlers), and a guard that genuinely dead code after a comment still gets flagged.Health suite: 927 passed. Full unit suite: 6806 passed, 1 skipped (the pre-existing
test_bundled_changelog_in_syncfailure on main was deselected; it is unrelated changelog drift).Rerunning the tree-wide sweep: unreachable false positives drop from 106 functions to zero, and the true-unreachable fixtures stay green.
Perf-neutral: a list-comprehension filter where a list copy already happened, plus one extra edge per try-with-finally.