fix(extract): shadow JS/TS catch bindings from indirect_call args

[imagineers-tyler]

Summary

A catch (e) clause binds its name through the clause's own parameter field and is never wrapped in a variable_declarator, so _js_local_bound_names never sees it. The binding is therefore absent from the shadow set guarding indirect_call argument resolution: passing it on as a plain call argument (handlers.get(e)) reads as an unresolved by-name reference, falls through to the corpus-wide label index, and fabricates an indirect_call edge (INFERRED, 0.8) to an unrelated same-named callable.

One-letter catch bindings make this land constantly against minified bundles, which define a private function for nearly every letter.

Reproduction

// vendor.min.js
var Lib=function(){function k(a){return a}return{k:k}}();

// a.js
export function run(handlers) {
  try { boom(); } catch (k) { handlers.get(k); }
}

from graphify.extract import extract
from pathlib import Path
r = extract([Path('a.js'), Path('vendor.min.js')], cache_root=Path('.'))
print([e for e in r['edges'] if e['relation'] == 'indirect_call'])

On v8 @ 9f25a3a:

indirect_call | a_run -> vendor_min_k | INFERRED 0.8 | source_location L2

Change — scoped to the clause, not the function

A catch binding is scoped to its own block. Folding it into the function-wide _js_local_bound_names set would fix the fabricated edge but suppress a legitimate by-name reference to a same-named callable elsewhere in the same function. So the fix uses the per-subtree extra_locals channel in walk_calls instead — the same shape as the untracked-closure fold added in #2241:

if (
    config.ts_module in ("tree_sitter_javascript", "tree_sitter_typescript")
    and node.type == "catch_clause"
):
    param = node.child_by_field_name("parameter")  # absent for ES2019 `catch {}`
    if param is not None:
        caught: set[str] = set()
        _js_collect_pattern_idents(param, source, caught)
        extra_locals = extra_locals | frozenset(caught)

_js_local_bound_names is untouched, so the variable_declarator path keeps its existing behaviour. All six shadow-set consumers already read local_bound_names.get(caller_nid, frozenset()) | extra_locals, so nothing else needed changing.

Notes: _js_local_bound_names cannot be reused on a catch node — it reads child_by_field_name("parameters") (plural), while catch_clause exposes parameter (singular). _js_collect_pattern_idents covers the pattern forms, so catch ({ cause }) works. ES2019 catch { } has no parameter field and is guarded.

Tests

New tests/test_indirect_call_catch_binding_shadow.py, modelled on test_indirect_call_nested_closure_shadow.py:

test	pins
test_catch_binding_emits_no_indirect_call	the reported shape produces no edge
test_catch_binding_destructured_emits_no_indirect_call	catch ({ cause }) shadows too
test_catch_binding_does_not_shadow_outside_its_block	a same-named module callable referenced after the try/catch still resolves
test_catch_genuine_reference_still_emits_indirect_call	a real by-name reference inside a catch block still emits
test_optional_catch_binding_unaffected	ES2019 catch { } — the param is not None guard

Measured on this test file, swapping only graphify/extractors/engine.py:

engine.py	result
v8 @ 9f25a3a	2 failed, 3 passed — the two suppression tests, i.e. the bug
function-wide variant (an earlier draft of this PR)	1 failed, 4 passed — does_not_shadow_outside_its_block
this change	5 passed

Regression run on this branch: test_indirect_call_catch_binding_shadow.py + test_indirect_call_nested_closure_shadow.py + test_indirect_dispatch*.py + test_extract.py + test_cross_language_call_resolution.py + test_node_id_canonical.py → 227 passed, 1 skipped.

Scope

Does not touch _js_local_bound_names, so it does not overlap #1985 (for_in_statement), which edits that function. The two changes are in different functions and do not conflict.

[graphify-labs]

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

---

Graphify review — findings

This PR extends _js_local_bound_names in the JS/TS extractor to also collect names bound by catch clauses (via the clause's parameter field), adding them to the set of locally-bound names used when deciding whether a call argument should produce an indirect_call edge. The stated intent is to prevent catch bindings (including destructured ones) from being treated as by-name references to same-named callables elsewhere in the corpus. It also adds a new test file covering catch bindings, destructured catch bindings, genuine references inside catch blocks, and catchless try/finally cases.

Worth a look

• Catch parameter suppresses by-name references outside its block — graphify/extractors/engine.py:1176 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 580 functions depend on the 187 functions this change touches.

Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):

• _extract_generic() — 18 callers, 20 callees (high)
• extract_xaml() — 19 callers, 17 callees (high)
• extract_objc() — 27 callers, 8 callees (high)
• extract_julia() — 16 callers, 7 callees (high)
• extract_vue() — 10 callers, 6 callees (high)
• walk() — 1 callers, 52 callees (high)
• extract_groovy() — 14 callers, 3 callees (high)
• extract_astro() — 6 callers, 5 callees (medium)
• …and 2 more

