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.