Merge pull request #295 from Wolfvin/fix/issue-294-jsx-handler-references

fix(dead-code): count JSX prop function references as usage (closes #294)

Author: Wolfvin

scripts/parsers/tsx_parser.py

@@ -66,6 +66,8 @@ def visit(node: Node, _, depth):
                 self._process_jsx_attribute(node, source, file_path, classes, ids)
             elif node.type in ('jsx_opening_element', 'jsx_self_closing_element'):
                 self._process_jsx_component(node, source, file_path, fn_declarations, edges)
+            elif node.type == 'jsx_expression':
+                self._process_jsx_expression(node, source, file_path, fn_declarations, edges)
             elif node.type == 'call_expression':
                 call_info = self._parse_call(node, source, fn_declarations)
                 if call_info:
@@ -442,6 +444,88 @@ def _process_jsx_component(self, node: Node, source: bytes,
             "via_jsx": True
         })
 
+    def _process_jsx_expression(self, node: Node, source: bytes,
+                                 file_path: str, fn_declarations: List[Dict],
+                                 edges: List):
+        """Emit usage edges for functions referenced inside a JSX expression
+        container (issue #294).
+
+        In React, an event handler is passed by *reference* as a prop value —
+        ``onClick={handleClick}`` — not by call. The per-call passes only see
+        ``call_expression`` nodes, so a bare identifier reference produces zero
+        edges, leaving the handler with ``ref_count=0`` and false-flagged dead.
+
+        This handler counts two reference shapes as usage, but ONLY when the
+        identifier resolves to a function declared in this file (guarded by
+        ``declared``) — arbitrary identifiers, DOM props, and non-function names
+        are never counted:
+
+        1. Attribute value / child reference: ``onClick={handleClick}`` — the
+           expression's direct child is the identifier.
+        2. Callback argument: ``{items.map(renderItem)}`` — the identifier is
+           passed as an argument to a call.
+
+        Double-counting is avoided by:
+          - skipping the ``function`` position of a ``call_expression`` (those
+            are already emitted by :meth:`_parse_call`), walking only its
+            ``arguments``;
+          - not descending into nested ``jsx_expression`` nodes (the outer
+            tree walk visits each one on its own);
+          - skipping ``member_expression`` (e.g. ``items.map`` / ``this.x``).
+        """
+        declared = {d["node"]["fn"] for d in fn_declarations}
+        if not declared:
+            return
+
+        # Resolve the enclosing function (innermost scope) as the edge source.
+        expr_line = self.get_line(node)
+        caller_id = None
+        best_scope_size = float('inf')
+        for decl in fn_declarations:
+            if decl["scope_start"] <= expr_line - 1 <= decl["scope_end"]:
+                scope_size = decl["scope_end"] - decl["scope_start"]
+                if scope_size < best_scope_size:
+                    best_scope_size = scope_size
+                    caller_id = decl["node"]["id"]
+        if not caller_id:
+            return
+
+        seen = set()
+
+        def emit(name: str):
+            if name in seen:
+                return
+            if name in self.SKIP_NAMES or name not in declared:
+                return
+            seen.add(name)
+            edges.append({
+                "from": caller_id,
+                "to_fn": name,
+                "via_jsx_ref": True,
+            })
+
+        def walk(n: Node):
+            for child in n.children:
+                t = child.type
+                if t == 'jsx_expression':
+                    # Handled by the outer tree walk — avoid re-processing.
+                    continue
+                if t == 'identifier':
+                    emit(self.get_text(child, source))
+                elif t == 'call_expression':
+                    # Function position is already counted by _parse_call;
+                    # only inspect arguments for callback references.
+                    args = child.child_by_field_name('arguments')
+                    if args:
+                        walk(args)
+                elif t == 'member_expression':
+                    # e.g. items.map, this.handler — not a bare function ref.
+                    continue
+                else:
+                    walk(child)
+
+        walk(node)
+
     def _parse_call(self, node: Node, source: bytes,
                      fn_declarations: List[Dict]) -> Optional[Dict]:
         """Parse a call expression and return an edge if it's within a known function.

tests/test_graph_accuracy_golden.py

@@ -153,6 +153,29 @@ def _callers_of(backend: dict, fn: str) -> set:
 """
 
 
