fix(extraction): name a function bound through a curried wrapper (#1747) (#2002)

* fix(extraction): name a function bound through a curried wrapper (#1747)

reactHookBoundName already names an anonymous function after the
variable_declarator that binds it. The shape is general, but the method
is bounded to three React hooks, so `const run =
Effect.fn("Session.run")(function* () {…})` produced no function node at
all -- and its body's calls were attributed to the enclosing container,
so a file gained an outgoing edge belonging to a function and the
callee's caller list named the file instead.

Bound the new path by a different, equally decidable test: the callee is
itself a call, i.e. a factory returning the wrapper. That admits
Effect.fn(...)(fn), connect(mapState)(fn) and a project's own
wrap("n")(fn), and excludes the single-call forms whose argument is a
computation -- useMemo(() => …, []), arr.map(…) -- which stay anonymous
exactly as before. Widening to "any callee" would have given useMemo a
function node and reddened react-hook-handlers.test.ts.

Generators are admitted 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, and it is what the report opens with --
broadening the callee test alone would not have fixed that repro.

Mirrored in the Rust kernel; kernel-parity.mjs reports byte-parity on
the new shape.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WgtWMywGTyu6twnk1Bu9nP
(cherry picked from commit 4f2b40080d)

* fix(extraction): name a curried-wrapper function that is an object member (#1747)

The curried-wrapper bound stopped at a variable_declarator parent, so
`return { getMode: Effect.fn("ACP.Session.getMode")(function* () {…}) }`
still produced no function node. An Effect service is usually an object a
factory returns, and on sst/opencode (df23b7f) these member positions were
the bulk of what the branch missed.

The member already has a name in the source: its property key. This repo
names `key: () => {}` pairs by that key (extractObjectLiteralFunctions /
objectKeyName), so a `pair` parent is now named the same way. The
callee-is-a-call rule is unchanged, so `key: useMemo(() => …, [])` and
`key: arr.map(…)` stay anonymous.

extractObjectLiteralFunctions also reaches such a member, because an
exported object with an inline function member is extracted member by member
and its initializer is never walked.

A wrapper passed as an argument to another call has no binding at all and is
left as it was.

Mirrored in the Rust kernel; kernel-parity.mjs reports byte-parity on the
new shapes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dfb3aazPY1ccmp5vam4k3L
(cherry picked from commit bbdef20542)

* chore: drop CHANGELOG entry (collected separately)

---------

Co-authored-by: Max Hsu <maxmilian@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-09-27 12:30:53 +00:00
committed by GitHub
parent d1df89e17d
commit 9fb2c25bd5
4 changed files with 314 additions and 0 deletions
+151
View File
@@ -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<typeof extractFromSource>, 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<T>(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<T>(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<typeof extractFromSource>) =>
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');
});
});
+11
View File
@@ -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" {
+72
View File
@@ -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<String> {
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
+80
View File
@@ -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`