mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-22 09:35:22 +08:00
fix(ts/js): scope a variable's initializer walk to the variable (#693 for TS/JS)
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 `<cfscript>` bodies are delegated to a separate extractor as if they were a whole module, so a `var x = helper()` local inside a `<cffunction>` 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).
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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)];
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
+18
-3
@@ -2383,8 +2383,16 @@ export class ToolHandler {
|
||||
// fed only to the dynamic-dispatch-links scan below.
|
||||
const dynNamed = new Map<string, Node>();
|
||||
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<string>();
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user