Skip to content

Commit 5fe25b9

Browse files
committed
fix(receiver): same-file function constructors win over cross-file class nodes
When multiple files define a symbol with the same name, the receiver-type edge for a `new C()` call in a file that defines `function C() {}` (pre-ES6 constructor) was pointing to a `class C` in a different file instead of the local constructor. Root cause: `RECEIVER_KINDS` / `receiver_kinds` did not include `function`, so the same-file `C` (kind=function) was filtered out, the same-file set became empty, and the global fallback found the wrong `class C` in another file. Fix: introduce `RECEIVER_KINDS_SAME_FILE` (TS) and `receiver_kinds_same_file` (Rust) that extend the base set with `function`. Same-file candidate lookup uses the wider set; the global cross-file fallback keeps the narrower set to avoid false positives from unrelated same-named functions in other files. Applied in both engines (call-resolver.ts + build_edges.rs) to maintain parity. Closes #1513
1 parent e5ea649 commit 5fe25b9

3 files changed

Lines changed: 151 additions & 20 deletions

File tree

crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,11 @@ struct EdgeContext<'a> {
140140
nodes_by_file: HashMap<&'a str, Vec<&'a NodeInfo>>,
141141
builtin_set: HashSet<&'a str>,
142142
receiver_kinds: HashSet<&'a str>,
143+
/// Same-file receiver lookup also accepts `function` to handle pre-ES6
144+
/// function constructors (e.g. `function C() {}` with `C.prototype = { … }`).
145+
/// Global fallback keeps the narrower set to avoid false positives from
146+
/// unrelated same-named functions in other files.
147+
receiver_kinds_same_file: HashSet<&'a str>,
143148
}
144149

145150
impl<'a> EdgeContext<'a> {
@@ -158,7 +163,17 @@ impl<'a> EdgeContext<'a> {
158163
let builtin_set: HashSet<&str> = builtin_receivers.iter().map(|s| s.as_str()).collect();
159164
let receiver_kinds: HashSet<&str> = ["class", "struct", "interface", "type", "module"]
160165
.iter().copied().collect();
161-
Self { nodes_by_name, nodes_by_name_and_file, nodes_by_file, builtin_set, receiver_kinds }
166+
let receiver_kinds_same_file: HashSet<&str> =
167+
["class", "struct", "interface", "type", "module", "function"]
168+
.iter().copied().collect();
169+
Self {
170+
nodes_by_name,
171+
nodes_by_name_and_file,
172+
nodes_by_file,
173+
builtin_set,
174+
receiver_kinds,
175+
receiver_kinds_same_file,
176+
}
162177
}
163178
}
164179

