fix(extraction): index const-bound functions inside a body as symbols - #1679
fix(extraction): index const-bound functions inside a body as symbols#1679danusha2345 wants to merge 1 commit into
Conversation
…colbymchenry#1669) `const handleClear = () => {…}` inside a component — every React handler that skips useCallback — was never a symbol: the body walker only named nested function declarations and hook-bound arrows, so the handler was absent from callers/impact ("Symbol not found", indistinguishable from "no callers") and its calls attributed to the component. Bind the arrow or function expression to its declarator the way module scope already does, in both the wasm walker and the kernel. A navigation such a handler makes is now the handler's own edge and a hop in the Screens `via` chain — the shape a useCallback handler already has — so the react-router and expo-router expectations follow that convention.
|
Verified on a real TypeScript repo (Chrome MV3 extension, 582 files, TS/JS/Vue/markdown, Windows 11, tree-sitter wasm walker, kernel off). Branch: this PR merged onto current Merges clean. The six closures we had listed as missing ( Two things the new nodes attract that were not there before:
|
|
Thanks — the fuzzy finding is real and #1709 is the right fix for it, so I took #1709 into my integration build ahead of this one as you suggested. Measured on two real repos with both PRs (and #1511) in, kernel on, same trees re-indexed before/after #1709: |
|
@danusha2345 before you sequence this behind #1709 — I ran your metric on a third repo and it moves the other way there. Details and full tables on #1709: #1709 (comment) Short version. Same vite checkout indexed at #1709's merge-base and at its tip, counting every edge by
Cross-tabbed against the edge-set diff: 12 edges lost (all fuzzy, all correct removals of unreachable nested functions) and 59 gained (all fuzzy — 52 calls, 7 imports), nearly all pointing I had assumed your metric simply couldn't see this failure mode. It can — that's why I measured before replying. On your two trees the filter only removes bad guesses (337 → 2, 142 → 0); on vite it leaves exactly one surviving candidate and the matcher commits to it. vite has ~12 symbols named So your measurement stands for your trees and #1709's removal half is doing real work. My concern is only with landing it first: the underlying bug is one layer up — when filtering leaves a single survivor, we commit without checking the call site's binding is internal — and until that's handled, #1709 is not a strict improvement on every tree. Nothing here affects this PR's own change. I'd just rather the ordering decision be made with the vite number in view than have it re-raised as a blocker on you later. |
|
Closing the loop I opened here. I raised the vite counter-example as a reason not to sequence this behind #1709; the underlying bug is now fixed in #1713, and I no longer think the sequencing concern should hold anything up. #1713 is standalone — one commit off So the objection I raised against ordering is spent. Nothing here needs to wait on me. Full tables and the two failure modes that remain: #1709 and #1713. |
Fixes #1669.
Problem
const handleClear = () => {…}inside a component — every React handler written withoutuseCallback— was never a symbol. The body walker names nested function declarations and hook-bound arrows (const x = useCallback(…)), but a plain declarator-bound arrow or function expression fell through, so the handler was absent fromcallers/callees/ impact ("Symbol not found", which reads exactly like "no callers") and its calls attributed to the component. The same declaration at module scope already names a function.Change
arrow_function/function_expressionthat is the wholevalueof avariable_declaratorwith a plain identifier name is extracted as a function, contained by the enclosing one, with its own calls.extractFunctionalready resolves the name from the declarator. A destructuring binding, an inline JSX arrow and a non-function value stay as they were. JS family only.torture.tsxgains the shape sokernel-tsjs-paritypins both arms.Graph shape
A navigation such a handler makes is now the handler's own
navigatesedge, and the handler is a hop in the Screensviachain — the shape auseCallbackhandler already has (see the Next.js test'snavs(sym('handleSubmit'))andpush.via). The react-router and expo-router expectations are updated to that convention; the screen still reaches the handler through itsonSubmit={…}reference and the links resolve as before, with the trigger now visible in Steps.Verification
__tests__/nested-declarator-functions.test.ts: arrow, function expression andlet-bound arrow becomeWidget::namefunctions with their own calls and containment; a value, a destructuring and an inline arrow do not; Python is untouched.npm run build:kernel && npm run build, full suite withCODEGRAPH_KERNEL_EXPECT=1: 237 files, 4233 passed, 9 skipped.Re-index after upgrading to pick up the new symbols.
🤖 Generated with Claude Code