fix(health): stop flagging reachable code as unreachable in the dataflow CFG (#760)

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.

Author: RaghavChamadiya

packages/core/src/repowise/core/analysis/health/dataflow/cfg.py

@@ -254,12 +254,20 @@ def _block_of(self, node: Node | None) -> Node | None:
     def _body_stmts(self, block: Node | None) -> list[Node]:
         """The statement sequence inside a container, unwrapping a single nested
         statement-container (Go wraps a ``block``'s statements in a
-        ``statement_list``; Python/TS expose them directly)."""
+        ``statement_list``; Python/TS expose them directly).
+
+        Comment nodes are dropped: tree-sitter emits them as named siblings in
+        the statement sequence (a trailing ``return x  # note`` puts the comment
+        *after* the return), and a comment threaded through ``_process_seq``
+        after a terminator would spawn a bogus ``unreachable`` block at the
+        terminator's own line. The substring match covers every grammar's
+        comment kinds (``comment`` / ``line_comment`` / ``block_comment`` /
+        ``doc_comment``), the same idiom nloc counting uses."""
         if block is None:
             return []
-        kids = list(block.named_children)
+        kids = [c for c in block.named_children if "comment" not in c.type]
         while len(kids) == 1 and kids[0].type in self.lmap.block_kinds:
-            kids = list(kids[0].named_children)
+            kids = [c for c in kids[0].named_children if "comment" not in c.type]
         return kids
 
     # -- the recursive walk ---------------------------------------------------
@@ -444,6 +452,13 @@ def _build_try(self, try_node: Node, cur: BasicBlock) -> BasicBlock:
         # try body (the protected region)
         body_entry = self._new()
         self._edge(cur, body_entry)
+        # The finally body runs on every path out of the protected region,
+        # including abrupt exits (a body / handler ``return`` or ``raise``)
+        # that never reach the normal join. Approximate with an edge from the
+        # region entry, mirroring the handler edges below; without it a
+        # ``finally`` after an always-returning body is flagged unreachable.
+        if finally_clause is not None:
+            self._edge(body_entry, normal_join)
         body_out = self._process_seq(
             self._body_stmts(try_node.child_by_field_name("body")), body_entry
         )

packages/core/src/repowise/core/analysis/health/dataflow/facts.py

@@ -124,13 +124,13 @@ def derive_facts(analysis: FunctionAnalysis) -> DataflowFacts:
     exit_defs = reaching.in_sets.get(cfg.exit_id, frozenset())
     flows_out = tuple(sorted({def_use.definitions[i].var for i in exit_defs if i >= n_param_seeds}))
 
-    unreachable = cfg.reachable_ids()
+    reachable = cfg.reachable_ids()
     unreachable_lines = tuple(
         sorted(
             {
                 stmt.start_line
                 for block in cfg.blocks
-                if block.id not in unreachable
+                if block.id not in reachable
                 for stmt in block.statements
             }
         )

tests/unit/health/test_dataflow_cfg.py

@@ -297,6 +297,149 @@ def f():
     assert unreachable[0].id not in cfg.reachable_ids()
 
 
+# -- comments are not statements --------------------------------------------------
+# Tree-sitter emits a trailing comment as a named sibling *after* its statement,
+# so a comment following a terminator used to spawn a bogus ``unreachable`` block
+# at the terminator's own line. These mirror the shapes found in the wild.
+
+
+def _assert_all_reachable(cfg) -> None:
+    reachable = cfg.reachable_ids()
+    assert [b.id for b in cfg.blocks if b.id not in reachable] == []
+    assert _by_kind(cfg, "unreachable") == []
+
+
+def test_guarded_bare_return_with_trailing_comment():
+    cfg = _cfg(
+        """
+        def f(token):
+            if not token:
+                return  # no token configured, skip verification
+            check(token)
+        """
+    )
+    _assert_all_reachable(cfg)
+
+
+def test_trailing_return_after_if_chain_with_comment():
+    cfg = _cfg(
+        """
+        def f(kind, is_test):
+            if not kind:
+                return True
+            if kind == "test":
+                return is_test
+            return False  # config / doc: symbols do not qualify
+        """
+    )
+    _assert_all_reachable(cfg)
+
+
+def test_post_guard_body_stays_reachable():
+    cfg = _cfg(
+        """
+        def f(ids, titles):
+            if not ids:
+                return None  # no siblings to compare
+            shared = titles & ids
+            return len(shared) / len(ids)
+        """
+    )
+    _assert_all_reachable(cfg)
+
+
+def test_if_guarded_return_with_comment_mid_function():
+    cfg = _cfg(
+        """
+        def f(include, targets):
+            if not targets:
+                return None
+            if include and "source" in include:
+                return None  # source mode provides its own truncation info
+            return build(targets)
+        """
+    )
+    _assert_all_reachable(cfg)
+
+
+def test_standalone_comment_after_return_is_not_a_statement():
+    cfg = _cfg(
+        """
+        def f(x):
+            if x:
+                return 1
+                # explains the early exit
+            return 2
+        """
+    )
+    _assert_all_reachable(cfg)
+
+
+def test_ts_mid_return_chain_with_trailing_comment():
+    src = textwrap.dedent(
+        """
+        function f(scope, dead, hot) {
+            if (dead && hot) return "unified";
+            if (dead) return "dead";
+            if (hot) return "hotfiles";
+            return scope; // "architecture" | "full"
+        }
+        """
+    )
+    result = build_cfgs_for_file("m.ts", "typescript", src.encode(), flagged_only=False)
+    if result.stats.functions_seen == 0:
+        pytest.skip("tree-sitter language pack missing for typescript")
+    _assert_all_reachable(result.functions[0].cfg)
+
+
+def test_finally_reachable_when_body_and_handlers_return():
+    # The finally body runs on every path out of the protected region, even
+    # when the body and all handlers terminate; it must never be flagged.
+    cfg = _cfg(
+        """
+        def f(store):
+            try:
+                write(store)
+                return True
+            except Exception:
+                return False
+            finally:
+                store.close()
+        """
+    )
+    _assert_all_reachable(cfg)
+
+
+def test_finally_reachable_after_always_returning_try_without_except():
+    cfg = _cfg(
+        """
+        def f(store):
+            try:
+                return read(store)
+            finally:
+                store.close()
+        """
+    )
+    _assert_all_reachable(cfg)
+
+
+def test_true_unreachable_after_comment_still_flagged():
+    # A comment between the terminator and real dead code must not hide the
+    # dead code: the statement itself stays flagged.
+    cfg = _cfg(
+        """
+        def f():
+            return 1
+            # a note about the exit
+            dead = 2
+        """
+    )
+    unreachable = _by_kind(cfg, "unreachable")
+    assert len(unreachable) == 1
+    assert [s.kind for s in unreachable[0].statements] == ["expression_statement"]
+    assert unreachable[0].statements[0].start_line == 5
+
+
 # -- determinism ----------------------------------------------------------------