@@ -1035,16 +1050,15 @@ fn emit_receiver_edge(
10351050
let type_entry = type_map.get(receiver.as_str());
10361051
let effective_receiver = type_entry.map(|&(t, _)| t).unwrap_or(receiver.as_str());
10371052

1038-
// Filter-before: apply receiver_kinds to same-file candidates first, then
1039-
// fall back to global candidates (also filtered) only when same-file yields
1040-
// nothing. This prevents an imported name emitted as kind='function' in the
1041-
// importing file from blocking the fallback to the actual class/struct/etc.
1042-
// node in the defining file.
1053+
// Same-file candidates use receiver_kinds_same_file (includes "function") so
1054+
// that pre-ES6 function constructors (e.g. `function C() {}`) in the same
1055+
// file are matched. Global fallback uses the narrower receiver_kinds to
1056+
// avoid false positives from unrelated same-named functions in other files.
10431057
let samefile_candidates: Vec<&NodeInfo> = ctx.nodes_by_name_and_file
10441058
.get(&(effective_receiver, rel_path))
10451059
.cloned().unwrap_or_default()
10461060
.into_iter()
1047-
.filter(|n| ctx.receiver_kinds.contains(n.kind.as_str()))
1061+
.filter(|n| ctx.receiver_kinds_same_file.contains(n.kind.as_str()))
10481062
.collect();
10491063
let receiver_nodes: Vec<&NodeInfo> = if !samefile_candidates.is_empty() {
10501064
samefile_candidates

src/domain/graph/builder/call-resolver.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ export interface CallNodeLookup {
2323

2424
export const RECEIVER_KINDS = new Set(['class', 'struct', 'interface', 'type', 'module']);
2525

26+
/**
27+
* Kinds accepted as receiver-type candidates when looking in the *same file*
28+
* as the call site. Same-file scope makes the match unambiguous, so we also
29+
* accept `function` to handle pre-ES6 function constructors (e.g.
30+
* `function C() {}` with `C.prototype = { … }`) that are extracted as
31+
* `kind='function'` rather than `kind='class'`.
32+
*
33+
* Global (cross-file) lookups deliberately exclude `function` to avoid false
34+
* positives from unrelated same-named functions in other files.
35+
*/
36+
export const RECEIVER_KINDS_SAME_FILE = new Set([...RECEIVER_KINDS, 'function']);
37+
2638
/**
2739
* Languages where bare `foo()` calls inside a class method are lexically scoped
2840
* to the module, not the class — there is no implicit this/class binding.
@@ -355,13 +367,15 @@ export function resolveCallTargets(
355367
* Returns the edge tuple to insert, or null if nothing matched or the edge
356368
* was already seen. Callers are responsible for the actual DB/array insert.
357369
*
358-
* Receiver resolution collects all same-file candidates first (no kind
359-
* filter), falls back to global candidates only when the same-file set is
360-
* entirely empty, then filters the chosen set by RECEIVER_KINDS. This
361-
* matches the native Rust build path: if a file imports a name that happens
362-
* to be emitted as `kind='function'` in the importer, the same-file set is
363-
* non-empty and blocks the global fallback, so no receiver edge is emitted.
364-
* Keeping this behaviour identical to the Rust path maintains engine parity.
370+
* Same-file candidates are filtered by RECEIVER_KINDS_SAME_FILE (which
371+
* includes `function` to handle pre-ES6 function constructors). If no
372+
* same-file candidate matches, the lookup falls back to global candidates
373+
* filtered by the narrower RECEIVER_KINDS (no `function`) to avoid false
374+
* positives from unrelated same-named functions in other files.
375+
*
376+
* This means a local `function C() {}` in the same file always wins over a
377+
* `class C` defined in a different file — fixing cross-file receiver
378+
* non-determinism when multiple files define the same constructor name.
365379
*/
366380
export function resolveReceiverEdge(
367381
lookup: CallNodeLookup,
@@ -382,14 +396,13 @@ export function resolveReceiverEdge(
382396
? ((typeEntry as { confidence?: number }).confidence ?? null)
383397
: null;
384398
const effectiveReceiver = typeName || call.receiver;
385-
// Filter-before: apply RECEIVER_KINDS to same-file candidates first, then
386-
// fall back to global candidates (also filtered) only when same-file yields
387-
// nothing. This prevents an imported name emitted as kind='function' in the
388-
// importing file from blocking the fallback to the actual class/struct/etc.
389-
// node in the defining file.
399+
// Same-file candidates use RECEIVER_KINDS_SAME_FILE (includes 'function') so
400+
// that pre-ES6 function constructors (e.g. `function C() {}`) in the same
401+
// file are matched. Global fallback uses the narrower RECEIVER_KINDS to
402+
// avoid false positives from unrelated same-named functions in other files.
390403
const sameFileCandidates = lookup
391404
.byNameAndFile(effectiveReceiver, relPath)
392-
.filter((n) => RECEIVER_KINDS.has(n.kind ?? ''));
405+
.filter((n) => RECEIVER_KINDS_SAME_FILE.has(n.kind ?? ''));
393406
const candidates =
394407
sameFileCandidates.length > 0
395408
? sameFileCandidates
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* Regression test for #1513: receiver edge for a function constructor must
3+
* point to the same-file definition when multiple files define the same name.
4+
*
5+
* Setup: two files both define a symbol named `C`.
6+
* - a.js: `function C() {}` with `C.prototype = { foo: function(){} }`
7+
* and a caller `run()` that does `new C(); v.foo()`
8+
* - b.js: `class C { bar() {} }` — same name, different file
9+
*
10+
* Expected receiver edge: `a.js::run → C` where C is the one in a.js (kind=function),
11+
* NOT the class C in b.js.
12+
*/
13+
14+
import fs from 'node:fs';
15+
import os from 'node:os';
16+
import path from 'node:path';
17+
import Database from 'better-sqlite3';
18+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
19+
import { buildGraph } from '../../src/domain/graph/builder.js';
20+
21+
const FILE_A = `
22+
function C() {}
23+
C.prototype = {
24+
foo: function() {},
25+
};
26+
export function run() {
27+
var v = new C();
28+
v.foo();
29+
}
30+
`;
31+
32+
// Same name C in a different file — must not steal the receiver edge from a.js
33+
const FILE_B = `
34+
export class C {
35+
bar() {}
36+
}
37+
`;
38+
39+
let tmpWasm: string;
40+
let tmpNative: string;
41+
42+
beforeAll(async () => {
43+
tmpWasm = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1513-wasm-'));
44+
fs.writeFileSync(path.join(tmpWasm, 'a.js'), FILE_A);
45+
fs.writeFileSync(path.join(tmpWasm, 'b.js'), FILE_B);
46+
47+
tmpNative = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1513-native-'));
48+
fs.writeFileSync(path.join(tmpNative, 'a.js'), FILE_A);
49+
fs.writeFileSync(path.join(tmpNative, 'b.js'), FILE_B);
50+
51+
await Promise.all([
52+
buildGraph(tmpWasm, { incremental: false, skipRegistry: true, engine: 'wasm' }),
53+
buildGraph(tmpNative, { incremental: false, skipRegistry: true, engine: 'native' }),
54+
]);
55+
});
56+
57+
afterAll(() => {
58+
fs.rmSync(tmpWasm, { recursive: true, force: true });
59+
fs.rmSync(tmpNative, { recursive: true, force: true });
60+
});
61+
62+
function getReceiverEdges(dbPath: string) {
63+
const db = new Database(dbPath, { readonly: true });
64+
try {
65+
return db
66+
.prepare(
67+
`SELECT n1.name AS src, n2.name AS tgt, n2.file AS tgt_file
68+
FROM edges e
69+
JOIN nodes n1 ON e.source_id = n1.id
70+
JOIN nodes n2 ON e.target_id = n2.id
71+
WHERE e.kind = 'receiver'
72+
ORDER BY n1.name, n2.name`,
73+
)
74+
.all() as Array<{ src: string; tgt: string; tgt_file: string }>;
75+
} finally {
76+
db.close();
77+
}
78+
}
79+
80+
describe('receiver same-file priority over cross-file (#1513)', () => {
81+
it('WASM: run() receiver edge targets C in a.js, not b.js', () => {
82+
const edges = getReceiverEdges(path.join(tmpWasm, '.codegraph', 'graph.db'));
83+
const edge = edges.find((e) => e.src === 'run');
84+
expect(edge).toBeDefined();
85+
expect(edge?.tgt).toBe('C');
86+
expect(edge?.tgt_file).toBe('a.js');
87+
});
88+
89+
it('Native: run() receiver edge targets C in a.js, not b.js', () => {
90+
const edges = getReceiverEdges(path.join(tmpNative, '.codegraph', 'graph.db'));
91+
const edge = edges.find((e) => e.src === 'run');
92+
expect(edge).toBeDefined();
93+
expect(edge?.tgt).toBe('C');
94+
expect(edge?.tgt_file).toBe('a.js');
95+
});
96+
97+
it('WASM and native produce the same receiver edges', () => {
98+
const wasmEdges = getReceiverEdges(path.join(tmpWasm, '.codegraph', 'graph.db'));
99+
const nativeEdges = getReceiverEdges(path.join(tmpNative, '.codegraph', 'graph.db'));
100+
const wasmSet = new Set(wasmEdges.map((e) => `${e.src}${e.tgt}@${e.tgt_file}`));
101+
const nativeSet = new Set(nativeEdges.map((e) => `${e.src}${e.tgt}@${e.tgt_file}`));
102+
expect(wasmSet).toEqual(nativeSet);
103+
});
104+
});

0 commit comments

Comments
 (0)