From f7d15c654f53e5e4df1ee589c12f5a0a1580fa7f Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Thu, 3 Sep 2026 15:30:47 +0300 Subject: [PATCH] fix(extraction): index const-bound functions inside a body as symbols (#1669) 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. --- CHANGELOG.md | 2 + __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, 174 insertions(+), 7 deletions(-) create mode 100644 __tests__/nested-declarator-functions.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1815c4150..a78c7ca3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -201,6 +201,8 @@ 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) + - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges. - **Production code under a `samples` or `examples` package path is no longer treated as test code.** A Kotlin or Java project whose package path runs through `com/google/samples/…` (Now in Android, for one) had nearly every file counted as a fixture, so the Map opened on `build-logic`, the entry points hid the app, and dead-code and test badges were wrong. Only the project layout above a `src/` folder decides now; the package path below it never does. diff --git a/__tests__/expo-router.test.ts b/__tests__/expo-router.test.ts index f4ecda82b..3463e111d 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 de27c0a8f..007792e96 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 000000000..0e0431637 --- /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 21d9505ae..fea8d2274 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 afe6361d5..d9f157e95 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 7ef90c273..1140c4b14 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -5222,6 +5222,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 declarator name a React handler hook binds an anonymous function to — * `const NAME = useCallback(, [...])` — or null for any other shape. @@ -5389,6 +5411,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.