diff --git a/__tests__/curried-wrapper-handlers.test.ts b/__tests__/curried-wrapper-handlers.test.ts new file mode 100644 index 000000000..0ccefd483 --- /dev/null +++ b/__tests__/curried-wrapper-handlers.test.ts @@ -0,0 +1,151 @@ +/** + * A function bound through a curried wrapper is named by its declarator (#1747). + * + * `react_hook_bound_name` already names an anonymous function after the + * `variable_declarator` that binds it, but only when the callee is one of three + * React hooks. The same shape with any other wrapper produced no function node + * at all — and the loss is not only a missing node: the body's calls attribute + * to the enclosing container instead, so a file or an outer function picks up an + * outgoing edge that belongs to the function, and the callee's caller list names + * the wrong thing. + * + * The bound used here is that the callee is itself a call — a factory that + * returns the wrapper. That admits `Effect.fn("x")(fn)`, `connect(m)(fn)` and a + * project's own `wrap("n")(fn)`, and leaves the single-call forms whose argument + * is a computation (`useMemo`, `arr.map`) anonymous exactly as before. + */ +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); + +const CODE = ` +declare const Effect: { fn: (n: string) => (b: unknown) => unknown }; +declare function connect(m: unknown): (c: unknown) => unknown; +declare function wrap(n: string): (c: unknown) => unknown; +declare function useMemo(f: () => T, d: unknown[]): T; + +function helper() { return 1; } + +const viaEffectGen = Effect.fn("Session.run")(function* () { return helper(); }); +const viaEffectArrow = Effect.fn("Session.go")(() => { return helper(); }); +const viaConnect = connect({})(function () { return helper(); }); +const viaWrapArrow = wrap("n")(() => { return helper(); }); + +const total = useMemo(() => 1 + 1, []); +const mapped = [1, 2].map(() => helper()); +`; + +describe('curried wrapper handlers', () => { + it('names the wrapped function after its declarator, for every callee shape', () => { + const result = extractFromSource('src/b.ts', CODE); + const names = result.nodes.filter((n) => n.kind === 'function').map((n) => n.name); + + // A generator is the common form in the ecosystem this comes from, and it + // is the shape the report opens with — `function*` is not accepted by + // reactHookBoundName, so broadening the callee test alone would not fix it. + expect(names).toContain('viaEffectGen'); + expect(names).toContain('viaEffectArrow'); + expect(names).toContain('viaConnect'); + expect(names).toContain('viaWrapArrow'); + }); + + it('leaves single-call arguments anonymous, so the graph does not gain nodes for computations', () => { + const result = extractFromSource('src/b.ts', CODE); + const names = result.nodes.filter((n) => n.kind === 'function').map((n) => n.name); + + // The guard that makes the bound meaningful. `useMemo(() => …, [])` and + // `arr.map(() => …)` are single calls whose argument is a computation; if + // these ever start producing function nodes, the callee test has been + // widened past what this change claims. + expect(names).not.toContain('total'); + expect(names).not.toContain('mapped'); + }); + + it('attributes the body calls to the function, not to the file', () => { + const result = extractFromSource('src/b.ts', CODE); + const fn = result.nodes.find((n) => n.kind === 'function' && n.name === 'viaEffectGen'); + expect(fn, 'no function node for viaEffectGen').toBeDefined(); + + // The half of the report that a node count alone does not cover: the file + // must not be the one calling helper. + expect(refsFrom(result, fn!.id)).toContain('helper'); + const file = result.nodes.find((n) => n.kind === 'file'); + expect(file, 'no file node').toBeDefined(); + expect(refsFrom(result, file!.id)).not.toContain('helper'); + }); +}); + +// Object members (#1747 follow-up). An Effect service is usually an object a +// factory returns, so the wrapper's result lands in a `pair`, not a declarator. +// The property key names it — the same key extractObjectLiteralFunctions +// already uses for `key: () => {}`. +const MEMBER_CODE = ` +declare const Effect: { fn: (n: string) => (b: unknown) => unknown }; +declare function wrap(n: string): (c: unknown) => unknown; +declare function useMemo(f: () => T, d: unknown[]): T; + +function helper() { return 1; } + +function make() { + return { + getMode: Effect.fn("ACP.Session.getMode")(function* (id: string) { return helper(); }), + 'quotedKey': wrap("n")(() => { return helper(); }), + computed: useMemo(() => 1 + 1, []), + mapped: [1, 2].map((x) => x + 1), + }; +} + +const service = { + run: Effect.fn("Service.run")(function* () { return helper(); }), + plain: () => helper(), +}; + +// Exported with an inline function member: extractObjectLiteralFunctions mints +// the members one by one and the initializer is not walked, so the curried +// member has to be reached from there too. +export const mixed = { + direct: () => helper(), + wrapped: Effect.fn("Mixed.wrapped")(function* () { return helper(); }), +}; +`; + +describe('curried wrapper handlers as object members', () => { + const fnNodes = (result: ReturnType) => + result.nodes.filter((n) => n.kind === 'function'); + + it('names the wrapped function after its property key', () => { + const names = fnNodes(extractFromSource('src/m.ts', MEMBER_CODE)).map((n) => n.name); + expect(names).toContain('getMode'); + expect(names).toContain('quotedKey'); + expect(names).toContain('run'); + expect(names).toContain('wrapped'); + }); + + it('leaves single-call member values anonymous, and does not duplicate plain members', () => { + const names = fnNodes(extractFromSource('src/m.ts', MEMBER_CODE)).map((n) => n.name); + expect(names).not.toContain('computed'); + expect(names).not.toContain('mapped'); + // Neither path may mint a member twice. + expect(names.filter((n) => n === 'run')).toHaveLength(1); + expect(names.filter((n) => n === 'wrapped')).toHaveLength(1); + expect(names.filter((n) => n === 'plain').length).toBeLessThanOrEqual(1); + }); + + it('attributes the member body calls to the member, not to the factory', () => { + const result = extractFromSource('src/m.ts', MEMBER_CODE); + const getMode = fnNodes(result).find((n) => n.name === 'getMode'); + expect(getMode, 'no function node for getMode').toBeDefined(); + expect(refsFrom(result, getMode!.id)).toContain('helper'); + const make = fnNodes(result).find((n) => n.name === 'make'); + expect(make, 'no function node for make').toBeDefined(); + expect(refsFrom(result, make!.id)).not.toContain('helper'); + }); +}); diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index 99fe07468..8af774945 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/codegraph-kernel/src/tsjs/extractors.rs @@ -559,6 +559,17 @@ impl<'t> Walker<'t> { if matches!(v.kind(), "arrow_function" | "function_expression") { let name = util::object_key_name(self.text(k)); self.extract_function(v, Some(name)); + } else if v.kind() == "call_expression" { + // `key: Effect.fn("…")(function* () {…})` — see + // curried_wrapper_bound_name. + let fun = v + .child_by_field_name("arguments") + .and_then(|a| a.named_child(0)); + if let Some(f) = fun { + if let Some(bound) = self.curried_wrapper_bound_name(f) { + self.extract_function(f, Some(bound)); + } + } } } } else if member.kind() == "method_definition" { diff --git a/codegraph-kernel/src/tsjs/mod.rs b/codegraph-kernel/src/tsjs/mod.rs index d9932ad6f..28f299f5e 100644 --- a/codegraph-kernel/src/tsjs/mod.rs +++ b/codegraph-kernel/src/tsjs/mod.rs @@ -752,6 +752,13 @@ impl<'t> Walker<'t> { self.extract_function(node, Some(bound)); return; } + // `const run = Effect.fn("Session.run")(function* () {…})` (#1747): + // the same declarator binding through a curried wrapper. Mirrors + // TreeSitterExtractor's curriedWrapperBoundName. + if let Some(bound) = self.curried_wrapper_bound_name(node) { + 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. @@ -842,6 +849,71 @@ impl<'t> Walker<'t> { Some(self.text(name_node).to_string()) } + /// The declarator name for an anonymous function passed to a CURRIED + /// wrapper call — `const NAME = factory(...)(function () {…})` — or the + /// property key when the call is an object member, `{ NAME: factory(...)(fn) }`; + /// else None. + /// + /// `react_hook_bound_name` above names a function through the declarator + /// that binds it; the shape is general, but that method is bounded to the + /// three React handler hooks. This is the same shape with a different, + /// equally decidable bound: the callee is itself a call, i.e. a factory + /// that returns the wrapper (#1747). Requiring that keeps it narrow — + /// `useMemo(|| …, [])` and `arr.map(…)` are single calls and stay + /// anonymous, exactly as before. + /// + /// Generators are admitted here and not in `react_hook_bound_name`: a + /// React handler is never a generator, while `function*` is the common + /// form in the ecosystem this shape comes from. + /// + /// Mirrors TreeSitterExtractor's curriedWrapperBoundName. + fn curried_wrapper_bound_name(&self, node: Node<'t>) -> Option { + if !matches!( + node.kind(), + "arrow_function" | "function_expression" | "generator_function" + ) { + return None; + } + let args = node.parent()?; + if args.kind() != "arguments" { + return None; + } + let first = args.named_child(0)?; + if first.start_byte() != node.start_byte() || first.end_byte() != node.end_byte() { + return None; + } + let call = args.parent()?; + if call.kind() != "call_expression" { + return None; + } + // The bound that replaces the hook allowlist: the thing being called + // is itself a call, so this is a curried wrapper's second application. + let callee = call.child_by_field_name("function")?; + if callee.kind() != "call_expression" { + return None; + } + let binder = call.parent()?; + // `{ getMode: Effect.fn("…")(function* () {…}) }`: an object member is + // named by its property key, as extract_object_literal_functions names + // `key: () => {}`. + if binder.kind() == "pair" { + let key = binder.child_by_field_name("key")?; + let value = binder.child_by_field_name("value")?; + if value.start_byte() != call.start_byte() || value.end_byte() != call.end_byte() { + return None; + } + return Some(util::object_key_name(self.text(key))); + } + if binder.kind() != "variable_declarator" { + return None; + } + let name_node = binder.child_by_field_name("name")?; + if name_node.kind() != "identifier" { + return None; + } + Some(self.text(name_node).to_string()) + } + /// extractName / extractNameRaw for the TS/JS configs. fn extract_name(&self, node: Node) -> String { // javascriptExtractor.resolveName: field_definition names its key the diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 633949b2f..d8fed3e5c 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -2306,6 +2306,11 @@ export class TreeSitterExtractor { const value = getChildByField(member, 'value'); if (key && value && (value.type === 'arrow_function' || value.type === 'function_expression')) { this.extractFunction(value, this.objectKeyName(key)); + } else if (value?.type === 'call_expression') { + // `key: Effect.fn("…")(function* () {…})` — see curriedWrapperBoundName. + const fn = getChildByField(value, 'arguments')?.namedChild(0); + const bound = fn ? this.curriedWrapperBoundName(fn) : null; + if (fn && bound) this.extractFunction(fn, bound); } } else if (member.type === 'method_definition') { // Method shorthand: `{ fetchUser() {...} }`. extractMethod deliberately @@ -5601,6 +5606,71 @@ export class TreeSitterExtractor { return nameNode?.type === 'identifier' ? getNodeText(nameNode, this.source) : null; } + /** + * The declarator name for an anonymous function passed to a CURRIED wrapper + * call — `const NAME = factory(...)(function () {…})` — or the property key + * when the call is an object member, `{ NAME: factory(...)(fn) }`; else null. + * + * `reactHookBoundName` above already names a function through the declarator + * that binds it; the shape is general, but that method is deliberately + * bounded to the three React handler hooks. This is the same shape with a + * different, equally decidable bound: the callee is itself a call, i.e. a + * factory that returns the wrapper (#1747). + * + * Requiring the callee to be a call is what keeps this narrow. It admits + * `Effect.fn("Session.run")(function* () {…})`, `connect(mapState)(fn)` and + * a project's own `wrap("name")(fn)`, and it does not admit the one-call + * forms where the argument is a computation rather than a body worth a node + * of its own — `useMemo(() => 1 + 1, [])`, `arr.map(() => …)` — which stay + * anonymous exactly as before. + * + * Generators are included here and not in `reactHookBoundName`: a React + * handler is never a generator, while `function*` is the common form in the + * ecosystem this shape comes from. + */ + private curriedWrapperBoundName(node: SyntaxNode): string | null { + if ( + this.language !== 'typescript' && + this.language !== 'javascript' && + this.language !== 'tsx' && + this.language !== 'jsx' + ) { + return null; + } + if ( + node.type !== 'arrow_function' && + node.type !== 'function_expression' && + node.type !== 'generator_function' + ) { + return null; + } + const args = node.parent; + if (!args || args.type !== 'arguments') return null; + const first = args.namedChild(0); + if (!first || first.startIndex !== node.startIndex || first.endIndex !== node.endIndex) return null; + const call = args.parent; + if (!call || call.type !== 'call_expression') return null; + // The bound that replaces the hook allowlist: the thing being called is + // itself a call, so this is the second application of a curried wrapper. + const callee = getChildByField(call, 'function'); + if (!callee || callee.type !== 'call_expression') return null; + const binder = call.parent; + if (!binder) return null; + // `{ getMode: Effect.fn("…")(function* () {…}) }` — a service is often an + // object a factory returns, so the wrapper's result lands in a `pair`. The + // property key names it, as extractObjectLiteralFunctions names + // `key: () => {}`. + if (binder.type === 'pair') { + const key = getChildByField(binder, 'key'); + const value = getChildByField(binder, 'value'); + if (!key || !value || value.startIndex !== call.startIndex || value.endIndex !== call.endIndex) return null; + return this.objectKeyName(key); + } + if (binder.type !== 'variable_declarator') return null; + const nameNode = getChildByField(binder, 'name'); + return nameNode?.type === 'identifier' ? getNodeText(nameNode, this.source) : null; + } + private visitFunctionBody(body: SyntaxNode, _functionId: string): void { if (!this.extractor) return; @@ -5738,6 +5808,16 @@ export class TreeSitterExtractor { this.extractFunction(node, hookBound); return; } + // `const run = Effect.fn("Session.run")(function* () {…})` (#1747) — + // the same declarator binding through a curried wrapper. Without a node + // the body's calls attribute to the enclosing container, so the file or + // the outer function picks up an outgoing edge that belongs to this + // function and the callee's caller list names the wrong thing. + const wrapperBound = this.curriedWrapperBoundName(node); + if (wrapperBound) { + this.extractFunction(node, wrapperBound); + 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`