From a10a7fa6f71dad8a5803e6a13020ed9beb8f967c Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Wed, 5 Aug 2026 22:44:33 +0300 Subject: [PATCH] fix(ts/js): scope a variable's initializer walk to the variable (#693 for TS/JS) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in one place — extractVariable's JS-family branch. 1. The initializer walk ran with only the FILE on the node stack, so `const cfg = loadConfig()` recorded the FILE as loadConfig's caller. That is literally the leak #693 described and #744 fixed for Go; the closing note there said "JS/TS didn't have this gap", which holds only for the `const f = () => …` shape (an arrow value delegates to extractFunction and gets its own node). 2. Object literals were excluded from the walk outright, on the grounds that their function-valued members are extracted individually below — but that only happens for EXPORTED consts. `const obj = { handler: () => target() }` therefore contributed nothing at all: no member node, and no call edge to anything. Fix: walk the initializer with the declared symbol pushed on the stack, and skip only the shapes whose members really are extracted one-by-one (exported object-of-functions, RTK endpoints, Pinia setup, Vue store collections) — walking those too would double-count each member arrow's calls. Two follow-on repairs the change surfaced: - CFML `` bodies are delegated to a separate extractor as if they were a whole module, so a `var x = helper()` local inside a `` minted a top-level variable node and the walk attributed `helper` to it. Those snippet-top-level non-callables are locals of the enclosing function; their refs now redirect to it, like the snippet's file-attributed refs already did. - codegraph_explore stopped listing a synthesized dynamic-dispatch link once the same pair also had a static call edge: it asked getCallers/getCallees, which return one row per NEIGHBOUR (the #1086 de-dup), so the static edge hid the heuristic one. It now asks the edges directly. This is exactly the RTK thunk case — `dispatch(innerThunk(n))` inside a thunk initializer is now a real static edge, which is the point, but the synthesized hop must still show up in the summary. Measured on three independent TypeScript trees (codegraph's own src/, evcc-ng, gv-grx — 499 files): 0 lost nodes/edges/refs, 780 refs re-attributed from the file node to the declaring constant, 228 genuinely new refs from object literals, node and edge counts unchanged. Kernel parity holds (324/324 non-deferred files byte-identical). --- CHANGELOG.md | 2 ++ __tests__/extraction.test.ts | 36 +++++++++++++++++++++ __tests__/fixtures/kernel-parity/torture.js | 6 ++++ codegraph-kernel/src/tsjs/extractors.rs | 30 ++++++++++------- src/extraction/cfml-extractor.ts | 16 ++++++++- src/extraction/tree-sitter.ts | 29 ++++++++++------- src/mcp/tools.ts | 21 ++++++++++-- 7 files changed, 113 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b9d9f094..e9ae9fff5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - On Windows, the Claude Code prompt hook written by `codegraph install` failed with "command not found" when hooks run through Git Bash, which needs the `.cmd` extension to find the launcher. The installer now writes the platform-correct command, and re-running `codegraph install` (or `codegraph upgrade`) repairs an existing install in place. (#1466) - Python classes used as values — `return SomeSerializer` from a factory method, `handler = SomeClass` aliases, registry dicts and lists, and classes passed as arguments — now produce reference edges in the graph. Previously these idioms were invisible, so on Django and Django REST Framework projects, asking for a serializer's callers or the impact of editing it missed the views that actually use it. Re-index after upgrading to pick up the new edges. (#1478) - When a file changed on disk after its last index sync, `codegraph_node` and `codegraph_explore` could return a different symbol's code under the requested name — current file bytes cut at outdated line positions — while presenting it as verbatim, trustworthy source. This hit hardest on projects queried through `projectPath` (for example, sub-projects of a monorepo), which have no live file watcher to flag pending edits. Both tools now verify each file against the index before showing sliced code: an out-of-date file is either shown whole with its full current source, or its code is withheld with a clear "changed on disk" notice — never served as a wrong slice. A fresh re-index restores normal output automatically. Thanks @inth3shadows for the thorough report and verification passes. (#1474) +- TypeScript and JavaScript module-level declarations now name themselves as the caller of whatever their initializer runs. `const cfg = loadConfig()` recorded the *file* as loadConfig's caller, which is no use for callers or impact; it now records `cfg`. And an object literal that wasn't exported — `const handlers = { onSave: () => persist() }` — was skipped entirely, so nothing inside it reached the graph at all. Re-index with `codegraph index -f` after upgrading to pick up the new edges. +- `codegraph_explore` again lists a dynamic-dispatch link when the same two symbols are also joined by an ordinary call, instead of dropping it from the summary. - Java fields initialized with a lambda or an anonymous class — `private final Runnable r = () -> doWork();`, `new LocationListener() { … }`, `Parcelable.Creator` — now contribute call edges, and the anonymous class and its overrides become real symbols instead of being invisible. Previously everything inside a field initializer was dropped, so a method reached only from one looked like it had no callers. Re-index with `codegraph index -f` after upgrading to pick up the new edges. - Kotlin properties that hold a lambda, a SAM callback or an anonymous object — `private val frameListener = CameraFrameListener { … }`, the way Android and MSDK callbacks are almost always declared — now contribute call edges. Previously everything inside such an initializer was dropped, so a function reached only through one of these callbacks looked like it had no callers at all and its blast radius came back far too small. Delegated properties (`by lazy { … }`) and plain initializers (`val x = compute()`) were affected the same way and are fixed too. Re-index with `codegraph index -f` after upgrading to pick up the new edges. - The blast-radius section of `codegraph_explore` flagged "no covering tests found" whenever no test called a symbol directly — falsely branding helpers that tests exercise through their callers as untested (about 40% of flagged symbols in a measured sample). The check now follows caller chains up to 3 hops and reports indirect coverage as "tested via callers"; when nothing is found it states exactly what was checked instead of an unconditional warning. Thanks @inth3shadows for measuring the false-positive rate. (#1475) diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 31827e8fe..92907b0a3 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -931,6 +931,42 @@ const token = getTokenMp(); ); expect(call).toBeDefined(); }); + + describe('initializer walk is scoped to the declared symbol (#693 for TS/JS)', () => { + const code = ` +const eager = load(); +const obj = { handler: () => target(), plain: target() }; +const list = [() => target()]; +export const exported = { handler: () => target() }; +`; + const callersOf = (name: string) => { + const result = extractFromSource('app.ts', code); + const byId = new Map(result.nodes.map((n) => [n.id, n])); + return result.unresolvedReferences + .filter((u) => u.referenceKind === 'calls' && u.referenceName === name) + .map((u) => byId.get(u.fromNodeId)) + .map((n) => (n ? `${n.kind}:${n.name}` : '?')) + .sort(); + }; + + it("a plain call initializer names the CONSTANT as caller, not the file", () => { + // The walk ran with only the file on the stack, so `load` recorded the + // file as its caller — useless for callers/impact. + expect(callersOf('load')).toEqual(['constant:eager']); + }); + + it('a non-exported object literal contributes calls (it was skipped outright)', () => { + // `exported`'s members are minted as their own function nodes, so its + // arrow's call comes from `handler`; the non-exported ones attribute to + // the declared constant. + expect(callersOf('target')).toEqual([ + 'constant:list', + 'constant:obj', + 'constant:obj', + 'function:handler', + ]); + }); + }); }); describe('File Node Extraction', () => { diff --git a/__tests__/fixtures/kernel-parity/torture.js b/__tests__/fixtures/kernel-parity/torture.js index 50ddd0246..98b6ea4ed 100644 --- a/__tests__/fixtures/kernel-parity/torture.js +++ b/__tests__/fixtures/kernel-parity/torture.js @@ -73,3 +73,9 @@ export default { }, }, }; + +// Initializer walks attributed to the declared symbol (#693). A plain call +// leaked to the FILE node; a non-exported object literal was skipped outright. +const eagerConfig = loadConfig(); +const handlerMap = { onSave: () => persist(eagerConfig), onLoad: loadConfig() }; +const lazyList = [() => persist(eagerConfig)]; diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index ad0440d6d..66679943d 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/codegraph-kernel/src/tsjs/extractors.rs @@ -398,18 +398,26 @@ impl<'t> Walker<'t> { } } - // Walk the initializer for calls — except the object/store shapes - // whose members are extracted method-by-method below. + // Walk the initializer for calls, ATTRIBUTED to the declared symbol + // (#693) — except the object/store shapes whose members are + // extracted method-by-method below (walking those too would + // double-count each member arrow's calls). Before this the walk ran + // with only the FILE on the stack (`const cfg = load()` recorded the + // file as load's caller) and object literals were skipped outright. + let members_extracted_separately = extract_object_methods + || rtk_endpoints.is_some() + || pinia_setup.is_some() + || !store_collections.is_empty(); if let Some(v) = value { - let vk = v.kind(); - if vk != "object" - && vk != "object_expression" - && !(extract_object_methods && vk == "call_expression") - && rtk_endpoints.is_none() - && pinia_setup.is_none() - && store_collections.is_empty() - { - self.visit_function_body(v); + if !members_extracted_separately { + match var_row { + Some(row) => { + self.stack.push(Scope { row, kind, name: name.clone() }); + self.visit_function_body(v); + self.stack.pop(); + } + None => self.visit_function_body(v), + } } } diff --git a/src/extraction/cfml-extractor.ts b/src/extraction/cfml-extractor.ts index 2f4bc4779..edb4a4955 100644 --- a/src/extraction/cfml-extractor.ts +++ b/src/extraction/cfml-extractor.ts @@ -356,6 +356,13 @@ export class CfmlExtractor { .filter((e) => e.kind === 'contains' && e.source === innerFileNodeId) .map((e) => e.target) ); + // Snippet-top-level non-callables: `var x = …` locals of the enclosing + // function that the fragment-as-module parse mints as declarations. + const localVarIds = new Set( + result.nodes + .filter((n) => topLevelIds.has(n.id) && (n.kind === 'variable' || n.kind === 'constant')) + .map((n) => n.id) + ); for (const node of result.nodes) { if (node.kind === 'file') continue; node.startLine += startLine; @@ -385,7 +392,14 @@ export class CfmlExtractor { // top-level script in a .cfm template, or any statement directly in // the snippet body) attribute to the filtered-out snippet file node by // default — redirect those (and any genuinely unset ones) to parentId. - if ((!ref.fromNodeId || ref.fromNodeId === innerFileNodeId) && parentId) ref.fromNodeId = parentId; + // Same for a snippet-top-level `var x = helper()`: the inner extractor + // parses the fragment as a whole module, so it mints a variable node and + // attributes the initializer's calls to it — but this fragment is a + // FUNCTION BODY, so `x` is a local and `helper` is the enclosing + // function's callee. Snippet-top-level FUNCTIONS keep their own calls. + if ((!ref.fromNodeId || ref.fromNodeId === innerFileNodeId || localVarIds.has(ref.fromNodeId)) && parentId) { + ref.fromNodeId = parentId; + } this.unresolvedReferences.push(ref); } for (const error of result.errors) { diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index dfb3b3e26..9b25f8fe5 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -2697,19 +2697,24 @@ export class TreeSitterExtractor { storeCollections.push(objectOfFns); } - // Visit the initializer body for calls — EXCEPT object literals (their - // function-valued properties are extracted below) and the store-factory - // / createApi / store-collection call whose nested objects we extract - // method-by-method below (walking the whole call would re-visit those - // method arrows and mis-attribute their inner calls to the file scope). - if (valueNode && - valueNode.type !== 'object' && - valueNode.type !== 'object_expression' && - !(extractObjectMethods && valueNode.type === 'call_expression') && - !rtkEndpoints && - !piniaSetup && - storeCollections.length === 0) { + // Visit the initializer body for calls, ATTRIBUTED to the declared + // symbol (#693) — EXCEPT the shapes whose members are extracted + // one-by-one below (the store-factory / createApi / store-collection + // objects), where walking the whole initializer would re-visit each + // member arrow and double-count its calls. + // + // Two things were wrong here before. The walk ran with only the FILE + // on the stack, so `const cfg = load()` recorded the FILE as load's + // caller — the exact leak Go's #693 fixed. And an object literal was + // skipped outright, so `const obj = { handler: () => target() }` + // contributed nothing at all unless the const was exported (only then + // does extractObjectLiteralFunctions mint the members). + const membersExtractedSeparately = + extractObjectMethods || !!rtkEndpoints || !!piniaSetup || storeCollections.length > 0; + if (valueNode && !membersExtractedSeparately) { + if (varNode) this.nodeStack.push(varNode.id); this.visitFunctionBody(valueNode, ''); + if (varNode) this.nodeStack.pop(); } if (extractObjectMethods && objectOfFns) { diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 51c81e16f..e69b1b791 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -2383,8 +2383,16 @@ export class ToolHandler { // fed only to the dynamic-dispatch-links scan below. const dynNamed = new Map(); const DYN_KINDS = new Set(['constant', 'variable', 'field', 'property']); + // RAW edges, not getCallers/getCallees: those return one row per NEIGHBOUR + // (the #1086 de-dup), so when a pair is joined by BOTH a static and a + // synthesized edge the static one wins and the synthesized one becomes + // invisible — which is exactly what happens once a thunk's `dispatch(x)` + // is walked statically. The question here is about the graph, not about + // callers, so ask the edges directly. const hasHeuristicEdge = (id: string): boolean => - [...cg.getCallers(id), ...cg.getCallees(id)].some(({ edge }) => edge.provenance === 'heuristic'); + [...cg.getIncomingEdges(id), ...cg.getOutgoingEdges(id)].some( + (e) => e.provenance === 'heuristic' + ); for (const t of tokens) { const hits = this.findAllSymbols(cg, t).nodes; const cands = hits.filter((n) => CALLABLE.has(n.kind)); @@ -2432,9 +2440,16 @@ export class ToolHandler { const synthSeen = new Set(); for (const n of [...named.values(), ...dynNamed.values()]) { if (synthLines.length >= 6) break; - for (const { node: other, edge } of [...cg.getCallers(n.id), ...cg.getCallees(n.id)]) { + // RAW edges for the same reason as hasHeuristicEdge above — a static + // edge over the same pair hides the synthesized one from getCallers. + const incident = [...cg.getIncomingEdges(n.id), ...cg.getOutgoingEdges(n.id)]; + for (const edge of incident) { if (synthLines.length >= 6) break; - if (edge.provenance !== 'heuristic' || other.id === n.id) continue; + if (edge.provenance !== 'heuristic') continue; + const otherId = edge.source === n.id ? edge.target : edge.source; + if (otherId === n.id) continue; + const other = cg.getNode(otherId); + if (!other) continue; if (skipInChain && skipInChain(edge)) continue; const src = edge.source === n.id ? n : other; const tgt = edge.source === n.id ? other : n;