From 8733c2880f0e338a23633d9a61f695e325f65179 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 8 Sep 2026 10:07:46 -0500 Subject: [PATCH] fix(extraction): index const-bound functions inside a body as symbols (#1669) (#1774) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. Co-authored-by: danusha2345 Co-authored-by: Colby McHenry --- CHANGELOG.md | 1 + __tests__/expo-router.test.ts | 3 +- __tests__/fixtures/kernel-parity/torture.tsx | 10 +++ __tests__/nested-declarator-functions.test.ts | 76 +++++++++++++++++++ __tests__/react-router.test.ts | 28 +++++-- codegraph-kernel/src/tsjs/mod.rs | 28 +++++++ src/extraction/tree-sitter.ts | 34 +++++++++ 7 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 __tests__/nested-declarator-functions.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 82d9cee..2ccebbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -213,6 +213,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer +- **Functions bound with `const` inside another function are symbols now.** `const handleClear = () => {…}` inside a React component — every handler that skips `useCallback` — was invisible to `callers`, `callees` and impact, answering "Symbol not found" exactly the way a function with no callers would. It is indexed like its module-level twin, contained by the enclosing function, with its own calls. Re-index after upgrading. (#1669) - `codegraph callers`, `callees`, and `query` now clearly report when their result limit hides additional matches, including exact totals in callers/callees JSON output; the `codegraph_callers` and `codegraph_callees` MCP answers carry the same "showing N of M" note. (#1639, #1674) - CommonJS controllers written as `exports.getItems = async (req, res) => {…}` or `module.exports.x = function () {…}` are now indexed as exported functions, so `node`, `callers` and impact find every Express handler in that style and the calls inside them belong to the handler instead of the file. Re-index JavaScript projects after upgrading. (#1675) - Python parameters annotated with a quoted forward reference — `def f(o: "Alpha")`, or anything under `from __future__ import annotations` — now resolve the methods called on them, the same as the unquoted annotation. Re-index Python projects after upgrading. (#1684) diff --git a/__tests__/expo-router.test.ts b/__tests__/expo-router.test.ts index f4ecda8..3463e11 100644 --- a/__tests__/expo-router.test.ts +++ b/__tests__/expo-router.test.ts @@ -560,7 +560,8 @@ describe('expo-router: end-to-end', () => { const detail = screens.screens.find((s) => s.path === '/object-detail')!; const tap = screens.links.find((l) => l.from === home.id && l.to === detail.id)!; expect(tap).toBeDefined(); - expect(tap.via.map((v) => v.name)).toEqual(['ItemCard', 'openObjectDetail']); + // `handlePress` is a symbol of its own (#1669), so the tap passes through it. + expect(tap.via.map((v) => v.name)).toEqual(['ItemCard', 'handlePress', 'openObjectDetail']); expect(tap.when).toBe('props.collected'); expect(tap.sites[0]!.href).toBe('/object-detail?detectionItem=${…}'); // Navigation nothing on a screen reaches is an origin, not dropped: the diff --git a/__tests__/fixtures/kernel-parity/torture.tsx b/__tests__/fixtures/kernel-parity/torture.tsx index de27c0a..007792e 100644 --- a/__tests__/fixtures/kernel-parity/torture.tsx +++ b/__tests__/fixtures/kernel-parity/torture.tsx @@ -212,3 +212,13 @@ import('./dynamic-module'); new NS.Widget(makeArg()); new Map(); super_weird?.(); + +// --- const-bound functions inside a body (#1669) ----------------------------- +export function NestedHandlers({ items, onPick }: { items: string[]; onPick: (a: unknown, b: unknown) => void }) { + const handleClear = () => { onPick(null, null); }; + const describe = function (item: string) { return formatLabel(item); }; + let later = (x: string) => parseLabel(x); + const count = items.length; + const [a, b] = [() => 1, () => 2]; + return items.map((i) => ); +} diff --git a/__tests__/nested-declarator-functions.test.ts b/__tests__/nested-declarator-functions.test.ts new file mode 100644 index 0000000..0e04316 --- /dev/null +++ b/__tests__/nested-declarator-functions.test.ts @@ -0,0 +1,76 @@ +/** + * A function bound by a `const` inside another function is a symbol (#1669). + * + * `const handleClear = () => {…}` inside a component is how every React + * handler that skips `useCallback` is written. At module scope the same + * declaration already names a function; inside a body it was skipped, so the + * handler was absent from callers / impact — "Symbol not found", which reads + * exactly like "no callers" — and its calls attributed to the component. + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; + +beforeAll(async () => { + await initGrammars(); + await loadAllGrammars(); +}); + +const refsFrom = (result: ReturnType, id: string) => + result.unresolvedReferences.filter((r) => r.fromNodeId === id).map((r) => r.referenceName); + +describe('declarator-bound functions inside a body', () => { + it('extracts const arrows and function expressions as functions of the enclosing one', () => { + const code = ` +import { formatLabel, parseLabel } from './labels' +export default function Widget({ items, onPick }) { + const handleClear = () => { + onPick(null, null) + } + const describe = function (item) { + return formatLabel(item) + } + let later = (x) => parseLabel(x) + const count = items.length + const [a, b] = [() => 1, () => 2] + return items.map((i) => ) +} +`; + const result = extractFromSource('src/widget.jsx', code); + const fns = result.nodes.filter((n) => n.kind === 'function'); + const names = fns.map((n) => n.name); + expect(names).toEqual(expect.arrayContaining(['Widget', 'handleClear', 'describe', 'later'])); + // A value, a destructuring and an inline arrow stay out. + expect(names).not.toContain('count'); + expect(names).not.toContain('a'); + expect(names.filter((n) => n === '')).toEqual([]); + + const widget = fns.find((n) => n.name === 'Widget')!; + const handleClear = fns.find((n) => n.name === 'handleClear')!; + const describeFn = fns.find((n) => n.name === 'describe')!; + expect(handleClear.qualifiedName).toBe('Widget::handleClear'); + expect(handleClear.startLine).toBe(4); + expect(describeFn.startLine).toBe(7); + + // The handler's calls are its own; the component keeps what it does itself. + expect(refsFrom(result, handleClear.id)).toContain('onPick'); + expect(refsFrom(result, widget.id)).not.toContain('onPick'); + expect(refsFrom(result, describeFn.id)).toContain('formatLabel'); + expect(refsFrom(result, widget.id)).toContain('handleClear'); + + // Containment: the component contains its handlers. + const contains = result.edges.filter((e) => e.kind === 'contains' && e.source === widget.id).map((e) => e.target); + expect(contains).toContain(handleClear.id); + expect(contains).toContain(describeFn.id); + }); + + it('does not apply outside the JS family', () => { + const code = ` +def outer(): + inner = lambda x: x + 1 + return inner(1) +`; + const result = extractFromSource('src/mod.py', code); + expect(result.nodes.filter((n) => n.kind === 'function').map((n) => n.name)).toEqual(['outer']); + }); +}); diff --git a/__tests__/react-router.test.ts b/__tests__/react-router.test.ts index 21d9505..fea8d22 100644 --- a/__tests__/react-router.test.ts +++ b/__tests__/react-router.test.ts @@ -229,6 +229,14 @@ describe('react-router: a routed app end to end', () => { if (!n) throw new Error(`no symbol ${name}`); return n; }; + // A handler written as `const submitHandler = () => {…}` inside a screen is a + // symbol of its own (#1669), so a navigation it makes is ITS edge — the same + // shape a `useCallback` handler has — and the screen reaches it by calling it. + const symIn = (name: string, file: string): Node => { + const n = cg.getNodesByName(name).find((n) => n.kind !== 'route' && n.kind !== 'file' && n.kind !== 'import' && n.filePath.endsWith(file)); + if (!n) throw new Error(`no symbol ${name} in ${file}`); + return n; + }; const navs = (from: Node) => cg.getOutgoingEdges(from.id).filter((e) => e.kind === 'navigates'); const hrefs = (from: Node) => navs(from) @@ -250,17 +258,24 @@ describe('react-router: a routed app end to end', () => { it('the payment screen pushes to both pages it leads to — the bounce out and the one on submit', () => { const payment = sym('PaymentScreen'); - expect(hrefs(payment)).toEqual(['/placeorder', '/shipping']); - const byHref = new Map(navs(payment).map((e) => [(e.metadata as Record).href, e])); + const submit = symIn('submitHandler', 'PaymentScreen.js'); + // The bounce-out is the component's own; the push on submit belongs to its handler. + expect(hrefs(payment)).toEqual(['/shipping']); + expect(hrefs(submit)).toEqual(['/placeorder']); + // `onSubmit={submitHandler}` is the screen's reference to it; the Screens + // walk below rides that hop. + expect(cg.getOutgoingEdges(payment.id).some((e) => e.target === submit.id && e.kind === 'references')).toBe(true); + const byHref = new Map([...navs(payment), ...navs(submit)].map((e) => [(e.metadata as Record).href, e])); expect(byHref.get('/shipping')!.target).toBe(route('/shipping').id); expect(byHref.get('/placeorder')!.target).toBe(route('/placeorder').id); expect(byHref.get('/placeorder')!.metadata).toMatchObject({ navMethod: 'push' }); }); it('history.replace navigates, and v6’s navigate() with a template hole reaches the :id route', () => { - expect(navs(sym('ShippingScreen'))[0]!.target).toBe(route('/payment').id); - expect(navs(sym('ShippingScreen'))[0]!.metadata).toMatchObject({ href: '/payment', navMethod: 'replace' }); - const product = navs(sym('ProductScreen')); + const shippingSubmit = symIn('submitHandler', 'ShippingScreen.js'); + expect(navs(shippingSubmit)[0]!.target).toBe(route('/payment').id); + expect(navs(shippingSubmit)[0]!.metadata).toMatchObject({ href: '/payment', navMethod: 'replace' }); + const product = navs(sym('addToCart')); expect(product).toHaveLength(1); expect(product[0]!.target).toBe(route('/cart/:id?').id); expect(product[0]!.metadata).toMatchObject({ href: '/cart/${…}', navMethod: 'navigate' }); @@ -288,7 +303,8 @@ describe('react-router: a routed app end to end', () => { const link = screens.links.find((l) => l.from === at('/payment').id && l.to === at('/placeorder').id)!; expect(link).toBeDefined(); expect(link.sites[0]).toMatchObject({ href: '/placeorder', method: 'push' }); - expect(link.via).toEqual([]); + // The submit handler is the hop between the screen and the push. + expect(link.via.map((v) => v.name)).toEqual(['submitHandler']); expect(screens.links.find((l) => l.from === at('/shipping').id && l.to === at('/payment').id)).toBeDefined(); expect(screens.links.find((l) => l.from === at('/product/:id').id && l.to === at('/cart/:id?').id)).toBeDefined(); }); diff --git a/codegraph-kernel/src/tsjs/mod.rs b/codegraph-kernel/src/tsjs/mod.rs index 7ce4dcd..736d24e 100644 --- a/codegraph-kernel/src/tsjs/mod.rs +++ b/codegraph-kernel/src/tsjs/mod.rs @@ -718,6 +718,13 @@ impl<'t> Walker<'t> { self.extract_function(node, Some(bound)); return; } + // `const handleClear = () => {…}` inside a body (#1669): named by + // its declarator, like at module scope. Mirrors + // TreeSitterExtractor's declaratorBoundFunction. + if self.declarator_bound_function(node) { + self.extract_function(node, None); + return; + } } if is_class_type(self.variant, kind) { @@ -742,6 +749,27 @@ impl<'t> Walker<'t> { // --- name / signature / modifier helpers ------------------------------------ + /// Whether an anonymous function is the whole value of a + /// `variable_declarator` with a plain identifier name — + /// `const NAME = () => {…}` / `= function () {…}`. + fn declarator_bound_function(&self, node: Node<'t>) -> bool { + if !matches!(node.kind(), "arrow_function" | "function_expression") { + return false; + } + let Some(declarator) = node.parent() else { return false }; + if declarator.kind() != "variable_declarator" { + return false; + } + let Some(value) = declarator.child_by_field_name("value") else { return false }; + if value.start_byte() != node.start_byte() || value.end_byte() != node.end_byte() { + return false; + } + declarator + .child_by_field_name("name") + .map(|n| n.kind() == "identifier") + .unwrap_or(false) + } + /// The declarator name a React handler hook binds an anonymous function /// to — `const NAME = useCallback(, [...])` (also `React.useCallback`, /// `useEffectEvent`, `useEvent`) — or None for any other shape. The node diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index a57ec89..c5cfb92 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -5325,6 +5325,28 @@ export class TreeSitterExtractor { targets.add(target); } + /** + * Whether an anonymous function is the whole value of a `variable_declarator` + * with a plain identifier name — `const NAME = () => {…}` / `= function () {…}`. + * JS-family only. + */ + private declaratorBoundFunction(node: SyntaxNode): boolean { + if ( + this.language !== 'typescript' && + this.language !== 'javascript' && + this.language !== 'tsx' && + this.language !== 'jsx' + ) { + return false; + } + if (node.type !== 'arrow_function' && node.type !== 'function_expression') return false; + const declarator = node.parent; + if (!declarator || declarator.type !== 'variable_declarator') return false; + const value = getChildByField(declarator, 'value'); + if (!value || value.startIndex !== node.startIndex || value.endIndex !== node.endIndex) return false; + return getChildByField(declarator, 'name')?.type === 'identifier'; + } + /** * The property a CommonJS export assignment binds a function to — * `exports.NAME = ` or `module.exports.NAME = ` — or null for @@ -5519,6 +5541,18 @@ export class TreeSitterExtractor { this.extractFunction(node, hookBound); return; } + // `const handleClear = () => {…}` inside a body (#1669) — the same + // binding that names a function at module scope names one here, and in + // a React component it is how every handler that skips `useCallback` + // is written. Without a node the handler is absent from callers / + // impact ("Symbol not found" reads like "no callers") and its calls + // attribute to the component. extractFunction resolves the name from + // the declarator; a destructuring or otherwise unnamed binding stays + // anonymous and falls through. + if (this.declaratorBoundFunction(node)) { + this.extractFunction(node); + return; + } } // Extract structural nodes found inside function bodies.