Verification — 580 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

• verification_scope: 521 function(s) in the blast radius were not formally verified this run

· 1 grounded finding(s) anchored inline below.

[graphify-labs]

Catch parameter suppresses by-name references outside its block — agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Graphify suggests a fix:

Suggested change

	elif c.type == "catch_clause":
	# `catch (e)` binds through the clause's `parameter` field, never a
	# variable_declarator, so the branch above misses it. A one-letter
	# binding then read as a call argument in the handler body was taken
	# for a by-name reference to a same-named callable elsewhere in the
	# corpus — minified bundles supply one for nearly every letter.
	param = c.child_by_field_name("parameter")
	if param is not None:
	_js_collect_pattern_idents(param, source, bound)
	walk(c)
	# NOTE: `catch (e)` is deliberately NOT handled here. A catch parameter
	# is scoped to its own catch block, so adding it to this function-wide
	# shadow set would suppress legitimate by-name references to a same-named
	# callable elsewhere in the function. The catch body is still walked below
	# so its var/let/const declarators are collected.
	walk(c)

[imagineers-tyler]

The finding is correct, and it is now fixed rather than argued with. Pushed b441b77.

Reproduced

The earlier draft folded the catch parameter into _js_local_bound_names, which is a flat function-wide set. That did suppress a legitimate by-name reference elsewhere in the same function:

function k(x){ return x; }
export function run(pool) {
  try { boom(); } catch (k) { log(k); }
  pool.submit(k);            // ← should resolve to the module-level k
}

Fixed by moving to the per-subtree channel

walk_calls already carries extra_locals (engine.py:4229), added in #2241 to fold an untracked closure's own bindings in "for its subtree only". A catch binding has exactly that shape, so it belongs there rather than in the function-wide set:

if (
    config.ts_module in ("tree_sitter_javascript", "tree_sitter_typescript")
    and node.type == "catch_clause"
):
    param = node.child_by_field_name("parameter")  # absent for ES2019 `catch {}`
    if param is not None:
        caught: set[str] = set()
        _js_collect_pattern_idents(param, source, caught)
        extra_locals = extra_locals | frozenset(caught)

_js_local_bound_names is now untouched by this PR, so the variable_declarator path is unchanged. All six shadow-set consumers already read local_bound_names.get(caller_nid, frozenset()) | extra_locals, so nothing else needed changing.

A new test pins the exact case the review flagged, test_catch_binding_does_not_shadow_outside_its_block. Swapping only graphify/extractors/engine.py against the same test file:

engine.py	result
v8 @ 9f25a3a	2 failed, 3 passed — the two suppression tests, i.e. the original bug
function-wide variant (the reviewed commit)	1 failed, 4 passed — precisely this finding
b441b77	5 passed

Regression run: 227 passed, 1 skipped across test_indirect_call_*, test_indirect_dispatch*, test_extract, test_cross_language_call_resolution, test_node_id_canonical.

On the inline suggestion

Not taking it, for the record: as written the suggestion block spans the whole added hunk, so applying it removes the fix entirely and leaves a comment asserting the omission was deliberate. Its text also states that a catch parameter is block-scoped "unlike" what the function collects — but _js_local_bound_names has no block scoping at all, so a block-scoped const over-suppresses identically today:

function k(x){ return x; }
export function run(pool) {
  if (1) { const k = 1; void k; }
  pool.submit(k);            // suppressed on v8 today, no change from this PR
}

That is a real pre-existing issue and the same extra_locals channel could address it, but it changes behaviour for every let/const in the codebase (one 52-file Node project showed ~900 affected suppressions versus 1 for catch), so it is not a drive-by — happy to open it separately if you want it.

[graphify-labs]

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

---

Graphify review — findings

This PR addresses a case where a JS/TS catch clause binding could produce a fabricated indirect_call edge, since catch bindings are declared through the clause's own parameter field rather than a variable_declarator and thus weren't captured by the existing local-name collection. In engine.py, it adds a branch in the call-walking logic that, for JS/TS catch_clause nodes, collects the caught pattern's identifiers and folds them into the per-subtree extra_locals shadow set. It also adds a new test file covering simple and destructured catch bindings, scoping behavior outside the block, genuine references, and the ES2019 optional (parameter-less) catch form, and adds a changelog entry.

No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 762 functions depend on the 369 functions this change touches.

Health — this change adds coupling hotspots:

• worse: walk_calls() — 1 callers, 14 callees

Verification — 762 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

• verification_scope: 703 function(s) in the blast radius were not formally verified this run

· 1 more finding(s) on lines outside this diff (see the check run).