+_JSX_HANDLER_TSX = """\
+// Guards #294: a function passed as a JSX prop value (onClick={handleClick}) is
+// a REFERENCE, not a call — before #294 it produced zero edges, so the handler
+// got rc=0 / status=dead (false positive on the primary React use case). A
+// function passed as a callback argument ({items.map(renderItem)}) must also
+// count. Genuinely-unused handlers must STILL be dead (control).
+function handleClick() { return 1; }
+function handleSubmit() { return 2; }
+function renderItem(x: number) { return x; }
+function directlyCalled() { return 3; }
+function unusedHandler() { return 4; }
+export function App({ items }: { items: number[] }) {
+  directlyCalled();
+  return (
+    <div onClick={handleClick}>
+      <form onSubmit={handleSubmit} />
+      <ul>{items.map(renderItem)}</ul>
+    </div>
+  );
+}
+"""
+
+
 _MODULE_LEVEL_CALL_PY = """\
 # Guards #291: Python calls at module top level (not inside any def/class) must
 # count toward the callee's rc — analogous to #219 for TS/JS. Before #291 the
@@ -191,6 +214,7 @@ def _build_workspace(tmp_path) -> str:
     (ws / "src" / "callback.ts").write_text(_INLINE_CALLBACK_TS)
     (ws / "src" / "same_file.rs").write_text(_SAME_FILE_USAGE_RS)
     (ws / "src" / "dead.ts").write_text(_GENUINELY_DEAD_TS)
+    (ws / "src" / "App.tsx").write_text(_JSX_HANDLER_TSX)
     return str(ws)
 
 
@@ -372,3 +396,32 @@ def test_genuinely_dead_still_detected(self, backend):
             f"control: trulyUnusedInternal should have rc 0, got {nodes[0].get('ref_count')} — "
             "a fix that inflates rc to hide false-positives would break real dead-code detection"
         )
+
+    def test_jsx_prop_handler_reference_counts_toward_rc(self, backend):
+        """#294: handlers passed as JSX prop values are USED, not dead."""
+        for fn in ("handleClick", "handleSubmit"):
+            rc = _rc(backend, fn)
+            assert rc >= 1, f"#294 regression: {fn} rc={rc}, expected >=1 (referenced via JSX prop)"
+            callers = _callers_of(backend, fn)
+            assert any("App.tsx" in c for c in callers), (
+                f"#294: expected a caller from App.tsx for {fn}, got {callers}"
+            )
+
+    def test_jsx_callback_argument_reference_counts_toward_rc(self, backend):
+        """#294: `{items.map(renderItem)}` — callback arg reference is usage."""
+        rc = _rc(backend, "renderItem")
+        assert rc >= 1, f"#294 regression: renderItem rc={rc}, expected >=1 (JSX callback arg)"
+
+    def test_jsx_direct_call_not_regressed(self, backend):
+        """#294 control: a directly-called function in the TSX file keeps rc>=1."""
+        rc = _rc(backend, "directlyCalled")
+        assert rc >= 1, f"#294: directlyCalled rc={rc}, expected >=1 (direct call must not regress)"
+
+    def test_jsx_genuinely_unused_handler_still_dead(self, backend):
+        """#294 control: a TSX function referenced nowhere IS still dead (rc 0)."""
+        nodes = _nodes_named(backend, "unusedHandler")
+        assert nodes, "control node unusedHandler missing"
+        assert nodes[0].get("ref_count", 0) == 0, (
+            f"#294 control: unusedHandler should have rc 0, got {nodes[0].get('ref_count')} — "
+            "the JSX-reference fix must not inflate rc for genuinely-unused handlers"
+        )