fix(extract): shadow a single unparenthesised arrow parameter from indirect_call args - #2569
Conversation
…direct_call args tree-sitter gives an arrow with one unparenthesised parameter a `parameter` field (singular) and no `parameters` list node, so `_js_local_bound_names` never saw it: `x => sink(x)` bound nothing, and `x` read as a by-name reference to any same-named callable in the corpus, fabricating an indirect_call edge (INFERRED, 0.8). The parenthesised form was always handled, so `(x) => …` and `x => …` behaved differently. Same singular/plural trap as `catch_clause.parameter`. Strictly subtractive on real code: +0/-88 indirect_call edges over 5,402 files of node_modules, +0/-98 over a 6,000-file mixed corpus.
There was a problem hiding this comment.
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 modifies _js_local_bound_names in the JavaScript extractor to also collect identifiers from an arrow function's singular parameter field, in addition to the existing parameters list field. This targets the case of single unparenthesised arrow parameters (e.g. x => f(x)), which previously had no parameters list node. It adds a new test file with five cases covering single-param, parenthesised, and async arrow forms, plus checks that genuine references outside the arrow and unshadowed references inside the arrow body still produce indirect_call edges. The changed-symbols list also references many other engine functions, but the diff shown only touches this one function and the new test file.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 590 functions depend on the 197 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, 22 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 — 590 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: 531 function(s) in the blast radius were not formally verified this run
Summary
_js_local_bound_namesreads only theparametersfield. tree-sitter gives an arrow withone unparenthesised parameter a
parameterfield (singular) and noparameterslist nodeat all, so that parameter never entered the shadow set:
x => sink(x)bound nothing, andxread as an unresolved by-name reference, resolved against the corpus-wide label index, and
fabricated an
indirect_calledge (INFERRED, 0.8) to an unrelated same-named callable.This is the same singular/plural trap as
catch_clause.parameter(#2517). The parenthesisedform was always handled, which is what makes it easy to miss —
(x) => …andx => …behaveddifferently.
Minified bundles name nearly every private function with a single letter and use this arrow
form heavily, so the two collide constantly.
Reproduction
On
v8@09a34ad(v0.9.37):v8k => sink(k)run -> vendor_min_kasync k => sink(k)(k) => sink(k)(control)Grammar check, so the field names are not taken on trust:
Change
Seven lines in
_js_local_bound_names, next to the existingparametersread:_js_collect_pattern_identsalready handles the pattern forms, andasync x => …is the samenode with the same field, so both are covered by the one read.
Scope note: the parameter is scoped to its arrow, and
_js_local_bound_namesis called perfunction, so this does not widen anything beyond that arrow — a same-named module callable
referenced from a different function still resolves. There is a test for exactly that.
Tests
New
tests/test_indirect_call_arrow_single_param_shadow.py, modelled ontest_indirect_call_nested_closure_shadow.py:test_single_unparenthesised_arrow_param_emits_no_indirect_calltest_parenthesised_arrow_param_still_shadowsparameterspath stays correcttest_async_single_param_arrow_shadowsasync x => …, same singular fieldtest_arrow_param_does_not_shadow_a_genuine_referencetest_genuine_reference_inside_the_arrow_still_emitsAgainst the same test file:
v8@09a34adRegression:
227 passed, 2 skippedacrosstest_indirect_call_*,test_indirect_dispatch*,test_extract,test_cross_language_call_resolution,test_node_id_canonical.Effect on real code
Strictly subtractive — it removes fabricated edges and adds none:
v8node_modules, 5,402 JS/TS filesindirect_callThose are my own measurements on my machine; the per-case tables and the grammar check above
are the parts you can reproduce directly from this description.
Scope
Touches only
_js_local_bound_names, and only by adding a second field read. It does notoverlap #1985 (
for_in_statement, same function but a different branch) or #2517(
catch_clause, inwalk_calls) — though all three are the same underlying class of bug: abinding form that never reaches the shadow set.