mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-26 12:30:04 +08:00
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
This commit is contained in:
@@ -295,6 +295,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
- Python calls and file dependencies through `from package import module as alias` now appear in the graph, so renamed imports no longer hide live callers or imported modules. Thanks @JoeyNPP. (#1626)
|
||||
|
||||
- **A function bound through a wrapper call is now part of the graph.** `const run = Effect.fn("Session.run")(function* () {…})` — and the same shape with Redux's `connect(mapState)(…)` or a project's own `wrap("name")(…)` — produced no function at all: only React's `useCallback`, `useEffectEvent` and `useEvent` were recognized as naming the function they wrap. The calls inside those bodies were attributed to the file instead, so a file appeared to call what the function called, and asking who calls a helper named the file rather than the function that really calls it. One codebase carried over a thousand of these. Any wrapper that is itself the result of a call now names the function it wraps, generators included; a callback passed straight to a single call, such as `useMemo` or `map`, is a computation and stays anonymous as before. Re-index after upgrading. (#1747)
|
||||
|
||||
## [1.6.0] - 2026-08-26
|
||||
|
||||
### Highlights
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* 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');
|
||||
});
|
||||
});
|
||||
@@ -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,58 @@ 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 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 declarator = call.parent()?;
|
||||
if declarator.kind() != "variable_declarator" {
|
||||
return None;
|
||||
}
|
||||
let name_node = declarator.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
|
||||
|
||||
@@ -5591,6 +5591,59 @@ 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 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 declarator = call.parent;
|
||||
if (!declarator || declarator.type !== 'variable_declarator') return null;
|
||||
const nameNode = getChildByField(declarator, 'name');
|
||||
return nameNode?.type === 'identifier' ? getNodeText(nameNode, this.source) : null;
|
||||
}
|
||||
|
||||
private visitFunctionBody(body: SyntaxNode, _functionId: string): void {
|
||||
if (!this.extractor) return;
|
||||
|
||||
@@ -5728,6 +5781,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`
|
||||
|
||||
Reference in New Issue
Block a user